Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:55:53Zhttps://gitlab.torproject.org/legacy/trac/-/issues/18685Fire a`STATUS_SERVER` event when the hibernation state changes.2020-06-13T14:55:53ZYawning AngelFire a`STATUS_SERVER` event when the hibernation state changes.Most of the hibernation related information is already exposed in a useful manner via `GETINFO`. As far as I can tell the only really other useful thing to do here is to fire an event whenever we enter/exit hibernation so that controlle...Most of the hibernation related information is already exposed in a useful manner via `GETINFO`. As far as I can tell the only really other useful thing to do here is to fire an event whenever we enter/exit hibernation so that controllers don't have to poll continuously via `GETINFO`.Tor: 0.2.9.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/18624Dir auths should only give Guard if Stable2020-06-13T14:55:30ZRoger DingledineDir auths should only give Guard if Stable```
$ grep "^s " cached-consensus |grep Guard|wc -l
2074
$ grep "^s " cached-consensus |grep Guard|grep -v Stable|wc -l
18
$ grep "^s " cached-consensus |grep Guard|grep -v Fast|wc -l
0
```
So right now, out of the 2074 Guards that we h...```
$ grep "^s " cached-consensus |grep Guard|wc -l
2074
$ grep "^s " cached-consensus |grep Guard|grep -v Stable|wc -l
18
$ grep "^s " cached-consensus |grep Guard|grep -v Fast|wc -l
0
```
So right now, out of the 2074 Guards that we have, every one of them has the Fast flag, and all but 18 of them have the Stable flag.
At the same time, we have some complicated logic in choose_random_entry_impl() and populate_live_entry_guards() that looks at need_uptime (aka Stable) and need_capacity (aka Fast), including ugly code like
```
if (!node && need_uptime) {
need_uptime = 0; /* try without that requirement */
goto retry;
}
if (!node && need_capacity) {
/* still no? last attempt, try without requiring capacity */
need_capacity = 0;
goto retry;
}
```
But worst of all, it produces this weird edge case for the small fraction of unlucky clients who picked a non-Stable Guard, since when they build circuits that require the Stable flag, they will always use their second Guard for those circuits.
In short, this is complexity that is cheap to get rid of. Let's do it.Tor: 0.2.9.x-finalRoger DingledineRoger Dingledinehttps://gitlab.torproject.org/legacy/trac/-/issues/18618Write a tool to move functions around from module to module2020-06-13T14:55:27ZNick MathewsonWrite a tool to move functions around from module to moduleIt's annoying to move code between modules, especially when doing a _bulk_ move, since you can't easily verify the move, and you can't easily review the move decision without reviewing huge patches.
I'm working on a tool to fix that by ...It's annoying to move code between modules, especially when doing a _bulk_ move, since you can't easily verify the move, and you can't easily review the move decision without reviewing huge patches.
I'm working on a tool to fix that by doing a two step annotate-then-move process.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18617Write a tool to check for modularity violations in Tor's codebase2020-06-13T14:55:26ZNick MathewsonWrite a tool to check for modularity violations in Tor's codebaseIt's easy to find out which C files look at symbols from which other C files. If we combine that with the ability to know which C files belong to which modules, and which functions should move to _other_ C files, we'll be in good shape ...It's easy to find out which C files look at symbols from which other C files. If we combine that with the ability to know which C files belong to which modules, and which functions should move to _other_ C files, we'll be in good shape for a low-impact redividing of our source code into well-considered modules, to further simplify our modulewise callgraph.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18613Tor could use a set of BUG_ON, BUG, etc macros to help us use tor_assert less.2020-06-13T14:55:21ZNick MathewsonTor could use a set of BUG_ON, BUG, etc macros to help us use tor_assert less.Every time we use tor_assert, it's kind of a DoS bug waiting to happen. We should have macros for recoverable handling of recoverable assertion-like conditions.Every time we use tor_assert, it's kind of a DoS bug waiting to happen. We should have macros for recoverable handling of recoverable assertion-like conditions.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/18607torspec: Fix typo and explain variable in prop 2242020-06-13T14:55:18Zteortorspec: Fix typo and explain variable in prop 224Please merge my fix-224-typo branch to torspec.
It's at https://github.com/teor2345/torspec.gitPlease merge my fix-224-typo branch to torspec.
It's at https://github.com/teor2345/torspec.gitTor: 0.2.9.x-finalteorteorhttps://gitlab.torproject.org/legacy/trac/-/issues/18362Tor could use a generic 'handle' implementation.2020-06-13T14:54:38ZNick MathewsonTor could use a generic 'handle' implementation.Frequently we want to have one object have a pointer to another, but we don't want to have the first object own the second. In these cases, we need to do one of the following ugly C dances:
* We make sure that the pointed-to object n...Frequently we want to have one object have a pointer to another, but we don't want to have the first object own the second. In these cases, we need to do one of the following ugly C dances:
* We make sure that the pointed-to object never outlives the pointing object.
* We make sure that when the pointed-to object is freed, the pointer to it is set to NULL.
* Instead of using a pointer, we use some kind of unique identifier, and look up the pointed-to object in a hash table.
The first two options are error-prone, and the third is slower than regular pointer access.
Instead of these choices, we could use a 'handle' pattern to create a standard way to look up objects indirectly; we could use some of the tricks from a usual 'weak reference' implementation. Ideally, we could write the interface in such a way as to permit more than one possible implementation.
The branch `weakref` in my public repository has some janky progress towards a solution here.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/16792Have a way to mark lines as "unreachable by unit tests"2020-06-13T15:15:02ZNick MathewsonHave a way to mark lines as "unreachable by unit tests"It would be great to have a way to tell the test coverage code "this line won't be reached by tests, so don't worry about it." This would let us have a prayer of reaching 100%.It would be great to have a way to tell the test coverage code "this line won't be reached by tests, so don't worry about it." This would let us have a prayer of reaching 100%.Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/legacy/trac/-/issues/14334Bugs when registering guard status in connection_or_connect()2020-06-13T14:42:02ZGeorge KadianakisBugs when registering guard status in connection_or_connect()In `connection_or_connect()` we can see the following snippet:
```
switch (connection_connect(TO_CONN(conn), conn->base_.address,
&addr, port, &socket_error)) {
case -1:
/* If the connection faile...In `connection_or_connect()` we can see the following snippet:
```
switch (connection_connect(TO_CONN(conn), conn->base_.address,
&addr, port, &socket_error)) {
case -1:
/* If the connection failed immediately, and we're using
* a proxy, our proxy is down. Don't blame the Tor server. */
if (conn->base_.proxy_state == PROXY_INFANT)
entry_guard_register_connect_status(conn->identity_digest,
0, 1, time(NULL));
connection_or_connect_failed(conn,
errno_to_orconn_end_reason(socket_error),
tor_socket_strerror(socket_error));
connection_free(TO_CONN(conn));
return NULL;
```
I see two problems here:
a) `connection_or_connect()` can fail in ways that are unrelated to the guard node. For example it can fail with this codepath, in which case the guard should not be marked as down:
```
s = tor_open_socket_nonblocking(protocol_family,SOCK_STREAM,IPPROTO_TCP);
if (! SOCKET_OK(s)) {
*socket_error = tor_socket_errno(-1);
log_warn(LD_NET,"Error creating network socket: %s",
tor_socket_strerror(*socket_error));
return -1;
}
```
b) Also, he comment seems to state the opposite than what the code does. That is, the comment claims that if we are using a proxy and it's down, we shouldn't mark the guard as down. But the code only marks the guard as down, if the proxy state is `PROXY_INFANT` which is the state of a proxy when we haven't connected to it yet. I think the bug was introduced by me in `a79bea40d`.
I think a better solution here is to check for `proxy_type == PROXY_NONE` before marking the guard as down.Tor: 0.2.9.x-finalGeorge KadianakisGeorge Kadianakis