Opened 15 months ago

Last modified 6 months ago

#26316 new defect

Windows newlines in extrainfo descriptor

Reported by: atagar Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-spec, postfreeze-ok, 040-deferred-20190220
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hi Network Team, the following extrainfo descriptor is making Stem's parser error with "Malformed dirreq-stats-end line: dirreq-stats-end 2018-06-05 06:11:40 (86400 s)"...

extra-info Jittor E062E8AD9FA8907D140ECFC406D5DAFD4B23F300
identity-ed25519
-----BEGIN ED25519 CERT-----
AQQABnvMATGms3ONiqeM1KNjSmQQVTcmVFwb7pHjW6b4qB4DhqG/AQAgBACmvl+Y
QFLCTyOfsf9cL9qL7IDZLOceNdcTJSNGPzevmvgB827O9wO0IDxY4V10wDxHRiNG
TPy43S5ag5hWvyKzI3hGA/sz7+8NwnGEqFrjg9nlZt/FADvdC0Ow9owIHwU=
-----END ED25519 CERT-----
published 2018-06-05 12:27:03
geoip-db-digest 765D2F123DDA7DAEA9C9263975E2B4BAFBD2B908
geoip6-db-digest 2AA568CC898BF9924FBACEA26946AA7ED10E7D87
dirreq-stats-end 2018-06-05 06:11:40 (86400 s)^M
dirreq-v3-ips ru=488,us=400,de=264,ua=168,fr=160,gb=128,it=96,br=80,es=80,jp=80,ca=64,in=56,nl=56,pl=56,ar=32,au=32,tw=32,by=24,ch=24,cz=24,fi=24,id=24,ir=24,mx=24,ae=16,at=16,be=16,bg=16,gr=16,hk=16,hu=16,ie=16,il=16,lt=16,lv=16,my=16,no=16,pt=16,ro=16,rs=16,se=16,th=16,vn=16,??=8,ba=8,bd=8,bh=8,bj=8,ci=8,cl=8,cn=8,co=8,cr=8,cw=8,cy=8,dk=8,do=8,dz=8,ec=8,ee=8,et=8,ge=8,gh=8,gp=8,gt=8,gy=8,hr=8,iq=8,is=8,ke=8,kr=8,kw=8,kz=8,lk=8,lu=8,ma=8,md=8,me=8,mg=8,mu=8,mv=8,na=8,nz=8,pe=8,ph=8,pk=8,sd=8,sg=8,si=8,sk=8,sv=8,sy=8,tc=8,td=8,tn=8,tr=8,ug=8,uy=8,ve=8,za=8^M
dirreq-v3-reqs us=1464,ru=1048,de=752,fr=440,ua=280,gb=200,br=144,ca=136,it=136,jp=136,es=120,nl=120,pl=88,in=80,by=56,tw=56,au=48,ch=48,cz=48,id=48,md=48,se=48,ar=40,ir=40,ae=32,at=32,lt=32,mx=32,no=32,??=24,fi=24,hu=24,ie=24,il=24,lv=24,pt=24,ro=24,sg=24,vn=24,be=16,bg=16,cl=16,co=16,gr=16,hk=16,hr=16,is=16,kr=16,my=16,ph=16,rs=16,th=16,tr=16,ve=16,ba=8,bd=8,bh=8,bj=8,ci=8,cn=8,cr=8,cw=8,cy=8,dk=8,do=8,dz=8,ec=8,ee=8,et=8,ge=8,gh=8,gp=8,gt=8,gy=8,iq=8,ke=8,kw=8,kz=8,lk=8,lu=8,ma=8,me=8,mg=8,mu=8,mv=8,na=8,nz=8,pe=8,pk=8,sd=8,si=8,sk=8,sv=8,sy=8,tc=8,td=8,tn=8,ug=8,uy=8,za=8^M
dirreq-v3-resp ok=6200,not-enough-sigs=16,unavailable=0,not-found=0,not-modified=1120,busy=0^M
dirreq-v3-direct-dl complete=16,timeout=0,running=0,min=229451,d1=229451,d2=516011,q1=1012307,d3=1012307,d4=1059359,md=1587774,d6=2175000,d7=2758125,q3=3023437,d8=3023437,d9=3105875,max=3332493^M
dirreq-v3-tunneled-dl complete=6048,timeout=136,running=0,min=360,d1=147601,d2=281819,q1=344834,d3=417296,d4=547425,md=705125,d6=916070,d7=1265607,q3=1371936,d8=1660923,d9=2477000,max=41697000^M
hidserv-stats-end 2018-06-05 06:11:40 (86400 s)^M
hidserv-rend-relayed-cells 5973866 delta_f=2048 epsilon=0.30 bin_size=1024^M
hidserv-dir-onions-seen 262 delta_f=8 epsilon=0.30 bin_size=8^M
router-sig-ed25519 kZGzsa4k+78QHsyb/0FXWBJYxttV8KvVHV/LiMH1muGAk/f9fj9c+lYusnhK9sql+AB7r5b0j6uaNR7ZTCXLBw
router-signature
-----BEGIN SIGNATURE-----
WjdbB6hbV9cyHWMqglNNMfjuT+WUvOmqP+IvMgwCke5/cXXEHOzqVXsVFdXmEBsg
HQOjB4vlmK4n6diTc2O0LkUzZTk1XIQwZOyKjtFNi0OrQye6mJA9gKaCIQUF3CDZ
XL6dW0QQaXgURTlm3oApAsZ+vRzI/Wfq9YxjTCLc9u4=
-----END SIGNATURE-----

