Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:44:53Zhttps://gitlab.torproject.org/legacy/trac/-/issues/31561hs-v3: Service can keep unused intro points in its list2020-06-13T15:44:53ZDavid Gouletdgoulet@torproject.orghs-v3: Service can keep unused intro points in its listTor always selects an extra number of intro points in addition to the configured `HiddenServiceNumIntroductionPoints`.
It launches all of them and the first `NumIntro...` to finish are used (considering #31548 is resolved).
Once the ci...Tor always selects an extra number of intro points in addition to the configured `HiddenServiceNumIntroductionPoints`.
It launches all of them and the first `NumIntro...` to finish are used (considering #31548 is resolved).
Once the circuit of the remaining intro opens, we notice that we have too many and then re-purpose the extra ones.
However, I've noticed that sometimes establishing an intro circuit timeouts during build, basically expiring due to our CBT policy. In that case, the circuit is simply close but the intro point remains in the service descriptor list.
This is bad because of #31548, this means an intro point can end up in the descriptor even though the service never established any circuits to it...
We simply need to callback into the HS subsystem when we are expiring an HS circuit so appropriate actions can be taken such as in this case, removing the IP from the list.Tor: unspecifiedDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/legacy/trac/-/issues/31392Explain Padding 1 and 2 in tor-spec.txt2020-06-13T15:44:14ZteorExplain Padding 1 and 2 in tor-spec.txtPadding=1 is declared by some versions of tor without Padding support. We will stop declaring support for it I once #31356 is merged into 0.4.1.
Padding=2 is the guaranteed working version once #31356 is merged into 0.4.1 and later.Padding=1 is declared by some versions of tor without Padding support. We will stop declaring support for it I once #31356 is merged into 0.4.1.
Padding=2 is the guaranteed working version once #31356 is merged into 0.4.1 and later.Tor: 0.4.2.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/31343appveyor: labs(time_t) is not allowed2020-06-13T15:44:03ZNick Mathewsonappveyor: labs(time_t) is not allowedMike points out that appveyor is failing with
```
../src/core/or/channeltls.c:1768:7: error: absolute value function 'labs' given an argument of type 'time_t' {aka 'long long int'} but has parameter of type 'long int' which may cause tr...Mike points out that appveyor is failing with
```
../src/core/or/channeltls.c:1768:7: error: absolute value function 'labs' given an argument of type 'time_t' {aka 'long long int'} but has parameter of type 'long int' which may cause truncation of value [-Werror=absolute-value]
1768 | if (labs(now - chan->conn->handshake_state->sent_versions_at) < 180) {
| ^~~~
```
That code has been there for ages, though. Looks like this is a new compiler warning rather than a new bug. It is a c issue, though, anywhere that time_t is bigger than long.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/31111Properly support two padding machines per circuit2020-06-13T15:43:29ZpullsProperly support two padding machines per circuitAs part of `circuitpadding.c`, in `circpad_add_matching_machines()`, the macros `FOR_EACH_CIRCUIT_MACHINE_BEGIN` and `SMARTLIST_FOREACH_REVERSE_BEGIN` currently expand to a for loop each. Below a slightly cropped (...) version of `circpa...As part of `circuitpadding.c`, in `circpad_add_matching_machines()`, the macros `FOR_EACH_CIRCUIT_MACHINE_BEGIN` and `SMARTLIST_FOREACH_REVERSE_BEGIN` currently expand to a for loop each. Below a slightly cropped (...) version of `circpad_add_matching_machines()`:
```
circpad_add_matching_machines(origin_circuit_t *on_circ,
smartlist_t *machines_sl)
{
...
FOR_EACH_CIRCUIT_MACHINE_BEGIN(i) {
...
SMARTLIST_FOREACH_REVERSE_BEGIN(machines_sl,
circpad_machine_spec_t *,
machine) {
...
if (circpad_negotiate_padding(on_circ, machine->machine_num,
machine->target_hopnum,
CIRCPAD_COMMAND_START) < 0) {
log_info(LD_CIRC, "Padding not negotiated. Cleaning machine");
circpad_circuit_machineinfo_free_idx(circ, i);
circ->padding_machine[i] = NULL;
on_circ->padding_negotiation_failed = 1;
} else {
/* Success. Don't try any more machines */
return;
}
}
} SMARTLIST_FOREACH_END(machine);
} FOR_EACH_CIRCUIT_MACHINE_END;
}
```
The outer loop goes over each machine index (currently 2, set by `CIRCPAD_MAX_MACHINES`), while the inner loop looks for a suitable machine for that index to negotiate. As soon as one is found and negotiated, currently, the function returns without looking for a machine for later indices in the outer loop. The `return` should be replaced by a `break` to continue looking for a machine for the next index.
See https://github.com/torproject/tor/pull/1168 for a PR.Tor: 0.4.3.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/31027Coverity: circpadding: dead code in circpad_machine_remove_token2020-06-13T15:43:15ZNick MathewsonCoverity: circpadding: dead code in circpad_machine_remove_tokenIn `circpad_machine_remove_token()`, we check for `state==NULL` on line 1087, and then again on line 1107. Since state is not assigned between these points, the second check is dead code.
This is CID 1447298.In `circpad_machine_remove_token()`, we check for `state==NULL` on line 1087, and then again on line 1107. Since state is not assigned between these points, the second check is dead code.
This is CID 1447298.Tor: 0.4.2.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/30686Better warnings when minherit/madvise fails2020-06-13T15:41:55ZNick MathewsonBetter warnings when minherit/madvise failsSee #30614 -- our current behavior here is to give a BUG message, which doesn't help the user understand what to do.See #30614 -- our current behavior here is to give a BUG message, which doesn't help the user understand what to do.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30309Rename tor_mem_is_zero to fast_mem_is_zero; use it consistently2020-06-13T15:41:04ZNick MathewsonRename tor_mem_is_zero to fast_mem_is_zero; use it consistentlyBy convention, tor_memeq() means "constant time" and fast_memeq() means "variable-time." But tor_mem_is_zero is variable-time, whereas its constant-time variant is safe_mem_is_zero.
I'm fine with the "safe_" name, but we should rename ...By convention, tor_memeq() means "constant time" and fast_memeq() means "variable-time." But tor_mem_is_zero is variable-time, whereas its constant-time variant is safe_mem_is_zero.
I'm fine with the "safe_" name, but we should rename tor_mem_is_zero() to indicate that it is the fast one, not the safe one. We should also audit its users, particularly tor_digest{256,}_is_zero()Tor: 0.4.1.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/30148Fix infrequent, unlikely memory leak on failure to create keys directory2020-06-13T15:40:38ZNick MathewsonFix infrequent, unlikely memory leak on failure to create keys directoryIn load_ed_keys(), if we can't create the key directory, we leak some memory.In load_ed_keys(), if we can't create the key directory, we leak some memory.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30147Fix "high-impact" coverity false-positive warnings outside of tests.2020-06-13T15:40:38ZNick MathewsonFix "high-impact" coverity false-positive warnings outside of tests.Let's fix all the current "high impact" coverity issues that are outside of the unit tests. (For other issues, see siblings of this ticket.)Let's fix all the current "high impact" coverity issues that are outside of the unit tests. (For other issues, see siblings of this ticket.)Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/30112Fix outdated comments in dirserv_read_measured_bandwidths()2020-06-13T15:40:33ZteorFix outdated comments in dirserv_read_measured_bandwidths()We refactored the function to use tor_getdelim(), but didn't remove the comments about fgets().We refactored the function to use tor_getdelim(), but didn't remove the comments about fgets().Tor: 0.4.1.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/30109Document that MapAddress is automatically strict, but does not handle redirects2020-06-13T15:40:32ZteorDocument that MapAddress is automatically strict, but does not handle redirectsWe should document that:
1. StrictNodes does not apply to MapAddress
2. MapAddress ~~falls back to a random exit by default~~ fails rather than falling back to a random exit
Edited to add:
3. If the site does a redirect, MapAddress does...We should document that:
1. StrictNodes does not apply to MapAddress
2. MapAddress ~~falls back to a random exit by default~~ fails rather than falling back to a random exit
Edited to add:
3. If the site does a redirect, MapAddress does not apply to the new siteTor: 0.4.0.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/30078shellcheck: src/test/fuzz/fixup_filenames.sh issues2020-06-13T15:40:25Zrl1987shellcheck: src/test/fuzz/fixup_filenames.sh issues```
In ./src/test/fuzz/fixup_filenames.sh line 11:
prev=`basename "$fn"`
^--------------^ SC2006: Use $(...) notation instead of legacy backticked `...`.
In ./src/test/fuzz/fixup_filenames.sh line 12:
post=`sha256sum ...```
In ./src/test/fuzz/fixup_filenames.sh line 11:
prev=`basename "$fn"`
^--------------^ SC2006: Use $(...) notation instead of legacy backticked `...`.
In ./src/test/fuzz/fixup_filenames.sh line 12:
post=`sha256sum "$fn" | sed -e 's/ .*//;'`
^-- SC2006: Use $(...) notation instead of legacy backticked `...`.
In ./src/test/fuzz/fixup_filenames.sh line 13:
if [ "$prev" == "$post" ] ; then
^-- SC2039: In POSIX sh, == in place of = is undefined.
For more information:
https://www.shellcheck.net/wiki/SC2039 -- In POSIX sh, == in place of = is ...
https://www.shellcheck.net/wiki/SC2006 -- Use $(...) notation instead of le...
```Tor: 0.4.1.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/29541Re-enable util/mmap_anon_no_fork2020-06-13T15:38:25ZNick MathewsonRe-enable util/mmap_anon_no_forkThe test above was disabled with #29534 in 0.4.0. We should re-enable it and make it pass everywhere. I think I know what the problem is: I think it's a matter of some flags existing in the headers but not in the kernel.The test above was disabled with #29534 in 0.4.0. We should re-enable it and make it pass everywhere. I think I know what the problem is: I think it's a matter of some flags existing in the headers but not in the kernel.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29436Add scripts to test unit test coverage determinism2020-06-13T15:38:03ZNick MathewsonAdd scripts to test unit test coverage determinismBack in #25908 I wrote some scripts to check whether our test coverage was deterministic, but I never cleaned them up or checked them in. I should do that.Back in #25908 I wrote some scripts to check whether our test coverage was deterministic, but I never cleaned them up or checked them in. I should do that.Tor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29435coverage script broken by library refactoring2020-06-13T15:38:02ZNick Mathewsoncoverage script broken by library refactoringThe fix is easy here; I'll write a patch.The fix is easy here; I'll write a patch.Tor: 0.3.5.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29310control-spec: "limits/max-mem-in-queues" appears to be undocumented2020-06-13T15:37:52Znusenucontrol-spec: "limits/max-mem-in-queues" appears to be undocumented```
GETINFO info/names
[...]
limits/max-mem-in-queues -- Actual limit on memory in queues
[...]
```
but it can not be found in:
https://gitweb.torproject.org/torspec.git/tree/control-spec.txt```
GETINFO info/names
[...]
limits/max-mem-in-queues -- Actual limit on memory in queues
[...]
```
but it can not be found in:
https://gitweb.torproject.org/torspec.git/tree/control-spec.txtTor: 0.4.1.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/29298Explicitly specify histogram bin endpoints/widths2020-06-13T15:41:38ZMike PerryExplicitly specify histogram bin endpoints/widthsGeorge has convinced me that we should explicitly specify the bin labels of our histograms rather than rely on a formula and lots of special cases for short histogram lengths.
The most straight-forward thing is to let researchers specif...George has convinced me that we should explicitly specify the bin labels of our histograms rather than rely on a formula and lots of special cases for short histogram lengths.
The most straight-forward thing is to let researchers specify a list of bin labels.. It might be more compact to encode if this list is a list of widths, but that may just be an encoding decision for the consensus. We should probably choose simplest possible representation here.Tor: 0.4.1.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/29122Intermittent test failure in circuitpadding/circuitpadding_wronghop2020-06-13T15:41:38ZNick MathewsonIntermittent test failure in circuitpadding/circuitpadding_wronghopTry running `circuitpadding_wronghop` in a loop, like this:
```
while ./src/test/test circuitpadding/circuitpadding_wronghop ; do true; done
```
After a minute or two it will fail with an error like this:
```
FAIL src/test/test_circui...Try running `circuitpadding_wronghop` in a loop, like this:
```
while ./src/test/test circuitpadding/circuitpadding_wronghop ; do true; done
```
After a minute or two it will fail with an error like this:
```
FAIL src/test/test_circuitpadding.c:1346: assert(n_client_cells OP_EQ 2): 3 vs 2
```
or sometimes like this
```
circuitpadding/circuitpadding_wronghop: [forking]
FAIL src/test/test_circuitpadding.c:1336: assert(n_client_cells OP_EQ 1): 2 vs 1
```Tor: 0.4.0.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/29070shellcheck: torify issue2020-06-13T15:36:45Zrl1987shellcheck: torify issue```
In torify line 56:
echo "$0: Failed to exec torsocks $@" >&2
^-- SC2145: Argument mixes string and array. Use * or separate argument.
``````
In torify line 56:
echo "$0: Failed to exec torsocks $@" >&2
^-- SC2145: Argument mixes string and array. Use * or separate argument.
```Tor: 0.4.1.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/29065shellcheck: test_switch_id.sh issues2020-06-13T15:36:43Zrl1987shellcheck: test_switch_id.sh issues```
In test_switch_id.sh line 3:
if test "`id -u`" != '0'; then
^-- SC2006: Use $(..) instead of legacy `..`.
In test_switch_id.sh line 8:
if test "`id -u nobody`" = ""; then
^-- SC2006: Use $(..) instead of legacy `...```
In test_switch_id.sh line 3:
if test "`id -u`" != '0'; then
^-- SC2006: Use $(..) instead of legacy `..`.
In test_switch_id.sh line 8:
if test "`id -u nobody`" = ""; then
^-- SC2006: Use $(..) instead of legacy `..`.
```Tor: 0.4.1.x-finalrl1987rl1987