Opened 4 years ago

Last modified 8 months ago

#20625 needs_revision enhancement

When a consensus doesn't have enough signatures, write it (and sigs) to a file

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, tor-dirauth, 034-triage-20180328, 034-removed-20180328
Cc: Actual Points:
Parent ID: #4539 Points: 1
Reviewer: Sponsor:


Just like we write 'unparseable-desc' and 'v3-status-votes' to a file, it would be great to write 'unpublished[-flavour]-consensus' to a file, when we log this message:

Nov 10 21:00:01.000 [warn] A consensus needs 5 good signatures from recognized authorities for us to accept it. This one has 4 (...). 4 (...) of the authorities we know didn't sign it.
Nov 10 21:00:01.000 [warn] Not enough info to publish pending ns consensus

Similarly, when we disagree about signatures, we should write 'unpublished-[-flavour]-signatures', when we log this message:

Nov 10 21:27:32.000 [warn] Problem adding detached signatures from <redacted-address-and-port>: Mismatched digest.

(Or, even better, write both matching and mismatching signatures to the file.)

One catch:
These new filenames need to be added to the sandbox.

Child Tickets

Change History (15)

comment:1 Changed 4 years ago by teor

We should only write the latest bad consensus to a file.

We should only write signatures from authorities we trust to the sigs file. Otherwise, an authority's disk could be filled by a malicious server pretending to be an authority.

Alternately, we could limit the file size(s).

comment:2 Changed 4 years ago by nickm

For the consensus:

 23:31 < nickm> in dirvote.c, the pending_consensus_field array the consensuses 
               that we generated,  which have not yet gotten enough signatures.
 23:33 < nickm> In dirvote_publish_consensus() might be the right place.

For signatures we can't apply, dirvote_add_signatures_to_pending_consensus is where we'll notice that we tried to add signatures, but couldn't.

comment:3 Changed 4 years ago by nickm

Points: 1

comment:4 Changed 4 years ago by jvsg

Please have a look at this patch.

This patch enables writing consensus to file. The latter half of the enhancement (writing unpublished signatures) will be addressed in the next patch by me.

comment:5 Changed 4 years ago by nickm

Status: newneeds_review

comment:6 Changed 4 years ago by teor

Status: needs_reviewneeds_revision

Hi jvsg, thanks for this patch!

I'm not sure if this is the code you meant to submit: as far as I can see, it reads the file "cached-[<flavour>-]consensus", rather than writing the file "unpublished-consensus".

Also, you will need to add both "unpublished-consensus" and "unpublished-microdesc-consensus" to the sandbox whitelist.

It also looks like you might have copied this code from somewhere.
If so, we try to refactor code into a static function and call it from multiple locations, rather than copying it. (If not, use tor_asprintf() rather than tor_snprintf(), it's much easier!)

comment:7 Changed 4 years ago by dgoulet

Keywords: easy tor-dirauth added; easy? tor-auth removed
Milestone: Tor: 0.3.0.x-finalTor: 0.3.1.x-final

Deferring to 031. Very cool to have but not critical for 030 feature freeze in a couple of days.

comment:8 Changed 3 years ago by arma

Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final

Deferring to 0.3.2 for the same reason.

comment:9 Changed 3 years ago by teor

We can now use storage_dir_save_labeled_to_file() or storage_dir_save_bytes_to_file() for this.

comment:10 Changed 3 years ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

Defer all needs_revision non-spec enhancements to 0.3.3.

comment:11 Changed 3 years ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

Mark a lot of assigned/needs_revision tickets as 0.3.4. If you think this should happen in 0.3.3 instead, just let me know?

comment:12 Changed 3 years ago by nickm

Keywords: 034-triage-20180328 added

comment:13 Changed 3 years ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:14 Changed 3 years ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

comment:15 Changed 8 months ago by teor

Parent ID: #4539

Seems like a duplicate of #4539.

Note: See TracTickets for help on using tickets.