The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2021-09-16T14:31:39Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23459prop224: Specialize interface of hs_circuitmap_get_rend_circ_client_side()2021-09-16T14:31:39ZGeorge Kadianakisprop224: Specialize interface of hs_circuitmap_get_rend_circ_client_side()We currently use `hs_circuitmap_get_rend_circ_client_side()` for two reasons:
a) To proceed with the rend protocol as a client when we receive an intro ack (in `handle_introduce_ack_success()`).
b) To close useless rend circuits in `clos...We currently use `hs_circuitmap_get_rend_circ_client_side()` for two reasons:
a) To proceed with the rend protocol as a client when we receive an intro ack (in `handle_introduce_ack_success()`).
b) To close useless rend circuits in `close_or_reextend_intro_circ()`.
To fit these two scenarios, the function `hs_circuitmap_get_rend_circ_client_side()` currently returns all sorts of rend circs (established and unestablished).
We can improve the logic and semantics here by splitting into two funcs. One that returns only established circs (used for (a)), and another that retuns all kinds of circs (used for (b)).Tor: 0.3.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23305hs: Maybe don't use REND_DESC_ID_V2_LEN_BASE32 as the length for a base32 rel...2021-09-16T14:32:00ZDavid Gouletdgoulet@torproject.orghs: Maybe don't use REND_DESC_ID_V2_LEN_BASE32 as the length for a base32 relay digest idFunction `hs_lookup_last_hid_serv_request()` defines an HSDir identity digest in base32 as:
```
char hsdir_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1];
```
Although the length is correct, semantically this is bad to use the base32 desc...Function `hs_lookup_last_hid_serv_request()` defines an HSDir identity digest in base32 as:
```
char hsdir_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1];
```
Although the length is correct, semantically this is bad to use the base32 descriptor ID length for this.Tor: 0.3.2.x-finalDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/23288refactor temporary file cleanup and make it more consistent2021-09-16T14:32:00ZTaylor Yurefactor temporary file cleanup and make it more consistentlegacy/trac#23271 is an example of a temporary file that doesn't properly get cleaned up. We should consider refactoring the creation and destruction of temporary files so they get cleaned up consistently.legacy/trac#23271 is an example of a temporary file that doesn't properly get cleaned up. We should consider refactoring the creation and destruction of temporary files so they get cleaned up consistently.https://gitlab.torproject.org/tpo/core/tor/-/issues/23271control_auth_cookie isn't deleted when tor stops2020-06-27T13:55:52Zyurivict271control_auth_cookie isn't deleted when tor stopsWhen tor is stopped gracefully ('service stop tor' on FreeBSD), it leaves the file /var/db/tor/control_auth_cookie.
Due to the sensitive nature of this file, it should be only present when tor runs and deleted once it stops gracefully.When tor is stopped gracefully ('service stop tor' on FreeBSD), it leaves the file /var/db/tor/control_auth_cookie.
Due to the sensitive nature of this file, it should be only present when tor runs and deleted once it stops gracefully.Tor: 0.3.3.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23237Add 'GETINFO ip-to-country/available'2020-06-27T13:55:54ZDamian JohnsonAdd 'GETINFO ip-to-country/available'Hi Nick. Very minor ask but if we had a 'GETINFO ip-to-country/available' option to determine if tor has a geoip database available that would simplify Stem a bit. Stem tracks 'is the geoip database available' so it can avoid 'GETINFO ip...Hi Nick. Very minor ask but if we had a 'GETINFO ip-to-country/available' option to determine if tor has a geoip database available that would simplify Stem a bit. Stem tracks 'is the geoip database available' so it can avoid 'GETINFO ip-to-country/*' requests that are doomed to fail anyway.
Thanks!Tor: 0.3.2.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23233Unexpected BUG violation in hsv3 decriptor decoding2020-06-27T13:55:54ZhaxxpopUnexpected BUG violation in hsv3 decriptor decoding
As you can see in hs_descriptor.c
```
/* Find the start of signature. */
sig_start = tor_memstr(encoded_desc, encoded_len, "\n" str_signature);
/* Getting here means the token parsing worked for the signature so if we
* can't ...
As you can see in hs_descriptor.c
```
/* Find the start of signature. */
sig_start = tor_memstr(encoded_desc, encoded_len, "\n" str_signature);
/* Getting here means the token parsing worked for the signature so if we
* can't find the start of the signature, we have a code flow issue. */
if (BUG(!sig_start)) {
goto err;
}
```
str_signature is "signature", so, if you send the "\n signature" (like in the attachment) instead of "\nsignature" tor_memstr will return null and violate `BUG(!sig_start)`
I suggest that we should just remove BUG and let it be `if (!sig_start) {`Tor: 0.3.1.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23168Guard sample calls relay descriptors a "consensus"2022-02-07T19:39:17ZteorGuard sample calls relay descriptors a "consensus"[info] router_load_routers_from_string: 96 elements to add
[info] sampled_guards_update_from_consensus: Updating sampled guard status based on received consensus.
The message should either say "received directory document(s)", or actual...[info] router_load_routers_from_string: 96 elements to add
[info] sampled_guards_update_from_consensus: Updating sampled guard status based on received consensus.
The message should either say "received directory document(s)", or actually describe the directory document it just received.https://gitlab.torproject.org/tpo/core/tor/-/issues/23092Stop ignoring CircuitIdleTimeout when it's lower than IDLE_TIMEOUT_WHILE_LEAR...2020-06-27T13:55:59ZteorStop ignoring CircuitIdleTimeout when it's lower than IDLE_TIMEOUT_WHILE_LEARNINGWhen I set CircuitIdleTimeout in chutney, it doesn't always work, because IDLE_TIMEOUT_WHILE_LEARNING is used instead. We should document this.
Or maybe we should use min(CircuitIdleTimeout, IDLE_TIMEOUT_WHILE_LEARNING) when learning?When I set CircuitIdleTimeout in chutney, it doesn't always work, because IDLE_TIMEOUT_WHILE_LEARNING is used instead. We should document this.
Or maybe we should use min(CircuitIdleTimeout, IDLE_TIMEOUT_WHILE_LEARNING) when learning?Tor: 0.3.2.x-finalteorteorhttps://gitlab.torproject.org/tpo/core/tor/-/issues/23066Test granularity, distribution, and inner range of crypto_rand* functions2021-09-16T14:32:00ZteorTest granularity, distribution, and inner range of crypto_rand* functionsIn legacy/trac#23061, we added tests for crypto_rand_double() that test:
* granularity: low bits being non-zero,
* distribution: some values above and below half, and
* inner range: mock crypto_rand() and generate lowest and highest vali...In legacy/trac#23061, we added tests for crypto_rand_double() that test:
* granularity: low bits being non-zero,
* distribution: some values above and below half, and
* inner range: mock crypto_rand() and generate lowest and highest valid results.
We should do the same for:
* crypto_rand()
* crypto_rand_int()
* crypto_rand_int_range()
* crypto_rand_uint64()
* crypto_rand_uint64_range()
* crypto_rand_time_range()
* crypto_rand_hostname()https://gitlab.torproject.org/tpo/core/tor/-/issues/22951NETINFO cells are mandatory, but tor-spec says "may"2021-07-22T16:22:40ZteorNETINFO cells are mandatory, but tor-spec says "may"In this context, "may" is ambiguous: NETINFO is actually a mandatory requirement:
```
cell (4.5). As soon as it gets the CERTS cell, the initiator knows
whether the responder is correctly authenticated. At this point the
- initi...In this context, "may" is ambiguous: NETINFO is actually a mandatory requirement:
```
cell (4.5). As soon as it gets the CERTS cell, the initiator knows
whether the responder is correctly authenticated. At this point the
- initiator may send a NETINFO cell if it does not wish to
+ initiator MUST send a NETINFO cell if it does not wish to
authenticate, or a CERTS cell, an AUTHENTICATE cell (4.4), and a NETINFO
cell if it does. When this handshake is in use, the first cell must
be VERSIONS, VPADDING or AUTHORIZE, and no other cell type is allowed to
intervene besides those specified, except for PADDING and VPADDING cells.
```
https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n482Tor: 0.3.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22948Padding, Keepalive and Drop cells should have random payloads2022-03-17T19:41:22ZteorPadding, Keepalive and Drop cells should have random payloadstor-spec says:
```
Link padding can be created by sending PADDING or VPADDING cells
along the connection; relay cells of type "DROP" can be used for
long-range padding. The contents of a PADDING, VPADDING, or DROP
cell SHOUL...tor-spec says:
```
Link padding can be created by sending PADDING or VPADDING cells
along the connection; relay cells of type "DROP" can be used for
long-range padding. The contents of a PADDING, VPADDING, or DROP
cell SHOULD be chosen randomly, and MUST be ignored.
```
https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1534
But padding cells sent by channelpadding_send_padding_cell_for_callback() and keepalive cells sent by run_connection_housekeeping() have a payload of all zero bytes.
I don't know if this is a security issue or not. It is probably ok, unless Tor has compression enabled on its TLS connections. If compression is enabled, all the padding data size calculations will be wrong.https://gitlab.torproject.org/tpo/core/tor/-/issues/22937Clarify how resolved values are encoded in cells2021-07-22T16:22:40ZteorClarify how resolved values are encoded in cellsT1. Is length 0 for error types?
T2. IPv4 and IPv6 addresses are in network byte order
T3. Hostnames are in standard DNS order (we might not need to say this)
T4. TTLs are left out in NETINFO cells
```
To find the address associated ...T1. Is length 0 for error types?
T2. IPv4 and IPv6 addresses are in network byte order
T3. Hostnames are in standard DNS order (we might not need to say this)
T4. TTLs are left out in NETINFO cells
```
To find the address associated with a hostname, the OP sends a
RELAY_RESOLVE cell containing the hostname to be resolved with a NUL
terminating byte. (For a reverse lookup, the OP sends a RELAY_RESOLVE
cell containing an in-addr.arpa address.) The OR replies with a
RELAY_RESOLVED cell containing any number of answers. Each answer is
of the form:
Type (1 octet)
Length (1 octet)
Value (variable-width)
TTL (4 octets)
"Length" is the length of the Value field.
"Type" is one of:
0x00 -- Hostname
0x04 -- IPv4 address
0x06 -- IPv6 address
0xF0 -- Error, transient
0xF1 -- Error, nontransient
If any answer has a type of 'Error', then no other answer may be given.
For backward compatibility, if there are any IPv4 answers, one of those
must be given as the first answer.
The RELAY_RESOLVE cell must use a nonzero, distinct streamID; the
corresponding RELAY_RESOLVED cell must use the same streamID. No stream
is actually created by the OR when resolving the name.
```
https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1480Tor: 0.3.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22882The v4 link protocol requires the initiator to set the most significant bit2021-07-22T16:22:39ZteorThe v4 link protocol requires the initiator to set the most significant bittor-spec.txt says:
```
To prevent CircID collisions, when one node sends a CREATE cell to
another, it chooses from only one half of the possible values based
on the ORs' public identity keys. In link protocol version 3 or
lo...tor-spec.txt says:
```
To prevent CircID collisions, when one node sends a CREATE cell to
another, it chooses from only one half of the possible values based
on the ORs' public identity keys. In link protocol version 3 or
lower, if the sending node has a lower key, it chooses a CircID with
an MSB of 0; otherwise, it chooses a CircID with an MSB of 1. (Public
keys are compared numerically by modulus.)
In link protocol version 4 or higher, whichever node initiated the
connection sets its MSB to 1, and whichever node didn't initiate the
connection sets its MSB to 0.
(An OP with no public key MAY choose any CircID it wishes, since an OP
never needs to process a CREATE cell.)
```
But this last sentence is only true for v3. For v4, if the most significant bit is not set by a client, relays close the connection with a message like:
```
Jul 11 20:20:18.000 [info] command_process_create_cell: Received create cell with unexpected circ_id 2147483647. Closing.
```Tor: 0.3.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22820Give the Exit flag to Exits that use the secure IRC port 66972020-06-27T13:56:13ZteorGive the Exit flag to Exits that use the secure IRC port 6697Igor Mitrofano suggests that the Exit flag should be given to relays that allow exiting to the default secure IRC port 6697.
From tor-relays:
https://lists.torproject.org/pipermail/tor-relays/2017-July/012585.html
The existing policy i...Igor Mitrofano suggests that the Exit flag should be given to relays that allow exiting to the default secure IRC port 6697.
From tor-relays:
https://lists.torproject.org/pipermail/tor-relays/2017-July/012585.html
The existing policy is "two ports from [80, 443, 6667]", or these options:
* 80, 443
* 80, 6667
* 443, 6667
We would change to "two ports from [80, 443, 6667-OR-6697]", adding these options:
* 80, 6697
* 443, 6697
We don't need a new consensus method for this, because the way Exit flags are combined stays as majority vote.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22812find_dl_min_and_max_delay's DL_SCHED_DETERMINISTIC case is never used2021-09-16T14:31:59Zteorfind_dl_min_and_max_delay's DL_SCHED_DETERMINISTIC case is never usedfind_dl_min_and_max_delay is never called with DL_SCHED_DETERMINISTIC schedules. So we should just remove that case entirely. And replace it with a BUG() return.find_dl_min_and_max_delay is never called with DL_SCHED_DETERMINISTIC schedules. So we should just remove that case entirely. And replace it with a BUG() return.Tor: 0.3.2.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22723Avoid double-quoting esc_for_log output2022-07-21T19:19:42ZteorAvoid double-quoting esc_for_log outputIn legacy/trac#22368, arma posted a log where we double-quote relay nicknames.
```
[warn] There is a router named """" in my declared family, but that isn't a legal nickname. Skipping it.
```
Let's stop double-quoting esc_for_log output ...In legacy/trac#22368, arma posted a log where we double-quote relay nicknames.
```
[warn] There is a router named """" in my declared family, but that isn't a legal nickname. Skipping it.
```
Let's stop double-quoting esc_for_log output across the codebase.Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22720Don't exit(0) on error: exit(1) instead2020-06-27T13:56:20ZNick MathewsonDon't exit(0) on error: exit(1) insteadper recent tor-talk thread, there are several places in our code where we exit(0) on an error, and we should exit(1) instead.
When fixing this, it's a good idea to check every exit(0) to see whether it's an error or an expected exit con...per recent tor-talk thread, there are several places in our code where we exit(0) on an error, and we should exit(1) instead.
When fixing this, it's a good idea to check every exit(0) to see whether it's an error or an expected exit condition.Tor: 0.3.1.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22700Stop relays using the Client*IPv6* options2020-07-28T03:40:49ZteorStop relays using the Client*IPv6* optionsSome operators add all the IPv6 options to their torrcs.
We should warn when they add client options on relays.Some operators add all the IPv6 options to their torrcs.
We should warn when they add client options on relays.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22668Add ed25519 public key to format_node_description and callers2020-10-27T13:44:56ZteorAdd ed25519 public key to format_node_description and callersNow that we are pinning relay ed25519 public keys, it would be great to log them along with RSA key fingerprints.Now that we are pinning relay ed25519 public keys, it would be great to log them along with RSA key fingerprints.Nick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/22638Typo in compress_none.c header comment2020-06-27T13:56:25ZteorTypo in compress_none.c header commentThe file name is wrong:
```
* \file compress_lzma.c
* \brief Compression backend for identity compression.
```The file name is wrong:
```
* \file compress_lzma.c
* \brief Compression backend for identity compression.
```Tor: 0.3.1.x-finalTaylor YuTaylor Yu