Closed Bug 1124649 Opened 10 years ago Closed 9 years ago

when processing a STS or PKP header fails, the console incorrectly always warns of an invalid header

Categories

(Core :: Security: PSM, defect)

Product:
Core
Component:
Security: PSM
Version:
35 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox42 --- fixed

People

(Reporter: mathias, Assigned: Cykesiopka)

References

Details

Attachments

(3 files, 3 obsolete files)

bug1124649_part1_add-specific-err-msgs_v3.patch
39.17 KB, patch
Cykesiopka
: review+
Details | Diff | Splinter Review
bug1124649_part2_add-sts-webconsole-msg-tests_v2.patch
10.31 KB, patch
Cykesiopka
: review+
Details | Diff | Splinter Review
bug1124649_part3_add-pkp-webconsole-msg-tests_v1.patch
9.91 KB, patch
Cykesiopka
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 Build ID: 20150108202552 Steps to reproduce: Added Strict-Transport-Security header to web page using Web.config file, so it applies to all content served. I used the value "max-age=31536000; includeSubdomains", as suggested on https://www.owasp.org/index.php/HTTP_Strict_Transport_Security. I also tried without the "includeSubdomains" and different values for max-age. All like described on MDN page (https://developer.mozilla.org/en-US/docs/Web/Security/HTTP_strict_transport_security). <!-- from Web.config --> <add name="Strict-Transport-Security" value="max-age=31536000; includeSubdomains"/> Actual results: Header was sent, tested with "Live HTTP headers" plugin. And Firefox dev tools show Console message "The site specified an invalid Strict-Transport-Security header.[Learn More]". https://localhost/ GET / HTTP/1.1 Host: localhost User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 Accept-Language: en-US,en;q=0.5 Accept-Encoding: gzip, deflate Cookie: AnonymousID=gid=ffa789a7-0870-47a6-b67b-04989e48384b; UserTrackingID=gid=b7985823-bf0e-4a6f-9d02-b89bdb725475; optimizelySegments=%7B%22206370730%22%3A%22ff%22%2C%22206333231%22%3A%22false%22%2C%22204948447%22%3A%22direct%22%7D; optimizelyEndUserId=oeu1421769850141r0.6186617974969267; optimizelyBuckets=%7B%7D; _ga=GA1.1.1497968550.1421769850; adv_targeting=gid=cd954c60-a34d-4aac-b65f-018d26821d5d; POPUPCHECK=1421944643493; ASP.NET_SessionId=42wvebtmajcsmnh5xejfdjyn; website#lang=de-CH; __AntiXsrfToken=03251177a7f44b41acbcc0452644eda7 Connection: keep-alive HTTP/1.1 200 OK Cache-Control: no-cache, no-store Pragma: no-cache Content-Type: text/html; charset=utf-8 Content-Encoding: gzip Content-Language: de Expires: -1 Vary: Accept-Encoding Server: Microsoft-IIS/7.5 X-UA-Compatible: IE=edge X-AspNet-Version: 4.0.30319 Set-Cookie: AnonymousID=gid=ffa789a7-0870-47a6-b67b-04989e48384b; expires=Fri, 22-Apr-2016 12:35:13 GMT; path=/ Set-Cookie: UserTrackingID=gid=b7985823-bf0e-4a6f-9d02-b89bdb725475; expires=Fri, 22-Apr-2016 12:35:14 GMT; path=/ Set-Cookie: adv_targeting=gid=cd954c60-a34d-4aac-b65f-018d26821d5d; expires=Fri, 22-Apr-2016 12:35:19 GMT; path=/ X-Powered-By: ASP.NET P3P: policyref="http://www.comparis.ch/w3c/p3p.xml", CP="IND" Strict-Transport-Security: max-age=31536000; includeSubdomains X-XSS-Protection: 1; mode=block X-Content-Type-Options: nosniff Date: Thu, 22 Jan 2015 13:35:19 GMT Content-Length: 22319 Expected results: No message should be displayed for a valid header.
Blocks: hpkp
Component: Untriaged → Security: PSM
Product: Firefox → Core
HPKP != HSTS :) It looks like you are trying to do this on localhost. Are you using a self-signed cert and getting certificate warnings when visiting the page? I think we don't store HSTS headers in the case where there are cert warnings, but David would know for sure.
No longer blocks: hpkp
Flags: needinfo?(dkeeler)
Thanks for taking a look on this :-) The issue occurs for localhost and for our dev environment. And you are right, we use self-signed certificates for both environments. But when I understand you right, the issue would not occur on our live system with a "real" certificate.
Monica is correct - the underlying transport must be secure (i.e. no warnings or certificate error overrides) for a host to be noted as an HSTS host. The error message in the console is misleading, though. It looks like we display that error message regardless of why processing the header failed (i.e. it's currently assumed that the header was invalid, but it could also be the case that the transport was insecure (or, indeed, any number of other reasons)).
Flags: needinfo?(dkeeler)
Summary: After adding seemingly valid Strict-Transport-Security header, Console message states "invalid STS header" → when processing a Strict-Transport-Security header fails, the console incorrectly always warns of an invalid STS header
I've been working on some WIPs to make the web console print out more specific errors, so assigning to myself for now.
Assignee: nobody → cykesiopka.bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Bug 1124649 - Add specific error messages for various types of HSTS header failures.
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. Still working on the remaining test cases, but feedback on the approach taken here would be nice.
Attachment #8640340 - Flags: feedback?(dkeeler)
https://reviewboard.mozilla.org/r/14311/#review12979 This looks great! Were you going to add strings for the HPKP errors too? I think it would be good to do that at the same time. ::: netwerk/protocol/http/nsHttpChannel.cpp:1183 (Diff revision 1) > - * Process a single security header. Only two types are suported HSTS and HPKP. > + * Process a single security header. Only two types are supported HSTS and HPKP. As long as we're here, we might as well change 'are supported' to 'are supported:'
Attachment #8640340 - Flags: feedback?(dkeeler) → feedback+
https://reviewboard.mozilla.org/r/14311/#review12979 I currently have the HPKP changes in a separate patch, but I guess it does make sense to just do it all at once.
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. Bug 1124649 - Add specific error messages for various types of STS and PKP header failures.
Attachment #8640340 - Attachment description: MozReview Request: Bug 1124649 - Add specific error messages for various types of HSTS header failures. → MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures.
Attachment #8640340 - Flags: feedback+
Bug 1124649 - Add additional invalid STS header web console tests.
Comment on attachment 8642342 [details] MozReview Request: Bug 1124649 - Add additional invalid STS header web console tests. Hi jryans, Do the changes here look reasonable? I plan to replicate the same sort of structure for the PKP tests. Feel free to forward to someone else if you're busy. Thanks!
Attachment #8642342 - Flags: feedback?(jryans)
Comment on attachment 8642342 [details] MozReview Request: Bug 1124649 - Add additional invalid STS header web console tests. Seems reasonable to me!
Attachment #8642342 - Flags: feedback?(jryans) → feedback+
Summary: when processing a Strict-Transport-Security header fails, the console incorrectly always warns of an invalid STS header → when processing a STS or PKP header fails, the console incorrectly always warns of an invalid header
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. Bug 1124649 - Add specific error messages for various types of STS and PKP header failures.
Attachment #8640340 - Flags: review?(jryans)
Attachment #8640340 - Flags: review?(hurley)
Attachment #8640340 - Flags: review?(dkeeler)
Comment on attachment 8642342 [details] MozReview Request: Bug 1124649 - Add additional invalid STS header web console tests. Bug 1124649 - Add additional invalid STS header web console tests.
Attachment #8642342 - Flags: feedback+ → review?(jryans)
Bug 1124649 - Add invalid PKP header web console tests.
Attachment #8642811 - Flags: review?(jryans)
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. I'm not a reviewer for DOM / network changes, but the error text seems reasonable to me.
Attachment #8640340 - Flags: review?(jryans) → feedback+
Comment on attachment 8642342 [details] MozReview Request: Bug 1124649 - Add additional invalid STS header web console tests. https://reviewboard.mozilla.org/r/14713/#review13401 Ship It!
Attachment #8642342 - Flags: review?(jryans) → review+
Comment on attachment 8642811 [details] MozReview Request: Bug 1124649 - Add invalid PKP header web console tests. https://reviewboard.mozilla.org/r/14849/#review13409 It looks like for STS, we add a "Learn More" link to the console output[1]. It would be nice to link to such a page for PKP errors too, perhaps [this page][2]. [1]: https://dxr.allizom.org/mozilla-central/rev/abc56d57f6e1aebade48949fb557d26eae555df8/browser/devtools/webconsole/webconsole.js#1679 [2]: https://developer.mozilla.org/en-US/docs/Web/Security/Public_Key_Pinning
Attachment #8640340 - Flags: review?(hurley) → review+
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. https://reviewboard.mozilla.org/r/14311/#review13461 Just 2 little stylistic nits, please fix them before landing. Otherwise, the bits under netwerk/ look good to me. ::: netwerk/protocol/http/nsHttpChannel.cpp:1151 (Diff revision 3) > +void mark this "static void" ::: netwerk/protocol/http/nsHttpChannel.cpp:1185 (Diff revision 3) > +void mark this "static void" as well
Comment on attachment 8640340 [details] MozReview Request: Bug 1124649 - Add specific error messages for various types of STS and PKP header failures. https://reviewboard.mozilla.org/r/14311/#review13473 This is awesome. Just a minor nit. ::: netwerk/protocol/http/nsHttpChannel.cpp:1152 (Diff revision 3) > +GetSTSConsoleErrorTag(nsAString& consoleErrorTag, uint32_t failureResult) Slight nit: consoleErrorTag is an output parameter, right? I would expect it to be listed last in the arguments. ::: netwerk/protocol/http/nsHttpChannel.cpp:1186 (Diff revision 3) > +GetPKPConsoleErrorTag(nsAString& consoleErrorTag, uint32_t failureResult) Same here.
Attachment #8640340 - Flags: review?(dkeeler) → review+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #20) > It would be nice to link to such a page for PKP errors too, perhaps [this > page][2]. > > [1]: > https://dxr.allizom.org/mozilla-central/rev/ > abc56d57f6e1aebade48949fb557d26eae555df8/browser/devtools/webconsole/ > webconsole.js#1679 > [2]: https://developer.mozilla.org/en-US/docs/Web/Security/Public_Key_Pinning Sounds reasonable - I filed Bug 1191335 for this.
+ Address nits
Attachment #8640340 - Attachment is obsolete: true
Attachment #8643720 - Flags: review+
Attachment #8642342 - Attachment is obsolete: true
Attachment #8643721 - Flags: review+
Attachment #8642811 - Attachment is obsolete: true
Attachment #8643724 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/fe549ef1f881 https://hg.mozilla.org/mozilla-central/rev/b3d049d0dbe8 https://hg.mozilla.org/mozilla-central/rev/f1808548c6f3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: