Tor issueshttps://gitlab.torproject.org/tpo/core/tor/-/issues2022-10-11T23:39:35Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40282relay: Stop DirPort self reachability test and stop publishing it2022-10-11T23:39:35ZDavid Gouletdgoulet@torproject.orgrelay: Stop DirPort self reachability test and stop publishing itWith tor#2667, we've now denied network re-entry for security reasons and thus relay self reachability test on the `DirPort` won't work anymore.
We should thus remove it at once and not stuck relays on this. Directory requests are happe...With tor#2667, we've now denied network re-entry for security reasons and thus relay self reachability test on the `DirPort` won't work anymore.
We should thus remove it at once and not stuck relays on this. Directory requests are happening on the ORPort since many years ago thus relay `DirPort` is less and less relevant. It is still important on directory authorities at the moment though but reachability test still won't work.
Talked to nickm about this and the highlights:
1. Client should ONLY use ORPort for directory content. We'll make an assessment on all supported versions of Tor to make sure.
2. `DirCache` should be fine without a `DirPort` according to the manpage but we should again assess that it is true on all supported versions.
```
DirCache 0|1
When this option is set, Tor caches all current directory documents
except extra info documents, and accepts client requests for them.
If DownloadExtraInfo is set, cached extra info documents are also
cached. Setting DirPort is not required for DirCache, because
clients connect via the ORPort by default. Setting either DirPort
or BridgeRelay and setting DirCache to 0 is not supported.
(Default: 1)
```
3. Make sure that directory and bridge authority still properly works without a `DirPort` selftest.
All this resulting in the removal of the `DirPort` selftest and entirely stop publishing it in the descriptor at once except for authorities.Tor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40261dos: Move subsystem to the subsys manager2022-10-11T23:39:35ZDavid Gouletdgoulet@torproject.orgdos: Move subsystem to the subsys managerIn order to be able to export `MetricsPort` data from the DoS subsystem (in part with tor#40259), we need the DoS subsystem to be added to the subsys manager so the `get_metrics()` method can be used.
Furthermore, to do it right, we nee...In order to be able to export `MetricsPort` data from the DoS subsystem (in part with tor#40259), we need the DoS subsystem to be added to the subsys manager so the `get_metrics()` method can be used.
Furthermore, to do it right, we need to move all configuration options within the module as well so we have better modularization.
This ticket is for this work.Tor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40253Extend the DoS subsystem to block addrs that connect too often too2022-10-11T23:39:35ZRoger DingledineExtend the DoS subsystem to block addrs that connect too often tooRight now we have a connection-based DoS component which triggers on *concurrent* ORPort connections. That is, *if* you have 3 concurrent connections *and* you [write me more]Right now we have a connection-based DoS component which triggers on *concurrent* ORPort connections. That is, *if* you have 3 concurrent connections *and* you [write me more]Tor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40189"tor-gencert --create-identity-key" fails with no clear error message if pass...2022-07-07T00:48:31ZRoger Dingledine"tor-gencert --create-identity-key" fails with no clear error message if passphrase is empty or shortRun tor-gencert to make a new key, but leave the PEM pass phrase empty (just hit enter):
```
$ ./tor-gencert --create-identity-key
Enter PEM pass phrase:
Nov 15 15:32:59.730 [err] Couldn't write identity key to ./authority_identity_key
N...Run tor-gencert to make a new key, but leave the PEM pass phrase empty (just hit enter):
```
$ ./tor-gencert --create-identity-key
Enter PEM pass phrase:
Nov 15 15:32:59.730 [err] Couldn't write identity key to ./authority_identity_key
Nov 15 15:32:59.730 [err] crypto error while Writing identity key: result too small (in UI routines:UI_set_result_ex)
Nov 15 15:32:59.730 [err] crypto error while Writing identity key: processing error (in UI routines:UI_process)
Nov 15 15:32:59.730 [err] crypto error while Writing identity key: problems getting password (in PEM routines:PEM_def_callback)
Nov 15 15:32:59.730 [err] crypto error while Writing identity key: read key (in PEM routines:do_pk8pkey)
```
It seems to me that having an empty pass phrase should work; but if we want it to not work, we should say that as an error message.
(Found by tech-exorcist on #tor)
In fact, I just tried it with short passphrases, and they also fail with these cryptic error messages. So it sounds like maybe we have a secret minimum passphrase length or something?Tor: 0.4.6.x-freezeNeel Chauhanneel@neelc.orgNeel Chauhanneel@neelc.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40285Bug: 2-hop circuit with purpose 5 has no guard state2022-07-07T00:47:09ZRoger DingledineBug: 2-hop circuit with purpose 5 has no guard stateWhen I issue an "extendcircuit 0 A,B" command with my controller, and the circuit gets built, my Tor logs a line like:
```
Feb 09 20:36:44.400 [warn] circuit_build_no_more_hops(): Bug: 2-hop circuit 0x5558a75e2190 with purpose 5 has no g...When I issue an "extendcircuit 0 A,B" command with my controller, and the circuit gets built, my Tor logs a line like:
```
Feb 09 20:36:44.400 [warn] circuit_build_no_more_hops(): Bug: 2-hop circuit 0x5558a75e2190 with purpose 5 has no guard state (on Tor 0.4.5.5-rc-dev f420eacf1858220f)
```
My extendprobe tester (https://gitlab.torproject.org/tpo/network-health/team/-/issues/16) makes many such circuits, and ends up with many such Bug lines.
See #24966 for a past version of this bug that got closed.
This issue isn't a big deal, except for the log spam, and maybe we still (hopefully we still?) have an invariant goal where Tor isn't supposed to issue any "Bug" lines during ordinary expected behavior.Tor: 0.4.6.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/19011Use of maxunmeasuredbw and bwweightscale is broken in consensus2021-09-16T14:33:29ZNick MathewsonUse of maxunmeasuredbw and bwweightscale is broken in consensusWhile refactoring, I noticed this code in dirvote.c:
```
if (params) {
if (strcmpstart(params, "bwweightscale=") == 0)
bw_weight_param = params;
else
bw_weight_param = strstr(params, " bwweightscale=");
...While refactoring, I noticed this code in dirvote.c:
```
if (params) {
if (strcmpstart(params, "bwweightscale=") == 0)
bw_weight_param = params;
else
bw_weight_param = strstr(params, " bwweightscale=");
}
if (bw_weight_param) {
int ok=0;
char *eq = strchr(bw_weight_param, '=');
if (eq) {
weight_scale = tor_parse_long(eq+1, 10, 1, INT32_MAX, &ok,
NULL);
if (!ok) {
log_warn(LD_DIR, "Bad element '%s' in bw weight param",
escaped(bw_weight_param));
weight_scale = BW_WEIGHT_SCALE;
}
} else {
log_warn(LD_DIR, "Bad element '%s' in bw weight param",
escaped(bw_weight_param));
weight_scale = BW_WEIGHT_SCALE;
}
}
```
Looking at the use of tor_parse_ulong(). Since "next" is NULL, any unconverted characters should make it give an error, making us use the default value.Tor: 0.4.6.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40222prop328: Implementation of the proposal "Make Relays Report When They Are Ove...2021-07-23T16:05:30ZDavid Gouletdgoulet@torproject.orgprop328: Implementation of the proposal "Make Relays Report When They Are Overloaded"This is a ticket to track the implementation of proposal 328 which is under review at https://gitlab.torproject.org/tpo/core/torspec/-/merge_requests/22 (and in tor#40158).
Also, the tor-dev@ thread: https://lists.torproject.org/piperma...This is a ticket to track the implementation of proposal 328 which is under review at https://gitlab.torproject.org/tpo/core/torspec/-/merge_requests/22 (and in tor#40158).
Also, the tor-dev@ thread: https://lists.torproject.org/pipermail/tor-dev/2020-December/014483.htmlTor: 0.4.6.x-freezeGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/tpo/core/tor/-/issues/40266hs-v2: Farewell Old Friend2021-06-23T17:22:08ZDavid Gouletdgoulet@torproject.orghs-v2: Farewell Old FriendThe 0.4.6 merge window has opened earlier this year, thus it is time to say goodbye to our great friend, the onion service version 2.
It has served us incredibly well, it has done its time and can now retire with pride and glory.
This ...The 0.4.6 merge window has opened earlier this year, thus it is time to say goodbye to our great friend, the onion service version 2.
It has served us incredibly well, it has done its time and can now retire with pride and glory.
This ticket is about the removal of v2 support from tor.git. As per our deprecation timeline:
https://lists.torproject.org/pipermail/tor-dev/2020-June/014365.htmlTor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40314dir auths should publish their wfu / tk / mtbf for each relay in each vote?2021-05-17T01:12:09ZRoger Dingledinedir auths should publish their wfu / tk / mtbf for each relay in each vote?I've just started running this patch on moria1:
```
diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_
routerstatus.c
index 252b2e61fe..660d9e5430 100644
--- a/src/feature/nodelist/fmt_routerstatus.c
+++ b/s...I've just started running this patch on moria1:
```
diff --git a/src/feature/nodelist/fmt_routerstatus.c b/src/feature/nodelist/fmt_
routerstatus.c
index 252b2e61fe..660d9e5430 100644
--- a/src/feature/nodelist/fmt_routerstatus.c
+++ b/src/feature/nodelist/fmt_routerstatus.c
@@ -21,6 +21,8 @@
#include "feature/nodelist/routerinfo_st.h"
#include "feature/nodelist/vote_routerstatus_st.h"
+#include "feature/stats/rephist.h"
+
#include "lib/crypt_ops/crypto_format.h"
/** Helper: write the router-status information in <b>rs</b> into a newly
@@ -194,6 +196,13 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version,
digest256_to_base64(ed_b64, (const char*)vrs->ed25519_id);
smartlist_add_asprintf(chunks, "id ed25519 %s\n", ed_b64);
}
+ time_t now = time(NULL);
+ long tk = rep_hist_get_weighted_time_known(rs->identity_digest, now);
+ double wfu =
+ rep_hist_get_weighted_fractional_uptime(rs->identity_digest, now);
+ double mtbf = rep_hist_get_stability(rs->identity_digest, now);
+ smartlist_add_asprintf(chunks, "stats wfu=%.3f tk=%lu mtbf=%.3f\n",
+ wfu, tk, mtbf);
}
}
```
where the goal is to make it explicit what moria1 thinks of each relay, to make it possible for relay operators to figure out why it votes the way it does, and to make it possible for network-wide debugging to find bugs in our tk calculations.
You can see it in action at<br>
https://freehaven.net/~arma/moria1-v3-status-votes
Is this something we want to upstream? Seems like maybe yes, on the theory that I wrote the patch and the transparency could be useful?
Some small improvements to consider:
* in the flag-thresholds line we print mtbf as a %lu even though it is a double, so maybe we want to make it a lu here too, since the numbers are big so decimal precision is kind of silly
* maybe we want a better paint color than "stats" for this line.
I tried to make the numbers that it publishes as close as possible to the numbers that the dir auths make their decisions on, to minimize the class of bugs where we still make flag decisions in weird ways but we can't figure out why from these published numbers.Tor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40229Audit strstr() calls to see which should be find_str_at_start_of_line().2021-03-03T20:05:24ZNick MathewsonAudit strstr() calls to see which should be find_str_at_start_of_line().Related to #40226 -- we sometimes have a string that we want to find at the start of a line only, but instead we look for it with `strstr()`. That's not right: if you just do `strstr(s, "target")` then you'll match a line like `prefixed...Related to #40226 -- we sometimes have a string that we want to find at the start of a line only, but instead we look for it with `strstr()`. That's not right: if you just do `strstr(s, "target")` then you'll match a line like `prefixed-target ...`. But if you do `strstr(s, "\n target")` then you'll miss `target` at the start of a file.
We should go through our uses of strstr() and see which of them, if any, should be modified to use find_str_at_start_of_line().Tor: 0.4.6.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40308dos: Overwrite of the connection stats in the heartbeat2021-02-23T14:30:02ZDavid Gouletdgoulet@torproject.orgdos: Overwrite of the connection stats in the heartbeatCommit 94b56eaa7597e4a0 merged yesterday had an issue I found out later that night.
The commit added the rejected connect we would do to the heartbeat but the code overwrites the existing stats of how many connections were closed from t...Commit 94b56eaa7597e4a0 merged yesterday had an issue I found out later that night.
The commit added the rejected connect we would do to the heartbeat but the code overwrites the existing stats of how many connections were closed from the concurrent detection. See, `conn_msg` is overwritten.
```
tor_asprintf(&conn_msg,
" %" PRIu64 " connections closed.",
conn_num_addr_rejected);
+ tor_asprintf(&conn_msg,
+ " %" PRIu64 " connect() connections closed.",
+ conn_num_addr_connect_rejected);
```Tor: 0.4.6.x-freezeDavid Gouletdgoulet@torproject.orgDavid Gouletdgoulet@torproject.orghttps://gitlab.torproject.org/tpo/core/tor/-/issues/40306Coverity deadcode warnings in HSv2-related code2021-02-22T13:37:53ZNick MathewsonCoverity deadcode warnings in HSv2-related codeLooks like there's some new dead code now that HSv2 has gone the way of Flash?
Of possible interest for @dgoulet or @asn
```
________________________________________________________________________________________________________
*** C...Looks like there's some new dead code now that HSv2 has gone the way of Flash?
Of possible interest for @dgoulet or @asn
```
________________________________________________________________________________________________________
*** CID 1473233: Possible Control flow issues (DEADCODE)
/src/feature/control/control_cmd.c: 1479 in handle_control_hsfetch()
1473 tor_assert_nonfatal_unreached();
1474 }
1475 }
1476
1477 /* Using a descriptor ID, we force the user to provide at least one
1478 * hsdir server using the SERVER= option. */
>>> CID 1473233: Possible Control flow issues (DEADCODE)
>>> Execution cannot reach the expression "hsdirs" inside this statement: "if (desc_id && (!hsdirs || ...".
1479 if (desc_id && (!hsdirs || !smartlist_len(hsdirs))) {
1480 control_write_endreply(conn, 512, "SERVER option is required");
1481 goto done;
1482 }
1483
1484 /* We are about to trigger HSDir fetch so send the OK now because after
** CID 1473232: Control flow issues (DEADCODE)
/src/core/or/connection_edge.c: 2020 in connection_ap_handle_onion()
________________________________________________________________________________________________________
*** CID 1473232: Control flow issues (DEADCODE)
/src/core/or/connection_edge.c: 2020 in connection_ap_handle_onion()
2014
2015 /* Lookup the given onion address. If invalid, stop right now.
2016 * Otherwise, we might have it in the cache or not. */
2017 unsigned int refetch_desc = 0;
2018 if (rend_cache_lookup_result < 0) {
2019 switch (-rend_cache_lookup_result) {
>>> CID 1473232: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "case 22:".
2020 case EINVAL:
2021 /* We should already have rejected this address! */
2022 log_warn(LD_BUG,"Invalid service name '%s'",
2023 safe_str_client(onion_address));
2024 connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
2025 return -1;
```Tor: 0.4.6.x-freezeGeorge KadianakisGeorge Kadianakishttps://gitlab.torproject.org/tpo/core/tor/-/issues/40204Travis chutney tests are borked by two bad commits2021-02-05T21:03:36ZGeorge KadianakisTravis chutney tests are borked by two bad commitsSeems like Travis chutney tests have been broken for the past 7 days. I did some investigation today using git-bisect and found out that there are two offending commits that both need to be reverted for the tests to pass.
These two comm...Seems like Travis chutney tests have been broken for the past 7 days. I did some investigation today using git-bisect and found out that there are two offending commits that both need to be reverted for the tests to pass.
These two commits are 1588767e655f87f49cf0f71d6e604117be52a135 and 4382e977f74e41f65170b4d9292678859c9e521f. Reverting just one of them won't do it and both of them need to be reverted for the tests to pass.
We should fix this so that we start getting a proper CI again.Tor: 0.4.6.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40196Deprecate rust-in-Tor build features for now, recommend arti rust implemntati...2021-01-08T19:52:25ZNick MathewsonDeprecate rust-in-Tor build features for now, recommend arti rust implemntation insteadWe still like Rust, but right now all the good Rust work is happening in @tpo/core/arti
The existing Rust-in-tor build system is just confusing people, and we should probably deprecate it for now. We can return to it if we want to do a...We still like Rust, but right now all the good Rust work is happening in @tpo/core/arti
The existing Rust-in-tor build system is just confusing people, and we should probably deprecate it for now. We can return to it if we want to do arti/tor integration in the future.Tor: 0.4.6.x-freezeNick MathewsonNick Mathewsonhttps://gitlab.torproject.org/tpo/core/tor/-/issues/40223Make ARGUMENT_OPTIONAL flags a bit smarter2020-12-08T14:54:48ZNick MathewsonMake ARGUMENT_OPTIONAL flags a bit smarterWe should teach our flags that other flags are unlikely to be their arguments. Fortunately, none of our current ARGUMENT_OPTIONAL values currently take any arguments starting with -.
Related to #40204.We should teach our flags that other flags are unlikely to be their arguments. Fortunately, none of our current ARGUMENT_OPTIONAL values currently take any arguments starting with -.
Related to #40204.Tor: 0.4.6.x-freezeNick MathewsonNick Mathewson