Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-07-31T12:42:39Zhttps://gitlab.torproject.org/legacy/trac/-/issues/20284consensus weight case 2b3 does not follow dir-spec2020-07-31T12:42:39Zpastlyconsensus weight case 2b3 does not follow dir-spec[dir-spec](https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n2681) says the following.
```
If M > T/3, then the Wmd weight above will become negative. Set it to 0
in this case:
Wmd = 0
Wgd = weight_scale - Wed
```
The ...[dir-spec](https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n2681) says the following.
```
If M > T/3, then the Wmd weight above will become negative. Set it to 0
in this case:
Wmd = 0
Wgd = weight_scale - Wed
```
The code dutifully sets `Wmd` to 0, but neglects `Wgd`.
I assume the spec is correct and the intended behavior. Branch incoming once I get a ticket number.Tor: unspecifiedpastlypastlyhttps://gitlab.torproject.org/legacy/trac/-/issues/26072Malformed connected cell closes connection but code continues2020-06-13T15:25:35ZMike PerryMalformed connected cell closes connection but code continuesIn connection_edge_process_relay_cell_not_open(), there is a clause that closes the connection if the connected cell is malformed. However, it does not return from the function. Every other clause where the connection is closed does retu...In connection_edge_process_relay_cell_not_open(), there is a clause that closes the connection if the connected cell is malformed. However, it does not return from the function. Every other clause where the connection is closed does return.
This looks like a bug. I couldn't immediately find any issues with this, though. Perhaps an assert if the connection gets marked twice..Tor: 0.3.4.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/22735prop224: HS desc overlap period func uses absolute times instead of slots2020-06-13T15:10:45ZGeorge Kadianakisprop224: HS desc overlap period func uses absolute times instead of slotsThe function that decides whether we are in HS desc overlap mode currently uses the following logic:
```
tor_gmtime_r(&consensus->valid_after, &valid_after_tm);
if (valid_after_tm.tm_hour > 0 && valid_after_tm.tm_hour < 12) {
ret...The function that decides whether we are in HS desc overlap mode currently uses the following logic:
```
tor_gmtime_r(&consensus->valid_after, &valid_after_tm);
if (valid_after_tm.tm_hour > 0 && valid_after_tm.tm_hour < 12) {
return 1;
}
```
While that logic is accurate wrt prop224 section 2.2.4, it's actually not a good idea since it uses absolute times, and it will fail to work as intended in the testnet.
We should refactor that logic to use the slot based system that we use for time periods and shared randomness, since semantically the HS desc overlap period is just the period between publishing a fresh SRV and starting the new time period.
We should also change the spec to reflect the new logic.Tor: unspecifiedGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/legacy/trac/-/issues/22534Stop parsing rend protocol versions greater than 7 in legacy hidden service d...2020-06-13T15:10:06ZteorStop parsing rend protocol versions greater than 7 in legacy hidden service descriptorsREND_PROTOCOL_VERSION_BITMASK_WIDTH is 16, but rend_encode_v2_descriptors() only checks bits 0 to 7.
This was introduced in 0a6480c in 0.2.4.3-alpha, which fixed #6827, which was itself a bugfix on c58675 in 0.2.0.10-alpha.
I think we ...REND_PROTOCOL_VERSION_BITMASK_WIDTH is 16, but rend_encode_v2_descriptors() only checks bits 0 to 7.
This was introduced in 0a6480c in 0.2.4.3-alpha, which fixed #6827, which was itself a bugfix on c58675 in 0.2.0.10-alpha.
I think we should wontfix this.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/20218Fix and refactor and redocument routerstatus_has_changed2020-06-13T15:01:41ZNick MathewsonFix and refactor and redocument routerstatus_has_changedThe routerstatus_has_changed() function is used by controllers to to tell which rs entries are new. But it only looks at a fraction of the fields which might change in a routerstatus. Also, it only checks for semantic changes that tor ...The routerstatus_has_changed() function is used by controllers to to tell which rs entries are new. But it only looks at a fraction of the fields which might change in a routerstatus. Also, it only checks for semantic changes that tor cares about (though this is not documented).Tor: 0.4.3.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/4369I can send (almost) any cell I want before the VERSIONS or NETINFO cell2020-06-13T14:14:19ZRoger DingledineI can send (almost) any cell I want before the VERSIONS or NETINFO cellWhile debugging #4368 I noticed that I can send a CREATE cell right out of the gate after the v2 handshake finishes, when the other side is expecting a VERSIONS cell or (later) a NETINFO cell. My cell will be quietly dropped with (by def...While debugging #4368 I noticed that I can send a CREATE cell right out of the gate after the v2 handshake finishes, when the other side is expecting a VERSIONS cell or (later) a NETINFO cell. My cell will be quietly dropped with (by default) a log_info message.
Similarly, I can send CREATE cells interspersed in the VERSIONS / CERTS / NETINFO cells in the v3 handshake, with no complaints louder than info.
But the spec says things like
```
No other intervening cell types are allowed.
```
and
```
When this handshake is in use, the first cell must
still be VERSIONS, and no other cell type is allowed to intervene
besides those specified, except for PADDING and VPADDING cells.
```
If this is a feature, meaning we're trying to be forgiving about arbitrary future behavior, we should make it clearer in the spec.
If it's a bug, we should think about how thoroughly to fix it.
I think at least some part of this is a bug, for example because we don't call or_handshake_state_record_cell() on the CREATE cells in the v3 handshake case since we drop them first.Tor: 0.2.3.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/4361Shouldn't the v3 client process the certs cell before sending her netinfo cell?2020-06-13T14:14:16ZRoger DingledineShouldn't the v3 client process the certs cell before sending her netinfo cell?The tor-spec used to say:
```
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
authenticate, ...The tor-spec used to say:
```
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
authenticate, or a CERTS cell, an AUTHENTICATE cell (4.4), and a NETINFO
cell if it does.
```
I changed it to:
```
The initiator can use the CERTS cell to confirm whether
the responder is correctly authenticated. If the initiator does not wish
to authenticate, it can send a NETINFO cell once it has received the
VERSIONS cell from the responder. If the initiator does wish to
authenticate, it waits until it gets the AUTH_CHALLENGE cell, and then
sends a CERTS cell, an AUTHENTICATE cell (4.4), and a NETINFO
cell.
```
since that's what the code does.
But troll_un points out that we should probably change the code so the client checks the CERTS cell and either hangs up then, or sends her NETINFO comfortable in the knowledge that she knows who she's sending the NETINFO cell to.
If we do change the code, we'd then want to revert (and probably still clean up a bit more) the spec change.Tor: 0.2.3.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/2767Another possible directory-handling bug2020-06-13T14:09:33ZRobert RansomAnother possible directory-handling bug> 2011-03-16 09:58 <tor-internal> parse_http_url() allows alone GET or POST, "If it's well-formed, strdup the second \%s" says that it's incorrect. second "eat_whitespace_no_nl" leaves nl, should be "eat_whitespace".> 2011-03-16 09:58 <tor-internal> parse_http_url() allows alone GET or POST, "If it's well-formed, strdup the second \%s" says that it's incorrect. second "eat_whitespace_no_nl" leaves nl, should be "eat_whitespace".Tor: 0.2.5.x-final