FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: fix edge case in authenticated encryption · nodejs/node@624e516 · GitHub

/ node Public

Commit 624e516

Browse files
tniessen
authored and
targos
committed
crypto: fix edge case in authenticated encryption
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: #22828 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
1 parent 329ac60 commit 624e516

3 files changed

Lines changed: 38 additions & 20 deletions

File tree

‎src/node_crypto.cc‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,6 +2891,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
28912891
return args.GetReturnValue().Set(false);
28922892
}
28932893

2894+
// TODO(tniessen): Throw if the authentication tag has already been set.
2895+
if (cipher->auth_tag_state_ == kAuthTagPassedToOpenSSL)
2896+
return args.GetReturnValue().Set(true);
2897+
28942898
unsigned int tag_len = Buffer::Length(args[0]);
28952899
const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get());
28962900
if (mode == EVP_CIPH_GCM_MODE) {
@@ -2923,6 +2927,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
29232927

29242928
// Note: we don't use std::min() here to work around a header conflict.
29252929
cipher->auth_tag_len_ = tag_len;
2930+
cipher->auth_tag_state_ = kAuthTagKnown;
29262931
if (cipher->auth_tag_len_ > sizeof(cipher->auth_tag_))
29272932
cipher->auth_tag_len_ = sizeof(cipher->auth_tag_);
29282933

@@ -2934,14 +2939,14 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
29342939

29352940

29362941
bool CipherBase::MaybePassAuthTagToOpenSSL() {
2937-
if (!auth_tag_set_ && auth_tag_len_ != kNoAuthTagLength) {
2942+
if (auth_tag_state_ == kAuthTagKnown) {
29382943
if (!EVP_CIPHER_CTX_ctrl(ctx_.get(),
29392944
EVP_CTRL_AEAD_SET_TAG,
29402945
auth_tag_len_,
29412946
reinterpret_cast<unsigned char*>(auth_tag_))) {
29422947
return false;
29432948
}
2944-
auth_tag_set_ = true;
2949+
auth_tag_state_ = kAuthTagPassedToOpenSSL;
29452950
}
29462951
return true;
29472952
}

‎src/node_crypto.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,11 @@ class CipherBase : public BaseObject {
363363
kErrorMessageSize,
364364
kErrorState
365365
};
366+
enum AuthTagState {
367+
kAuthTagUnknown,
368+
kAuthTagKnown,
369+
kAuthTagPassedToOpenSSL
370+
};
366371
static const unsigned kNoAuthTagLength = static_cast<unsigned>(-1);
367372

368373
void Init(const char* cipher_type,
@@ -404,7 +409,7 @@ class CipherBase : public BaseObject {
404409
: BaseObject(env, wrap),
405410
ctx_(nullptr),
406411
kind_(kind),
407-
auth_tag_set_(false),
412+
auth_tag_state_(kAuthTagUnknown),
408413
auth_tag_len_(kNoAuthTagLength),
409414
pending_auth_failed_(false) {
410415
MakeWeak();
@@ -413,7 +418,7 @@ class CipherBase : public BaseObject {
413418
private:
414419
DeleteFnPtr<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free> ctx_;
415420
const CipherKind kind_;
416-
bool auth_tag_set_;
421+
AuthTagState auth_tag_state_;
417422
unsigned int auth_tag_len_;
418423
char auth_tag_[EVP_GCM_TLS_TAG_LEN];
419424
bool pending_auth_failed_;

‎test/parallel/test-crypto-authenticated.js‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -579,27 +579,35 @@ for (const test of TEST_CASES) {
579579
}
580580

581581
// Test that the authentication tag can be set at any point before calling
582-
// final() in GCM mode.
582+
// final() in GCM or OCB mode.
583583
{
584584
const plain = Buffer.from('Hello world', 'utf8');
585585
const key = Buffer.from('0123456789abcdef', 'utf8');
586586
const iv = Buffer.from('0123456789ab', 'utf8');
587587

588-
const cipher = crypto.createCipheriv('aes-128-gcm', key, iv);
589-
const ciphertext = Buffer.concat([cipher.update(plain), cipher.final()]);
590-
const authTag = cipher.getAuthTag();
591-
592-
for (const authTagBeforeUpdate of [true, false]) {
593-
const decipher = crypto.createDecipheriv('aes-128-gcm', key, iv);
594-
if (authTagBeforeUpdate) {
595-
decipher.setAuthTag(authTag);
596-
}
597-
const resultUpdate = decipher.update(ciphertext);
598-
if (!authTagBeforeUpdate) {
599-
decipher.setAuthTag(authTag);
588+
for (const mode of ['gcm', 'ocb']) {
589+
for (const authTagLength of mode === 'gcm' ? [undefined, 8] : [8]) {
590+
const cipher = crypto.createCipheriv(`aes-128-${mode}`, key, iv, {
591+
authTagLength
592+
});
593+
const ciphertext = Buffer.concat([cipher.update(plain), cipher.final()]);
594+
const authTag = cipher.getAuthTag();
595+
596+
for (const authTagBeforeUpdate of [true, false]) {
597+
const decipher = crypto.createDecipheriv(`aes-128-${mode}`, key, iv, {
598+
authTagLength
599+
});
600+
if (authTagBeforeUpdate) {
601+
decipher.setAuthTag(authTag);
602+
}
603+
const resultUpdate = decipher.update(ciphertext);
604+
if (!authTagBeforeUpdate) {
605+
decipher.setAuthTag(authTag);
606+
}
607+
const resultFinal = decipher.final();
608+
const result = Buffer.concat([resultUpdate, resultFinal]);
609+
assert(result.equals(plain));
610+
}
600611
}
601-
const resultFinal = decipher.final();
602-
const result = Buffer.concat([resultUpdate, resultFinal]);
603-
assert(result.equals(plain));
604612
}
605613
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL