1411659 - Issue parsing multi-line headers (CSP in this case)

Status: RESOLVED DUPLICATE of bug 1050329

(Core :: Networking: HTTP, defect, P3)

Description

[Joey2250]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce:

Using the following multi-line CSP header in NGINX

    add_header Content-Security-Policy "
        default-src 'self' *.google.com *.youtube.com *.facebook.com *.fonts.google.com *.fonts.googleapis.com *.google-analytics.com *.googleapis.com cdnjs.cloudflare.com code.jquery.com connect.facebook.net *.imgur.com *.500px.com www.reddit.com www.flickr.com c1.staticflickr.com maxcdn.bootstrapcdn.com code.ionicframework.com cdn.fontawesome.com;
        script-src 'self' 'unsafe-inline' 'unsafe-eval' *.discinsights.com *.google-analytics.com ajax.googleapis.com *.facebook.net *.facebook.com *.addthis.com *.zoho.com *.zohostatic.com *.addthisedge.com *.braintreegateway.com www.vimeo.com vimeo.com *.vimeocdn.com;
        style-src 'self' 'unsafe-inline' *.discinsights.com *.googleapis.com *.zoho.com *.zohostatic.com *.zohopublic.com;
        img-src 'self' *.discinsights.com *.google-analytics.com *.facebook.com *.doubleclick.net *.google.com *.paypalobjects.com *.vimeocdn.com data:;
        connect-src 'self' *.discinsights.com *.facebook.com *.zoho.com *.zohopublic.com *.addthis.com wss://vts.zohopublic.com;
        font-src 'self' *.discinsights.com themes.googleusercontent.com fonts.gstatic.com *.zohostatic.com data:;
        object-src 'none';
        media-src 'self';
        form-action 'self' *.discinsights.com *.facebook.com *.zoho.com;
        frame-src *.discinsights.com *.expedia.com *.facebook.com *.zendesk.com *.addthis.com *.braintreegateway.com *.vimeo.com http://*.vimeo.com;
        frame-ancestors *.discinsights.com theholyspirit.com *.peoplekeys.com studentkeys.com;
        report-uri https://peoplekeys.report-uri.io/r/default/csp/enforce;
    " always;


Actual results:

Firefox literally displayed nothing. The page didnt show an error, the existing page in the tab stayed active. Other web browsers worked with no issue (and chrome for sure parsed and followed the CSP)

The network inspector tab showed the correct SSL cert and I incorrectly thought it was due to the TLS setup stage. My troubleshooting of the SSL didnt turn up anything. 

I was then pointed to the HTTP logging in firefox and determined a decode error happened after my CSP header was sent. I removed the CSP header and the page displayed correctly. 

I readded the CSP header without line breaks and the page now displayed as well.

Removing the line breaks and using the following header worked in FireFox:
    add_header Content-Security-Policy "default-src 'self' *.google.com *.youtube.com *.facebook.com *.fonts.google.com *.fonts.googleapis.com *.google-analytics.com *.googleapis.com cdnjs.cloudflare.com code.jquery.com connect.facebook.net *.imgur.com *.500px.com www.reddit.com www.flickr.com c1.staticflickr.com maxcdn.bootstrapcdn.com code.ionicframework.com cdn.fontawesome.com; script-src 'self' 'unsafe-inline' 'unsafe-eval' *.discinsights.com *.google-analytics.com ajax.googleapis.com *.facebook.net *.facebook.com *.addthis.com *.zoho.com *.zohostatic.com *.addthisedge.com *.braintreegateway.com www.vimeo.com vimeo.com *.vimeocdn.com; style-src 'self' 'unsafe-inline' *.discinsights.com *.googleapis.com *.zoho.com *.zohostatic.com *.zohopublic.com; img-src 'self' *.discinsights.com *.google-analytics.com *.facebook.com *.doubleclick.net *.google.com *.paypalobjects.com *.vimeocdn.com data:; connect-src 'self' *.discinsights.com *.facebook.com *.zoho.com *.zohopublic.com *.addthis.com wss://vts.zohopublic.com; font-src 'self' *.discinsights.com themes.googleusercontent.com fonts.gstatic.com *.zohostatic.com data:; object-src 'none'; media-src 'self'; form-action 'self' *.discinsights.com *.facebook.com *.zoho.com; frame-src *.discinsights.com *.expedia.com *.facebook.com *.zendesk.com *.addthis.com *.braintreegateway.com *.vimeo.com http://*.vimeo.com; frame-ancestors *.discinsights.com theholyspirit.com *.peoplekeys.com studentkeys.com; report-uri https://peoplekeys.report-uri.io/r/default/csp/enforce;" always;

