[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover
#27190: disparate duplicate subproto handling in protover
----------------------------------------------+----------------------------
Reporter: cyberpunks | Owner: (none)
Type: defect | Status: new
Priority: Medium | Milestone: Tor:
| unspecified
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: rust, 033-backport, 034-backport | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
----------------------------------------------+----------------------------
Comment (by teor):
Replying to [comment:4 cyberpunks]:
> Replying to [comment:2 teor]:
> > No, duplicate subprotocol entries are allowed by the spec:
>
> That spec seems ambiguous and could be clarified in either direction.
>
> If it's clarified to forbid duplicates--it already says the entries
should be sorted alphabetically--it would become simpler to also verify
that there aren't any overlapping ranges, which the spec says shouldn't
happen but the C implementation doesn't check for right now.
"must" is an absolute requirement, but "should" is a recommendation:
SHOULD This word, or the adjective "RECOMMENDED", mean that there
may exist valid reasons in particular circumstances to ignore a
particular item, but the full implications must be understood and
carefully weighed before choosing a different course.
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n19
https://tools.ietf.org/html/rfc2119
Therefore, Tor's parser can tolerate duplicates, unsorted ranges, and
overlapping ranges. I think that it's worth accepting them, to support a
broad range of relay implementations. As a trivial example, it's worth
accepting both "Foo=1,2" and "Foo=1-2".
But all directory authority implementations MUST generate identical
consensuses:
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n1614
Therefore:
* all directory authority parser implementations MUST interpret edge cases
as the same protocol versions, and
* all directory authority serialisation implementations MUST generate
exactly the same text for the same protocol versions.
So I think we should fix the Rust to match the C.
We can also open another ticket to clarify the spec, but I'm not sure
exactly what changes to make. We don't try to specify the exact format of
a consensus in the spec - we only provide enough detail to parse a
consensus.
> Also, `make test-network-all` passes on branch protodupes1, at least.
Unfortunately, those tests don't test all the edge cases. And apparently
neither do our unit tests. So we should add more unit tests.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/27190#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs