Opened 7 years ago

Closed 6 years ago

#6350 closed defect (not a bug)

Extra-info descriptors contain extra space between nickname and fingerprint

Reported by: karsten Owned by:
Priority: Very Low Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-relay
Cc: atagar, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Starting 16 hours ago, a relay started publishing the following extra-info descriptor that makes metrics-lib choke:

extra-info free  EB2798A84E9D962DF13C61FA055C2513DDDAF800
published 2012-07-08 13:36:56
router-signature
-----BEGIN SIGNATURE-----
nn7itViCioERo/QzaKYB5AKu/ufMjukORZBU4QXqL0PPSvjDVix1+ckK8J5xYJc7
6fie9moyZpUK+bdz4xzIf/YECMbGwT3lJOZO59f+vMwQycI6tRWHyCWc9A941Mf4
6hrvwRj7IKgIcJdhxFp3FevREx5CCA3f+yhWJdAbQ5s=
-----END SIGNATURE-----

metrics-lib would expect a single SP between nickname and fingerprint, which is the default for most descriptor lines. I'm not entirely certain that dir-spec.txt doesn't allow two SP though:

    "extra-info" Nickname Fingerprint NL

The space between Nickname and Fingerprint could be interpreted as SP (which is what I did in many places that didn't have an explicit definition of SP), or it could be considered WS which is defined as WS = (SP | TAB)+.

So, should the directory authorities reject this descriptor for being malformed? Or should metrics-lib (and stem) interpret multiple SP as well as any combination of SP and TAB as keyword separator whenever there's no explicit definition in dir-spec.txt?

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by atagar

Agreed that this is an ambiguity in the spec. It's more formally defined that the whitespace between the keyword and value can be any size...
https://gitweb.torproject.org/torspec.git/blob/HEAD:/dir-spec.txt#l203

But I'm not aware of anything that specifies the spaces within a KeywordLine's 'ArgumentChar+'. Personally I thought that they needed to be single spaces too, and vote for this being malformed data on tor's part.

As for stem, this shouldn't effect us due to how split() works...

>>> value = "free  EB2798A84E9D962DF13C61FA055C2513DDDAF800"
>>> value.split()
['free', 'EB2798A84E9D962DF13C61FA055C2513DDDAF800']

comment:2 Changed 7 years ago by karsten

Cc: nickm added

I can change metrics-lib to accept [SP|TAB]+ wherever it would expect a single SP right now. But I'd want to hear first if that's considered valid due to current dir-spec.txt. Nick?

comment:3 Changed 7 years ago by nickm

It's valid according to the spec, but we still shouldn't generate it.

comment:4 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-final

comment:5 Changed 7 years ago by nickm

Keywords: easy added

comment:6 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:7 Changed 7 years ago by nickm

Component: Tor RelayTor

comment:8 Changed 6 years ago by nickm

Priority: normaltrivial

I don't see how our code could generate this, unless you somehow got a nickname with a space in it, which ought to be impossible. Have there been any more of these? What versions of Tor generate them?

comment:9 Changed 6 years ago by karsten

Resolution: not a bug
Status: newclosed

I checked all descriptors from 2012-07 to 2012-12, and there hasn't been a single descriptor with two spaces in an extra-info line, except for the one posted above. I think it's safe to pretend this never happened and move on. Closing.

Note: See TracTickets for help on using tickets.