1905691 - (CVE-2024-7531) PK11_Encrypt using CKM_CHACHA20 and aliased input / output buffers reveals plaintext on Intel Sandy Bridge machines

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Lars Eggert [:lars]]

With the libnss3-dev_2%3a3.98-1build1_amd64.deb package installed via apt and used for neqo on an Ubuntu 24.04 GitHub CI runner, we're seeing two CHACHA20 tests fail.

Because of @martinthomson's comment on GitHub, I'm marking this as a security bug. (It may not be.) He said:

That failure is pretty troubling. If that were to happen in practice, it would mean a complete failure to encrypt, without hitting the error path.

Looking at the function that fails, we have a completely fresh HpKey instance that is successfully created. If something went wrong there, that's not likely to result in that sort of error, so we'll skip that.

The failure has to be in the hp.mask() call. That function takes a 16 byte buffer of zeros and encrypts it using ChaCha20. What we're seeing here is a complete failure to do anything to those zero values, but the function is returning OK. The only way I can see that happening is if NSS is somehow returning OK. Maybe someone on the crypto team would be able to help there. I can't spot the broken link.

A docker image built during CI from the PR also sees all the CHACHA20-related interop tests with other QUIC stacks failing. (C20 tags in this comment.)

The odd thing is that I tried to recreate the issue in a local Ubuntu 24.04 VM on my Mac, and the tests pass there using the NSS apt package. They also pass in GitHub when compiling NSS from source.

I also briefly tried an older Ubuntu 22.04 GitHub runner, which now also has NSS 3.98 available via apt, and it sees the same failure.

Comment 1

[Daniel Veditz [:dveditz]]

John: Please take a look and see if this is a security problem for Firefox or not, or pass it along to the right person on the team to look at ChaCha20 issues.

Comment 2

[John Schanck [:jschanck]]

Neqo-crypto's HpKey::mask function calls the NSS PK11_Encrypt function with mechanism == CKM_CHACHA20 and out == data. This ultimately calls ChaCha20Xor with output == block. Here's the full source for ChaCha20Xor as of NSS 3.72 / Bug 1727555 after preprocessing for x86-64:

$ unifdef -DNSS_X64 -UNSS_DISABLE_AVX2 -UNSS_DISABLE_SSE3 <source>
void
ChaCha20Xor(uint8_t *output, uint8_t *block, uint32_t len, uint8_t *k, uint8_t *nonce, uint32_t ctr)
{
    if (avx2_support()) {
        Hacl_Chacha20_Vec256_chacha20_encrypt_256(len, output, block, k, nonce, ctr);
    }

    if (ssse3_support() && sse4_1_support() && avx_support()) {
        Hacl_Chacha20_Vec128_chacha20_encrypt_128(len, output, block, k, nonce, ctr);
    }

    {
        Hacl_Chacha20_chacha20_encrypt(len, output, block, k, nonce, ctr);
    }
}

On machines where avx2_support(), ssse3_support(), sse4_1_support() and avx_support() all evaluate to true, this is equivalent to:

void
ChaCha20Xor(uint8_t *output, uint8_t *block, uint32_t len, uint8_t *k, uint8_t *nonce, uint32_t ctr)
{
        Hacl_Chacha20_Vec256_chacha20_encrypt_256(len, output, block, k, nonce, ctr);
        Hacl_Chacha20_Vec128_chacha20_encrypt_128(len, output, block, k, nonce, ctr);
        Hacl_Chacha20_chacha20_encrypt(len, output, block, k, nonce, ctr);
}

Which is computationally wasteful but harmless.

However, on machines where avx2_support() evaluates to false and the others evaluate to true (e.g. Intel Sandy Bridge) we get:

void
ChaCha20Xor(uint8_t *output, uint8_t *block, uint32_t len, uint8_t *k, uint8_t *nonce, uint32_t ctr)
{
        Hacl_Chacha20_Vec128_chacha20_encrypt_128(len, output, block, k, nonce, ctr);
        Hacl_Chacha20_chacha20_encrypt(len, output, block, k, nonce, ctr);
}

Because output == block, the call to Hacl_Chacha20_chacha20_encrypt has the effect of decrypting the output in place.

As far as I can tell, HpKey::mask is the only function in mozilla-central using PK11_Encrypt in this way. I don't know enough about QUIC header protection to assess the consequences, but it's safe to say this is a sec-high for NSS.

I should also note that PK11_Encrypt with mechanism == CKM_CHACHA20_POLY1305 is not affected, as that ultimately calls
ChaCha20Poly1305_Seal instead of ChaCha20Xor. Despite some superficial similarities between the two functions, ChaCha20Poly1305_Seal does not contain the same bug (note the goto's). Otherwise this would be a sec-crit.

Comment 3

[Anna Weine]

Attachment: Bug 1905691 - ChaChaXor to return after the function

Comment 4

[Anna Weine]

Hi,
Is it possible to check that the attached patch solves the problem?

Thanks

Comment 5

[Lars Eggert [:lars]]

I'd say yes, except that when I compile NSS from source, the bug doesn't occur, probably because during the configure stage, the defines are initialized correctly. I think the issue happens when the configure-time detected CPU instruction support differs from the one at runtime.

(That said, your patch appears correct.)

Comment 6

[Daniel Veditz [:dveditz]]

I assume this will affect ESR-115 as well—or is this exposed by a more recent necko change?

Comment 7

[John Schanck [:jschanck]]

I'd like to hear more about the impact on Firefox before backporting the fix to NSS 3.90 / Firefox 115. We might be able to treat this as a sec-high for NSS and a sec-moderate or sec-low for Firefox.

The bug can also be avoided from the neqo side by using separate input and output buffers, as is done in the ssl_CreateMaskInner function in libssl: https://searchfox.org/mozilla-central/rev/01aaa47e62a2015e7641f26ab0bc2bb00ab579b8/security/nss/lib/ssl/sslprimitive.c#402-415. That might be a simpler / more robust fix for older versions of Firefox and platforms that might rely on system NSS.

Comment 8

[Martin Thomson [:mt:]]

Let's do that. It's not that much more work and probably not any slower to do it that way. Well, aside from the multiple passes, I guess. I can do that; I think we can get away with a public pull request as long as we get the fix in Firefox in quickly. Are there patch releases we might need to get in for? Are we going to uplift?

Comment 9

[Martin Thomson [:mt:]]

I worked out that I can hide the change: https://github.com/mozilla/neqo/pull/1978

Note that we already outed this as a potential issue in public comments, so this is probably already out of the bag if someone was really looking to attack this.

Comment 10

[Anna Weine]

Attachment: Bug 1905691 - ChaChaXor to return after the function

Original Revision: https://phabricator.services.mozilla.com/D215892

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/nss/rev/525c5044cc9e53f5015c697b04b1405df91003ac

We're nearly into RC week for this cycle, so we need to decide pretty quickly what if anything needs to be done for ESR115, ESR128, and Fx129 (Beta).

Comment 12

[John Schanck [:jschanck]]

I'm backporting this to

• NSS 3.90.4 for ESR 115,
• NSS 3.101.2 for ESR 128, and
• NSS 3.102.1 for Beta 129.

Uplift requests coming shortly.

Comment 13

[Donal Meehan [:dmeehan]]

Bug 1907387 uplifted to beta
Bug 1909713 uplifted to esr128
Bug 1909714 uplifted to esr115

Comment 14

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/b0b0b68f8c764bf7848ca94f8b011e4bf192dc3f (3.90.4)
https://hg.mozilla.org/projects/nss/rev/7e33e341dbdfd24a5dece35fa8a616e1e51b7c3f (3.101.2)
https://hg.mozilla.org/projects/nss/rev/a66b3cb8c642abf1dfec3b7767f7e11578e68d2d (3.102.1)

Comment 15

[Martin Thomson [:mt:]]

Since Tyson asked:

This affects Firefox, but not in a security-relevant way. We only use this cipher in one very specific way: QUIC header protection.

On certain hardware, Firefox would fail to apply QUIC header protection when the ChaCha20-Poly1305 cipher suite was negotiated. This would likely result in QUIC connection failures, though it is possible that connections would continue with extremely high packet loss rates (the odds of this are low, but implausibly so; the best case is 1/4096 packets getting through, though 1/8192 is a more relevant number).

The use of QUIC header protection is designed to counter a minor privacy risk where packets from the same connection can be identified as coming from the same source when the connection moves to a new network path. This encryption failure negates that privacy protection.

So there is a privacy loss when:

• a ChaCha20-Poly1305 cipher suite is negotiated and the connection does not immediately fail due to extremely high packet loss,
• the Firefox client moves to a new network path due to forces outside of our control (we don't deliberately migrate to new networks, so this is also quite unlikely), and
• there is an observer in a position to observe the unencrypted packet sequence numbers.

Oh, and because we are not in control of the network path change, there is a very high chance that we're sending packets with a connection ID that provides the observer with a far easier way of linking the activity across paths. If we were in control of the path change, we'd switch that connection ID for a fresh one, but we don't have the ability to do that yet.

All of which to say is that this is not a big deal for Firefox, as disastrous as this might seem.

Comment 16

[Tyson Smith [:tsmith]]

Attachment: advisory.txt