[tor-bugs] #12930 [Obfuscation/Pluggable transport]: Someone, somewhere needs to unescape pluggable transport "SMETHOD ARGS" arguments.

[Tor Bug Tracker & Wiki]

#12930: Someone, somewhere needs to unescape pluggable transport "SMETHOD ARGS"
arguments.
---------------------------------------------+---------------------
 Reporter:  yawning                          |          Owner:  asn
     Type:  defect                           |         Status:  new
 Priority:  Medium                           |      Milestone:
Component:  Obfuscation/Pluggable transport  |        Version:
 Severity:  Normal                           |     Resolution:
 Keywords:  goptlib                          |  Actual Points:
Parent ID:                                   |         Points:
 Reviewer:                                   |        Sponsor:
---------------------------------------------+---------------------

Comment (by dcf):

 Replying to [comment:10 catalyst]:
 > 1.  `SMETHOD ARGS`
 >  - comma-separated key=value pairs
 >  - `,` and `=` (but not `\` -- at least not consistently, e.g. goptlib)
 escaped by `\` ([https://gitweb.torproject.org/torspec.git/tree/pt-
 spec.txt#n566 pt-spec.txt#n566])
 >  - no provision for escaping whitespace

 Is there a place where goptlib doesn't escape `\`? If so it is probably a
 bug. `\` has to be escaped in order to make the escaping reversible, even
 though the spec doesn't explicitly call for it ("Equal signs and commas
 MUST be escaped with a backslash"); my comment in goptlib interpolates
 "[and backslashes]".

 The `encodeSmethodArgs` function has
 {{{
 // "Equal signs and commas [and backslashes] must be escaped with a
 backslash."
         escape := func(s string) string {
                 return backslashEscape(s, []byte{'=', ','})
         }
 }}}
 but that doesn't mean `\` isn't itself escaped; `backslashEscape` escapes
 `\` ''in addition to'' the other bytes that are listed.

 > 5. encoded in SOCKS username/password
 >  - semicolon-separated key=value pairs
 >  - `;`, `=`, and `\` escaped by `\`
 ([https://gitweb.torproject.org/torspec.git/tree/pt-spec.txt#n638 pt-
 spec.txt#n638])
 >  - currently, `transports.c` doesn't escape `=`, contrary to spec
 ([https://gitweb.torproject.org/tor.git/tree/src/or/transports.c#n1668
 transports.c#n1668])
 >  - on the other hand,
 [https://gitweb.torproject.org/torspec.git/tree/proposals/180-pluggable-
 transport.txt#n157 180-pluggable-transport.txt#n157] doesn't specify
 escaping of `=` when sending to the transport's SOCKS proxy, so maybe this
 can remain unchanged

 Here, I felt that the lack of backslashing equals signs was a bug in the
 spec and interpolated into a comment above `parseClientParameters`:
 {{{
 // "If a key or value value must contain [an equals sign or] a semicolon
 // or a backslash, it is escaped with a backslash."
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/12930#comment:11>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online