Re: [tor-bugs] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by atagar):

 Related a few past tickets where this has bitten us.

 Just a quick note that dirauths could use Stem as a tool for rejecting
 malformed content. It does stricter validation than the tor binary that
 descriptors are conformant with the spec. I've been performing this check
 through DocTor since 2013, filing tickets each time more bad data makes it
 into the consensus...

 https://gitweb.torproject.org/doctor.git/tree/descriptor_checker.py

 Bad data chokes not only Stem, but metrics-lib and anything else that
 ingests it.

 Clearly in an ideal world the tor binary itself would do better validation
 but in the absence of that if we took advantage of Stem's validator I
 wouldn't need to keep filing tickets every few months. Using Stem on
 dirauths to reject malformed descriptors would prevent these issues
 upfront, saving Karsten and I hassle.

 If that's a no-go I could also redirect the DocTor check I mention above
 to email other folks (Nick? teor? Maybe the network team list?) so I don't
 need to file tickets each time this comes up.

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Replying to [comment:7 teor]:
 > Since the UTF-8 checking function accepts a length, we should probably
 also check for NUL. If we fail on the first NUL, this check becomes a
 last-ditch memory safety check, as most bytes in RAM are NUL.
 >
 > We should probably also log a bug warning if the function encounters a
 NUL byte:

 Receiving a NUL byte isn't a bug though. This function is processing
 untrusted input from `fetch_from_buf_http()` that might contain anything
 including NUL bytes.

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Replying to [comment:8 teor]:
 > Oh, and I think that the changes file still mentions relays?

 The changes file was reverted actually. Fixed.

 But this ticket says 'and relays.' Could edit the summary and have a
 separate ticket for making relays reject in the future?

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 I opened #27389 to allow failures on that target, until the compiler is
 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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 The same thing went wrong with the rebuild:
 https://ci.appveyor.com/project/teor2345/tor/build/1.0.111/job/8nf6lhj1oag6nbmf

 I wonder if the compiler that we're using has been updated to a broken
 version.

 I pushed the common ancestor of your branch and master to master-unicode-
 descriptors1, and the current master to master. If they both fail, I'll
 open a ticket to disable that build:
 https://github.com/teor2345/tor/branches

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Something went wrong with one of the Windows builds in files you didn't
 change, so I restarted it:
 https://ci.appveyor.com/project/teor2345/tor/build/1.0.111

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Oh, and I think that the changes file still mentions relays?

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Thanks, I pushed it to CI, if it passes, a green tick will show up next to
 the branch on https://github.com/teor2345/tor/branches

 When we check different kinds of directory documents, we'll want the BOM
 check as part of a descriptor UTF-8 checking function in util_string.c.
 Would you mind moving it in this branch?

 Since the UTF-8 checking function accepts a length, we should probably
 also check for NUL. If we fail on the first NUL, this check becomes a
 last-ditch memory safety check, as most bytes in RAM are NUL.

 We should probably also log a bug warning if the function encounters a NUL
 byte:
 {{{
 if (BUG(failure-condition) {
   failure-action;
 }
 }}}
 or
 {{{
 if (success-condition) {
   return
 }
 tor_assert_nonfatal_unreached();
 }}}

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Right, just authorities at first. Undid the other commit and pushed a BOM
 check.

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Hi, the CI failed:
 * https://ci.appveyor.com/project/teor2345/tor/build/1.0.109
 * https://travis-ci.org/teor2345/tor/builds/422038871

 We can't merge this branch as-is, because it doesn't implement the
 proposal:
 * there are no byte-order mark checks in this branch:
 https://gitweb.torproject.org/torspec.git/tree/proposals/285-utf-8.txt#n90
 * the proposal involves a staged deployment:
 https://gitweb.torproject.org/torspec.git/tree/proposals/285-utf-8.txt#n27

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.6.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * status:  new => needs_review
 * milestone:  Tor: unspecified => Tor: 0.3.6.x-final


Comment:

 I pushed your branch to ​https://github.com/teor2345/tor/branches to get
 CI.

 We usually create valid descriptors, votes, and consensuses using chutney:
 https://gitweb.torproject.org/chutney.git

 In this case, an unpatched tor will happily copy non-unicode characters
 from ContactInfo into its descriptor, and sign that descriptor.

 Here are some existing test descriptors and votes:
 https://gitweb.torproject.org/tor.git/tree/src/test/test_descriptors.inc

 And here is the function that parses those descriptors:
 https://gitweb.torproject.org/tor.git/tree/src/test/test_helpers.c#n90

 If we're going to test for invalid descriptors, then we should probably
 put the test in test_dir.c.

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Not sure how to test, but made an attempt at this in branch unicode-
 descriptors1 at https://gitgud.io/onionk/tor.git

--
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] #27367 [Core Tor/Tor]: Authorities and relays should reject non-UTF-8 in relay descriptors (was: Authorities should reject non-UTF-8 in relay descriptors)

[Tor Bug Tracker & Wiki]

#27367: Authorities and relays should reject non-UTF-8 in relay descriptors
-+-
 Reporter:  teor |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  rust-wants, prop285, |  Actual Points:
  034-triage-20180328, 034-removed-20180328  |
Parent ID:  #24033   | Points:
 Reviewer:   |Sponsor:
-+-
Description changed by teor:

Old description:

> Part of #24033.

New description:

 Part of #24033.

 https://gitweb.torproject.org/torspec.git/tree/proposals/285-utf-8.txt#n29

--

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