1730935 - (CVE-2021-38507) Opportunistic Security for HTTP/2 opt-in checking partial bypass

Status: RESOLVED FIXED

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

Description

[Takeshi Terada]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/93.0.4577.63 Safari/537.36

Steps to reproduce:

This report is about a Firefox's bug in opt-in process of OE (Opportunistic Encryption, RFC 8164).

Let's say the association between 80 (unencrypted) and 443 (encrypted) is made by opt-in checking using .well-known JSON. Then content from port 443 (i.e. alternative service) is treated as if it is provided from port 80. It means, by design, opted-in encrypted content can be read by unencrypted content thanks to Same Origin Policy.

---

■example.jp (IPAddr: 1.1.1.1)
80 - Open to the internet. /.well-known/http-opportunistic is placed.
443 (h2) - Same as port 80.
8443 (TLS) - It serves secret file.

The problem is that Firefox performs opt-in checking only in the first h2 connection to alternative service. This means active network attacker can forward second or sebsequent connection to a port other than 443 (8443 port above, encrypted, not opted-in). The encrypted content from 8443 port can be read by unencrypted content. This way, Firefox's implementation of OE put non opted-in TLS ports at risk.

An attack example is as follows. The attacker controls DNS records and has a server (2.2.2.2) on the internet. Port 8443 on example.jp (1.1.1.1) is the attack target.

---

■Attacker's server (IPAddr: 2.2.2.2)
80 - It serves a copy of .well-known JSON file and a page for attack.

(1)
Attacker manipulates DNS record: example.jp. 0 IN A 2.2.2.2.
Attacker configures port forwarding: 2.2.2.2:8443 to 1.1.1.1:443.

(2)
Victim user navigates to http://example.jp/ (2.2.2.2).

Alt-Svc: h2="example.jp:8443"; ma=120

<iframe src="/test"></iframe>
<script>
// Get iframe source repeatedly
setInterval(() => {
var i = frames[0];
i.location.reload();
setTimeout(() => {console.log(i.document.body.textContent)}, 1000);
}, 5000);
</script>

Opt-in is OK because h2 connection to example.jp:8443 (2.2.2.2:8443) is forwarded to 1.1.1.1:443.

(3)
Attacker restores DNS record: example.jp. 0 IN A 1.1.1.1.
Attacker then shuts down port forwarding running at 2.2.2.2:8443.
JavaScript in (2) now fetches content from 8443 TLS with no errors.

Possible fix for this issue is to perform opt-in checking upon every new h2 connection to alternative service (obviously, non opted-in ports could detect this kind of attack by checking Alt-Used or Host header (port number), but necessity for such defense is not mentioned in RFC).

One more thing to note is that, incompliant with last paragraph of RFC 7838 section 2.4, Firefox checks ALPN protocol name ONLY on the first connection to alternative service. In other words, no protocol name checking is performed in the second and subsequent connections (same goes for upgrading http/1.1 TLS to h2). This increases the possibility of successful exploitation including cross-protocol attack, so it's safer to limit alternative service connections to h2/h3 only.

Comment 1

[Martin Thomson [:mt:]]

Seems pretty serious. A service needs to opt-in, but it seems like any would do. However, if a pair of ports (one unsecured, one secured) opt in, then all secured ports on the same host are vulnerable to this sort of exfiltration.

I think that the answer is essentially as suggested: request /.well-known/http-opportunistic from the alternative. What I suspect is happening here is that we do that, but the request to the alternative is being cached with the original http://example.jp origin. Attributing the request to that origin is what the spec requires. However, we probably aren't being sufficiently careful about segmenting the cache for these requests. Adding an additional origin attribute with the identity of the alternative for this entry might be necessary to avoid picking up a cache entry that was made on a previous connection to an alternative.

+Dragana who might be better able to work through the networking code here.

I'm less sure about the ALPN name check. In part because that check is not especially effective. That text was added to the spec to manage the risk of protocol version downgrade, but we have since learned a lot more about that (and invented new ways to downgrade things without detection; thanks QUIC). I don't think that we need to do anything that acknowledge that we are not compliant with that requirement. I don't see any significant security consequence from that. Yes, we might get worse TLS configuration if we attempt http/1.1, but our baseline profile is not so far from the h2 requirements that this is worth scrambling to put a fix in. That said, if we can ensure that we validate ALPN when we make requests to alternatives, it wouldn't hurt to do so. (I might track that part separately.)

Comment 2

[Dragana Damjanovic [:dragana]]

