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

crypto: fix edge case in authenticated encryption

Browse Source 
Restricting the authentication tag length and calling update or
setAAD before setAuthTag caused an incorrect authentication tag to
be passed to OpenSSL: The auth_tag_len_ field was already set, so
the implementation assumed that the tag itself was known as well.

PR-URL: https://github.com/nodejs/node/pull/22828
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This commit is contained in:
Tobias Nießen 2018-09-13 00:48:35 +02:00
parent 47a0d041d1
commit a9e7369b11
No known key found for this signature in database
GPG Key ID: 718207F8FD156B70
3 changed files with 37 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/node_crypto.cc
View File

@ -2897,6 +2897,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
return args.GetReturnValue().Set(false);
}
// TODO(tniessen): Throw if the authentication tag has already been set.
if (cipher->auth_tag_state_ == kAuthTagPassedToOpenSSL)
return args.GetReturnValue().Set(true);
unsigned int tag_len = Buffer::Length(args[0]);
const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get());
bool is_valid;
@ -2921,6 +2925,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
}
cipher->auth_tag_len_ = tag_len;
cipher->auth_tag_state_ = kAuthTagKnown;
CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_));
memset(cipher->auth_tag_, 0, sizeof(cipher->auth_tag_));
@ -2931,14 +2936,14 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
bool CipherBase::MaybePassAuthTagToOpenSSL() {
if (!auth_tag_set_ && auth_tag_len_ != kNoAuthTagLength) {
if (auth_tag_state_ == kAuthTagKnown) {
if (!EVP_CIPHER_CTX_ctrl(ctx_.get(),
EVP_CTRL_AEAD_SET_TAG,
auth_tag_len_,
reinterpret_cast<unsigned char*>(auth_tag_))) {
return false;
}
auth_tag_set_ = true;
auth_tag_state_ = kAuthTagPassedToOpenSSL;
}
return true;
}

9
src/node_crypto.h
View File

@ -363,6 +363,11 @@ class CipherBase : public BaseObject {
kErrorMessageSize,
kErrorState
};
enum AuthTagState {
kAuthTagUnknown,
kAuthTagKnown,
kAuthTagPassedToOpenSSL
};
static const unsigned kNoAuthTagLength = static_cast<unsigned>(-1);
void Init(const char* cipher_type,
@ -404,7 +409,7 @@ class CipherBase : public BaseObject {
: BaseObject(env, wrap),
ctx_(nullptr),
kind_(kind),
auth_tag_set_(false),
auth_tag_state_(kAuthTagUnknown),
auth_tag_len_(kNoAuthTagLength),
pending_auth_failed_(false) {
MakeWeak();
@ -413,7 +418,7 @@ class CipherBase : public BaseObject {
private:
DeleteFnPtr<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free> ctx_;
const CipherKind kind_;
bool auth_tag_set_;
AuthTagState auth_tag_state_;
unsigned int auth_tag_len_;
char auth_tag_[EVP_GCM_TLS_TAG_LEN];
bool pending_auth_failed_;

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

@ -557,27 +557,35 @@ for (const test of TEST_CASES) {
}
// Test that the authentication tag can be set at any point before calling
// final() in GCM mode.
// final() in GCM or OCB mode.
{
const plain = Buffer.from('Hello world', 'utf8');
const key = Buffer.from('0123456789abcdef', 'utf8');
const iv = Buffer.from('0123456789ab', 'utf8');
const cipher = crypto.createCipheriv('aes-128-gcm', key, iv);
const ciphertext = Buffer.concat([cipher.update(plain), cipher.final()]);
const authTag = cipher.getAuthTag();
for (const mode of ['gcm', 'ocb']) {
for (const authTagLength of mode === 'gcm' ? [undefined, 8] : [8]) {
const cipher = crypto.createCipheriv(`aes-128-${mode}`, key, iv, {
authTagLength
});
const ciphertext = Buffer.concat([cipher.update(plain), cipher.final()]);
const authTag = cipher.getAuthTag();
for (const authTagBeforeUpdate of [true, false]) {
const decipher = crypto.createDecipheriv('aes-128-gcm', key, iv);
if (authTagBeforeUpdate) {
decipher.setAuthTag(authTag);
for (const authTagBeforeUpdate of [true, false]) {
const decipher = crypto.createDecipheriv(`aes-128-${mode}`, key, iv, {
authTagLength
});
if (authTagBeforeUpdate) {
decipher.setAuthTag(authTag);
}
const resultUpdate = decipher.update(ciphertext);
if (!authTagBeforeUpdate) {
decipher.setAuthTag(authTag);
}
const resultFinal = decipher.final();
const result = Buffer.concat([resultUpdate, resultFinal]);
assert(result.equals(plain));
}
}
const resultUpdate = decipher.update(ciphertext);
if (!authTagBeforeUpdate) {
decipher.setAuthTag(authTag);
}
const resultFinal = decipher.final();
const result = Buffer.concat([resultUpdate, resultFinal]);
assert(result.equals(plain));
}
}