The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2020-07-24T18:03:39Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16831Cover dns.c with unit tests2020-07-24T18:03:39Zrl1987Cover dns.c with unit testsTor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16829tor: src/common/log.c:484: logv: Assertion `severity >= 3 && severity <= 7' f...2020-06-27T14:00:49ZRoger Dingledinetor: src/common/log.c:484: logv: Assertion `severity >= 3 && severity <= 7' failed.On moria1:
```
#0 0x00007fbf4e9c9625 in raise () from /lib64/libc.so.6
#1 0x00007fbf4e9cae05 in abort () from /lib64/libc.so.6
#2 0x00007fbf4e9c274e in __assert_fail_base () from /lib64/libc.so.6
#3 0x00007fbf4e9c2810 in __assert_fai...On moria1:
```
#0 0x00007fbf4e9c9625 in raise () from /lib64/libc.so.6
#1 0x00007fbf4e9cae05 in abort () from /lib64/libc.so.6
#2 0x00007fbf4e9c274e in __assert_fail_base () from /lib64/libc.so.6
#3 0x00007fbf4e9c2810 in __assert_fail () from /lib64/libc.so.6
#4 0x00007fbf5019bc61 in logv (severity=1, domain=16384,
funcname=0x7fbf5022e9d0 "dirserv_get_status_impl", suffix=0x0,
format=0x7fbf5022ddd0 "Not marking '%s' valid because of address '%s'",
ap=0x7ffc23068590) at src/common/log.c:484
#5 0x00007fbf5019be5c in log_fn_ (severity=<value optimized out>,
domain=<value optimized out>, fn=<value optimized out>,
format=<value optimized out>) at src/common/log.c:688
#6 0x00007fbf501696b9 in dirserv_get_status_impl (
id_digest=<value optimized out>, nickname=0x7fbf55542780 "[nickname]",
addr=401543737, or_port=9001, platform=<value optimized out>,
msg=0x7ffc23068888, severity=1) at src/or/dirserv.c:369
#7 0x00007fbf50169850 in dirserv_router_get_status (router=0x7fbf55ac9400,
msg=0x7ffc23068888, severity=6) at src/or/dirserv.c:291
[...]
```
Here's the function prototype for dirserv_get_status_impl:
```
static uint32_t
dirserv_get_status_impl(const char *fp, const char *nickname,
uint32_t addr, uint16_t or_port,
const char *platform, const char **msg,
int should_log);
```
And now here's a place where we call it with the last argument being reasonable:
```
return dirserv_get_status_impl(d, router->nickname,
router->addr, router->or_port,
router->platform, msg, 1);
```
and here's a place where we call it with...a severity as the last argument?
```
res = dirserv_get_status_impl(rs->identity_digest, rs->nickname,
rs->addr, rs->or_port,
NULL, NULL, LOG_DEBUG);
```
Looks like the problem came in during commit 20254907d7 ?Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16826Add a mechanism to allow callgraph generator to find vtbl-like constructions2020-07-24T18:03:49ZNick MathewsonAdd a mechanism to allow callgraph generator to find vtbl-like constructionsIf we had an explicit, unified way of having our code make C++-like vtbls, we could have our callgraph-generation code take that into account, and get a more accurate callgraph. Otherwise, we may be missing important things when we try ...If we had an explicit, unified way of having our code make C++-like vtbls, we could have our callgraph-generation code take that into account, and get a more accurate callgraph. Otherwise, we may be missing important things when we try to modularize our code.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16825client with explicit EntryNodes, no cached-* files never finds its entrynodes2020-06-27T14:00:49Zstarlightclient with explicit EntryNodes, no cached-* files never finds its entrynodesClient-only 'tor' with explicit guard
nodes configured will stall forever at
Bootstrapped 80% or 85% on cold-start
with cached-* files removed.
Shutdown and fresh start of daemon
required to clear state.Client-only 'tor' with explicit guard
nodes configured will stall forever at
Bootstrapped 80% or 85% on cold-start
with cached-* files removed.
Shutdown and fresh start of daemon
required to clear state.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16823extra tor_free() for create_cell_t in command_process_create_cell()2020-06-27T14:00:49ZIsis Lovecruftextra tor_free() for create_cell_t in command_process_create_cell()In `command_process_create_cell()` (on master, as of commit da04fed865b6df09b33e6b632d51d34b3eb20d14)
```
memset(&created_cell, 0, sizeof(created_cell));
len = onion_skin_server_handshake(ONION_HANDSHAKE_TYPE_FAST,
...In `command_process_create_cell()` (on master, as of commit da04fed865b6df09b33e6b632d51d34b3eb20d14)
```
memset(&created_cell, 0, sizeof(created_cell));
len = onion_skin_server_handshake(ONION_HANDSHAKE_TYPE_FAST,
create_cell->onionskin,
create_cell->handshake_len,
NULL,
created_cell.reply,
keys, CPATH_KEY_MATERIAL_LEN,
rend_circ_nonce);
tor_free(create_cell);
if (len < 0) {
log_warn(LD_OR,"Failed to generate key material. Closing.");
circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL);
tor_free(create_cell);
return;
}
```
Which is a double-free (somewhat dependent on what the `PREDICT_LIKELY` macro generates).
I haven't tested yet, but it might be possible to crash relays with this bug. We should probably patch this ASAP.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16822make certificate lifetime accessible through Tor's ControlPort2022-06-17T18:46:01Zpropermake certificate lifetime accessible through Tor's ControlPortI am referring to the following. Sometimes user Tor logs contain something like this.
```
Sep 03 10:32:59.000 [warn] Certificate already expired. Either their clock is set wrong, or your clock is wrong.
Sep 03 10:32:59.000 [warn] (certi...I am referring to the following. Sometimes user Tor logs contain something like this.
```
Sep 03 10:32:59.000 [warn] Certificate already expired. Either their clock is set wrong, or your clock is wrong.
Sep 03 10:32:59.000 [warn] (certificate lifetime runs from Aug 16 00:00:00 2014 GMT through Jul 29 23:59:59 2015 GMT. Your time is Sep 03 10:32:59 2015 UTC.)
```
This information is interesting in context for anonymity distributions and secure network time synchronization, usability and whatnot. Used by Tails' [tordate](https://git-tails.immerda.ch/tails/tree/config/chroot_local-includes/etc/NetworkManager/dispatcher.d/20-time.sh) or Whonix's [anondate](https://www.whonix.org/wiki/Dev/TimeSync#anondate).
However, these tools rely on parsing Tor's log, which is [fragile](https://labs.riseup.net/code/issues/8977).
It would be nice, if something like
* `certificate/valid-after`
* and `certificate/valid-until`
where accessible through Tor's ControlPort.https://gitlab.torproject.org/tpo/core/tor/-/issues/16821additional /var/run/tor/log default log2020-06-27T14:00:50Zproperadditional /var/run/tor/log default logProposal:
* `/var/log/tor/log` contains Tor's log across reboots
* `/var/run/tor/log` contains Tor's log since last reboot
Both are useful. `/var/run/tor/log` contains less, more relevant information.
`/usr/share/tor/tor-service-defau...Proposal:
* `/var/log/tor/log` contains Tor's log across reboots
* `/var/run/tor/log` contains Tor's log since last reboot
Both are useful. `/var/run/tor/log` contains less, more relevant information.
`/usr/share/tor/tor-service-defaults-torrc` currently contains:
`Log notice file /var/log/tor/log`
To implement this ticket, it would only require one additional line:
`Log notice file /run/tor/log`
What do you think?Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16811Mark TestingTorNetwork PREDICT_UNLIKELY2020-07-24T18:04:31ZteorMark TestingTorNetwork PREDICT_UNLIKELYI'd like to see every instance of:
`if (options->TestingTorNetwork) {` (or similarly, `get_options()->`)
transformed to:
`if (PREDICT_UNLIKELY(options->TestingTorNetwork)) {`
This should give us a slight performance boost in relays and ...I'd like to see every instance of:
`if (options->TestingTorNetwork) {` (or similarly, `get_options()->`)
transformed to:
`if (PREDICT_UNLIKELY(options->TestingTorNetwork)) {`
This should give us a slight performance boost in relays and authorities, at the cost of a slight slowdown due to missed branch predictions in test networks.
This would be best done using a coccinelle, perl, or sed script.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16810Unit tests on circuit/relay functions2020-07-24T18:04:07ZNick MathewsonUnit tests on circuit/relay functionsWe have functions for maintaining, using, etc circuits and relay cells. These have almost no unit tests, and the fact makes me twitchy. We should remedy that.We have functions for maintaining, using, etc circuits and relay cells. These have almost no unit tests, and the fact makes me twitchy. We should remedy that.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16809High coverage on node/path selection functions2020-07-24T18:04:11ZNick MathewsonHigh coverage on node/path selection functionsThese functions are stupidly fragile, since bad behavior tends to happen only under rare circumstances. We should make sure that, through integration and unit tests, we hit all the cases. Note that coverage isn't enough if we don't chec...These functions are stupidly fragile, since bad behavior tends to happen only under rare circumstances. We should make sure that, through integration and unit tests, we hit all the cases. Note that coverage isn't enough if we don't check to make sure that the statistical distribution on paths is good, and the constraints are actually obeyed.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16808High coverage on connection_edge, addressmap2020-07-24T17:53:57ZNick MathewsonHigh coverage on connection_edge, addressmapThese functions get more than their share of bugs, and aren't very well reached by our current tests. We can use integration and unit tests for these.These functions get more than their share of bugs, and aren't very well reached by our current tests. We can use integration and unit tests for these.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16805Improve unit-test coverage on old and/or pure-ish functions/modules in src/or2020-07-24T17:54:31ZNick MathewsonImprove unit-test coverage on old and/or pure-ish functions/modules in src/orThese files have the most old code in src/or right now, and they have a lot of functions that are mostly pure-ish functions. Let's call these low-hanging fruit.
```
routerparse.c.gcov 717 1620 69.32
routerlist.c.gcov 1496 588 28.21
node...These files have the most old code in src/or right now, and they have a lot of functions that are mostly pure-ish functions. Let's call these low-hanging fruit.
```
routerparse.c.gcov 717 1620 69.32
routerlist.c.gcov 1496 588 28.21
nodelist.c.gcov 503 165 24.70
rendcache.c.gcov 332 1 0.30
networkstatus.c.gcov 681 144 17.45
policies.c.gcov 244 506 67.47
rephist.c.gcov 931 278 22.99
buffers.c.gcov 361 506 58.36
```Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16803Unit tests for sandbox failures2021-09-04T16:25:49ZNick MathewsonUnit tests for sandbox failuresWe should check that our sandboxing code blocks what it's supposed to block. This seems like something to do in a program to be invoked from a shell script.We should check that our sandboxing code blocks what it's supposed to block. This seems like something to do in a program to be invoked from a shell script.https://gitlab.torproject.org/tpo/core/tor/-/issues/16801Increase torgzip coverage as high as possible2020-06-27T14:00:51ZNick MathewsonIncrease torgzip coverage as high as possibleWe've actually had bugs in this one that caused needless bandwidth wastage. (eg legacy/trac#11824 and legacy/trac#11787.) Let's see if there are any more!
```
x/torgzip.c.gcov 88 136 60.71
```We've actually had bugs in this one that caused needless bandwidth wastage. (eg legacy/trac#11824 and legacy/trac#11787.) Let's see if there are any more!
```
x/torgzip.c.gcov 88 136 60.71
```Tor: 0.2.9.x-finalNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16800Raise tortls unit test coverage as high as possible2020-06-27T14:00:51ZNick MathewsonRaise tortls unit test coverage as high as possibleOur TLS wrappers really ought to be completely covered by unit tests of one kind or another. Right now, they're among the worst-covered functions in src/common:
```
x/tortls.c.gcov 593 355 37.45
TOTAL 593 355 37.45
```
It might also be...Our TLS wrappers really ought to be completely covered by unit tests of one kind or another. Right now, they're among the worst-covered functions in src/common:
```
x/tortls.c.gcov 593 355 37.45
TOTAL 593 355 37.45
```
It might also be a good idea to split up these functions into backend wrappers vs functions based on those wrappers for how tor uses TLSTor: 0.2.8.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16799Raise utility testing over 95%2020-07-24T17:54:40ZNick MathewsonRaise utility testing over 95%These modules perform utility functionality that we should be able to get to a very high testing coverage rate.
```
x/memarea.c.gcov 20 85 80.95
x/util.c.gcov 484 1263 72.30
x/util_format.c.gcov 11 183 94.33
x/workqueue.c.gcov 34 117 77...These modules perform utility functionality that we should be able to get to a very high testing coverage rate.
```
x/memarea.c.gcov 20 85 80.95
x/util.c.gcov 484 1263 72.30
x/util_format.c.gcov 11 183 94.33
x/workqueue.c.gcov 34 117 77.48
x/address.c.gcov 77 595 88.54
x/log.c.gcov 274 274 50.00
```
(In the case of one or two of these, they might be actually compatibility stuff. In which case, compatibility stuff should move into new compat_* modules to be considered under legacy/trac#16798)Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16798Raise compat_* testing to over 80%2020-07-24T17:54:43ZNick MathewsonRaise compat_* testing to over 80%```
compat.c.gcov 360 426 54.20
compat_libevent.c.gcov 92 32 25.81
compat_pthreads.c.gcov 22 72 76.60
compat_threads.c.gcov 48 51 51.52
```
Getting 95% coverage for stuff that relies on OS functionality might not be feasible, but it wou...```
compat.c.gcov 360 426 54.20
compat_libevent.c.gcov 92 32 25.81
compat_pthreads.c.gcov 22 72 76.60
compat_threads.c.gcov 48 51 51.52
```
Getting 95% coverage for stuff that relies on OS functionality might not be feasible, but it would be great to get this as high as possible.
(Stubbing OS calls out is probably *not* the best answer here, even though you might think it would be. We're not only interested in whether our compatibility wrappers do the right thing when the OS behaves the way we expect: we're also interested in whether they do the right thing on the actual OS. )Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16796Raise coverage on containers, di_ops to 100%2020-06-27T14:00:51ZNick MathewsonRaise coverage on containers, di_ops to 100%These modules could be at 100% unit test coverage, and they should be.These modules could be at 100% unit test coverage, and they should be.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16795Process-related utility code should have unit test coverage over 95%2020-06-27T14:00:51ZNick MathewsonProcess-related utility code should have unit test coverage over 95%```
x/procmon.c.gcov 45 0 0.00
x/util_process.c.gcov 5 33 86.84
``````
x/procmon.c.gcov 45 0 0.00
x/util_process.c.gcov 5 33 86.84
```Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/16794All cryptography unit test coverage should be over 95%; all should have test ...2020-06-27T14:00:52ZNick MathewsonAll cryptography unit test coverage should be over 95%; all should have test vectorsIt's high, but it's not there yet:
```
x/crypto.c.gcov 204 738 78.34
x/crypto_curve25519.c.gcov 8 74 90.24
x/crypto_ed25519.c.gcov 17 95 84.82
x/crypto_format.c.gcov 5 95 95.00
x/crypto_pwbox.c.gcov 2 59 96.72
x/crypto_s2k.c.gcov 14 152...It's high, but it's not there yet:
```
x/crypto.c.gcov 204 738 78.34
x/crypto_curve25519.c.gcov 8 74 90.24
x/crypto_ed25519.c.gcov 17 95 84.82
x/crypto_format.c.gcov 5 95 95.00
x/crypto_pwbox.c.gcov 2 59 96.72
x/crypto_s2k.c.gcov 14 152 91.57
x/aes.c.gcov 0 17 100.00
TOTAL 250 1230 83.11
```Tor: 0.2.9.x-finalNick MathewsonNick Mathewson