Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T15:07:46Zhttps://gitlab.torproject.org/legacy/trac/-/issues/21919hs: Change trunnel prop224 cell's namespace2020-06-13T15:07:46ZDavid Gouletdgoulet@torproject.orghs: Change trunnel prop224 cell's namespaceCurrently, the trunnel namespace for hidden service cells (in `src/trunnel/hs/`) is prefixed with `hs_cell_*`. We want to change this for two reasons.
First, if we could have something in the name indicating that it is trunnel, it would...Currently, the trunnel namespace for hidden service cells (in `src/trunnel/hs/`) is prefixed with `hs_cell_*`. We want to change this for two reasons.
First, if we could have something in the name indicating that it is trunnel, it would make it better for code semantic and separation.
Second, we want to create `hs_cells.[ch]` so we can put in there the cell creation/parsing/handling instead of growing `hs_circuit.c` to the "hydra size".
So for the renaming, here are some suggestions:
1. `tr_cell_*`
2. `tr_hs_cell_*`
3. `trunnel_cell_*`
4. `trnl_cell_*`
Considering that an `ESTABLISH_INTRO` or `INTRODUCE1` cell is only for hidden service, probably the `hs` in there is superfluous?Tor: 0.3.1.x-finalDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/legacy/trac/-/issues/21910Refactor connection_edge_process_relay_cell()2020-06-13T15:07:45ZDavid Gouletdgoulet@torproject.orgRefactor connection_edge_process_relay_cell()Ticket #16706 is one of the possible many issues we had and will have with this function.
It is quite big with many many return callsite and it is confusing on how it behaves. For instance, if `-reason` is returned, the caller should te...Ticket #16706 is one of the possible many issues we had and will have with this function.
It is quite big with many many return callsite and it is confusing on how it behaves. For instance, if `-reason` is returned, the caller should teardown the circuit and log warn but yet this functions already does many `LOG_PROTOCOL_WARN` in that case.
One thing we could do is maybe return a different error code (or set an error code) depending on what's happening (should close circ, cell dropped, error). For instance, currently, returning 0 can either mean that a cell was dropped or successfully relayed.
Auditing every callsite of this function would be important to understand how it is actually used so we can properly improve it and make it less error prone with dubious logging (or improved logging).Tor: unspecifiedNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/legacy/trac/-/issues/21585Check code that uses consensus membership to find clients2020-06-13T15:06:47ZteorCheck code that uses consensus membership to find clientsIn #21406, we identify clients by checking peer authentication information, as CREATE_FAST has been phased out.
Most code that needs to know if the other end of a connection is a client uses membership in the consensus, but this is unre...In #21406, we identify clients by checking peer authentication information, as CREATE_FAST has been phased out.
Most code that needs to know if the other end of a connection is a client uses membership in the consensus, but this is unreliable, as relays drop out of the consensus.
So we should check code that uses node_t to identify clients, and switch it over to using this flag instead.
This might mean creating an accessor function that takes an or_connection_t and returns whether the channel is a client.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/21423Refactor choose_good_entry_server based on different usecases2020-06-13T15:06:11ZNick MathewsonRefactor choose_good_entry_server based on different usecasesThe choose_good_entry_server function was used in four ways: picking out guards for the old (pre-prop271) guard algorithm; picking out guards for circuits; picking out guards for testing circuits on non-bridgees; picking out entries when...The choose_good_entry_server function was used in four ways: picking out guards for the old (pre-prop271) guard algorithm; picking out guards for circuits; picking out guards for testing circuits on non-bridgees; picking out entries when entry guards are disabled. These options should be disentangled.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/21349Split up very long functions in entrynodes.c2020-06-13T15:05:49ZNick MathewsonSplit up very long functions in entrynodes.cSome functions in entrynodes.c are now pretty huge, especially:
* sampled_guards_update_from_consensus()
* entry_guards_update_primary()
* select_entry_guard_for_circuit()
It would be good for testing and maintenance to split them...Some functions in entrynodes.c are now pretty huge, especially:
* sampled_guards_update_from_consensus()
* entry_guards_update_primary()
* select_entry_guard_for_circuit()
It would be good for testing and maintenance to split them up.Tor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/21039Refactor and simplify guard code of circuit_send_next_onion_skin()2020-06-13T15:04:47ZGeorge KadianakisRefactor and simplify guard code of circuit_send_next_onion_skin()As part of prop271 (#19877), we added about 70 fresh lines of code to `circuit_send_next_onion_skin()` which is an already convoluted function.
Ideally we should abstract this new circuit-related guard code and put it in its own functio...As part of prop271 (#19877), we added about 70 fresh lines of code to `circuit_send_next_onion_skin()` which is an already convoluted function.
Ideally we should abstract this new circuit-related guard code and put it in its own functions, to reduce complexity of `circuit_send_next_onion_skin()` and maybe even make it unittestable.Tor: unspecifiedGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/legacy/trac/-/issues/20918Switch onion.c to use TRUNNEL_OPAQUE2020-06-13T15:04:10ZNick MathewsonSwitch onion.c to use TRUNNEL_OPAQUEThe TRUNNEL_OPAQUE macro stops trunnel from exposing object internals in its headers; we should use that in onion.c. (And possibly elsewhere.) Noted by dgoulet during code review.The TRUNNEL_OPAQUE macro stops trunnel from exposing object internals in its headers; we should use that in onion.c. (And possibly elsewhere.) Noted by dgoulet during code review.Tor: unspecifiedNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/20895Split node_supports_ed25519_link_authentication into two or three separate fu...2020-06-13T15:04:05ZNick MathewsonSplit node_supports_ed25519_link_authentication into two or three separate functionsAs of our #15056 code to implement the circuit-side part of prop220, we have a function, `node_supports_ed25519_link_authentication`, which isn't quite right.
Sometimes, when we use it, we mean, "If we try to connect to this node, shoul...As of our #15056 code to implement the circuit-side part of prop220, we have a function, `node_supports_ed25519_link_authentication`, which isn't quite right.
Sometimes, when we use it, we mean, "If we try to connect to this node, should we expect that we will authenticate its ed25519 identity?"
Sometimes, we mean "If we try to make a connection through some random node to this node, authenticating with its ed25519 identity, will that work?"
And sometimes we mean "I'm thinking of asking _that_ node to extend a circuit to _this_ node. Should I tell it about _this_ node's Ed25519 identity, or would it take it the wrong way?"
I wrote a patch here in response to dgoulet's review of my #15056 branch, but on reflection, it isn't right. I'll attach it, but it's a bad start, and it's too complex, and maybe you should ignore it?Tor: 0.3.3.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/20887DIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d instead2020-06-13T15:04:02ZteorDIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d insteadA FreeBSD relay operator reports the message:
```
[WARN] Being a directory cache (default) with less than DIRCACHE_MIN_MB_BANDWIDTH MB of memory is not recommended and may consume most of the available resources, consider disabling this ...A FreeBSD relay operator reports the message:
```
[WARN] Being a directory cache (default) with less than DIRCACHE_MIN_MB_BANDWIDTH MB of memory is not recommended and may consume most of the available resources, consider disabling this functionality by setting the DirCache option to 0
```
So we should print the macro value the same way we do every other value, using "%d" in the string, and passing it as an argument.
(This macro changed name in #20684.)Tor: 0.3.4.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/20853rend_config_services should use service_is_ephemeral rather than old/new->dir...2020-06-13T15:03:52Zteorrend_config_services should use service_is_ephemeral rather than old/new->directoryWe missed this in the earlier refactor.
(Does that make it a bug? I think so.)We missed this in the earlier refactor.
(Does that make it a bug? I think so.)Tor: 0.3.0.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/20835Refactor choose_good_entry_server so it is (almost) never used2020-06-13T15:03:49ZNick MathewsonRefactor choose_good_entry_server so it is (almost) never usedFrom my prop271 branch:
```
* XXXX prop271 this function is used in four ways: picking out guards for
* the old (pre-prop271) guard algorithm; picking out guards for circuits;
* picking out guards for testing circuits on non-bridg...From my prop271 branch:
```
* XXXX prop271 this function is used in four ways: picking out guards for
* the old (pre-prop271) guard algorithm; picking out guards for circuits;
* picking out guards for testing circuits on non-bridgees;
* picking out entries when entry guards are disabled. These options
* should be disentangled.
```Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/20077Make is_sensitive_dir_purpose and purpose_needs_anonymity consistent2020-06-13T15:02:10ZteorMake is_sensitive_dir_purpose and purpose_needs_anonymity consistentI'm not sure why these both exist, but it's very confusing.I'm not sure why these both exist, but it's very confusing.Tor: 0.3.0.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/19981Make sure we build with OpenSSL 1.1.0 with all deprecated APIs removed2020-06-13T15:00:36ZNick MathewsonMake sure we build with OpenSSL 1.1.0 with all deprecated APIs removedThere's an option to build openssl 1.1.0 with all the deprecated APIs turned off. We should make sure that Tor still works without using any of those.There's an option to build openssl 1.1.0 with all the deprecated APIs turned off. We should make sure that Tor still works without using any of those.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19979Use OpenSSL 1.1.0 HKDF in Tor when available.2020-06-13T15:00:35ZNick MathewsonUse OpenSSL 1.1.0 HKDF in Tor when available.OpenSSL 1.1.0 now includes HKDF support. We should consider using their implementation instead of ours when it's available.OpenSSL 1.1.0 now includes HKDF support. We should consider using their implementation instead of ours when it's available.Tor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/legacy/trac/-/issues/19429Clean up our OpenSSL 1.1 support.2020-06-13T14:58:43ZYawning AngelClean up our OpenSSL 1.1 support.In an ideal world, we should support OpenSSL 1.1 built with the deprecated APIs entirely disabled. Additionally while the code that uses the new opaque wrappers for the RSA stuff is functional and maintainable, it would be cleaner if we...In an ideal world, we should support OpenSSL 1.1 built with the deprecated APIs entirely disabled. Additionally while the code that uses the new opaque wrappers for the RSA stuff is functional and maintainable, it would be cleaner if we provided our own wrappers for a more unified codepath.
Neither of these are high priority as the current code works, the changes suggested would just make things better.Tor: 0.3.4.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/19377Consider retry/backoff behavior when building new circuits2020-06-13T14:58:36ZAndrea ShepardConsider retry/backoff behavior when building new circuitsRetrying connections is the wrong level of abstraction at which to think about circuit failure behavior IMO (see comment on #15942); we should consider whether, as a client or an HS, we're ever doing anything like repeatedly retrying to ...Retrying connections is the wrong level of abstraction at which to think about circuit failure behavior IMO (see comment on #15942); we should consider whether, as a client or an HS, we're ever doing anything like repeatedly retrying to build a circuit without smart backoff behavior for the sake of DoS resistance though.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19328Try not to log from inside functions called from inside log functions2020-06-13T14:58:32ZNick MathewsonTry not to log from inside functions called from inside log functionsOur logging code is technically recursive now. Logging asks what the time is and tries to format it. Formatting the time can fail. Failures can log.Our logging code is technically recursive now. Logging asks what the time is and tries to format it. Formatting the time can fail. Failures can log.Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/19310Make code-movement functionality of module tool easier to preview2020-06-13T14:58:25ZNick MathewsonMake code-movement functionality of module tool easier to previewTor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19309Make target to count and list module callgraph violations2020-06-13T14:58:25ZNick MathewsonMake target to count and list module callgraph violationsThis target should make it easy to find regressions. This will require upstream changes to the modules tool.This target should make it easy to find regressions. This will require upstream changes to the modules tool.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19308Group C files into module-groups for module callgraph purposes2020-06-13T14:58:24ZNick MathewsonGroup C files into module-groups for module callgraph purposesTor: unspecified