Opened 2 years ago

Last modified 20 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:


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
-----BEGIN ED25519 CERT-----
-----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

You can fetch this descriptor for yourself with...


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 on Linux.

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


Child Tickets

Change History (9)

comment:1 Changed 2 years 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 2 years 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.

comment:3 Changed 2 years ago by nickm

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

comment:4 in reply to:  2 Changed 2 years 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.

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 2 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.6.x-final

comment:6 Changed 2 years ago by teor

Keywords: 035-roadmap-proposed removed

Remove 035-roadmap-proposed from items in 0.3.6.

comment:7 Changed 2 years 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 22 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 20 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.