Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:
  |  needs_revision
 Priority:  Medium|  Milestone:  Tor:
  |  0.3.5.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  rust, 033-backport, 034-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * milestone:  Tor: unspecified => Tor: 0.3.5.x-final


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:
  |  needs_revision
 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:
--+
Changes (by teor):

 * cc: nickm (added)


Comment:

 Since my last comment raises some spec issues, I'm cc'ing nickm.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:
  |  needs_revision
 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:
--+
Changes (by teor):

 * status:  new => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#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: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#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 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.

 Also, `make test-network-all` passes on branch protodupes1, at least.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  rust, 033-backport, 034-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * keywords:  rust => rust, 033-backport, 034-backport


Comment:

 No, duplicate subprotocol entries are allowed by the spec:
 https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n780

 The rust code is wrong, and should be fixed.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#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:
--+
Changes (by teor):

 * milestone:   => Tor: unspecified


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by cyberpunks):

 The C code already has to check for duplicates to prevent double-voting.
 Could both C and Rust be changed to consider it an error to have duplicate
 subprotocol entries?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #27190 [Core Tor/Tor]: disparate duplicate subproto handling in protover

[Tor Bug Tracker & Wiki]

#27190: disparate duplicate subproto handling in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal|   Keywords:  rust
Actual Points:|  Parent ID:
   Points:|   Reviewer:
  Sponsor:|
--+
 `protover.c` treats `Link=1 Link=1` and `Link=1` identically, allowing
 duplicate entries without complaint, though it does explicitly check for
 duplicates to avoid double-counting it as two votes for the same version.

 `protover.c` also treats `Link=1 Link=2` and `Link=1-2` the same, while
 the rust implementation of protover treats `Link=1 Link=2` as if it were
 `Link=2`.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs