From c9126f1a619ba76a0ec90936e32c4a5165fa3c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 23 Apr 2018 16:14:22 +0200 Subject: [PATCH 1/3] crypto: use kNoAuthTagLength in InitAuthenticated Refs: https://github.com/nodejs/node/pull/20039 --- src/node_crypto.cc | 30 +++++++++++++++++++++--------- src/node_crypto.h | 7 ++++--- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 255dc6e2d42e64..ff7d1db734c267 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2629,7 +2629,7 @@ void CipherBase::New(const FunctionCallbackInfo& args) { void CipherBase::Init(const char* cipher_type, const char* key_buf, int key_buf_len, - int auth_tag_len) { + unsigned int auth_tag_len) { HandleScope scope(env()->isolate()); #ifdef NODE_FIPS_MODE @@ -2700,10 +2700,16 @@ void CipherBase::Init(const FunctionCallbackInfo& args) { const node::Utf8Value cipher_type(args.GetIsolate(), args[0]); const char* key_buf = Buffer::Data(args[1]); ssize_t key_buf_len = Buffer::Length(args[1]); - CHECK(args[2]->IsInt32()); + // Don't assign to cipher->auth_tag_len_ directly; the value might not // represent a valid length at this point. - int auth_tag_len = args[2].As()->Value(); + unsigned int auth_tag_len; + if (args[2]->IsUint32()) { + auth_tag_len = args[2]->Uint32Value(); + } else { + CHECK(args[2]->IsInt32() && args[2]->Int32Value() == -1); + auth_tag_len = kNoAuthTagLength; + } cipher->Init(*cipher_type, key_buf, key_buf_len, auth_tag_len); } @@ -2714,7 +2720,7 @@ void CipherBase::InitIv(const char* cipher_type, int key_len, const char* iv, int iv_len, - int auth_tag_len) { + unsigned int auth_tag_len) { HandleScope scope(env()->isolate()); const EVP_CIPHER* const cipher = EVP_get_cipherbyname(cipher_type); @@ -2788,10 +2794,16 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { iv_buf = Buffer::Data(args[2]); iv_len = Buffer::Length(args[2]); } - CHECK(args[3]->IsInt32()); + // Don't assign to cipher->auth_tag_len_ directly; the value might not // represent a valid length at this point. - int auth_tag_len = args[3].As()->Value(); + unsigned int auth_tag_len; + if (args[3]->IsUint32()) { + auth_tag_len = args[3]->Uint32Value(); + } else { + CHECK(args[3]->IsInt32() && args[3]->Int32Value() == -1); + auth_tag_len = kNoAuthTagLength; + } cipher->InitIv(*cipher_type, key_buf, key_len, iv_buf, iv_len, auth_tag_len); } @@ -2802,7 +2814,7 @@ static bool IsValidGCMTagLength(unsigned int tag_len) { } bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, - int auth_tag_len) { + unsigned int auth_tag_len) { CHECK(IsAuthenticatedMode()); // TODO(tniessen) Use EVP_CTRL_AEAD_SET_IVLEN when migrating to OpenSSL 1.1.0 @@ -2815,7 +2827,7 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, const int mode = EVP_CIPHER_CTX_mode(ctx_); if (mode == EVP_CIPH_CCM_MODE) { - if (auth_tag_len < 0) { + if (auth_tag_len == kNoAuthTagLength) { char msg[128]; snprintf(msg, sizeof(msg), "authTagLength required for %s", cipher_type); env()->ThrowError(msg); @@ -2850,7 +2862,7 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, } else { CHECK_EQ(mode, EVP_CIPH_GCM_MODE); - if (auth_tag_len >= 0) { + if (auth_tag_len != kNoAuthTagLength) { if (!IsValidGCMTagLength(auth_tag_len)) { char msg[50]; snprintf(msg, sizeof(msg), diff --git a/src/node_crypto.h b/src/node_crypto.h index 3c166f5dcc89fc..a706c2805726bf 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -364,14 +364,15 @@ class CipherBase : public BaseObject { void Init(const char* cipher_type, const char* key_buf, int key_buf_len, - int auth_tag_len); + unsigned int auth_tag_len); void InitIv(const char* cipher_type, const char* key, int key_len, const char* iv, int iv_len, - int auth_tag_len); - bool InitAuthenticated(const char *cipher_type, int iv_len, int auth_tag_len); + unsigned int auth_tag_len); + bool InitAuthenticated(const char *cipher_type, int iv_len, + unsigned int auth_tag_len); bool CheckCCMMessageLength(int message_len); UpdateResult Update(const char* data, int len, unsigned char** out, int* out_len); From 217b6065abb992080cabf90d3847731d5c54fcfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Tue, 24 Apr 2018 10:36:34 +0200 Subject: [PATCH 2/3] fixup! crypto: use kNoAuthTagLength in InitAuthenticated --- src/node_crypto.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index ff7d1db734c267..de642544a6936a 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2705,9 +2705,9 @@ void CipherBase::Init(const FunctionCallbackInfo& args) { // represent a valid length at this point. unsigned int auth_tag_len; if (args[2]->IsUint32()) { - auth_tag_len = args[2]->Uint32Value(); + auth_tag_len = args[2].As()->Value(); } else { - CHECK(args[2]->IsInt32() && args[2]->Int32Value() == -1); + CHECK(args[2]->IsInt32() && args[2].As()->Value() == -1); auth_tag_len = kNoAuthTagLength; } @@ -2799,9 +2799,9 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { // represent a valid length at this point. unsigned int auth_tag_len; if (args[3]->IsUint32()) { - auth_tag_len = args[3]->Uint32Value(); + auth_tag_len = args[3].As()->Value(); } else { - CHECK(args[3]->IsInt32() && args[3]->Int32Value() == -1); + CHECK(args[3]->IsInt32() && args[3].As()->Value() == -1); auth_tag_len = kNoAuthTagLength; } From ab028e484b3abf1d7ec82096faa74b7fb48b7777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Tue, 24 Apr 2018 16:21:27 +0200 Subject: [PATCH 3/3] crypto: add using directives for v8::Int32, Uint32 --- src/node_crypto.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index de642544a6936a..cc63b09b2e276e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -72,6 +72,7 @@ using v8::External; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::Int32; using v8::Integer; using v8::Isolate; using v8::Local; @@ -84,6 +85,7 @@ using v8::PropertyAttribute; using v8::ReadOnly; using v8::Signature; using v8::String; +using v8::Uint32; using v8::Value; @@ -2705,9 +2707,9 @@ void CipherBase::Init(const FunctionCallbackInfo& args) { // represent a valid length at this point. unsigned int auth_tag_len; if (args[2]->IsUint32()) { - auth_tag_len = args[2].As()->Value(); + auth_tag_len = args[2].As()->Value(); } else { - CHECK(args[2]->IsInt32() && args[2].As()->Value() == -1); + CHECK(args[2]->IsInt32() && args[2].As()->Value() == -1); auth_tag_len = kNoAuthTagLength; } @@ -2799,9 +2801,9 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { // represent a valid length at this point. unsigned int auth_tag_len; if (args[3]->IsUint32()) { - auth_tag_len = args[3].As()->Value(); + auth_tag_len = args[3].As()->Value(); } else { - CHECK(args[3]->IsInt32() && args[3].As()->Value() == -1); + CHECK(args[3]->IsInt32() && args[3].As()->Value() == -1); auth_tag_len = kNoAuthTagLength; } @@ -3002,7 +3004,7 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { CHECK_EQ(args.Length(), 2); CHECK(args[1]->IsInt32()); - int plaintext_len = args[1].As()->Value(); + int plaintext_len = args[1].As()->Value(); if (!cipher->SetAAD(Buffer::Data(args[0]), Buffer::Length(args[0]), plaintext_len))