Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#1956 closed defect (fixed)

networkstatus_check_consensus_signature() prints wrong stats

Reported by: arma Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.2.15-alpha
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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"?

Child Tickets

Change History (6)

comment:1 Changed 9 years ago by arma

Check out branch bug1956 in arma's git for the instrumenting code (not meant to be merged into anything official).

comment:2 Changed 9 years ago by nickm

Milestone: Tor: 0.2.2.x-final
Owner: set to nickm
Status: newassigned

comment:3 Changed 9 years ago by nickm

Status: assignedneeds_review

See branch bug1956 in my public. I think it's right.

comment:4 Changed 9 years ago by arma

Looks fine to me. Merge when you like.

comment:5 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged; closing.

comment:6 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.