You can fetch this descriptor for yourself with...

curl http://154.35.175.225:80/tor/extra/fp/E062E8AD9FA8907D140ECFC406D5DAFD4B23F300

Note the "^M" trailing a handful (but not all) of lines. Those are Windows carriage returns (\r\n line endings). There's two issues here, first that somehow this is getting published on metric lines and second that the dirauths aren't rejecting this descriptor.

According to its server descriptor this relay (Jittor) is running Tor 0.3.3.6 on Linux.

That's right. Windows newlines on Linux. Pity we don't note the distro. :P

Thanks!

Child Tickets

Change History (9)

comment:1 Changed 15 months ago by nickm

Keywords: 035-proposed added
Milestone: Tor: unspecified
Severity: NormalMinor

Well, that's a confusing one!

My guess is that this relay was migrated from windows to Linux, and that the stats lines that end with CRLF were originally stored on Windows, and then copied verbatim from those files when they were reloaded. Or possible somebody was monkeying around with these files in a windows text-editor? That doesn't seem as likely though.

From dir-spec's POV, I think these lines are valid: ArgumentChar can be "any printing ascii character other than NL" currently according to the spec, and CR is printable.

comment:2 Changed 15 months ago by atagar

Severity: MinorNormal

Hi Nick. I disagree that these are valid according to the spec. If they were I'd adjust things on Stem's side. For example, here's the spec for dirreq-stats-end lines. It does not specify allowing a CR before the NL.

https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n919

comment:3 Changed 14 months ago by nickm

Keywords: 035-roadmap-proposed added; 035-proposed removed

comment:4 in reply to:  2 Changed 10 months ago by teor

Keywords: tor-spec added

Replying to atagar:

Hi Nick. I disagree that these are valid according to the spec. If they were I'd adjust things on Stem's side. For example, here's the spec for dirreq-stats-end lines. It does not specify allowing a CR before the NL.

https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n919

But the metadocument format allows lines containing CR:

    NL = The ascii LF character (hex value 0x0a).
    Document ::= (Item | NL)+
    Item ::= KeywordLine Object*
    KeywordLine ::= Keyword NL | Keyword WS ArgumentChar+ NL
    Keyword = KeywordChar+
    KeywordChar ::= 'A' ... 'Z' | 'a' ... 'z' | '0' ... '9' | '-'
    ArgumentChar ::= any printing ASCII character except NL.
    WS = (SP | TAB)+

And requires implementations to accept extra arguments:

   For forward compatibility, each item MUST allow extra arguments at the
   end of the line unless otherwise noted.  So if an item's description below
   is given as:
       "thing" int int int NL
   then implementations SHOULD accept this string as well:
       "thing 5 9 11 13 16 12" NL
   but not this string:
       "thing 5" NL
   and not this string:
       "thing 5 10 thing" NL
   .

   Whenever an item DOES NOT allow extra arguments, we will tag it with
   "no extra arguments".

If the line was:

dirreq-stats-end 2018-06-05 06:11:40 (86400 s) ^M

Then the trailing CR would be an "extra argument".

But since there is no WS between the final argument and the CR:

dirreq-stats-end 2018-06-05 06:11:40 (86400 s)^M

The final argument is malformed.

Let's resolve this issue by patching torspec to match the current behaviour of tor and metrics.

We could:

  • allow a trailing CR on each line
  • make CR part of WS
  • (something else?)

comment:5 Changed 10 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.6.x-final

comment:6 Changed 10 months ago by teor

Keywords: 035-roadmap-proposed removed

Remove 035-roadmap-proposed from items in 0.3.6.

comment:7 Changed 10 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:8 Changed 7 months ago by nickm

Keywords: postfreeze-ok added

Mark some tickets as postfreeze-ok, to indicate that I think they are okay to accept in 0.4.0 post-freeze. Does not indicate that they are all necessary to do postfreeze.

comment:9 Changed 6 months ago by nickm

Keywords: 040-deferred-20190220 added
Milestone: Tor: 0.4.0.x-finalTor: unspecified

Deferring 51 tickets from 0.4.0.x-final. Tagging them with 040-deferred-20190220 for visibility. These are the tickets that did not get 040-must, 040-can, or tor-ci.

Note: See TracTickets for help on using tickets.