1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

crypto: throw in setAuthTag on invalid length

Browse Source 
The current implementation performs limited checks only and silently
ignores superfluous bytes of the authentication tag. This change makes
setAuthTag throw when
- the user-specified authTagLength does not match the actual tag length,
  especially when the authentication tag is longer than 16 bytes, and
  when
- the mode is GCM, no authTagLength option has been specified and the
  tag length is not a valid GCM tag length.

This change makes the conditional assignment in SetAuthTag unnecessary,
which is replaced with a CHECK.

Refs: https://github.com/nodejs/node/pull/17825

PR-URL: https://github.com/nodejs/node/pull/20040
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Yihong Wang <yh.wang@ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Tobias Nießen 2018-04-15 02:58:25 +02:00
parent cb3d049bad
commit faf449ca04
No known key found for this signature in database
GPG Key ID: 718207F8FD156B70
2 changed files with 25 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/node_crypto.cc
View File

@ -2806,9 +2806,7 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_len,
return false;
}
// When decrypting in CCM mode, this field will be set in setAuthTag().
if (kind_ == kCipher)
auth_tag_len_ = auth_tag_len;
auth_tag_len_ = auth_tag_len;
// The message length is restricted to 2 ^ (8 * (15 - iv_len)) - 1 bytes.
CHECK(iv_len >= 7 && iv_len <= 13);
@ -2824,7 +2822,7 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_len,
if (!IsValidGCMTagLength(auth_tag_len)) {
char msg[50];
snprintf(msg, sizeof(msg),
"Invalid GCM authentication tag length: %u", auth_tag_len);
"Invalid authentication tag length: %u", auth_tag_len);
env()->ThrowError(msg);
return false;
}
@ -2891,21 +2889,26 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
// Restrict GCM tag lengths according to NIST 800-38d, page 9.
unsigned int tag_len = Buffer::Length(args[0]);
const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get());
bool is_valid;
if (mode == EVP_CIPH_GCM_MODE) {
if ((cipher->auth_tag_len_ != kNoAuthTagLength &&
cipher->auth_tag_len_ != tag_len) ||
!IsValidGCMTagLength(tag_len)) {
char msg[50];
snprintf(msg, sizeof(msg),
"Invalid GCM authentication tag length: %u", tag_len);
return cipher->env()->ThrowError(msg);
}
is_valid = (cipher->auth_tag_len_ == kNoAuthTagLength ||
cipher->auth_tag_len_ == tag_len) &&
IsValidGCMTagLength(tag_len);
} else {
CHECK_EQ(mode, EVP_CIPH_CCM_MODE);
CHECK_NE(cipher->auth_tag_len_, kNoAuthTagLength);
is_valid = cipher->auth_tag_len_ == tag_len;
}
if (!is_valid) {
char msg[50];
snprintf(msg, sizeof(msg),
"Invalid authentication tag length: %u", tag_len);
return cipher->env()->ThrowError(msg);
}
// Note: we don't use std::min() here to work around a header conflict.
cipher->auth_tag_len_ = tag_len;
if (cipher->auth_tag_len_ > sizeof(cipher->auth_tag_))
cipher->auth_tag_len_ = sizeof(cipher->auth_tag_);
CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_));
memset(cipher->auth_tag_, 0, sizeof(cipher->auth_tag_));
memcpy(cipher->auth_tag_, Buffer::Data(args[0]), cipher->auth_tag_len_);

12
test/parallel/test-crypto-authenticated.js
View File

@ -724,7 +724,7 @@ for (const test of TEST_CASES) {
decrypt.setAuthTag(Buffer.from('1'.repeat(length)));
}, {
type: Error,
message: `Invalid GCM authentication tag length: ${length}`
message: `Invalid authentication tag length: ${length}`
});
common.expectsError(() => {
@ -736,7 +736,7 @@ for (const test of TEST_CASES) {
});
}, {
type: Error,
message: `Invalid GCM authentication tag length: ${length}`
message: `Invalid authentication tag length: ${length}`
});
common.expectsError(() => {
@ -748,7 +748,7 @@ for (const test of TEST_CASES) {
});
}, {
type: Error,
message: `Invalid GCM authentication tag length: ${length}`
message: `Invalid authentication tag length: ${length}`
});
}
}
@ -783,7 +783,7 @@ for (const test of TEST_CASES) {
decipher.setAuthTag(Buffer.from('1'.repeat(12)));
}, {
type: Error,
message: 'Invalid GCM authentication tag length: 12'
message: 'Invalid authentication tag length: 12'
});
// The Decipher object should be left intact.
@ -985,7 +985,7 @@ for (const test of TEST_CASES) {
}
}
// Test that setAAD throws in CCM mode when no authentication tag is provided.
// Test that final() throws in CCM mode when no authentication tag is provided.
{
if (!common.hasFipsCrypto) {
const key = Buffer.from('1ed2233fa2223ef5d7df08546049406c', 'hex');
@ -1000,6 +1000,8 @@ for (const test of TEST_CASES) {
decrypt.setAAD(Buffer.from('63616c76696e', 'hex'), {
plaintextLength: ct.length
});
decrypt.update(ct);
decrypt.final();
}, errMessages.state);
}
}