Skip to content

Commit 4d96e53

Browse files
panvaaduh95
authored andcommitted
crypto: refactor WebCrypto AEAD algorithms auth tag handling
PR-URL: #62169 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 93d7771 commit 4d96e53

9 files changed

Lines changed: 91 additions & 227 deletions

‎lib/internal/crypto/aes.js‎

Lines changed: 6 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
'use strict';
22

33
const{
4-
ArrayBufferIsView,
5-
ArrayBufferPrototypeSlice,
64
ArrayFrom,
75
ArrayPrototypePush,
86
SafeSet,
9-
TypedArrayPrototypeSlice,
107
}=primordials;
118

129
const{
@@ -28,8 +25,6 @@ const {
2825
kKeyVariantAES_GCM_256,
2926
kKeyVariantAES_KW_256,
3027
kKeyVariantAES_OCB_256,
31-
kWebCryptoCipherDecrypt,
32-
kWebCryptoCipherEncrypt,
3328
}=internalBinding('crypto');
3429

3530
const{
@@ -143,80 +138,33 @@ function asyncAesKwCipher(mode, key, data) {
143138
getVariant('AES-KW',key[kAlgorithm].length)));
144139
}
145140

146-
asyncfunctionasyncAesGcmCipher(mode,key,data,algorithm){
141+
functionasyncAesGcmCipher(mode,key,data,algorithm){
147142
const{ tagLength =128}=algorithm;
148-
149143
consttagByteLength=tagLength/8;
150-
lettag;
151-
switch(mode){
152-
casekWebCryptoCipherDecrypt: {
153-
constslice=ArrayBufferIsView(data) ?
154-
TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice;
155-
tag=slice(data,-tagByteLength);
156-
157-
// Refs: https://www.w3.org/TR/WebCryptoAPI/#aes-gcm-operations
158-
//
159-
// > If *plaintext* has a length less than *tagLength* bits, then `throw`
160-
// > an `OperationError`.
161-
if(tagByteLength>tag.byteLength){
162-
throwlazyDOMException(
163-
'The provided data is too small.',
164-
'OperationError');
165-
}
166144

167-
data=slice(data,0,-tagByteLength);
168-
break;
169-
}
170-
casekWebCryptoCipherEncrypt:
171-
tag=tagByteLength;
172-
break;
173-
}
174-
175-
returnawaitjobPromise(()=>newAESCipherJob(
145+
returnjobPromise(()=>newAESCipherJob(
176146
kCryptoJobAsync,
177147
mode,
178148
key[kKeyObject][kHandle],
179149
data,
180150
getVariant('AES-GCM',key[kAlgorithm].length),
181151
algorithm.iv,
182-
tag,
152+
tagByteLength,
183153
algorithm.additionalData));
184154
}
185155

186-
asyncfunctionasyncAesOcbCipher(mode,key,data,algorithm){
156+
functionasyncAesOcbCipher(mode,key,data,algorithm){
187157
const{ tagLength =128}=algorithm;
188-
189158
consttagByteLength=tagLength/8;
190-
lettag;
191-
switch(mode){
192-
casekWebCryptoCipherDecrypt: {
193-
constslice=ArrayBufferIsView(data) ?
194-
TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice;
195-
tag=slice(data,-tagByteLength);
196-
197-
// Similar to GCM, OCB requires the tag to be present for decryption
198-
if(tagByteLength>tag.byteLength){
199-
throwlazyDOMException(
200-
'The provided data is too small.',
201-
'OperationError');
202-
}
203159

204-
data=slice(data,0,-tagByteLength);
205-
break;
206-
}
207-
casekWebCryptoCipherEncrypt:
208-
tag=tagByteLength;
209-
break;
210-
}
211-
212-
returnawaitjobPromise(()=>newAESCipherJob(
160+
returnjobPromise(()=>newAESCipherJob(
213161
kCryptoJobAsync,
214162
mode,
215163
key[kKeyObject][kHandle],
216164
data,
217165
getVariant('AES-OCB',key.algorithm.length),
218166
algorithm.iv,
219-
tag,
167+
tagByteLength,
220168
algorithm.additionalData));
221169
}
222170

‎lib/internal/crypto/chacha20_poly1305.js‎

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
'use strict';
22

33
const{
4-
ArrayBufferIsView,
5-
ArrayBufferPrototypeSlice,
64
ArrayFrom,
75
SafeSet,
8-
TypedArrayPrototypeSlice,
96
}=primordials;
107

118
const{
129
ChaCha20Poly1305CipherJob,
1310
KeyObjectHandle,
1411
kCryptoJobAsync,
15-
kWebCryptoCipherDecrypt,
16-
kWebCryptoCipherEncrypt,
1712
}=internalBinding('crypto');
1813

1914
const{
@@ -46,35 +41,13 @@ function validateKeyLength(length) {
4641
throwlazyDOMException('Invalid key length','DataError');
4742
}
4843

49-
asyncfunctionc20pCipher(mode,key,data,algorithm){
50-
lettag;
51-
switch(mode){
52-
casekWebCryptoCipherDecrypt: {
53-
constslice=ArrayBufferIsView(data) ?
54-
TypedArrayPrototypeSlice : ArrayBufferPrototypeSlice;
55-
56-
if(data.byteLength<16){
57-
throwlazyDOMException(
58-
'The provided data is too small.',
59-
'OperationError');
60-
}
61-
62-
tag=slice(data,-16);
63-
data=slice(data,0,-16);
64-
break;
65-
}
66-
casekWebCryptoCipherEncrypt:
67-
tag=16;
68-
break;
69-
}
70-
71-
returnawaitjobPromise(()=>newChaCha20Poly1305CipherJob(
44+
functionc20pCipher(mode,key,data,algorithm){
45+
returnjobPromise(()=>newChaCha20Poly1305CipherJob(
7246
kCryptoJobAsync,
7347
mode,
7448
key[kKeyObject][kHandle],
7549
data,
7650
algorithm.iv,
77-
tag,
7851
algorithm.additionalData));
7952
}
8053

‎src/crypto/crypto_aes.cc‎

Lines changed: 27 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -76,39 +76,35 @@ WebCryptoCipherStatus AES_Cipher(Environment* env,
7676
}
7777

7878
size_t tag_len = 0;
79+
size_t data_len = in.size();
7980

8081
if (params.cipher.isGcmMode() || params.cipher.isOcbMode()) {
82+
tag_len = params.length;
8183
switch (cipher_mode) {
8284
casekWebCryptoCipherDecrypt: {
83-
// If in decrypt mode, the auth tag must be set in the params.tag.
84-
CHECK(params.tag);
85+
// In decrypt mode, the auth tag is appended to the end of the
86+
// ciphertext. Split it off and set it on the cipher context.
87+
if (data_len < tag_len) {
88+
return WebCryptoCipherStatus::FAILED;
89+
}
90+
data_len -= tag_len;
8591

86-
// For OCB mode, we need to set the auth tag length before setting the
87-
// tag
8892
if (params.cipher.isOcbMode()) {
89-
if (!ctx.setAeadTagLength(params.tag.size())) {
93+
if (!ctx.setAeadTagLength(tag_len)) {
9094
return WebCryptoCipherStatus::FAILED;
9195
}
9296
}
9397

9498
ncrypto::Buffer<constchar> buffer = {
95-
.data = params.tag.data<char>(),
96-
.len = params.tag.size(),
99+
.data = in.data<char>() + data_len,
100+
.len = tag_len,
97101
};
98102
if (!ctx.setAeadTag(buffer)) {
99103
return WebCryptoCipherStatus::FAILED;
100104
}
101105
break;
102106
}
103107
casekWebCryptoCipherEncrypt: {
104-
// In encrypt mode, we grab the tag length here. We'll use it to
105-
// ensure that that allocated buffer has enough room for both the
106-
// final block and the auth tag. Unlike our other AES-GCM implementation
107-
// in CipherBase, in WebCrypto, the auth tag is concatenated to the end
108-
// of the generated ciphertext and returned in the same ArrayBuffer.
109-
tag_len = params.length;
110-
111-
// For OCB mode, we need to set the auth tag length
112108
if (params.cipher.isOcbMode()) {
113109
if (!ctx.setAeadTagLength(tag_len)) {
114110
return WebCryptoCipherStatus::FAILED;
@@ -122,7 +118,7 @@ WebCryptoCipherStatus AES_Cipher(Environment* env,
122118
}
123119

124120
size_t total = 0;
125-
int buf_len = in.size() + ctx.getBlockSize() + tag_len;
121+
int buf_len = data_len + ctx.getBlockSize() + (encrypt ? tag_len : 0);
126122
int out_len;
127123

128124
ncrypto::Buffer<constunsignedchar> buffer = {
@@ -148,9 +144,9 @@ WebCryptoCipherStatus AES_Cipher(Environment* env,
148144
// Refs: https://github.com/nodejs/node/pull/38913#issuecomment-866505244
149145
buffer = {
150146
.data = in.data<unsignedchar>(),
151-
.len = in.size(),
147+
.len = data_len,
152148
};
153-
if (in.empty()) {
149+
if (data_len == 0) {
154150
out_len = 0;
155151
} elseif (!ctx.update(buffer, ptr, &out_len)) {
156152
return WebCryptoCipherStatus::FAILED;
@@ -381,42 +377,17 @@ bool ValidateCounter(
381377
returntrue;
382378
}
383379

384-
boolValidateAuthTag(
385-
Environment* env,
386-
CryptoJobMode mode,
387-
WebCryptoCipherMode cipher_mode,
388-
Local<Value> value,
389-
AESCipherConfig* params) {
390-
switch (cipher_mode) {
391-
casekWebCryptoCipherDecrypt: {
392-
if (!IsAnyBufferSource(value)) {
393-
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
394-
returnfalse;
395-
}
396-
ArrayBufferOrViewContents<char> tag_contents(value);
397-
if (!tag_contents.CheckSizeInt32()) [[unlikely]] {
398-
THROW_ERR_OUT_OF_RANGE(env, "tagLength is too big");
399-
returnfalse;
400-
}
401-
params->tag = mode == kCryptoJobAsync
402-
? tag_contents.ToCopy()
403-
: tag_contents.ToByteSource();
404-
break;
405-
}
406-
casekWebCryptoCipherEncrypt: {
407-
if (!value->IsUint32()) {
408-
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
409-
returnfalse;
410-
}
411-
params->length = value.As<Uint32>()->Value();
412-
if (params->length > 128) {
413-
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
414-
returnfalse;
415-
}
416-
break;
417-
}
418-
default:
419-
UNREACHABLE();
380+
boolValidateAuthTag(Environment* env,
381+
Local<Value> value,
382+
AESCipherConfig* params) {
383+
if (!value->IsUint32()) {
384+
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
385+
returnfalse;
386+
}
387+
params->length = value.As<Uint32>()->Value();
388+
if (params->length > 128) {
389+
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
390+
returnfalse;
420391
}
421392
returntrue;
422393
}
@@ -451,8 +422,7 @@ AESCipherConfig::AESCipherConfig(AESCipherConfig&& other) noexcept
451422
cipher(other.cipher),
452423
length(other.length),
453424
iv(std::move(other.iv)),
454-
additional_data(std::move(other.additional_data)),
455-
tag(std::move(other.tag)) {}
425+
additional_data(std::move(other.additional_data)) {}
456426

457427
AESCipherConfig& AESCipherConfig::operator=(AESCipherConfig&& other) noexcept {
458428
if (&other == this) return *this;
@@ -466,7 +436,6 @@ void AESCipherConfig::MemoryInfo(MemoryTracker* tracker) const {
466436
if (mode == kCryptoJobAsync) {
467437
tracker->TrackFieldWithSize("iv", iv.size());
468438
tracker->TrackFieldWithSize("additional_data", additional_data.size());
469-
tracker->TrackFieldWithSize("tag", tag.size());
470439
}
471440
}
472441

@@ -510,7 +479,7 @@ Maybe<void> AESCipherTraits::AdditionalConfig(
510479
return Nothing<void>();
511480
}
512481
} elseif (params->cipher.isGcmMode() || params->cipher.isOcbMode()) {
513-
if (!ValidateAuthTag(env, mode, cipher_mode, args[offset + 2], params) ||
482+
if (!ValidateAuthTag(env, args[offset + 2], params) ||
514483
!ValidateAdditionalData(env, mode, args[offset + 3], params)) {
515484
return Nothing<void>();
516485
}

‎src/crypto/crypto_aes.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ struct AESCipherConfig final : public MemoryRetainer {
5252
size_t length;
5353
ByteSource iv; // Used for both iv or counter
5454
ByteSource additional_data;
55-
ByteSource tag; // Used only for authenticated modes (GCM)
5655

5756
AESCipherConfig() = default;
5857

0 commit comments

Comments
 (0)