-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Update BoringSSL, add SHA3 to WebCrypto and node:crypto #29323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e6ca20c
Update BoringSSL and add SHA3 support
Jarred-Sumner 14bef08
[autofix.ci] apply automated fixes
autofix-ci[bot] 8ce301b
Update process.versions boringssl hash in test snapshot
Jarred-Sumner 62a8a4d
Update baseline allowlist for namespaced BoringSSL x25519 ADX symbols
Jarred-Sumner 22a55d1
Allow SHA3 digests in PBKDF2 now that BoringSSL supports them
Jarred-Sumner 8d39b78
Address review: RSA+SHA3 JWK export, CryptoHasher HMAC, test fixes
Jarred-Sumner dc79dd8
[autofix.ci] apply automated fixes
autofix-ci[bot] 4514cea
Remove unused throw scope from SubtleCrypto::digest
Jarred-Sumner 4be6752
Properly check throw scope after normalizeCryptoAlgorithmParameters i…
Jarred-Sumner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /* | ||
| * Copyright (C) 2024 Apple Inc. All rights reserved. | ||
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions | ||
| * are met: | ||
| * 1. Redistributions of source code must retain the above copyright | ||
| * notice, this list of conditions and the following disclaimer. | ||
| * 2. Redistributions in binary form must reproduce the above copyright | ||
| * notice, this list of conditions and the following disclaimer in the | ||
| * documentation and/or other materials provided with the distribution. | ||
| * | ||
| * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' | ||
| * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, | ||
| * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS | ||
| * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR | ||
| * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF | ||
| * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS | ||
| * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN | ||
| * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) | ||
| * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF | ||
| * THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
|
|
||
| #include "config.h" | ||
| #include "CryptoAlgorithmSHA3.h" | ||
|
|
||
| #if ENABLE(WEB_CRYPTO) | ||
|
|
||
| #include "CryptoDigest.h" | ||
| #include "ScriptExecutionContext.h" | ||
|
|
||
| namespace WebCore { | ||
|
|
||
| static void dispatchDigest(PAL::CryptoDigest::Algorithm algorithm, | ||
| Vector<uint8_t>&& message, CryptoAlgorithm::VectorCallback&& callback, | ||
| CryptoAlgorithm::ExceptionCallback&& exceptionCallback, | ||
| ScriptExecutionContext& context, WorkQueue& workQueue) | ||
| { | ||
| auto digest = PAL::CryptoDigest::create(algorithm); | ||
| if (!digest) { | ||
| exceptionCallback(OperationError, ""_s); | ||
| return; | ||
| } | ||
|
|
||
| if (message.size() < 64) { | ||
| auto moved = WTF::move(message); | ||
| digest->addBytes(moved.begin(), moved.size()); | ||
| auto result = digest->computeHash(); | ||
| ScriptExecutionContext::postTaskTo(context.identifier(), | ||
| [callback = WTF::move(callback), result = WTF::move(result)](auto&) { | ||
| callback(result); | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| workQueue.dispatch(context.globalObject(), | ||
| [digest = WTF::move(digest), message = WTF::move(message), | ||
| callback = WTF::move(callback), | ||
| contextIdentifier = context.identifier()]() mutable { | ||
| digest->addBytes(message.begin(), message.size()); | ||
| auto result = digest->computeHash(); | ||
| ScriptExecutionContext::postTaskTo(contextIdentifier, | ||
| [callback = WTF::move(callback), result = WTF::move(result)](auto&) { | ||
| callback(result); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| #define DEFINE_SHA3(ClassName, DigestAlgo) \ | ||
| Ref<CryptoAlgorithm> ClassName::create() { return adoptRef(*new ClassName); } \ | ||
| CryptoAlgorithmIdentifier ClassName::identifier() const { return s_identifier; } \ | ||
| void ClassName::digest(Vector<uint8_t>&& message, VectorCallback&& callback, \ | ||
| ExceptionCallback&& exceptionCallback, ScriptExecutionContext& context, \ | ||
| WorkQueue& workQueue) \ | ||
| { \ | ||
| dispatchDigest(DigestAlgo, WTF::move(message), WTF::move(callback), \ | ||
| WTF::move(exceptionCallback), context, workQueue); \ | ||
| } | ||
|
|
||
| DEFINE_SHA3(CryptoAlgorithmSHA3_256, PAL::CryptoDigest::Algorithm::SHA3_256) | ||
| DEFINE_SHA3(CryptoAlgorithmSHA3_384, PAL::CryptoDigest::Algorithm::SHA3_384) | ||
| DEFINE_SHA3(CryptoAlgorithmSHA3_512, PAL::CryptoDigest::Algorithm::SHA3_512) | ||
|
|
||
| #undef DEFINE_SHA3 | ||
|
|
||
| } // namespace WebCore | ||
|
|
||
| #endif // ENABLE(WEB_CRYPTO) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /* | ||
| * Copyright (C) 2024 Apple Inc. All rights reserved. | ||
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions | ||
| * are met: | ||
| * 1. Redistributions of source code must retain the above copyright | ||
| * notice, this list of conditions and the following disclaimer. | ||
| * 2. Redistributions in binary form must reproduce the above copyright | ||
| * notice, this list of conditions and the following disclaimer in the | ||
| * documentation and/or other materials provided with the distribution. | ||
| * | ||
| * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' | ||
| * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, | ||
| * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS | ||
| * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR | ||
| * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF | ||
| * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS | ||
| * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN | ||
| * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) | ||
| * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF | ||
| * THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| #if ENABLE(WEB_CRYPTO) | ||
|
|
||
| #include "CryptoAlgorithm.h" | ||
|
|
||
| namespace WebCore { | ||
|
|
||
| class CryptoAlgorithmSHA3_256 final : public CryptoAlgorithm { | ||
| public: | ||
| static constexpr ASCIILiteral s_name = "SHA3-256"_s; | ||
| static constexpr CryptoAlgorithmIdentifier s_identifier = CryptoAlgorithmIdentifier::SHA3_256; | ||
| static Ref<CryptoAlgorithm> create(); | ||
|
|
||
| private: | ||
| CryptoAlgorithmSHA3_256() = default; | ||
| CryptoAlgorithmIdentifier identifier() const final; | ||
| void digest(Vector<uint8_t>&&, VectorCallback&&, ExceptionCallback&&, ScriptExecutionContext&, WorkQueue&) final; | ||
| }; | ||
|
|
||
| class CryptoAlgorithmSHA3_384 final : public CryptoAlgorithm { | ||
| public: | ||
| static constexpr ASCIILiteral s_name = "SHA3-384"_s; | ||
| static constexpr CryptoAlgorithmIdentifier s_identifier = CryptoAlgorithmIdentifier::SHA3_384; | ||
| static Ref<CryptoAlgorithm> create(); | ||
|
|
||
| private: | ||
| CryptoAlgorithmSHA3_384() = default; | ||
| CryptoAlgorithmIdentifier identifier() const final; | ||
| void digest(Vector<uint8_t>&&, VectorCallback&&, ExceptionCallback&&, ScriptExecutionContext&, WorkQueue&) final; | ||
| }; | ||
|
|
||
| class CryptoAlgorithmSHA3_512 final : public CryptoAlgorithm { | ||
| public: | ||
| static constexpr ASCIILiteral s_name = "SHA3-512"_s; | ||
| static constexpr CryptoAlgorithmIdentifier s_identifier = CryptoAlgorithmIdentifier::SHA3_512; | ||
| static Ref<CryptoAlgorithm> create(); | ||
|
|
||
| private: | ||
| CryptoAlgorithmSHA3_512() = default; | ||
| CryptoAlgorithmIdentifier identifier() const final; | ||
| void digest(Vector<uint8_t>&&, VectorCallback&&, ExceptionCallback&&, ScriptExecutionContext&, WorkQueue&) final; | ||
| }; | ||
|
|
||
| } // namespace WebCore | ||
|
|
||
| #endif // ENABLE(WEB_CRYPTO) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 When exporting an HMAC-SHA3-256/384/512 key as JWK, no alg field is emitted (the switch in exportKey falls through with a bare break at lines 182-188 of CryptoAlgorithmHMAC.cpp), and the import checkAlgCallback at lines 125-131 accepts any SHA3 JWK whenever alg is null — so a SHA3-256 key exported to JWK can be silently re-imported as SHA3-512. The JWK roundtrip is lossy with respect to the hash variant; consider throwing NotSupportedError for SHA3 HMAC JWK export/import, or documenting that JWK does not preserve the SHA3 hash variant.
Extended reasoning...
What the bug is and how it manifests
When exportKey('jwk', hmacSha3Key) is called, CryptoAlgorithmHMAC.cpp lines 182-188 match all three SHA3 variants and execute a bare break without setting jwk.alg. This is intentional — no IANA-registered JWK alg identifiers exist for HMAC-SHA3. The exported JWK therefore looks like {kty:'oct', k:'...'} with no alg field for any SHA3 variant.
The specific code path that triggers it
During importKey('jwk', jwkData, {name:'HMAC', hash:'SHA3-512'}), the code reaches CryptoKeyHMAC::importJwk which calls the checkAlgCallback lambda at lines 125-131. For all three SHA3 identifiers the callback returns alg.isNull(). Since the exported JWK never sets alg regardless of which SHA3 variant was used, alg.isNull() is always true — the check passes silently. The import then proceeds with SHA3_512 as the hash but with the key material of the original SHA3-256 key.
Why existing code does not prevent it
For SHA-2 HMAC, exportKey always sets jwk.alg to a specific string (HS256, HS384, etc.), and the import callback verifies alg == 'HS512' (for example). Cross-variant confusion is rejected at import time. For SHA3, no analogous IANA-registered alg string exists, so the export produces no alg field and the import check is reduced to the vacuous alg.isNull() condition — which is true for all SHA3 export JWKs and therefore cannot distinguish between variants.
What the impact would be
The JWK roundtrip does not preserve the SHA3 hash variant. A developer who exports a SHA3-256 HMAC key, transmits the JWK, and re-imports it with hash:'SHA3-512' will get an HMAC-SHA3-512 key backed by 32-byte key material without any error. HMAC values computed with the original key will not match those from the re-imported key, silently breaking correctness guarantees.
Step-by-step proof
How to fix it
Option A (clean): Return NotSupportedError from exportKey and importKey for SHA3 HMAC in JWK format, making the limitation explicit and consistent with the existing rejection of SHA3 as a standalone importKey algorithm in SubtleCrypto.cpp lines 381-387.
Option B (best-effort): Emit a non-standard alg string (e.g. HS3-256) and verify it in the import callback, preventing cross-variant confusion at the cost of interoperability with other implementations that follow the JWK spec strictly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches Node.js's webcrypto behavior:
There's no IETF-registered JWK
algvalue for HMAC-SHA3 or RSA-SHA3, so omitting it is the only correct behavior — emitting a made-up identifier would break import in other implementations. The hash variant must be supplied at import time, same as it would be for analg-less JWK from any source. Throwing on export would make the feature useless for key persistence with no real safety benefit (re-importing with wrong hash → wrong-size signatures → fails immediately on first verify, not silent corruption).