(In reply to Martin Thomson [:mt:] from comment #1)

Seems pretty serious. A service needs to opt-in, but it seems like any would do. However, if a pair of ports (one unsecured, one secured) opt in, then all secured ports on the same host are vulnerable to this sort of exfiltration.

I think that the answer is essentially as suggested: request /.well-known/http-opportunistic from the alternative. What I suspect is happening here is that we do that, but the request to the alternative is being cached with the original http://example.jp origin. Attributing the request to that origin is what the spec requires. However, we probably aren't being sufficiently careful about segmenting the cache for these requests. Adding an additional origin attribute with the identity of the alternative for this entry might be necessary to avoid picking up a cache entry that was made on a previous connection to an alternative.

I am not sure that I understand the last sentence. Are you suggesting to make the cache entry connected to a of a H2 connection. As long as a connection that was verified is alive the AltSvc can be used?

Anyway knowing our implementation, this can be fix (somewhat easy) but the consequence will be that the feature is even less often used.
Currently we do not block requests and wait for a validation to happen. If we require validation every time and we do not block, the feature will be use even less often.
Doing a proper fix, i.e. blocking requests, will need a bigger refactoring. Usage of this feature is not high to justify the refactoring at the moment.
I would suggest rather disabling the feature.

This would have been mitigated if the altSvc port was authenticated as well, but it is not.

+Dragana who might be better able to work through the networking code here.

I'm less sure about the ALPN name check. In part because that check is not especially effective. That text was added to the spec to manage the risk of protocol version downgrade, but we have since learned a lot more about that (and invented new ways to downgrade things without detection; thanks QUIC). I don't think that we need to do anything that acknowledge that we are not compliant with that requirement. I don't see any significant security consequence from that. Yes, we might get worse TLS configuration if we attempt http/1.1, but our baseline profile is not so far from the h2 requirements that this is worth scrambling to put a fix in. That said, if we can ensure that we validate ALPN when we make requests to alternatives, it wouldn't hurt to do so. (I might track that part separately.)

Comment 3

[Martin Thomson [:mt:]]

I'm suggesting that you perform the validation for every connection that is used this way, but you allow the confirmation request to be cached to make it more performant, using an expanded cache key that includes the identity of the alternative. That would make the validation fast in those cases where the alternative doesn't move, but would ensure that validation is robust to changes in the alternative (including hostnames).

Of course, if this is easier to fix another way, even if it it means slower validation, that might be better.

(We can't validate the port number. That is in most ways a good thing, because validating ports is not a useful concept. I'm glad our forebears didn't authenticate it as port numbers are a poor substitute for identifying a service. I would say the same for ALPN as a protocol is not a service either. Not authenticating port is only not good in the sense that we have to deal with the ways that ports can swap around, like this case here.)

Comment 4

[Kershaw Chang [:kershaw]]

Changing severity to S3 because this feature is already existed for a long time.
We'll discuss this at our team meeting and decide the next step.

Comment 5

[Daniel Veditz [:dveditz]]

Calling this sec-high because it's a same-origin policy violation against the port that didn't opt-in. How serious would depend on how many servers use this kind of set up (an opted-in TLS port and other TLS services not opted-in).

Comment 6

[Dragana Damjanovic [:dragana]]

Attachment: Disable Opportunistic Encryption, the feature has a low usage.

Comment 7

[Dragana Damjanovic [:dragana]]

Comment on attachment 9244176 [details]
Disable Opportunistic Encryption, the feature has a low usage.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The patch just disable a feature that is not use a lot. The patch or comments do not show what is the real reason for disabling the feature.
• 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?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: It is easy, this is only a pref flip.
• How likely is this patch to cause regressions; how much testing does it need?: Not likely. It disable a feature by a pref.

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9244176 [details]
Disable Opportunistic Encryption, the feature has a low usage.

Approved to land and uplift

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Disable Opportunistic Encryption, the feature has a low usage. r=kershaw,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/acc9a2f97b7e93cfcdfc36306d22f68d3f0f4964
https://hg.mozilla.org/mozilla-central/rev/acc9a2f97b7e

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta & ESR91 approval.

Comment 12

[Dragana Damjanovic [:dragana]]

Comment on attachment 9244176 [details]
Disable Opportunistic Encryption, the feature has a low usage.

Beta/Release Uplift Approval Request

• User impact if declined: This affect servers and this is a same-origin policy violation against the port that didn't opt-in.
  User will not see an effect.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only disables a feature via a pref.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high
• User impact if declined: This affect servers and this is a same-origin policy violation against the port that didn't opt-in.
  User will not see an effect.
• Fix Landed on Version: 95
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only disables a feature via a pref.
• String or UUID changes made by this patch:

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9244176 [details]
Disable Opportunistic Encryption, the feature has a low usage.

Approved for 94.0b6

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1fb919a4ac2e

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9244176 [details]
Disable Opportunistic Encryption, the feature has a low usage.

Approved for 91.3esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/821ded0b92ba

Comment 17

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: advisory.txt