Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#21673 closed defect (worksforme)

prop140: Handle signatures correctly

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop140 TorCoreTeam201705
Cc: Actual Points: .2
Parent ID: #13339 Points: .1
Reviewer: Sponsor: Sponsor4

Description

For diffs to work properly, we need to check the input document and the output document in their entirety, including their signatures. Otherwise, the diffs won't apply correctly when they change the signatures!

But for *that* to work, we need to do what we can to minimize the odds that anybody has a consensus with different signatures, or with signatures organized differently.

As an alternative, we could change the diff format so that it always replaces all the old signatures with the new ones.

Child Tickets

Attachments (2)

diff-seed.txt (4.5 KB) - added by nickm 3 years ago.
diff-apply-seed.txt (2.7 KB) - added by nickm 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by nickm

Owner: set to nickm
Status: newaccepted

comment:2 Changed 3 years ago by nickm

My #21643 branch switches to sha3 and covers the entire document, but doesn't yet create or enforce a consistent signature ordering.

Changed 3 years ago by nickm

Attachment: diff-seed.txt added

Changed 3 years ago by nickm

Attachment: diff-apply-seed.txt added

comment:3 Changed 3 years ago by dgoulet

Keywords: prop140 added

comment:4 Changed 3 years ago by dgoulet

Sponsor: Sponsor4

comment:5 Changed 3 years ago by nickm

According to the spec:

     The signatures at the end of a consensus document are sorted in
     ascending order by identity digest.

We should note that in a more clever place, and we should make sure the code really does it.

comment:6 Changed 2 years ago by nickm

Points: .1

comment:7 Changed 2 years ago by nickm

Actual Points: .1
Resolution: worksforme
Status: acceptedclosed

Indeed, we do this by emitting the signatures in networkstatus_format_signatures() in the order of consensus->voters, which is in turn sorted by ID. Closing this as "nothing to do".

comment:8 Changed 2 years ago by nickm

Actual Points: .1.2
Keywords: TorCoreTeam201705 added

Actually, there was an additional corner-case here, that we fixed by a proposal patch in torspec commit 28816242f9eaa5509dc400a48ade1e7c4a591717.

The problem was that clients would, when contacting caches, identify consensuses by the sha3 digest of the entire consensus, including signatures. But there are multiple valid encodings for a set of signatures, meaning that a malicious cache could serve each client a different encoding, and recognize the clients using the sha3 digests in their requests.

The first part of the solution is to fetch consensuses diffs based only on the consensus's digest-as-signed: the digest of the consensus with no signatures on it.

The second part of the solution is to generate diffs using the <n>,$d format to first remove all trailing signatures, so that the diffs will apply to any valid consensus, no matter how the signatures are encoded.

See #22143 for implementation work here.

Note: See TracTickets for help on using tickets.