Re: [tor-bugs] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces 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:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:  nickm|Sponsor:
-+-
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Sorry for the delay -- I do think we should use the metaformat from the
 rest of the spec, and change the spec to WS here.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:  nickm|Sponsor:
-+-
Changes (by teor):

 * status:  needs_information => needs_review
 * cc: nickm (removed)
 * reviewer:   => nickm


Comment:

 Replying to [comment:7 cyberpunks]:
 > Visual inspection of the code made them pretty apparent.
 >
 > Also, it's been 3 weeks now. Is this gonna get reviewed before Friday?

 We aim to review code within a week, but we've been busy with the 0.3.4
 release:
 https://lists.torproject.org/pipermail/tor-
 announce/2018-September/000164.html
 And the 0.3.5 code freeze:
 
https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases

 If it helps, you're not the only person waiting for code reviews:
 
https://trac.torproject.org/projects/tor/query?status=needs_review=Core+Tor%2FTor=id=summary=status=type=priority=milestone=component=priority

 I'm still waiting on nickm's opinion on spec vs code changes:

 Replying to [comment:4 teor]:
 > We could also update the protover spec to allow any number of spaces and
 tabs (WS), rather than a single space (SP), and update the C and Rust
 implementations to match.
 >
 > That would be more consistent with Tor's directory document meta-format:
 > https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n193
 >
 > I'd like to get nickm's opinion before we move forward.

 I'll try setting him as reviewer on this ticket instead.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_information
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Visual inspection of the code made them pretty apparent.

 Also, it's been 3 weeks now. Is this gonna get merged before Friday?

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_information
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * cc: nickm (added)


Comment:

 If I cc nickm, I'm more likely to get his opinion.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_information
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Also, I'm not sure how you're finding all these protover bugs, but you
 might be interested in protover fuzzing in #24265. Someone already tried
 in #26265, but they didn't do it using Tor's existing fuzzing
 infrastructure, and we needed to be able to fuzz the rust and C functions
 in the same process.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_information
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * status:  needs_review => needs_information


Comment:

 We could also update the protover spec to allow any number of spaces and
 tabs (WS), rather than a single space (SP), and update the C and Rust
 implementations to match.

 That would be more consistent with Tor's directory document meta-format:
 https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n193

 I'd like to get nickm's opinion before we move forward.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.2.9.4-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, 029-backport, 032-backport,|  Actual Points:
  033-backport, 034-backport |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * keywords:  rust => rust, 029-backport, 032-backport, 033-backport,
 034-backport
 * milestone:   => 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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.4-alpha
 Severity:  Normal| Resolution:
 Keywords:  rust  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  new => needs_review


Comment:

 Replying to [comment:1 cyberpunks]:
 > The protospaces2 branch at ​​https://gitgud.io/onionk/tor.git changes
 the C implementation to reject them. Nothing actually produces extra
 spaces like this, right?

 Extra spaces are not allowed by the protover spec. Each separator is
 exactly one space:
 https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n780

 But we should run `make test-network-all` using
 [https://gitweb.torproject.org/chutney.git chutney] to make sure that
 standard tor clients aren't inserting extra spaces.

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

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

Comment (by cyberpunks):

 The protospaces2 branch at ​​https://gitgud.io/onionk/tor.git changes the
 C implementation to reject them. Nothing actually produces extra spaces
 like this, right?

--
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] #27191 [Core Tor/Tor]: handling double spaces in protover

[Tor Bug Tracker & Wiki]

#27191: handling double spaces in protover
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.4-alpha
 Severity:  Normal|   Keywords:  rust
Actual Points:|  Parent ID:
   Points:|   Reviewer:
  Sponsor:|
--+
 `protover.c` accepts trailing spaces and extra spaces between subprotocol
 entries like `"Link=1-4  LinkAuth=1 "`, but rejects leading spaces like `"
 Link=1-4"`. It has since its
 
[https://gitweb.torproject.org/tor.git/commit/?id=b2b2e1c7f24d9b65059e3d089768d6c49ba4f58f
 introduction.]

 The Rust implementation rejects all extra spaces in any position. It's at
 least consistent.

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