diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index 0474060d394c99..c0765f75642189 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -1,12 +1,9 @@ 'use strict'; const { - ArrayBufferIsView, - ArrayBufferPrototypeSlice, ArrayFrom, ArrayPrototypePush, SafeSet, - TypedArrayPrototypeSlice, } = primordials; const { @@ -28,8 +25,6 @@ const { kKeyVariantAES_GCM_256, kKeyVariantAES_KW_256, kKeyVariantAES_OCB_256, - kWebCryptoCipherDecrypt, - kWebCryptoCipherEncrypt, } = internalBinding('crypto'); const { @@ -143,80 +138,33 @@ function asyncAesKwCipher(mode, key, data) { getVariant('AES-KW', key[kAlgorithm].length))); } -async function asyncAesGcmCipher(mode, key, data, algorithm) { +function asyncAesGcmCipher(mode, key, data, algorithm) { const { tagLength = 128 } = algorithm; - const tagByteLength = tagLength / 8; - let tag; - switch (mode) { - case kWebCryptoCipherDecrypt: { - const slice = ArrayBufferIsView(data) ? - TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice; - tag = slice(data, -tagByteLength); - - // Refs: https://www.w3.org/TR/WebCryptoAPI/#aes-gcm-operations - // - // > If *plaintext* has a length less than *tagLength* bits, then `throw` - // > an `OperationError`. - if (tagByteLength > tag.byteLength) { - throw lazyDOMException( - 'The provided data is too small.', - 'OperationError'); - } - data = slice(data, 0, -tagByteLength); - break; - } - case kWebCryptoCipherEncrypt: - tag = tagByteLength; - break; - } - - return await jobPromise(() => new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], data, getVariant('AES-GCM', key[kAlgorithm].length), algorithm.iv, - tag, + tagByteLength, algorithm.additionalData)); } -async function asyncAesOcbCipher(mode, key, data, algorithm) { +function asyncAesOcbCipher(mode, key, data, algorithm) { const { tagLength = 128 } = algorithm; - const tagByteLength = tagLength / 8; - let tag; - switch (mode) { - case kWebCryptoCipherDecrypt: { - const slice = ArrayBufferIsView(data) ? - TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice; - tag = slice(data, -tagByteLength); - - // Similar to GCM, OCB requires the tag to be present for decryption - if (tagByteLength > tag.byteLength) { - throw lazyDOMException( - 'The provided data is too small.', - 'OperationError'); - } - data = slice(data, 0, -tagByteLength); - break; - } - case kWebCryptoCipherEncrypt: - tag = tagByteLength; - break; - } - - return await jobPromise(() => new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], data, getVariant('AES-OCB', key.algorithm.length), algorithm.iv, - tag, + tagByteLength, algorithm.additionalData)); } diff --git a/lib/internal/crypto/chacha20_poly1305.js b/lib/internal/crypto/chacha20_poly1305.js index 0979d7aaddbb61..a2b7c1fb04fb89 100644 --- a/lib/internal/crypto/chacha20_poly1305.js +++ b/lib/internal/crypto/chacha20_poly1305.js @@ -1,19 +1,14 @@ 'use strict'; const { - ArrayBufferIsView, - ArrayBufferPrototypeSlice, ArrayFrom, SafeSet, - TypedArrayPrototypeSlice, } = primordials; const { ChaCha20Poly1305CipherJob, KeyObjectHandle, kCryptoJobAsync, - kWebCryptoCipherDecrypt, - kWebCryptoCipherEncrypt, } = internalBinding('crypto'); const { @@ -46,35 +41,13 @@ function validateKeyLength(length) { throw lazyDOMException('Invalid key length', 'DataError'); } -async function c20pCipher(mode, key, data, algorithm) { - let tag; - switch (mode) { - case kWebCryptoCipherDecrypt: { - const slice = ArrayBufferIsView(data) ? - TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice; - - if (data.byteLength < 16) { - throw lazyDOMException( - 'The provided data is too small.', - 'OperationError'); - } - - tag = slice(data, -16); - data = slice(data, 0, -16); - break; - } - case kWebCryptoCipherEncrypt: - tag = 16; - break; - } - - return await jobPromise(() => new ChaCha20Poly1305CipherJob( +function c20pCipher(mode, key, data, algorithm) { + return jobPromise(() => new ChaCha20Poly1305CipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], data, algorithm.iv, - tag, algorithm.additionalData)); } diff --git a/src/crypto/crypto_aes.cc b/src/crypto/crypto_aes.cc index b5495e59737eb6..fa619696ffd5b2 100644 --- a/src/crypto/crypto_aes.cc +++ b/src/crypto/crypto_aes.cc @@ -76,24 +76,28 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, } size_t tag_len = 0; + size_t data_len = in.size(); if (params.cipher.isGcmMode() || params.cipher.isOcbMode()) { + tag_len = params.length; switch (cipher_mode) { case kWebCryptoCipherDecrypt: { - // If in decrypt mode, the auth tag must be set in the params.tag. - CHECK(params.tag); + // In decrypt mode, the auth tag is appended to the end of the + // ciphertext. Split it off and set it on the cipher context. + if (data_len < tag_len) { + return WebCryptoCipherStatus::FAILED; + } + data_len -= tag_len; - // For OCB mode, we need to set the auth tag length before setting the - // tag if (params.cipher.isOcbMode()) { - if (!ctx.setAeadTagLength(params.tag.size())) { + if (!ctx.setAeadTagLength(tag_len)) { return WebCryptoCipherStatus::FAILED; } } ncrypto::Buffer buffer = { - .data = params.tag.data(), - .len = params.tag.size(), + .data = in.data() + data_len, + .len = tag_len, }; if (!ctx.setAeadTag(buffer)) { return WebCryptoCipherStatus::FAILED; @@ -101,14 +105,6 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, break; } case kWebCryptoCipherEncrypt: { - // In encrypt mode, we grab the tag length here. We'll use it to - // ensure that that allocated buffer has enough room for both the - // final block and the auth tag. Unlike our other AES-GCM implementation - // in CipherBase, in WebCrypto, the auth tag is concatenated to the end - // of the generated ciphertext and returned in the same ArrayBuffer. - tag_len = params.length; - - // For OCB mode, we need to set the auth tag length if (params.cipher.isOcbMode()) { if (!ctx.setAeadTagLength(tag_len)) { return WebCryptoCipherStatus::FAILED; @@ -122,7 +118,7 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, } size_t total = 0; - int buf_len = in.size() + ctx.getBlockSize() + tag_len; + int buf_len = data_len + ctx.getBlockSize() + (encrypt ? tag_len : 0); int out_len; ncrypto::Buffer buffer = { @@ -148,9 +144,9 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, // Refs: https://github.com/nodejs/node/pull/38913#issuecomment-866505244 buffer = { .data = in.data(), - .len = in.size(), + .len = data_len, }; - if (in.empty()) { + if (data_len == 0) { out_len = 0; } else if (!ctx.update(buffer, ptr, &out_len)) { return WebCryptoCipherStatus::FAILED; @@ -381,42 +377,17 @@ bool ValidateCounter( return true; } -bool ValidateAuthTag( - Environment* env, - CryptoJobMode mode, - WebCryptoCipherMode cipher_mode, - Local value, - AESCipherConfig* params) { - switch (cipher_mode) { - case kWebCryptoCipherDecrypt: { - if (!IsAnyBufferSource(value)) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env); - return false; - } - ArrayBufferOrViewContents tag_contents(value); - if (!tag_contents.CheckSizeInt32()) [[unlikely]] { - THROW_ERR_OUT_OF_RANGE(env, "tagLength is too big"); - return false; - } - params->tag = mode == kCryptoJobAsync - ? tag_contents.ToCopy() - : tag_contents.ToByteSource(); - break; - } - case kWebCryptoCipherEncrypt: { - if (!value->IsUint32()) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env); - return false; - } - params->length = value.As()->Value(); - if (params->length > 128) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env); - return false; - } - break; - } - default: - UNREACHABLE(); +bool ValidateAuthTag(Environment* env, + Local value, + AESCipherConfig* params) { + if (!value->IsUint32()) { + THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env); + return false; + } + params->length = value.As()->Value(); + if (params->length > 128) { + THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env); + return false; } return true; } @@ -451,8 +422,7 @@ AESCipherConfig::AESCipherConfig(AESCipherConfig&& other) noexcept cipher(other.cipher), length(other.length), iv(std::move(other.iv)), - additional_data(std::move(other.additional_data)), - tag(std::move(other.tag)) {} + additional_data(std::move(other.additional_data)) {} AESCipherConfig& AESCipherConfig::operator=(AESCipherConfig&& other) noexcept { if (&other == this) return *this; @@ -466,7 +436,6 @@ void AESCipherConfig::MemoryInfo(MemoryTracker* tracker) const { if (mode == kCryptoJobAsync) { tracker->TrackFieldWithSize("iv", iv.size()); tracker->TrackFieldWithSize("additional_data", additional_data.size()); - tracker->TrackFieldWithSize("tag", tag.size()); } } @@ -510,7 +479,7 @@ Maybe AESCipherTraits::AdditionalConfig( return Nothing(); } } else if (params->cipher.isGcmMode() || params->cipher.isOcbMode()) { - if (!ValidateAuthTag(env, mode, cipher_mode, args[offset + 2], params) || + if (!ValidateAuthTag(env, args[offset + 2], params) || !ValidateAdditionalData(env, mode, args[offset + 3], params)) { return Nothing(); } diff --git a/src/crypto/crypto_aes.h b/src/crypto/crypto_aes.h index 401ef70a5eba9f..5627f9020bad54 100644 --- a/src/crypto/crypto_aes.h +++ b/src/crypto/crypto_aes.h @@ -52,7 +52,6 @@ struct AESCipherConfig final : public MemoryRetainer { size_t length; ByteSource iv; // Used for both iv or counter ByteSource additional_data; - ByteSource tag; // Used only for authenticated modes (GCM) AESCipherConfig() = default; diff --git a/src/crypto/crypto_chacha20_poly1305.cc b/src/crypto/crypto_chacha20_poly1305.cc index bfe904c49ad771..0fd3e0517317ca 100644 --- a/src/crypto/crypto_chacha20_poly1305.cc +++ b/src/crypto/crypto_chacha20_poly1305.cc @@ -54,63 +54,6 @@ bool ValidateIV(Environment* env, return true; } -bool ValidateAuthTag(Environment* env, - CryptoJobMode mode, - WebCryptoCipherMode cipher_mode, - Local value, - ChaCha20Poly1305CipherConfig* params) { - switch (cipher_mode) { - case kWebCryptoCipherDecrypt: { - if (!IsAnyBufferSource(value)) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH( - env, "Authentication tag must be a buffer"); - return false; - } - - ArrayBufferOrViewContents tag(value); - if (!tag.CheckSizeInt32()) [[unlikely]] { - THROW_ERR_OUT_OF_RANGE(env, "tag is too large"); - return false; - } - - if (tag.size() != kChaCha20Poly1305TagSize) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH( - env, "Invalid authentication tag length"); - return false; - } - - if (mode == kCryptoJobAsync) { - params->tag = tag.ToCopy(); - } else { - params->tag = tag.ToByteSource(); - } - break; - } - case kWebCryptoCipherEncrypt: { - // For encryption, the value should be the tag length (passed from - // JavaScript) We expect it to be the tag size constant for - // ChaCha20-Poly1305 - if (!value->IsUint32()) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env, "Tag length must be a number"); - return false; - } - - uint32_t tag_length = value.As()->Value(); - if (tag_length != kChaCha20Poly1305TagSize) { - THROW_ERR_CRYPTO_INVALID_TAG_LENGTH( - env, "Invalid tag length for ChaCha20-Poly1305"); - return false; - } - // Tag is generated during encryption, not provided - break; - } - default: - UNREACHABLE(); - } - - return true; -} - bool ValidateAdditionalData(Environment* env, CryptoJobMode mode, Local value, @@ -138,8 +81,7 @@ ChaCha20Poly1305CipherConfig::ChaCha20Poly1305CipherConfig( : mode(other.mode), cipher(other.cipher), iv(std::move(other.iv)), - additional_data(std::move(other.additional_data)), - tag(std::move(other.tag)) {} + additional_data(std::move(other.additional_data)) {} ChaCha20Poly1305CipherConfig& ChaCha20Poly1305CipherConfig::operator=( ChaCha20Poly1305CipherConfig&& other) noexcept { @@ -154,7 +96,6 @@ void ChaCha20Poly1305CipherConfig::MemoryInfo(MemoryTracker* tracker) const { if (mode == kCryptoJobAsync) { tracker->TrackFieldWithSize("iv", iv.size()); tracker->TrackFieldWithSize("additional_data", additional_data.size()); - tracker->TrackFieldWithSize("tag", tag.size()); } } @@ -179,17 +120,9 @@ Maybe ChaCha20Poly1305CipherTraits::AdditionalConfig( return Nothing(); } - // Authentication tag parameter (only for decryption) or tag length (for - // encryption) - if (static_cast(args.Length()) > offset + 1) { - if (!ValidateAuthTag(env, mode, cipher_mode, args[offset + 1], params)) { - return Nothing(); - } - } - // Additional authenticated data parameter (optional) - if (static_cast(args.Length()) > offset + 2) { - if (!ValidateAdditionalData(env, mode, args[offset + 2], params)) { + if (static_cast(args.Length()) > offset + 1) { + if (!ValidateAdditionalData(env, mode, args[offset + 1], params)) { return Nothing(); } } @@ -229,23 +162,25 @@ WebCryptoCipherStatus ChaCha20Poly1305CipherTraits::DoCipher( return WebCryptoCipherStatus::FAILED; } - size_t tag_len = 0; + size_t tag_len = kChaCha20Poly1305TagSize; + size_t data_len = in.size(); switch (cipher_mode) { case kWebCryptoCipherDecrypt: { - if (params.tag.size() != kChaCha20Poly1305TagSize) { + if (data_len < tag_len) { return WebCryptoCipherStatus::FAILED; } + data_len -= tag_len; + if (!ctx.setAeadTag(ncrypto::Buffer{ - .data = params.tag.data(), - .len = params.tag.size(), + .data = in.data() + data_len, + .len = tag_len, })) { return WebCryptoCipherStatus::FAILED; } break; } case kWebCryptoCipherEncrypt: { - tag_len = kChaCha20Poly1305TagSize; break; } default: @@ -253,7 +188,7 @@ WebCryptoCipherStatus ChaCha20Poly1305CipherTraits::DoCipher( } size_t total = 0; - int buf_len = in.size() + ctx.getBlockSize() + tag_len; + int buf_len = data_len + ctx.getBlockSize() + (encrypt ? tag_len : 0); int out_len; // Process additional authenticated data if present @@ -271,9 +206,9 @@ WebCryptoCipherStatus ChaCha20Poly1305CipherTraits::DoCipher( // Process the input data buffer = { .data = in.data(), - .len = in.size(), + .len = data_len, }; - if (in.empty()) { + if (data_len == 0) { if (!ctx.update({}, ptr, &out_len)) { return WebCryptoCipherStatus::FAILED; } diff --git a/src/crypto/crypto_chacha20_poly1305.h b/src/crypto/crypto_chacha20_poly1305.h index 5b4d5cde2c3929..f56b1a2e74a152 100644 --- a/src/crypto/crypto_chacha20_poly1305.h +++ b/src/crypto/crypto_chacha20_poly1305.h @@ -17,7 +17,6 @@ struct ChaCha20Poly1305CipherConfig final : public MemoryRetainer { ncrypto::Cipher cipher; ByteSource iv; ByteSource additional_data; - ByteSource tag; ChaCha20Poly1305CipherConfig() = default; diff --git a/test/parallel/test-crypto-webcrypto-aes-decrypt-tag-too-small.js b/test/parallel/test-crypto-webcrypto-aes-decrypt-tag-too-small.js index 589a2f91a17cc2..2f0612db2bc253 100644 --- a/test/parallel/test-crypto-webcrypto-aes-decrypt-tag-too-small.js +++ b/test/parallel/test-crypto-webcrypto-aes-decrypt-tag-too-small.js @@ -24,6 +24,5 @@ subtle.importKey( }, k, new Uint8Array(0)); }, { name: 'OperationError', - message: /The provided data is too small/, }) ).then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-aead-decrypt-detached-buffer.js b/test/parallel/test-webcrypto-aead-decrypt-detached-buffer.js new file mode 100644 index 00000000000000..a96e709095430f --- /dev/null +++ b/test/parallel/test-webcrypto-aead-decrypt-detached-buffer.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { hasOpenSSL } = require('../common/crypto'); +const { subtle } = globalThis.crypto; + +async function test(algorithmName, keyLength, ivLength, format = 'raw') { + const key = await subtle.importKey( + format, + new Uint8Array(keyLength), + { name: algorithmName }, + false, + ['encrypt', 'decrypt'], + ); + + const data = new Uint8Array(32); + data.buffer.transfer(); + + await assert.rejects( + subtle.decrypt({ name: algorithmName, iv: new Uint8Array(ivLength) }, key, data), + { name: 'OperationError' }, + ); +} + +const tests = [ + test('AES-GCM', 32, 12), +]; + +if (hasOpenSSL(3)) { + tests.push(test('AES-OCB', 32, 12, 'raw-secret')); +} + +if (!process.features.openssl_is_boringssl) { + tests.push(test('ChaCha20-Poly1305', 32, 12, 'raw-secret')); +} + +Promise.all(tests).then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-encrypt-decrypt-chacha20-poly1305.js b/test/parallel/test-webcrypto-encrypt-decrypt-chacha20-poly1305.js index aea9528f2463db..5362484288089d 100644 --- a/test/parallel/test-webcrypto-encrypt-decrypt-chacha20-poly1305.js +++ b/test/parallel/test-webcrypto-encrypt-decrypt-chacha20-poly1305.js @@ -189,7 +189,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) { const iv = globalThis.crypto.getRandomValues(new Uint8Array(12)); await assert.rejects( subtle.decrypt({ name: 'ChaCha20-Poly1305', iv }, secretKey, new Uint8Array(8)), - { name: 'OperationError', message: /The provided data is too small/ } + { name: 'OperationError' } ); // Test invalid tagLength values