(see my ask a question: https://support.mozilla.org/en-US/questions/1181489 ) 

This was tested on v49 and v56 on Windows and v56.0.1 on OSX



Expected results:

There should have been an error page displayed. At the very least something letting me know that there was an error processing the page (or as fine-grained as saying there was an error processing the headers).

It literally just stopped loading the page and told me nothing.

Comment 1

[Daniel Veditz [:dveditz]]

What do you mean by "multi-line"? You mean there were line breaks in an HTTP header? That might be breaking in HTTP handling before it even gets to CSP if the next line doesn't look like an HTTP header.

What error did you see in the HTTP log?

Were there any errors on the _browser_ console? Sometimes errors are logged there that for various reasons don't make it to the devtools "web" console. You can open the browser console via the Tools menu, or Ctrl-Shift-J

Do you have a url that links to a broken test page so we can inspect this?

Comment 2

[Joey2250]

(In reply to Daniel Veditz [:dveditz] from comment #1)
> What do you mean by "multi-line"? You mean there were line breaks in an HTTP
> header? That might be breaking in HTTP handling before it even gets to CSP
> if the next line doesn't look like an HTTP header.
The line breaks were in the Nginx config, which was all enclosed by the "quotes", so I assume the line breaks were sent as well.
In Chrome it handled this and I was able to test my CSP and add what was needed to build out the full CSP I have listed. I assumed it would work in all browsers, which was my fault for assuming lol.

> 
> What error did you see in the HTTP log?
[Socket Thread]: I/nsHttp Http2Stream::ConvertResponseHeaders 0x12978f360 decode Error
This showed up right after the CSP header, (which by the way had line breaks in the HTTP log)


> 
> Were there any errors on the _browser_ console? Sometimes errors are logged
> there that for various reasons don't make it to the devtools "web" console.
> You can open the browser console via the Tools menu, or Ctrl-Shift-J
It wouldn' progress that far to log anything, as far as I could tell. I had the browser console open as well as the devtools, but nothing would get logged. It would literally stop and fall back to the existing page in the tab as if I never typed in the new URL. Nothing changed or indicated that anything happened, except for the network console. It showed on the timings tab it stopping during TLS setup, and it would load the SSL cert info. But then nothing. I have screenshots of my network console tab on my original support question at https://support.mozilla.org/en-US/questions/1181489

> 
> Do you have a url that links to a broken test page so we can inspect this?
I can set one up so we can see. I'll reply back with the URL.

Comment 3

[Joey2250]

OK I have set up the additional page with the multi-line header.

https://discinsights.com/csp-test.html

There is a simple hello world page there, along with a JS and CSS file loaded from the HEAD. The CSP is not set up to allow those to be loaded. 

You can see in Chrome they successfully get blocked by the CSP, but in FireFox the page doesn't even load. 

To see the same CSP, but with no line breaks go to https://discinsights.com/ It loads successfully in FF.

Comment 4

[Joey2250]

It looks like Chrome parses each line into a separate CSP header(judging by the chrome devtools network tab) and can interpret them as a whole that way.

Comment 5

[Daniel Veditz [:dveditz]]

This is not even getting to the CSP code in Firefox. Headers are defined as single lines separated by line-breaks and we just don't know what to do with what we get. Throwing over to the Networking component to see if whatever Chrome is doing for error recovery is reasonable, but that seems like it's opening the door to header injection.

Comment 6

[Honza Bambas (:mayhemer)]

Patrick, Nick, this happens inside the h2 header parser.  What is the expected outcome for parsing header values that are quoted and having linebreaks in the quoted span?  Looks like chrome somewhat accepts that, but I agree with Daniel's comment 5 (injection).

Comment 7

[Patrick McManus [:mcmanus]]

we're doing the right thing - 7540 10.3 mandates a whole session protocol error in the presence of a newline in a header value.


   While most of the values that can be encoded will not alter header
   field parsing, carriage return (CR, ASCII 0xd), line feed (LF, ASCII
   0xa), and the zero character (NUL, ASCII 0x0) might be exploited by
   an attacker if they are translated verbatim.  Any request or response
   that contains a character not permitted in a header field value MUST
   be treated as malformed (Section 8.1.2.6).  Valid characters are
   defined by the "field-content" ABNF rule in Section 3.2 of [RFC7230].

we can keep this bug open to consider a better error page but that's the only action I would take.

I'll mention it to chrome; they'll probably tighten up.

Comment 8

[u408661]

(In reply to Patrick McManus [:mcmanus] from comment #7)
> we can keep this bug open to consider a better error page but that's the
> only action I would take.

FWIW, I have bug 1050329 (that says when we receive GOAWAY, but I should change it to any GOAWAY sent or received) that I'm hoping to get to sometime in Q1 (in addition to my regularly scheduled stuff).

Comment 9

[Patrick McManus [:mcmanus]]

OP you might want to file an nginx bug - they should never produce invalid protocol frames.

Comment 10

[Joey2250]

Even a better error would, I think, be very beneficial. At the moment it just bails out without any notification at all to the user. Nailing down the actual issue here was very difficult without help from others.

I will try and see if I can get something submitted to NGINX as well.

Comment 11

[Honza Bambas (:mayhemer)]

Thanks Patrick.

So, this could be marked as duplicate of bug 1050329 then - report a better error message.

Comment 12

[Joey2250]

(In reply to Patrick McManus [:mcmanus] from comment #9)
> OP you might want to file an nginx bug - they should never produce invalid
> protocol frames.

What should I tell the NGINX team is the issue they need to look at? I mean is there anything technical I can reference that will help them know what is going on right off of the bat?

Comment 13

[Patrick McManus [:mcmanus]]

(In reply to Honza Bambas (:mayhemer) from comment #11)
> Thanks Patrick.
> 
> So, this could be marked as duplicate of bug 1050329 then - report a better
> error message.

I wouldn't dupe it.. the cause is different so it probably needs to be triggered from a second call site. Maybe nick would be happy to have it assigned though.

in the end its going to have to be somewhat generic - there are literally an infinite number of errors.

Comment 14

[Patrick McManus [:mcmanus]]

(In reply to Joey2250 from comment #12)
> (In reply to Patrick McManus [:mcmanus] from comment #9)
> > OP you might want to file an nginx bug - they should never produce invalid
> > protocol frames.
> 
> What should I tell the NGINX team is the issue they need to look at? I mean
> is there anything technical I can reference that will help them know what is
> going on right off of the bat?

I think comment 7 is decent.. it refers to rfc 7540 section 10.3 https://tools.ietf.org/html/rfc7540#section-10.3

Comment 15

[Patrick McManus [:mcmanus]]

and thanks to brad (thanks brad!) there's now a chrome bug, so I would expect them to change to also reject this.

https://bugs.chromium.org/p/chromium/issues/detail?id=787581

Comment 16

[Joey2250]

Submitted ticket #1435 to NGINX.

https://trac.nginx.org/nginx/ticket/1435

Comment 17

[zsebkecske]

FYI Now Chrome also rejects this, https://crbug.com/787581 is Fixed.  This will come out in release 64.

Comment 19

[u408661]

I am going to go ahead and dupe this to the GOAWAY error page bug. My intent there is to handle as many of the cases as are reasonable, and then we can add followups for new cases and/or make a generic h2 catchall that can say "just log it".