1.99 KB, patch
|Details | Diff | Splinter Review|
2.02 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2342.2 Safari/537.36 Steps to reproduce: (1) Launch https://mallory.csrf.jp:8020/altsvccertbypass/ (2) You can see the certificate error page. This is a correct behavior. (3) Launch http://csrf.jp/altsvccertbypass/ . This page sets Alt-Svc header h2="mallory.csrf.jp:8020". (4) Reload the page. (5) Instead of the page shown in (3), https://mallory.csrf.jp:8020/altsvccertbypass/ is shown as an alternative http/2 service. (6) Open a new tab and launch https://mallory.csrf.jp:8020/altsvccertbypass/ again. (7) You can NOT see the certificate error page. Actual results: In above (5) and (7), invalid certificate page https://mallory.csrf.jp:8020/altsvccertbypass/ is shown. Expected results: The server certificate used by mallory.csrf.jp:8020 is only valid for 'csrf.jp' and 'alice.csrf.jp'. Firefox should not show the page.
Further my investigation revealed that above attack can bypass HPKP and it works on the latest stable release (36.0.4). 1. Set a record "184.108.40.206 twitter.com" to your iptables or hosts file. 2. Open about:config and set security.cert_pinning.enforcement_level = 2(Strict) 3. Open http://csrf.jp/altsvc2/ 4. Reload the page. 5. Open a new tab and open https://twitter.com:8020/altsvc2/ At 5, you can see fake twitter.com with bypassing preloading HPKP. Expected behavior here is the "Secure Connection Failed" error page is shown.
It seems the cause is mis-implementation of opportunistic encryption on http/2. Section 4 of Opportunistic Security for HTTP specification document indicates that "https:" can be used only when the server certificate is successfully authenticated. https://tools.ietf.org/html/draft-ietf-httpbis-http2-encryption-01#section-4 However, Firefox permits "https:" host with invalid certificate after receiving Alt-Svc.
Patrick: I assume this is in your ballpark?
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking: HTTP
Ever confirmed: true
Product: Firefox → Core
Confirmed on Linux.
OS: Windows 8 → All
Hardware: x86 → All
still looking; but an update: [Launch http://csrf.jp/altsvccertbypass/ . This page sets Alt-Svc header h2="mallory.csrf.jp:8020".] the certificate on mallory.csrf.jp:8020 is signed and valid for csrf.jp and alice.csrf.jp - though not for mallory.csrf.jp Cross host use of alt-svc requrires authorization, and in this case mallory 8020 presents a valid signed cert (according to our trust anchor list) for the correct origin (csrf.jp) so the mapping is allowed. So steps 1 through 5 of comment 0 appear to be operating correctly. (at least for me in step 5 there is no change of origin to https as the comment indicates - that would be wrong, but I think the comment just means the content came back from mallory 8020 as intended) I haven't figured out yet how this makes https://mallory.csrf.jp:8020 validate
(In reply to Patrick McManus [:mcmanus] from comment #6) > > Cross host use of alt-svc requrires authorization, and in this case mallory > s/authorization/authentication
Comment 1 may or may not be the same issue - I haven't triaged it yet. The issue with Comment 0 is the TLS session resumption (and or session ticket) cache. We go to make a connection to https://malory:8020 and somehow manage to resume the http://csrf.jp connection that correctly used that same route earlier. Resumption implicitly bypasses the validation. security.ssl.disable_session_identifiers set to true gives the expected behavior - fingering that interaction. thanks :keeler Its not immediately obvious how to fix the root cause, so we should disable the feature. That should just be a pref flip but I want to run through a few tests before I tee that up.
It seems like it could be that we're attaching certain status to cached information when we connect successfully. I've seen patches on Chromium that fragment the NSS session cache to prevent various problems (specifically bug 1134548). Maybe there is something to look at there.
Thank you Muneakai These attacks don't rely on OE; they are a general alt-svc implementation problem. Comment 1 seems to boil down to the same trigger as security.ssl.disable_session_identifiers set to true breaks that STR as well.. it also requires DNS cache poisoning to an arbitrary domain. (There is also the odd problem that they do not reproduce on a clean profile for reasons as yet unknown - but they do with my normal profile. note to self to be careful when verifying) I can verify that disabling via "network.http.atsvc.enabled = false" fixes the problem. Note the embarrassing typo in that pref name. We should also set the correct spelling (s/atsvc/altsvc)to false to match the intention of the code (though its a nop because of the typo) as well as the network.a[l]tsvc.oe to false simply because it depends on the other pref for consistency - though its not strictly required.
I need to do childcare for a couple hours, I'll come back then with a patch.. but if there is a need to do it in the interim setting the 4 prefs below to false should be sufficient network.http.atsvc.enabled [*] network.http.atsvc.oe network.http.altsvc.enabled network.http.altsvc.oe [*] this is the only one strictly required - I would like all 4 though.
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
So that patch is for >=37. Due to the typo in pref name in the default config file, is plausible that this code could be exploited 35 (where it landed) or 36 as well. Telemetry shows only a tiny usage of the feature in general on those branches: 19 transactions on 35 (which could very well have been me) and a little less than 800 on 36. Dan, I know we have a mechanism for pushing pref changes independent of builds. I don't really know how it works - would that make sense here for 35,36 (maybe 37?)
(In reply to Patrick McManus [:mcmanus] from comment #14) This just makes the test coverage match the new prefs.
Lawrence: we need to chemspill/hotfix/respin this fix for Firefox 37.
Comment on attachment 8586432 [details] [diff] [review] disable alt-svc r=dveditz We don't want to land this patch until we are shipping a fix or hotfix to users. [Security approval request comment] How easily could an exploit be constructed based on the patch? - it's not a direct path, but it may invite investigation into this new feature and it's likely someone else would think to check this. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? - No Which older supported branches are affected by this flaw? - Alt-Svc was first landed in 35 If not all supported branches, which bug introduced the flaw? - The Alt-Svc implementation first landed in bug 1003448 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? - We're just disabling the feature: same patch, no risk How likely is this patch to cause regressions; how much testing does it need? - Hardly anyone uses this feature, none rely on it. Approval Request Comment [Feature/regressing bug #]: bug 1003448 [User impact if declined]: MITM your bank, even cert-pinned sites [Describe test coverage new/current, TreeHerder]: [Risks and why]: none [String/UUID change made/needed]: none
After speaking with dveditz I understand that this bug does not warrant a chemspill but does need to ship in a matter of days. We are going to release a 37.0.1 point release for stability issues. This bug can be included in the release. If we can't get the 37.0.1 release together in the next couple of days, we should make a decision about shipping this fix using an add-on hotfix.
If we're going with the point release I'll make a rollup patch that includes the test disabling to match the pref change.
Please do the roll up patch and have it ready, if you would.
rollup patch. I checked that it applies to each of 40, 39, 38 (with a little line fuzz). 37 requires a different patch because of a whitespace cleanup only in the test.
Attachment #8587015 - Flags: review+
Approval Request Comment See Comment 17
Attachment #8586432 - Attachment is obsolete: true
Attachment #8586484 - Attachment is obsolete: true
Attachment #8586432 - Flags: sec-approval?
Attachment #8586432 - Flags: approval-mozilla-release?
Attachment #8586432 - Flags: approval-mozilla-beta?
Attachment #8586432 - Flags: approval-mozilla-aurora?
Attachment #8587017 - Flags: review+
Attachment #8587017 - Flags: approval-mozilla-release?
Comment on attachment 8587015 [details] [diff] [review] Disable Alt Svc and test for 38, 39, 40 Approval Request Comment See Comment 17
(In reply to Patrick McManus [:mcmanus] from comment #21) > 37 requires a different patch because of a whitespace cleanup only in the > test. To be clear, a whitespace cleanup in the test landed on 38 at some time in the past. That creates a merge conflict with the disabling patch and is why a different version is needed for 37. The two versions can trivially be seen to be the same other than whitespace in the line being deleted in the test in both.
Duplicate of this bug: 1150203
Comment on attachment 8587017 [details] [diff] [review] Disable Alt-Svc and Test for release 37 We're gtb with 37.0.1 tonight. Release+
Attachment #8587017 - Flags: approval-mozilla-release? → approval-mozilla-release+
Comment on attachment 8587015 [details] [diff] [review] Disable Alt Svc and test for 38, 39, 40 Approving the disablement of this feature on all branches. Note that Nightly, Aurora, and Release should be patched on Apr 3. Beta won't receive an update unless we need to do one out of band until Apr 7. Beta+ Aurora+
The hotfix commit msg ended up referencing this bug instead of the hotfix bug, so for anyone who arrives here looking for the other one: bug 1150731
Depends on: 1150731
We tried to reproduce the issues from comment 0 and comment 1 using Firefox 37.0, on a Linux machine and two Windows machines but without success. Please let me know if there is something in particular we could setup to reproduce these problems (comment 10 suggests this does not reproduce on a clean profile).
Patrick, could you perhaps verify this on your machine with the profile that reproduces the issue?
Florin, can you see a red page with 'HTTP/2 Host' at step (4) on comment 1? If no, Alt-Svc doesn't work on your environment and it may be the reason why you don't reproduce.
(In reply to Muneaki Nishimura from comment #35) > Florin, can you see a red page with 'HTTP/2 Host' at step (4) on comment 1? > If no, Alt-Svc doesn't work on your environment and it may be the reason why > you don't reproduce. No, at step 4 I get the same page as at step 3.
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #36) > No, at step 4 I get the same page as at step 3. Thanks, can you show this page without error? https://alice.csrf.jp:8020 This page uses h2 with valid certificate. If you cannot see it, your network prohibits to access port 8020 or your Firefox cannot speak h2 correctly.
(In reply to Muneaki Nishimura from comment #37) > (In reply to Florin Mezei, QA (:FlorinMezei) from comment #36) > > No, at step 4 I get the same page as at step 3. > > Thanks, can you show this page without error? > https://alice.csrf.jp:8020 > > This page uses h2 with valid certificate. > If you cannot see it, your network prohibits to access port 8020 or your > Firefox cannot speak h2 correctly. I can see it without error in Chrome, but I get the Untrusted Connection page with Firefox 37.0.1. By the way Muneaki, do you think you could confirm this fix with the 37.0.1 build: ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/37.0.1-candidates/build1/?
If you use 37.0.1 and you got same page as step (3) at step(4) as written in comment 36, this is correct behavior since Alt-Svc may be successfully disabled. However, you should see https://alice.csrf.jp:8020 on 37.0.1 without any error. I downloaded 37.0.1 for OSX from above ftp and I could confirmed the page is correctly shown without error.
I can confirm that the STR from comment 0 is reproducible in 37 and not reproducible (i.e. fixed) in 37.0.1 for me on both linux and windows.
Thank you both for confirming this! I'm marking this as verified for Firefox 37.0.1.
Firefox for Android 37.0.1 Nexus 5 ( Android 5.0.1) I've tried to reproduce this on mobile side. I get the same behavior describe by Florin from Bug 1148328#c35 to Comment 39. ( same page after reload and "This Connection is Untrusted") I am not able to verify this. Is there a change any of you can try it on mobile side too?
Ioana - I can repro on 37.0 on my android phone. can you point me at the 37.0.1 candidate for android?
its android 4.4.4
(In reply to Patrick McManus [:mcmanus] from comment #43) > Ioana - I can repro on 37.0 on my android phone. can you point me at the > 37.0.1 candidate for android? I'm not Ioana, but builds are up at ftp://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/37.0.1-candidates/build1/
ftp://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/37.0.1-candidates/build1/android-api-11/en-US/fennec-37.0.1.en-US.android-arm.apk on android 4.4.4 confirm repro on 37 and fix on 37.0.1
(In reply to Patrick McManus [:mcmanus] from comment #10) > (There is also the odd problem that they do not reproduce on a clean profile > for reasons as yet unknown - but they do with my normal profile. note to > self to be careful when verifying) > I wanted to report back on this item, because it played a factor in verifying the fix. The reason the profile state mattered was that the alternate service that was being verified (mallory.csrf.jp) was expected to have a cert valid for the origin (csrf.jp) - and it did have the correct leaf cert but it did not bundle the complete set of intermediates. The validation failed on a clean profile with SEC_ERROR_UNKNOWN_ISSUER (normal for that problem) if the state of the browser did not include the necessary intermediates (from startssl iirc), and it passed (and enabled the alternate) if the intermediate was in the browser state. So particular detail is working correctly.
You need to log in before you can comment on or make changes to this bug.