networkstatus_check_consensus_signature() prints wrong stats
Sep 20 01:16:33.215 [info] A consensus needs 5 good signatures from recognized authorities for us to accept it. This one has 8. 2 of the authorities we know didn't sign it. It has 2 signatures from authorities we don't recognize.
Wait, it has 8 signatures, and also 2 of the authorities we know didn't sign it? It turns out that second statement is false.
I instrumented my Tor to explain when it bumps various counters in this function, and got this output:
Sep 20 01:42:33.344 [notice] Bootstrapped 25%: Loading networkstatus consensus.
Sep 20 01:42:34.337 [notice] looking at new voter tor26: 14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4
Sep 20 01:42:34.337 [notice] looking at new sig 14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4: 0 0 1
Sep 20 01:42:34.338 [notice] ++good_here
Sep 20 01:42:34.338 [notice] ++n_good
Sep 20 01:42:34.338 [notice] looking at new voter ides: 27B6B5996C426270A5C95488AA5BCEB6BCC86956
Sep 20 01:42:34.339 [notice] looking at new sig 27B6B5996C426270A5C95488AA5BCEB6BCC86956: 0 0 1
Sep 20 01:42:34.339 [notice] ++good_here
Sep 20 01:42:34.339 [notice] ++n_good
Sep 20 01:42:34.340 [notice] looking at new voter maatuska: 49015F787433103580E3B66A1707A00E60F2D15B
Sep 20 01:42:34.342 [notice] looking at new sig 49015F787433103580E3B66A1707A00E60F2D15B: 0 0 1
Sep 20 01:42:34.343 [notice] ++good_here
Sep 20 01:42:34.343 [notice] ++n_good
Sep 20 01:42:34.343 [notice] looking at new voter dannenberg: 585769C78764D58426B8B52B6651A5A71137189A
Sep 20 01:42:34.343 [notice] looking at new sig 585769C78764D58426B8B52B6651A5A71137189A: 0 0 1
Sep 20 01:42:34.347 [notice] ++good_here
Sep 20 01:42:34.347 [notice] ++n_good
Sep 20 01:42:34.347 [notice] looking at new voter urras: 80550987E1D626E3EBA5E5E75A458DE0626D088C
Sep 20 01:42:34.347 [notice] looking at new sig 80550987E1D626E3EBA5E5E75A458DE0626D088C: 0 0 1
Sep 20 01:42:34.350 [notice] ++good_here
Sep 20 01:42:34.350 [notice] ++n_good
Sep 20 01:42:34.351 [notice] looking at new voter gabelmoo-legacy: 81349FC1F2DBA2C2C11B45CB9706637D480AB913
Sep 20 01:42:34.351 [notice] looking at new sig 81349FC1F2DBA2C2C11B45CB9706637D480AB913: 0 0 1
Sep 20 01:42:34.351 [notice] ++n_unknown
Sep 20 01:42:34.351 [notice] ++n_no_signature
Sep 20 01:42:34.351 [notice] looking at new voter moria1: D586D18309DED4CD6D57C18FDB97EFA96D330566
Sep 20 01:42:34.352 [notice] looking at new sig D586D18309DED4CD6D57C18FDB97EFA96D330566: 0 0 1
Sep 20 01:42:34.356 [notice] ++good_here
Sep 20 01:42:34.356 [notice] ++n_good
Sep 20 01:42:34.356 [notice] looking at new voter moria1-legacy: E2A2AF570166665D738736D0DD58169CC61D8A8B
Sep 20 01:42:34.356 [notice] looking at new sig E2A2AF570166665D738736D0DD58169CC61D8A8B: 0 0 1
Sep 20 01:42:34.356 [notice] ++n_unknown
Sep 20 01:42:34.357 [notice] ++n_no_signature
Sep 20 01:42:34.357 [notice] looking at new voter dizum: E8A9C45EDE6D711294FADF8E7951F4DE6CA56B58
Sep 20 01:42:34.357 [notice] looking at new sig E8A9C45EDE6D711294FADF8E7951F4DE6CA56B58: 0 0 1
Sep 20 01:42:34.358 [notice] ++good_here
Sep 20 01:42:34.358 [notice] ++n_good
Sep 20 01:42:34.358 [notice] looking at new voter gabelmoo: ED03BB616EB2F60BEC80151114BB25CEF515B226
Sep 20 01:42:34.358 [notice] looking at new sig ED03BB616EB2F60BEC80151114BB25CEF515B226: 0 0 1
Sep 20 01:42:34.359 [notice] ++good_here
Sep 20 01:42:34.359 [notice] ++n_good
Sep 20 01:42:34.360 [warn] Consensus includes unrecognized authority 'gabelmoo-legacy' at 80.190.246.100:8180 (contact n/a; identity 81349FC1F2DBA2C2C11B45CB9706637D480AB913)
Sep 20 01:42:34.360 [warn] Consensus includes unrecognized authority 'moria1-legacy' at 128.31.0.34:9131 (contact n/a; identity E2A2AF570166665D738736D0DD58169CC61D8A8B)
Sep 20 01:42:34.360 [warn] A consensus needs 5 good signatures from recognized authorities for us to accept it. This one has 8. 2 of the authorities we know didn't sign it. It has 2 signatures from authorities we don't recognize.
Sep 20 01:42:34.360 [notice] n_good 8, n_v3_authorities 8, n_required 5, n_missing_key 0
So it turns out we're bumping n_no_signature when we're bumping n_unknown. That happens because we end the
SMARTLIST_FOREACH_BEGIN(consensus->voters, networkstatus_voter_info_t *,
loop without having set good_here, bad_here, etc, so it falls to the bottom of the if statements:
} else
++n_no_signature;
Something is not right, here. Perhaps it is as simple as "we are mis-describing what n_no_signature means"?