Spec doesn't mention password quotes

[atagar]

Section 5.1 of the control-spec [1] provides a nice description of authentication, but doesn't mention how to handle quotes in the password. Unsurprisingly controllers are expected to provide escaped quotes...

<pre>
atagar@morrigan:~$ tor --hash-password "this has a \" in it"

16:E6DC1BCEDF55EDCA607ADDB8781795772E42AAC75F7B7630B6227232E4

atagar@morrigan:~$ telnet localhost 9051

Connected to localhost.
AUTHENTICATE "this has a \" in it"
250 OK

</pre>

I'm gonna guess that only quotes should be escaped by controllers.

I've been finding it a little frustrating to figure out when and what escaping is expected so I'm generally working from the assumption that I should ignore escaping unless specifically called out by the spec (like it is for authentication cookie paths, though that wasn't enough to work from alone [2]).

Cheers! -Damian

[1] ​https://gitweb.torproject.org/torspec.git/blob/HEAD:/control-spec.txt#l1924
[2] ​https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/socket.py#l54

[atagar]

Oops, sorry - forgot trac has different preformatted blocks.

[nickm]

Is this something you can write a spec patch for?

I see the relevant functions as being the ones with names like *_escaped* in control.c. They're called by conrol_setconf_helper, handle_control_authenticate, handle_control_getinfo, handle_control_postdescriptor, control_event_or_authdir_new_descriptor, and conrol_event_networkstatus_changed_helper.

[nickm]

Replying to nickm:

Is this something you can write a spec patch for?

If it isn't, I'd have an easier time if you can rephrase the bug in terms of a list of questions that you want the spec to answer. I get that AUTHENTICATE should document when it accepts quoted strings, -- what else?

[rransom]

Replying to atagar:

I've been finding it a little frustrating to figure out when and what escaping is expected so I'm generally working from the assumption that I should ignore escaping unless specifically called out by the spec (like it is for authentication cookie paths, though that wasn't enough to work from alone [2]).

[2] ​https://gitweb.torproject.org/stem.git/blob/HEAD:/stem/socket.py#l54

esc_for_log is the only function Tor uses to escape strings sent to the control port. Every quoted string Tor emits on its control port should be quoted (and escaped) by a call to esc_for_log.

Unrelated: getinfo_helper_listeners is not dead code; a macro expands listeners in some ‘column’ of the list of GETINFO items (and the helpers that handle them) to getinfo_helper_listeners, and then the function that handles GETINFO commands calls it through a function pointer in the GETINFO item array.

[atagar]

Is this something you can write a spec patch for?

I can write spec patches for what I think it should be if you'd like.

I'd have an easier time if you can rephrase the bug in terms of a list of questions that you want the spec to answer

My first thought when I saw that AUTHENTICATE took a quoted passphrase was "Hm, guess the passphrase can't contain quotes. Hopefully --hash-password gives an error when users try to create hashed passphrases with quotes.". After a bit of experimenting my next thought was "Uhh, are quotes the only escaped value?".

I'd like the spec to specify...

• when and what escaping is expected for requests
• when and what unescaping should be applied to replies
• a note that says that if there's escaping outside of this then it's either a tor or spec bug

Cheers! -Damian

[nickm]

I'd still take a spec patch for this, but it apparently isn't happening in 0.2.3. Fortunately , the spec isn't tied to the tor release cycle.

[nickm]

I think I got a fix for this in 9716716527fc5e8047b57f14870afbb3c864a197. The issue was that QuotedString wasn't actually defined, I think.

[atagar]

Hi Nick. Sorry about re-opening this old ticket but while looking at #8471 I realized that ​9716716 doesn't match ​esc_for_log. Presently ​stem escapes the characters mentioned by esc_for_log.

Two questions:

1. Should QuotedString also mention that single quotes, tabs, etc are escaped?
2. rransom mentioned that tor only does escaping in a few specific places, are you sure that it's appropriate to say that all QuotedStrings are escaped?

[nickm]

Deferring to 0.2.5 after discussion with atagar: making the metaformat more uniform in 0.2.5 will make this stuff far far easier to document.

[andrea]

Triage for 0.2.5: keep and document as-is

[nickm]

This is actually nontrivial since as far as I can tell there are two single-line escaped-string formats here: The one that control.c generates (which uses esc_for_log), and the one we accept (which uses decode_escaped_string). I don't think control-spec does a great job distinguishing between them: it seems to call them both QuotedString, which is just wrong.

[nickm]

Closing in favor of #15000.

[teor]

Milestone renamed

[nickm]

Milestone deleted