From 926503b66910d9ec895c33c7fd94361fd78dea72 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 24 Jul 2024 10:08:42 -0700 Subject: [PATCH] src: shift more crypto impl details to ncrypto PR-URL: https://github.com/nodejs/node/pull/54028 Reviewed-By: Yagiz Nizipli --- deps/ncrypto/ncrypto.cc | 460 +++++++++++++++++++++++++++++++++-- deps/ncrypto/ncrypto.h | 98 ++++++-- src/crypto/crypto_aes.cc | 25 +- src/crypto/crypto_common.cc | 310 +---------------------- src/crypto/crypto_common.h | 1 - src/crypto/crypto_context.cc | 2 +- src/crypto/crypto_dh.cc | 91 ++++--- src/crypto/crypto_dsa.cc | 4 +- src/crypto/crypto_ec.cc | 24 +- src/crypto/crypto_random.cc | 41 ++-- src/crypto/crypto_rsa.cc | 28 +-- src/crypto/crypto_sig.cc | 30 +-- src/crypto/crypto_util.cc | 16 +- src/crypto/crypto_util.h | 2 +- 14 files changed, 653 insertions(+), 479 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 6822aac4ee74b6..9a70b52d8504b1 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -3,11 +3,19 @@ #include #include "openssl/bn.h" #include "openssl/evp.h" +#include "openssl/pkcs12.h" +#include "openssl/x509v3.h" #if OPENSSL_VERSION_MAJOR >= 3 #include "openssl/provider.h" #endif namespace ncrypto { +namespace { +static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON = + XN_FLAG_RFC2253 & + ~ASN1_STRFLGS_ESC_MSB & + ~ASN1_STRFLGS_ESC_CTRL; +} // namespace // ============================================================================ @@ -64,6 +72,53 @@ std::optional CryptoErrorList::pop_front() { return error; } +// ============================================================================ +DataPointer DataPointer::Alloc(size_t len) { + return DataPointer(OPENSSL_malloc(len), len); +} + +DataPointer::DataPointer(void* data, size_t length) + : data_(data), len_(length) {} + +DataPointer::DataPointer(const Buffer& buffer) + : data_(buffer.data), len_(buffer.len) {} + +DataPointer::DataPointer(DataPointer&& other) noexcept + : data_(other.data_), len_(other.len_) { + other.data_ = nullptr; + other.len_ = 0; +} + +DataPointer& DataPointer::operator=(DataPointer&& other) noexcept { + if (this == &other) return *this; + this->~DataPointer(); + return *new (this) DataPointer(std::move(other)); +} + +DataPointer::~DataPointer() { reset(); } + +void DataPointer::reset(void* data, size_t length) { + if (data_ != nullptr) { + OPENSSL_clear_free(data_, len_); + } + data_ = data; + len_ = length; +} + +void DataPointer::reset(const Buffer& buffer) { + reset(buffer.data, buffer.len); +} + +Buffer DataPointer::release() { + Buffer buf { + .data = data_, + .len = len_, + }; + data_ = nullptr; + len_ = 0; + return buf; +} + // ============================================================================ bool isFipsEnabled() { #if OPENSSL_VERSION_MAJOR >= 3 @@ -106,9 +161,20 @@ bool testFipsEnabled() { // Bignum BignumPointer::BignumPointer(BIGNUM* bignum) : bn_(bignum) {} +BignumPointer::BignumPointer(const unsigned char* data, size_t len) + : BignumPointer(BN_bin2bn(data, len, nullptr)) {} + BignumPointer::BignumPointer(BignumPointer&& other) noexcept : bn_(other.release()) {} +BignumPointer BignumPointer::New() { + return BignumPointer(BN_new()); +} + +BignumPointer BignumPointer::NewSecure() { + return BignumPointer(BN_secure_new()); +} + BignumPointer& BignumPointer::operator=(BignumPointer&& other) noexcept { if (this == &other) return *this; this->~BignumPointer(); @@ -121,47 +187,106 @@ void BignumPointer::reset(BIGNUM* bn) { bn_.reset(bn); } +void BignumPointer::reset(const unsigned char* data, size_t len) { + reset(BN_bin2bn(data, len, nullptr)); +} + BIGNUM* BignumPointer::release() { return bn_.release(); } -size_t BignumPointer::byteLength() { +size_t BignumPointer::byteLength() const { if (bn_ == nullptr) return 0; return BN_num_bytes(bn_.get()); } -std::vector BignumPointer::encode() { - return encodePadded(bn_.get(), byteLength()); +DataPointer BignumPointer::encode() const { + return EncodePadded(bn_.get(), byteLength()); } -std::vector BignumPointer::encodePadded(size_t size) { - return encodePadded(bn_.get(), size); +DataPointer BignumPointer::encodePadded(size_t size) const { + return EncodePadded(bn_.get(), size); } -std::vector BignumPointer::encode(const BIGNUM* bn) { - return encodePadded(bn, bn != nullptr ? BN_num_bytes(bn) : 0); +size_t BignumPointer::encodeInto(unsigned char* out) const { + if (!bn_) return 0; + return BN_bn2bin(bn_.get(), out); } -std::vector BignumPointer::encodePadded(const BIGNUM* bn, size_t s) { - if (bn == nullptr) return std::vector(0); - size_t size = std::max(s, static_cast(BN_num_bytes(bn))); - std::vector buf(size); - BN_bn2binpad(bn, buf.data(), size); +size_t BignumPointer::encodePaddedInto(unsigned char* out, size_t size) const { + if (!bn_) return 0; + return BN_bn2binpad(bn_.get(), out, size); +} + +DataPointer BignumPointer::Encode(const BIGNUM* bn) { + return EncodePadded(bn, bn != nullptr ? BN_num_bytes(bn) : 0); +} + +bool BignumPointer::setWord(unsigned long w) { + if (!bn_) return false; + return BN_set_word(bn_.get(), w) == 1; +} + +unsigned long BignumPointer::GetWord(const BIGNUM* bn) { + return BN_get_word(bn); +} + +unsigned long BignumPointer::getWord() const { + if (!bn_) return 0; + return GetWord(bn_.get()); +} + +DataPointer BignumPointer::EncodePadded(const BIGNUM* bn, size_t s) { + if (bn == nullptr) return DataPointer(); + size_t size = std::max(s, static_cast(GetByteCount(bn))); + auto buf = DataPointer::Alloc(size); + BN_bn2binpad(bn, reinterpret_cast(buf.get()), size); return buf; } +size_t BignumPointer::EncodePaddedInto(const BIGNUM* bn, unsigned char* out, size_t size) { + if (bn == nullptr) return 0; + return BN_bn2binpad(bn, out, size); +} + +int BignumPointer::operator<=>(const BignumPointer& other) const noexcept { + if (bn_ == nullptr && other.bn_ != nullptr) return -1; + if (bn_ != nullptr && other.bn_ == nullptr) return 1; + if (bn_ == nullptr && other.bn_ == nullptr) return 0; + return BN_cmp(bn_.get(), other.bn_.get()); +} -bool BignumPointer::operator==(const BignumPointer& other) noexcept { - if (bn_ == nullptr && other.bn_ != nullptr) return false; - if (bn_ != nullptr && other.bn_ == nullptr) return false; - if (bn_ == nullptr && other.bn_ == nullptr) return true; - return BN_cmp(bn_.get(), other.bn_.get()) == 0; +int BignumPointer::operator<=>(const BIGNUM* other) const noexcept { + if (bn_ == nullptr && other != nullptr) return -1; + if (bn_ != nullptr && other == nullptr) return 1; + if (bn_ == nullptr && other == nullptr) return 0; + return BN_cmp(bn_.get(), other); } -bool BignumPointer::operator==(const BIGNUM* other) noexcept { - if (bn_ == nullptr && other != nullptr) return false; - if (bn_ != nullptr && other == nullptr) return false; - if (bn_ == nullptr && other == nullptr) return true; - return BN_cmp(bn_.get(), other) == 0; +DataPointer BignumPointer::toHex() const { + if (!bn_) return {}; + char* hex = BN_bn2hex(bn_.get()); + if (!hex) return {}; + return DataPointer(hex, strlen(hex)); +} + +int BignumPointer::GetBitCount(const BIGNUM* bn) { + return BN_num_bits(bn); +} + +int BignumPointer::GetByteCount(const BIGNUM *bn) { + return BN_num_bytes(bn); +} + +bool BignumPointer::isZero() const { + return bn_ && BN_is_zero(bn_.get()); +} + +bool BignumPointer::isOne() const { + return bn_ && BN_is_one(bn_.get()); +} + +const BIGNUM* BignumPointer::One() { + return BN_value_one(); } // ============================================================================ @@ -263,7 +388,7 @@ BIOPointer ExportPublicKey(const char* input, size_t length) { if (PEM_write_bio_PUBKEY(bio.get(), pkey.get()) <= 0) return { }; - return std::move(bio); + return bio; } Buffer ExportChallenge(const char* input, size_t length) { @@ -289,4 +414,293 @@ Buffer ExportChallenge(const char* input, size_t length) { return {}; } +// ============================================================================ +namespace { +enum class AltNameOption { + NONE, + UTF8, +}; + +bool IsSafeAltName(const char* name, size_t length, AltNameOption option) { + for (size_t i = 0; i < length; i++) { + char c = name[i]; + switch (c) { + case '"': + case '\\': + // These mess with encoding rules. + // Fall through. + case ',': + // Commas make it impossible to split the list of subject alternative + // names unambiguously, which is why we have to escape. + // Fall through. + case '\'': + // Single quotes are unlikely to appear in any legitimate values, but they + // could be used to make a value look like it was escaped (i.e., enclosed + // in single/double quotes). + return false; + default: + if (option == AltNameOption::UTF8) { + // In UTF8 strings, we require escaping for any ASCII control character, + // but NOT for non-ASCII characters. Note that all bytes of any code + // point that consists of more than a single byte have their MSB set. + if (static_cast(c) < ' ' || c == '\x7f') { + return false; + } + } else { + // Check if the char is a control character or non-ASCII character. Note + // that char may or may not be a signed type. Regardless, non-ASCII + // values will always be outside of this range. + if (c < ' ' || c > '~') { + return false; + } + } + } + } + return true; +} + +void PrintAltName(const BIOPointer& out, + const char* name, + size_t length, + AltNameOption option = AltNameOption::NONE, + const char* safe_prefix = nullptr) { + if (IsSafeAltName(name, length, option)) { + // For backward-compatibility, append "safe" names without any + // modifications. + if (safe_prefix != nullptr) { + BIO_printf(out.get(), "%s:", safe_prefix); + } + BIO_write(out.get(), name, length); + } else { + // If a name is not "safe", we cannot embed it without special + // encoding. This does not usually happen, but we don't want to hide + // it from the user either. We use JSON compatible escaping here. + BIO_write(out.get(), "\"", 1); + if (safe_prefix != nullptr) { + BIO_printf(out.get(), "%s:", safe_prefix); + } + for (size_t j = 0; j < length; j++) { + char c = static_cast(name[j]); + if (c == '\\') { + BIO_write(out.get(), "\\\\", 2); + } else if (c == '"') { + BIO_write(out.get(), "\\\"", 2); + } else if ((c >= ' ' && c != ',' && c <= '~') || + (option == AltNameOption::UTF8 && (c & 0x80))) { + // Note that the above condition explicitly excludes commas, which means + // that those are encoded as Unicode escape sequences in the "else" + // block. That is not strictly necessary, and Node.js itself would parse + // it correctly either way. We only do this to account for third-party + // code that might be splitting the string at commas (as Node.js itself + // used to do). + BIO_write(out.get(), &c, 1); + } else { + // Control character or non-ASCII character. We treat everything as + // Latin-1, which corresponds to the first 255 Unicode code points. + const char hex[] = "0123456789abcdef"; + char u[] = { '\\', 'u', '0', '0', hex[(c & 0xf0) >> 4], hex[c & 0x0f] }; + BIO_write(out.get(), u, sizeof(u)); + } + } + BIO_write(out.get(), "\"", 1); + } +} + +// This function emulates the behavior of i2v_GENERAL_NAME in a safer and less +// ambiguous way. "othername:" entries use the GENERAL_NAME_print format. +bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) { + if (gen->type == GEN_DNS) { + ASN1_IA5STRING* name = gen->d.dNSName; + BIO_write(out.get(), "DNS:", 4); + // Note that the preferred name syntax (see RFCs 5280 and 1034) with + // wildcards is a subset of what we consider "safe", so spec-compliant DNS + // names will never need to be escaped. + PrintAltName(out, reinterpret_cast(name->data), name->length); + } else if (gen->type == GEN_EMAIL) { + ASN1_IA5STRING* name = gen->d.rfc822Name; + BIO_write(out.get(), "email:", 6); + PrintAltName(out, reinterpret_cast(name->data), name->length); + } else if (gen->type == GEN_URI) { + ASN1_IA5STRING* name = gen->d.uniformResourceIdentifier; + BIO_write(out.get(), "URI:", 4); + // The set of "safe" names was designed to include just about any URI, + // with a few exceptions, most notably URIs that contains commas (see + // RFC 2396). In other words, most legitimate URIs will not require + // escaping. + PrintAltName(out, reinterpret_cast(name->data), name->length); + } else if (gen->type == GEN_DIRNAME) { + // Earlier versions of Node.js used X509_NAME_oneline to print the X509_NAME + // object. The format was non standard and should be avoided. The use of + // X509_NAME_oneline is discouraged by OpenSSL but was required for backward + // compatibility. Conveniently, X509_NAME_oneline produced ASCII and the + // output was unlikely to contains commas or other characters that would + // require escaping. However, it SHOULD NOT produce ASCII output since an + // RFC5280 AttributeValue may be a UTF8String. + // Newer versions of Node.js have since switched to X509_NAME_print_ex to + // produce a better format at the cost of backward compatibility. The new + // format may contain Unicode characters and it is likely to contain commas, + // which require escaping. Fortunately, the recently safeguarded function + // PrintAltName handles all of that safely. + BIO_printf(out.get(), "DirName:"); + BIOPointer tmp(BIO_new(BIO_s_mem())); + NCRYPTO_ASSERT_TRUE(tmp); + if (X509_NAME_print_ex(tmp.get(), + gen->d.dirn, + 0, + kX509NameFlagsRFC2253WithinUtf8JSON) < 0) { + return false; + } + char* oline = nullptr; + long n_bytes = BIO_get_mem_data(tmp.get(), &oline); // NOLINT(runtime/int) + NCRYPTO_ASSERT_TRUE(n_bytes >= 0); + PrintAltName(out, oline, static_cast(n_bytes), + ncrypto::AltNameOption::UTF8, nullptr); + } else if (gen->type == GEN_IPADD) { + BIO_printf(out.get(), "IP Address:"); + const ASN1_OCTET_STRING* ip = gen->d.ip; + const unsigned char* b = ip->data; + if (ip->length == 4) { + BIO_printf(out.get(), "%d.%d.%d.%d", b[0], b[1], b[2], b[3]); + } else if (ip->length == 16) { + for (unsigned int j = 0; j < 8; j++) { + uint16_t pair = (b[2 * j] << 8) | b[2 * j + 1]; + BIO_printf(out.get(), (j == 0) ? "%X" : ":%X", pair); + } + } else { +#if OPENSSL_VERSION_MAJOR >= 3 + BIO_printf(out.get(), "", ip->length); +#else + BIO_printf(out.get(), ""); +#endif + } + } else if (gen->type == GEN_RID) { + // Unlike OpenSSL's default implementation, never print the OID as text and + // instead always print its numeric representation. + char oline[256]; + OBJ_obj2txt(oline, sizeof(oline), gen->d.rid, true); + BIO_printf(out.get(), "Registered ID:%s", oline); + } else if (gen->type == GEN_OTHERNAME) { + // The format that is used here is based on OpenSSL's implementation of + // GENERAL_NAME_print (as of OpenSSL 3.0.1). Earlier versions of Node.js + // instead produced the same format as i2v_GENERAL_NAME, which was somewhat + // awkward, especially when passed to translatePeerCertificate. + bool unicode = true; + const char* prefix = nullptr; + // OpenSSL 1.1.1 does not support othername in GENERAL_NAME_print and may + // not define these NIDs. +#if OPENSSL_VERSION_MAJOR >= 3 + int nid = OBJ_obj2nid(gen->d.otherName->type_id); + switch (nid) { + case NID_id_on_SmtpUTF8Mailbox: + prefix = "SmtpUTF8Mailbox"; + break; + case NID_XmppAddr: + prefix = "XmppAddr"; + break; + case NID_SRVName: + prefix = "SRVName"; + unicode = false; + break; + case NID_ms_upn: + prefix = "UPN"; + break; + case NID_NAIRealm: + prefix = "NAIRealm"; + break; + } +#endif // OPENSSL_VERSION_MAJOR >= 3 + int val_type = gen->d.otherName->value->type; + if (prefix == nullptr || + (unicode && val_type != V_ASN1_UTF8STRING) || + (!unicode && val_type != V_ASN1_IA5STRING)) { + BIO_printf(out.get(), "othername:"); + } else { + BIO_printf(out.get(), "othername:"); + if (unicode) { + auto name = gen->d.otherName->value->value.utf8string; + PrintAltName(out, + reinterpret_cast(name->data), name->length, + AltNameOption::UTF8, prefix); + } else { + auto name = gen->d.otherName->value->value.ia5string; + PrintAltName(out, + reinterpret_cast(name->data), name->length, + AltNameOption::NONE, prefix); + } + } + } else if (gen->type == GEN_X400) { + // TODO(tniessen): this is what OpenSSL does, implement properly instead + BIO_printf(out.get(), "X400Name:"); + } else if (gen->type == GEN_EDIPARTY) { + // TODO(tniessen): this is what OpenSSL does, implement properly instead + BIO_printf(out.get(), "EdiPartyName:"); + } else { + // This is safe because X509V3_EXT_d2i would have returned nullptr in this + // case already. + unreachable(); + } + + return true; +} +} // namespace + + +bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext) { + auto ret = OBJ_obj2nid(X509_EXTENSION_get_object(ext)); + NCRYPTO_ASSERT_EQUAL(ret, NID_subject_alt_name, "unexpected extension type"); + + GENERAL_NAMES* names = static_cast(X509V3_EXT_d2i(ext)); + if (names == nullptr) + return false; + + bool ok = true; + + for (int i = 0; i < sk_GENERAL_NAME_num(names); i++) { + GENERAL_NAME* gen = sk_GENERAL_NAME_value(names, i); + + if (i != 0) + BIO_write(out.get(), ", ", 2); + + if (!(ok = ncrypto::PrintGeneralName(out, gen))) { + break; + } + } + sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free); + + return ok; +} + +bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) { + auto ret = OBJ_obj2nid(X509_EXTENSION_get_object(ext)); + NCRYPTO_ASSERT_EQUAL(ret, NID_info_access, "unexpected extension type"); + + AUTHORITY_INFO_ACCESS* descs = + static_cast(X509V3_EXT_d2i(ext)); + if (descs == nullptr) + return false; + + bool ok = true; + + for (int i = 0; i < sk_ACCESS_DESCRIPTION_num(descs); i++) { + ACCESS_DESCRIPTION* desc = sk_ACCESS_DESCRIPTION_value(descs, i); + + if (i != 0) + BIO_write(out.get(), "\n", 1); + + char objtmp[80]; + i2t_ASN1_OBJECT(objtmp, sizeof(objtmp), desc->method); + BIO_printf(out.get(), "%s - ", objtmp); + if (!(ok = ncrypto::PrintGeneralName(out, desc->location))) { + break; + } + } + sk_ACCESS_DESCRIPTION_pop_free(descs, ACCESS_DESCRIPTION_free); + +#if OPENSSL_VERSION_MAJOR < 3 + BIO_write(out.get(), "\n", 1); +#endif + + return ok; +} + } // namespace ncrypto diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 979c177b4a8c65..633309aa0cbc6c 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -1,11 +1,11 @@ #pragma once +#include #include #include #include #include #include -#include #include "openssl/bn.h" #include #include @@ -82,6 +82,15 @@ namespace ncrypto { void* operator new(size_t) = delete; \ void operator delete(void*) = delete; +[[noreturn]] inline void unreachable() { +#ifdef __GNUC__ + __builtin_unreachable(); +#elif defined(_MSC_VER) + __assume(false); +#else +#endif +} + // ============================================================================ // Error handling utilities @@ -190,30 +199,92 @@ using SSLPointer = DeleteFnPtr; using SSLSessionPointer = DeleteFnPtr; using X509Pointer = DeleteFnPtr; +// An unowned, unmanaged pointer to a buffer of data. +template +struct Buffer { + T* data = nullptr; + size_t len = 0; +}; + +// A managed pointer to a buffer of data. When destroyed the underlying +// buffer will be freed. +class DataPointer final { + public: + static DataPointer Alloc(size_t len); + + DataPointer() = default; + explicit DataPointer(void* data, size_t len); + explicit DataPointer(const Buffer& buffer); + DataPointer(DataPointer&& other) noexcept; + DataPointer& operator=(DataPointer&& other) noexcept; + NCRYPTO_DISALLOW_COPY(DataPointer) + ~DataPointer(); + + inline bool operator==(std::nullptr_t) noexcept { return data_ == nullptr; } + inline operator bool() const { return data_ != nullptr; } + inline void* get() const noexcept { return data_; } + inline size_t size() const noexcept { return len_; } + void reset(void* data = nullptr, size_t len = 0); + void reset(const Buffer& buffer); + + // Releases ownership of the underlying data buffer. It is the caller's + // responsibility to ensure the buffer is appropriately freed. + Buffer release(); + + // Returns a Buffer struct that is a view of the underlying data. + inline operator const Buffer() const { + return { + .data = data_, + .len = len_, + }; + } + + private: + void* data_ = nullptr; + size_t len_ = 0; +}; + class BignumPointer final { public: BignumPointer() = default; explicit BignumPointer(BIGNUM* bignum); + explicit BignumPointer(const unsigned char* data, size_t len); BignumPointer(BignumPointer&& other) noexcept; BignumPointer& operator=(BignumPointer&& other) noexcept; NCRYPTO_DISALLOW_COPY(BignumPointer) ~BignumPointer(); - bool operator==(const BignumPointer& other) noexcept; - bool operator==(const BIGNUM* other) noexcept; - inline bool operator==(std::nullptr_t) noexcept { return bn_ == nullptr; } + int operator<=>(const BignumPointer& other) const noexcept; + int operator<=>(const BIGNUM* other) const noexcept; inline operator bool() const { return bn_ != nullptr; } inline BIGNUM* get() const noexcept { return bn_.get(); } void reset(BIGNUM* bn = nullptr); + void reset(const unsigned char* data, size_t len); BIGNUM* release(); - size_t byteLength(); + bool isZero() const; + bool isOne() const; - std::vector encode(); - std::vector encodePadded(size_t size); + bool setWord(unsigned long w); + unsigned long getWord() const; - static std::vector encode(const BIGNUM* bn); - static std::vector encodePadded(const BIGNUM* bn, size_t size); + size_t byteLength() const; + + DataPointer toHex() const; + DataPointer encode() const; + DataPointer encodePadded(size_t size) const; + size_t encodeInto(unsigned char* out) const; + size_t encodePaddedInto(unsigned char* out, size_t size) const; + + static BignumPointer New(); + static BignumPointer NewSecure(); + static DataPointer Encode(const BIGNUM* bn); + static DataPointer EncodePadded(const BIGNUM* bn, size_t size); + static size_t EncodePaddedInto(const BIGNUM* bn, unsigned char* out, size_t size); + static int GetBitCount(const BIGNUM* bn); + static int GetByteCount(const BIGNUM* bn); + static unsigned long GetWord(const BIGNUM* bn); + static const BIGNUM* One(); private: DeleteFnPtr bn_; @@ -269,12 +340,6 @@ bool testFipsEnabled(); // ============================================================================ // Various utilities -template -struct Buffer { - T* data = nullptr; - size_t len = 0; -}; - bool CSPRNG(void* buffer, size_t length) NCRYPTO_MUST_USE_RESULT; // This callback is used to avoid the default passphrase callback in OpenSSL @@ -286,6 +351,9 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u); int PasswordCallback(char* buf, int size, int rwflag, void* u); +bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext); +bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext); + // ============================================================================ // SPKAC diff --git a/src/crypto/crypto_aes.cc b/src/crypto/crypto_aes.cc index 774030d408711c..fa93f767574299 100644 --- a/src/crypto/crypto_aes.cc +++ b/src/crypto/crypto_aes.cc @@ -185,10 +185,7 @@ BignumPointer GetCounter(const AESCipherConfig& params) { if (remainder == 0) { unsigned int byte_length = params.length / CHAR_BIT; - return BignumPointer(BN_bin2bn( - data + params.iv.size() - byte_length, - byte_length, - nullptr)); + return BignumPointer(data + params.iv.size() - byte_length, byte_length); } unsigned int byte_length = @@ -199,7 +196,7 @@ BignumPointer GetCounter(const AESCipherConfig& params) { data + params.iv.size()); counter[0] &= ~(0xFF << remainder); - return BignumPointer(BN_bin2bn(counter.data(), counter.size(), nullptr)); + return BignumPointer(counter.data(), counter.size()); } std::vector BlockWithZeroedCounter( @@ -269,23 +266,22 @@ WebCryptoCipherStatus AES_CTR_Cipher( const AESCipherConfig& params, const ByteSource& in, ByteSource* out) { - BignumPointer num_counters(BN_new()); - if (!BN_lshift(num_counters.get(), BN_value_one(), params.length)) + auto num_counters = BignumPointer::New(); + if (!BN_lshift(num_counters.get(), BignumPointer::One(), params.length)) return WebCryptoCipherStatus::FAILED; BignumPointer current_counter = GetCounter(params); - BignumPointer num_output(BN_new()); + auto num_output = BignumPointer::New(); - if (!BN_set_word(num_output.get(), CeilDiv(in.size(), kAesBlockSize))) + if (!num_output.setWord(CeilDiv(in.size(), kAesBlockSize))) return WebCryptoCipherStatus::FAILED; // Just like in chromium's implementation, if the counter will // be incremented more than there are counter values, we fail. - if (BN_cmp(num_output.get(), num_counters.get()) > 0) - return WebCryptoCipherStatus::FAILED; + if (num_output > num_counters) return WebCryptoCipherStatus::FAILED; - BignumPointer remaining_until_reset(BN_new()); + auto remaining_until_reset = BignumPointer::New(); if (!BN_sub(remaining_until_reset.get(), num_counters.get(), current_counter.get())) { @@ -298,7 +294,7 @@ WebCryptoCipherStatus AES_CTR_Cipher( // Also just like in chromium's implementation, if we can process // the input without wrapping the counter, we'll do it as a single // call here. If we can't, we'll fallback to the a two-step approach - if (BN_cmp(remaining_until_reset.get(), num_output.get()) >= 0) { + if (remaining_until_reset >= num_output) { auto status = AES_CTR_Cipher2(key_data, cipher_mode, params, @@ -309,8 +305,7 @@ WebCryptoCipherStatus AES_CTR_Cipher( return status; } - BN_ULONG blocks_part1 = BN_get_word(remaining_until_reset.get()); - BN_ULONG input_size_part1 = blocks_part1 * kAesBlockSize; + BN_ULONG input_size_part1 = remaining_until_reset.getWord() * kAesBlockSize; // Encrypt the first part... auto status = diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 98f29b5a6a1351..15f25e3cc42091 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -1,8 +1,10 @@ #include "crypto/crypto_common.h" #include "base_object-inl.h" +#include "crypto/crypto_util.h" #include "env-inl.h" #include "memory_tracker-inl.h" #include "nbytes.h" +#include "ncrypto.h" #include "node.h" #include "node_buffer.h" #include "node_crypto.h" @@ -47,11 +49,6 @@ static constexpr int kX509NameFlagsMultiline = XN_FLAG_SEP_MULTILINE | XN_FLAG_FN_SN; -static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON = - XN_FLAG_RFC2253 & - ~ASN1_STRFLGS_ESC_MSB & - ~ASN1_STRFLGS_ESC_CTRL; - X509Pointer SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert) { X509_STORE* store = SSL_CTX_get_cert_store(ctx); DeleteFnPtr store_ctx( @@ -469,13 +466,13 @@ MaybeLocal GetExponentString( Environment* env, const BIOPointer& bio, const BIGNUM* e) { - uint64_t exponent_word = static_cast(BN_get_word(e)); + uint64_t exponent_word = static_cast(BignumPointer::GetWord(e)); BIO_printf(bio.get(), "0x%" PRIx64, exponent_word); return ToV8Value(env, bio); } Local GetBits(Environment* env, const BIGNUM* n) { - return Integer::New(env->isolate(), BN_num_bits(n)); + return Integer::New(env->isolate(), BignumPointer::GetBitCount(n)); } MaybeLocal GetModulusString( @@ -505,11 +502,11 @@ MaybeLocal GetRawDERCertificate(Environment* env, X509* cert) { MaybeLocal GetSerialNumber(Environment* env, X509* cert) { if (ASN1_INTEGER* serial_number = X509_get_serialNumber(cert)) { - BignumPointer bn(ASN1_INTEGER_to_BN(serial_number, nullptr)); - if (bn) { - char* data = BN_bn2hex(bn.get()); - ByteSource buf = ByteSource::Allocated(data, strlen(data)); - if (buf) return OneByteString(env->isolate(), buf.data()); + if (auto bn = BignumPointer(ASN1_INTEGER_to_BN(serial_number, nullptr))) { + if (auto hex = bn.toHex()) { + return OneByteString(env->isolate(), + static_cast(hex.get())); + } } } @@ -580,291 +577,6 @@ MaybeLocal GetValidFrom( return ToV8Value(env, bio); } -static inline bool IsSafeAltName(const char* name, size_t length, bool utf8) { - for (size_t i = 0; i < length; i++) { - char c = name[i]; - switch (c) { - case '"': - case '\\': - // These mess with encoding rules. - // Fall through. - case ',': - // Commas make it impossible to split the list of subject alternative - // names unambiguously, which is why we have to escape. - // Fall through. - case '\'': - // Single quotes are unlikely to appear in any legitimate values, but they - // could be used to make a value look like it was escaped (i.e., enclosed - // in single/double quotes). - return false; - default: - if (utf8) { - // In UTF8 strings, we require escaping for any ASCII control character, - // but NOT for non-ASCII characters. Note that all bytes of any code - // point that consists of more than a single byte have their MSB set. - if (static_cast(c) < ' ' || c == '\x7f') { - return false; - } - } else { - // Check if the char is a control character or non-ASCII character. Note - // that char may or may not be a signed type. Regardless, non-ASCII - // values will always be outside of this range. - if (c < ' ' || c > '~') { - return false; - } - } - } - } - return true; -} - -static inline void PrintAltName(const BIOPointer& out, const char* name, - size_t length, bool utf8, - const char* safe_prefix) { - if (IsSafeAltName(name, length, utf8)) { - // For backward-compatibility, append "safe" names without any - // modifications. - if (safe_prefix != nullptr) { - BIO_printf(out.get(), "%s:", safe_prefix); - } - BIO_write(out.get(), name, length); - } else { - // If a name is not "safe", we cannot embed it without special - // encoding. This does not usually happen, but we don't want to hide - // it from the user either. We use JSON compatible escaping here. - BIO_write(out.get(), "\"", 1); - if (safe_prefix != nullptr) { - BIO_printf(out.get(), "%s:", safe_prefix); - } - for (size_t j = 0; j < length; j++) { - char c = static_cast(name[j]); - if (c == '\\') { - BIO_write(out.get(), "\\\\", 2); - } else if (c == '"') { - BIO_write(out.get(), "\\\"", 2); - } else if ((c >= ' ' && c != ',' && c <= '~') || (utf8 && (c & 0x80))) { - // Note that the above condition explicitly excludes commas, which means - // that those are encoded as Unicode escape sequences in the "else" - // block. That is not strictly necessary, and Node.js itself would parse - // it correctly either way. We only do this to account for third-party - // code that might be splitting the string at commas (as Node.js itself - // used to do). - BIO_write(out.get(), &c, 1); - } else { - // Control character or non-ASCII character. We treat everything as - // Latin-1, which corresponds to the first 255 Unicode code points. - const char hex[] = "0123456789abcdef"; - char u[] = { '\\', 'u', '0', '0', hex[(c & 0xf0) >> 4], hex[c & 0x0f] }; - BIO_write(out.get(), u, sizeof(u)); - } - } - BIO_write(out.get(), "\"", 1); - } -} - -static inline void PrintLatin1AltName(const BIOPointer& out, - const ASN1_IA5STRING* name, - const char* safe_prefix = nullptr) { - PrintAltName(out, reinterpret_cast(name->data), name->length, - false, safe_prefix); -} - -static inline void PrintUtf8AltName(const BIOPointer& out, - const ASN1_UTF8STRING* name, - const char* safe_prefix = nullptr) { - PrintAltName(out, reinterpret_cast(name->data), name->length, - true, safe_prefix); -} - -// This function emulates the behavior of i2v_GENERAL_NAME in a safer and less -// ambiguous way. "othername:" entries use the GENERAL_NAME_print format. -static bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) { - if (gen->type == GEN_DNS) { - ASN1_IA5STRING* name = gen->d.dNSName; - BIO_write(out.get(), "DNS:", 4); - // Note that the preferred name syntax (see RFCs 5280 and 1034) with - // wildcards is a subset of what we consider "safe", so spec-compliant DNS - // names will never need to be escaped. - PrintLatin1AltName(out, name); - } else if (gen->type == GEN_EMAIL) { - ASN1_IA5STRING* name = gen->d.rfc822Name; - BIO_write(out.get(), "email:", 6); - PrintLatin1AltName(out, name); - } else if (gen->type == GEN_URI) { - ASN1_IA5STRING* name = gen->d.uniformResourceIdentifier; - BIO_write(out.get(), "URI:", 4); - // The set of "safe" names was designed to include just about any URI, - // with a few exceptions, most notably URIs that contains commas (see - // RFC 2396). In other words, most legitimate URIs will not require - // escaping. - PrintLatin1AltName(out, name); - } else if (gen->type == GEN_DIRNAME) { - // Earlier versions of Node.js used X509_NAME_oneline to print the X509_NAME - // object. The format was non standard and should be avoided. The use of - // X509_NAME_oneline is discouraged by OpenSSL but was required for backward - // compatibility. Conveniently, X509_NAME_oneline produced ASCII and the - // output was unlikely to contains commas or other characters that would - // require escaping. However, it SHOULD NOT produce ASCII output since an - // RFC5280 AttributeValue may be a UTF8String. - // Newer versions of Node.js have since switched to X509_NAME_print_ex to - // produce a better format at the cost of backward compatibility. The new - // format may contain Unicode characters and it is likely to contain commas, - // which require escaping. Fortunately, the recently safeguarded function - // PrintAltName handles all of that safely. - BIO_printf(out.get(), "DirName:"); - BIOPointer tmp(BIO_new(BIO_s_mem())); - CHECK(tmp); - if (X509_NAME_print_ex(tmp.get(), - gen->d.dirn, - 0, - kX509NameFlagsRFC2253WithinUtf8JSON) < 0) { - return false; - } - char* oline = nullptr; - long n_bytes = BIO_get_mem_data(tmp.get(), &oline); // NOLINT(runtime/int) - CHECK_GE(n_bytes, 0); - CHECK_IMPLIES(n_bytes != 0, oline != nullptr); - PrintAltName(out, oline, static_cast(n_bytes), true, nullptr); - } else if (gen->type == GEN_IPADD) { - BIO_printf(out.get(), "IP Address:"); - const ASN1_OCTET_STRING* ip = gen->d.ip; - const unsigned char* b = ip->data; - if (ip->length == 4) { - BIO_printf(out.get(), "%d.%d.%d.%d", b[0], b[1], b[2], b[3]); - } else if (ip->length == 16) { - for (unsigned int j = 0; j < 8; j++) { - uint16_t pair = (b[2 * j] << 8) | b[2 * j + 1]; - BIO_printf(out.get(), (j == 0) ? "%X" : ":%X", pair); - } - } else { -#if OPENSSL_VERSION_MAJOR >= 3 - BIO_printf(out.get(), "", ip->length); -#else - BIO_printf(out.get(), ""); -#endif - } - } else if (gen->type == GEN_RID) { - // Unlike OpenSSL's default implementation, never print the OID as text and - // instead always print its numeric representation. - char oline[256]; - OBJ_obj2txt(oline, sizeof(oline), gen->d.rid, true); - BIO_printf(out.get(), "Registered ID:%s", oline); - } else if (gen->type == GEN_OTHERNAME) { - // The format that is used here is based on OpenSSL's implementation of - // GENERAL_NAME_print (as of OpenSSL 3.0.1). Earlier versions of Node.js - // instead produced the same format as i2v_GENERAL_NAME, which was somewhat - // awkward, especially when passed to translatePeerCertificate. - bool unicode = true; - const char* prefix = nullptr; - // OpenSSL 1.1.1 does not support othername in GENERAL_NAME_print and may - // not define these NIDs. -#if OPENSSL_VERSION_MAJOR >= 3 - int nid = OBJ_obj2nid(gen->d.otherName->type_id); - switch (nid) { - case NID_id_on_SmtpUTF8Mailbox: - prefix = "SmtpUTF8Mailbox"; - break; - case NID_XmppAddr: - prefix = "XmppAddr"; - break; - case NID_SRVName: - prefix = "SRVName"; - unicode = false; - break; - case NID_ms_upn: - prefix = "UPN"; - break; - case NID_NAIRealm: - prefix = "NAIRealm"; - break; - } -#endif // OPENSSL_VERSION_MAJOR >= 3 - int val_type = gen->d.otherName->value->type; - if (prefix == nullptr || - (unicode && val_type != V_ASN1_UTF8STRING) || - (!unicode && val_type != V_ASN1_IA5STRING)) { - BIO_printf(out.get(), "othername:"); - } else { - BIO_printf(out.get(), "othername:"); - if (unicode) { - PrintUtf8AltName(out, gen->d.otherName->value->value.utf8string, - prefix); - } else { - PrintLatin1AltName(out, gen->d.otherName->value->value.ia5string, - prefix); - } - } - } else if (gen->type == GEN_X400) { - // TODO(tniessen): this is what OpenSSL does, implement properly instead - BIO_printf(out.get(), "X400Name:"); - } else if (gen->type == GEN_EDIPARTY) { - // TODO(tniessen): this is what OpenSSL does, implement properly instead - BIO_printf(out.get(), "EdiPartyName:"); - } else { - // This is safe because X509V3_EXT_d2i would have returned nullptr in this - // case already. - UNREACHABLE(); - } - - return true; -} - -bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext) { - CHECK_EQ(OBJ_obj2nid(X509_EXTENSION_get_object(ext)), NID_subject_alt_name); - - GENERAL_NAMES* names = static_cast(X509V3_EXT_d2i(ext)); - if (names == nullptr) - return false; - - bool ok = true; - - for (int i = 0; i < sk_GENERAL_NAME_num(names); i++) { - GENERAL_NAME* gen = sk_GENERAL_NAME_value(names, i); - - if (i != 0) - BIO_write(out.get(), ", ", 2); - - if (!(ok = PrintGeneralName(out, gen))) { - break; - } - } - sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free); - - return ok; -} - -bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) { - CHECK_EQ(OBJ_obj2nid(X509_EXTENSION_get_object(ext)), NID_info_access); - - AUTHORITY_INFO_ACCESS* descs = - static_cast(X509V3_EXT_d2i(ext)); - if (descs == nullptr) - return false; - - bool ok = true; - - for (int i = 0; i < sk_ACCESS_DESCRIPTION_num(descs); i++) { - ACCESS_DESCRIPTION* desc = sk_ACCESS_DESCRIPTION_value(descs, i); - - if (i != 0) - BIO_write(out.get(), "\n", 1); - - char objtmp[80]; - i2t_ASN1_OBJECT(objtmp, sizeof(objtmp), desc->method); - BIO_printf(out.get(), "%s - ", objtmp); - if (!(ok = PrintGeneralName(out, desc->location))) { - break; - } - } - sk_ACCESS_DESCRIPTION_pop_free(descs, ACCESS_DESCRIPTION_free); - -#if OPENSSL_VERSION_MAJOR < 3 - BIO_write(out.get(), "\n", 1); -#endif - - return ok; -} - v8::MaybeLocal GetSubjectAltNameString(Environment* env, X509* cert, const BIOPointer& bio) { @@ -875,7 +587,7 @@ v8::MaybeLocal GetSubjectAltNameString(Environment* env, X509_EXTENSION* ext = X509_get_ext(cert, index); CHECK_NOT_NULL(ext); - if (!SafeX509SubjectAltNamePrint(bio, ext)) { + if (!ncrypto::SafeX509SubjectAltNamePrint(bio, ext)) { CHECK_EQ(BIO_reset(bio.get()), 1); return v8::Null(env->isolate()); } @@ -893,7 +605,7 @@ v8::MaybeLocal GetInfoAccessString(Environment* env, X509_EXTENSION* ext = X509_get_ext(cert, index); CHECK_NOT_NULL(ext); - if (!SafeX509InfoAccessPrint(bio, ext)) { + if (!ncrypto::SafeX509InfoAccessPrint(bio, ext)) { CHECK_EQ(BIO_reset(bio.get()), 1); return v8::Null(env->isolate()); } diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h index a11565c1ac0e6c..449f06ae021627 100644 --- a/src/crypto/crypto_common.h +++ b/src/crypto/crypto_common.h @@ -129,7 +129,6 @@ v8::MaybeLocal GetSerialNumber(Environment* env, X509* cert); v8::MaybeLocal GetRawDERCertificate(Environment* env, X509* cert); v8::Local ToV8Value(Environment* env, const BIOPointer& bio); -bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext); v8::MaybeLocal GetSubject(Environment* env, X509* cert, diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 7b8fa8a589a62c..a60443a2aa151e 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -920,7 +920,7 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo& args) { const BIGNUM* p; DH_get0_pqg(dh.get(), &p, nullptr, nullptr); - const int size = BN_num_bits(p); + const int size = BignumPointer::GetBitCount(p); if (size < 1024) { return THROW_ERR_INVALID_ARG_VALUE( env, "DH parameter is less than 1024 bits"); diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index dac37f52b9687c..57dd1c0b36bb47 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -142,12 +142,27 @@ void DiffieHellman::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackFieldWithSize("dh", dh_ ? kSizeOf_DH : 0); } +namespace { +bool SetDhParams(DH* dh, BignumPointer* p, BignumPointer* g) { + // If DH_set0_pqg returns 0, ownership of the input parameters has + // not been transferred to the DH object. If the return value is 1, + // ownership has been transferred and we need to release them. + // The documentation for DH_set0_pqg is not clear on this point. + // It says that ownership is transfered when the method is called + // but there is an internal check that returns 0 if the input is + // not valid, and in that case ownership is not transferred. + if (DH_set0_pqg(dh, p->get(), nullptr, g->get()) == 0) return false; + p->release(); + g->release(); + return true; +} +} // namespace + bool DiffieHellman::Init(BignumPointer&& bn_p, int g) { dh_.reset(DH_new()); CHECK_GE(g, 2); - BignumPointer bn_g(BN_new()); - return bn_g && BN_set_word(bn_g.get(), g) && - DH_set0_pqg(dh_.get(), bn_p.release(), nullptr, bn_g.release()) && + auto bn_g = BignumPointer::New(); + return bn_p && bn_g.setWord(g) && SetDhParams(dh_.get(), &bn_p, &bn_g) && VerifyContext(); } @@ -163,14 +178,10 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) { DH_R_BAD_GENERATOR, __FILE__, __LINE__); return false; } - BignumPointer bn_p( - BN_bin2bn(reinterpret_cast(p), p_len, nullptr)); - BignumPointer bn_g(BN_new()); - if (bn_p == nullptr || bn_g == nullptr || !BN_set_word(bn_g.get(), g) || - !DH_set0_pqg(dh_.get(), bn_p.release(), nullptr, bn_g.release())) { - return false; - } - return VerifyContext(); + BignumPointer bn_p(reinterpret_cast(p), p_len); + auto bn_g = BignumPointer::New(); + return bn_p && bn_g.setWord(g) && SetDhParams(dh_.get(), &bn_p, &bn_g) && + VerifyContext(); } bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) { @@ -185,24 +196,14 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) { DH_R_BAD_GENERATOR, __FILE__, __LINE__); return false; } - BignumPointer bn_g( - BN_bin2bn(reinterpret_cast(g), g_len, nullptr)); - if (BN_is_zero(bn_g.get()) || BN_is_one(bn_g.get())) { + BignumPointer bn_g(reinterpret_cast(g), g_len); + if (!bn_g || bn_g.isZero() || bn_g.isOne()) { ERR_put_error(ERR_LIB_DH, DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR, __FILE__, __LINE__); return false; } - BignumPointer bn_p( - BN_bin2bn(reinterpret_cast(p), p_len, nullptr)); - if (!DH_set0_pqg(dh_.get(), bn_p.get(), nullptr, bn_g.get())) { - return false; - } - // The DH_set0_pqg call above takes ownership of the bignums on success, - // so we should release them here so we don't end with a possible - // use-after-free or double free. - bn_p.release(); - bn_g.release(); - return VerifyContext(); + BignumPointer bn_p(reinterpret_cast(p), p_len); + return bn_p && SetDhParams(dh_.get(), &bn_p, &bn_g) && VerifyContext(); } constexpr int kStandardizedGenerator = 2; @@ -303,16 +304,16 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { std::unique_ptr bs; { - const int size = BN_num_bytes(pub_key); + const int size = BignumPointer::GetByteCount(pub_key); CHECK_GE(size, 0); NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); bs = ArrayBuffer::NewBackingStore(env->isolate(), size); } - CHECK_EQ(static_cast(bs->ByteLength()), - BN_bn2binpad(pub_key, - static_cast(bs->Data()), - bs->ByteLength())); + CHECK_EQ( + bs->ByteLength(), + BignumPointer::EncodePaddedInto( + pub_key, static_cast(bs->Data()), bs->ByteLength())); Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); Local buffer; @@ -335,16 +336,15 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, std::unique_ptr bs; { - const int size = BN_num_bytes(num); + const int size = BignumPointer::GetByteCount(num); CHECK_GE(size, 0); NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); bs = ArrayBuffer::NewBackingStore(env->isolate(), size); } - CHECK_EQ(static_cast(bs->ByteLength()), - BN_bn2binpad(num, - static_cast(bs->Data()), - bs->ByteLength())); + CHECK_EQ(bs->ByteLength(), + BignumPointer::EncodePaddedInto( + num, static_cast(bs->Data()), bs->ByteLength())); Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); Local buffer; @@ -396,7 +396,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { ArrayBufferOrViewContents key_buf(args[0]); if (UNLIKELY(!key_buf.CheckSizeInt32())) return THROW_ERR_OUT_OF_RANGE(env, "secret is too big"); - BignumPointer key(BN_bin2bn(key_buf.data(), key_buf.size(), nullptr)); + BignumPointer key(key_buf.data(), key_buf.size()); std::unique_ptr bs; { @@ -452,9 +452,9 @@ void DiffieHellman::SetKey(const FunctionCallbackInfo& args, ArrayBufferOrViewContents buf(args[0]); if (UNLIKELY(!buf.CheckSizeInt32())) return THROW_ERR_OUT_OF_RANGE(env, "buf is too big"); - BIGNUM* num = BN_bin2bn(buf.data(), buf.size(), nullptr); - CHECK_NOT_NULL(num); - CHECK_EQ(1, set_field(dh->dh_.get(), num)); + BignumPointer num(buf.data(), buf.size()); + CHECK(num); + CHECK_EQ(1, set_field(dh->dh_.get(), num.release())); } void DiffieHellman::SetPublicKey(const FunctionCallbackInfo& args) { @@ -532,8 +532,7 @@ Maybe DhKeyGenTraits::AdditionalConfig( THROW_ERR_OUT_OF_RANGE(env, "prime is too big"); return Nothing(); } - params->params.prime = BignumPointer( - BN_bin2bn(input.data(), input.size(), nullptr)); + params->params.prime = BignumPointer(input.data(), input.size()); } CHECK(args[*offset + 1]->IsInt32()); @@ -552,16 +551,12 @@ EVPKeyCtxPointer DhKeyGenTraits::Setup(DhKeyPairGenConfig* params) { if (!dh) return EVPKeyCtxPointer(); - BIGNUM* prime = prime_fixed_value->get(); - BignumPointer bn_g(BN_new()); - if (!BN_set_word(bn_g.get(), params->params.generator) || - !DH_set0_pqg(dh.get(), prime, nullptr, bn_g.get())) { + auto bn_g = BignumPointer::New(); + if (!bn_g.setWord(params->params.generator) || + !SetDhParams(dh.get(), prime_fixed_value, &bn_g)) { return EVPKeyCtxPointer(); } - prime_fixed_value->release(); - bn_g.release(); - key_params = EVPKeyPointer(EVP_PKEY_new()); CHECK(key_params); CHECK_EQ(EVP_PKEY_assign_DH(key_params.get(), dh.release()), 1); diff --git a/src/crypto/crypto_dsa.cc b/src/crypto/crypto_dsa.cc index 3fa4a415dc911a..57f336dff1d880 100644 --- a/src/crypto/crypto_dsa.cc +++ b/src/crypto/crypto_dsa.cc @@ -143,8 +143,8 @@ Maybe GetDsaKeyDetail( DSA_get0_pqg(dsa, &p, &q, nullptr); - size_t modulus_length = BN_num_bits(p); - size_t divisor_length = BN_num_bits(q); + size_t modulus_length = BignumPointer::GetBitCount(p); + size_t divisor_length = BignumPointer::GetBitCount(q); if (target ->Set( diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index e8653c7db88590..cd6bee36dae102 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -273,10 +273,11 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { std::unique_ptr bs; { NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); - bs = ArrayBuffer::NewBackingStore(env->isolate(), BN_num_bytes(b)); + bs = ArrayBuffer::NewBackingStore(env->isolate(), + BignumPointer::GetByteCount(b)); } - CHECK_EQ(static_cast(bs->ByteLength()), - BN_bn2binpad( + CHECK_EQ(bs->ByteLength(), + BignumPointer::EncodePaddedInto( b, static_cast(bs->Data()), bs->ByteLength())); Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); @@ -295,8 +296,7 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo& args) { if (UNLIKELY(!priv_buffer.CheckSizeInt32())) return THROW_ERR_OUT_OF_RANGE(env, "key is too big"); - BignumPointer priv(BN_bin2bn( - priv_buffer.data(), priv_buffer.size(), nullptr)); + BignumPointer priv(priv_buffer.data(), priv_buffer.size()); if (!priv) { return THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to convert Buffer to BN"); @@ -372,13 +372,13 @@ bool ECDH::IsKeyValidForCurve(const BignumPointer& private_key) { CHECK(private_key); // Private keys must be in the range [1, n-1]. // Ref: Section 3.2.1 - http://www.secg.org/sec1-v2.pdf - if (BN_cmp(private_key.get(), BN_value_one()) < 0) { + if (private_key < BignumPointer::One()) { return false; } - BignumPointer order(BN_new()); + auto order = BignumPointer::New(); CHECK(order); return EC_GROUP_get_order(group_, order.get(), nullptr) && - BN_cmp(private_key.get(), order.get()) < 0; + private_key < order; } bool ECDH::IsKeyPairValid() { @@ -767,8 +767,8 @@ Maybe ExportJWKEcKey( int degree_bytes = (degree_bits / CHAR_BIT) + (7 + (degree_bits % CHAR_BIT)) / 8; - BignumPointer x(BN_new()); - BignumPointer y(BN_new()); + auto x = BignumPointer::New(); + auto y = BignumPointer::New(); if (!EC_POINT_get_affine_coordinates(group, pub, x.get(), y.get(), nullptr)) { ThrowCryptoError(env, ERR_get_error(), @@ -1005,9 +1005,9 @@ size_t GroupOrderSize(const ManagedEVPPKey& key) { const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(key.get()); CHECK_NOT_NULL(ec); const EC_GROUP* group = EC_KEY_get0_group(ec); - BignumPointer order(BN_new()); + auto order = BignumPointer::New(); CHECK(EC_GROUP_get_order(group, order.get(), nullptr)); - return BN_num_bytes(order.get()); + return order.byteLength(); } } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index 843eaeeabcfffc..7ef2526e986617 100644 --- a/src/crypto/crypto_random.cc +++ b/src/crypto/crypto_random.cc @@ -9,6 +9,7 @@ #include #include +#include namespace node { @@ -73,13 +74,14 @@ Maybe RandomPrimeTraits::EncodeOutput( const RandomPrimeConfig& params, ByteSource* unused, v8::Local* result) { - size_t size = BN_num_bytes(params.prime.get()); + size_t size = params.prime.byteLength(); std::shared_ptr store = ArrayBuffer::NewBackingStore(env->isolate(), size); - CHECK_EQ(static_cast(size), - BN_bn2binpad(params.prime.get(), - reinterpret_cast(store->Data()), - size)); + CHECK_EQ(size, + BignumPointer::EncodePaddedInto( + params.prime.get(), + reinterpret_cast(store->Data()), + size)); *result = ArrayBuffer::New(env->isolate(), store); return Just(true); } @@ -99,7 +101,7 @@ Maybe RandomPrimeTraits::AdditionalConfig( if (!args[offset + 2]->IsUndefined()) { ArrayBufferOrViewContents add(args[offset + 2]); - params->add.reset(BN_bin2bn(add.data(), add.size(), nullptr)); + params->add.reset(add.data(), add.size()); if (!params->add) { THROW_ERR_CRYPTO_OPERATION_FAILED(env, "could not generate prime"); return Nothing(); @@ -108,7 +110,7 @@ Maybe RandomPrimeTraits::AdditionalConfig( if (!args[offset + 3]->IsUndefined()) { ArrayBufferOrViewContents rem(args[offset + 3]); - params->rem.reset(BN_bin2bn(rem.data(), rem.size(), nullptr)); + params->rem.reset(rem.data(), rem.size()); if (!params->rem) { THROW_ERR_CRYPTO_OPERATION_FAILED(env, "could not generate prime"); return Nothing(); @@ -120,7 +122,7 @@ Maybe RandomPrimeTraits::AdditionalConfig( CHECK_GT(bits, 0); if (params->add) { - if (BN_num_bits(params->add.get()) > bits) { + if (BignumPointer::GetBitCount(params->add.get()) > bits) { // If we allowed this, the best case would be returning a static prime // that wasn't generated randomly. The worst case would be an infinite // loop within OpenSSL, blocking the main thread or one of the threads @@ -129,19 +131,17 @@ Maybe RandomPrimeTraits::AdditionalConfig( return Nothing(); } - if (params->rem) { - if (BN_cmp(params->add.get(), params->rem.get()) != 1) { - // This would definitely lead to an infinite loop if allowed since - // OpenSSL does not check this condition. - THROW_ERR_OUT_OF_RANGE(env, "invalid options.rem"); - return Nothing(); - } + if (params->rem && params->add <= params->rem) { + // This would definitely lead to an infinite loop if allowed since + // OpenSSL does not check this condition. + THROW_ERR_OUT_OF_RANGE(env, "invalid options.rem"); + return Nothing(); } } params->bits = bits; params->safe = safe; - params->prime.reset(BN_secure_new()); + params->prime = BignumPointer::NewSecure(); if (!params->prime) { THROW_ERR_CRYPTO_OPERATION_FAILED(env, "could not generate prime"); return Nothing(); @@ -171,8 +171,7 @@ bool RandomPrimeTraits::DeriveBits(Environment* env, } void CheckPrimeConfig::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackFieldWithSize( - "prime", candidate ? BN_num_bytes(candidate.get()) : 0); + tracker->TrackFieldWithSize("prime", candidate ? candidate.byteLength() : 0); } Maybe CheckPrimeTraits::AdditionalConfig( @@ -182,11 +181,7 @@ Maybe CheckPrimeTraits::AdditionalConfig( CheckPrimeConfig* params) { ArrayBufferOrViewContents candidate(args[offset]); - params->candidate = - BignumPointer(BN_bin2bn( - candidate.data(), - candidate.size(), - nullptr)); + params->candidate = BignumPointer(candidate.data(), candidate.size()); CHECK(args[offset + 1]->IsInt32()); // Checks params->checks = args[offset + 1].As()->Value(); diff --git a/src/crypto/crypto_rsa.cc b/src/crypto/crypto_rsa.cc index 100f94460686bd..7a9e083de111e9 100644 --- a/src/crypto/crypto_rsa.cc +++ b/src/crypto/crypto_rsa.cc @@ -49,9 +49,8 @@ EVPKeyCtxPointer RsaKeyGenTraits::Setup(RsaKeyPairGenConfig* params) { // 0x10001 is the default RSA exponent. if (params->params.exponent != 0x10001) { - BignumPointer bn(BN_new()); - CHECK_NOT_NULL(bn.get()); - CHECK(BN_set_word(bn.get(), params->params.exponent)); + auto bn = BignumPointer::New(); + CHECK(bn.setWord(params->params.exponent)); // EVP_CTX accepts ownership of bn on success. if (EVP_PKEY_CTX_set_rsa_keygen_pubexp(ctx.get(), bn.get()) <= 0) return EVPKeyCtxPointer(); @@ -529,13 +528,11 @@ Maybe GetRsaKeyDetail( RSA_get0_key(rsa, &n, &e, nullptr); - size_t modulus_length = BN_num_bits(n); - if (target - ->Set( - env->context(), - env->modulus_length_string(), - Number::New(env->isolate(), static_cast(modulus_length))) + ->Set(env->context(), + env->modulus_length_string(), + Number::New(env->isolate(), + static_cast(BignumPointer::GetBitCount(n)))) .IsNothing()) { return Nothing(); } @@ -543,13 +540,14 @@ Maybe GetRsaKeyDetail( std::unique_ptr public_exponent; { NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); - public_exponent = - ArrayBuffer::NewBackingStore(env->isolate(), BN_num_bytes(e)); + public_exponent = ArrayBuffer::NewBackingStore( + env->isolate(), BignumPointer::GetByteCount(e)); } - CHECK_EQ(BN_bn2binpad(e, - static_cast(public_exponent->Data()), - public_exponent->ByteLength()), - static_cast(public_exponent->ByteLength())); + CHECK_EQ(BignumPointer::EncodePaddedInto( + e, + static_cast(public_exponent->Data()), + public_exponent->ByteLength()), + public_exponent->ByteLength()); if (target ->Set(env->context(), diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 3441d7e7718ad6..0474bfff319725 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -6,6 +6,7 @@ #include "crypto/crypto_util.h" #include "env-inl.h" #include "memory_tracker-inl.h" +#include "openssl/ec.h" #include "threadpoolwork-inl.h" #include "v8.h" @@ -39,11 +40,10 @@ bool ValidateDSAParameters(EVP_PKEY* key) { #endif const DSA* dsa = EVP_PKEY_get0_DSA(key); const BIGNUM* p; - DSA_get0_pqg(dsa, &p, nullptr, nullptr); - size_t L = BN_num_bits(p); const BIGNUM* q; - DSA_get0_pqg(dsa, nullptr, &q, nullptr); - size_t N = BN_num_bits(q); + DSA_get0_pqg(dsa, &p, &q, nullptr); + int L = BignumPointer::GetBitCount(p); + int N = BignumPointer::GetBitCount(q); return (L == 1024 && N == 160) || (L == 2048 && N == 224) || @@ -125,7 +125,7 @@ unsigned int GetBytesOfRS(const ManagedEVPPKey& pkey) { if (base_id == EVP_PKEY_DSA) { const DSA* dsa_key = EVP_PKEY_get0_DSA(pkey.get()); // Both r and s are computed mod q, so their width is limited by that of q. - bits = BN_num_bits(DSA_get0_q(dsa_key)); + bits = BignumPointer::GetBitCount(DSA_get0_q(dsa_key)); } else if (base_id == EVP_PKEY_EC) { const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(pkey.get()); const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key); @@ -146,10 +146,12 @@ bool ExtractP1363( if (!asn1_sig) return false; - const BIGNUM* pr = ECDSA_SIG_get0_r(asn1_sig.get()); - const BIGNUM* ps = ECDSA_SIG_get0_s(asn1_sig.get()); + const BIGNUM* pr; + const BIGNUM* ps; + ECDSA_SIG_get0(asn1_sig.get(), &pr, &ps); - return BN_bn2binpad(pr, out, n) > 0 && BN_bn2binpad(ps, out + n, n) > 0; + return BignumPointer::EncodePaddedInto(pr, out, n) > 0 && + BignumPointer::EncodePaddedInto(ps, out + n, n) > 0; } // Returns the maximum size of each of the integers (r, s) of the DSA signature. @@ -206,13 +208,11 @@ ByteSource ConvertSignatureToDER( ECDSASigPointer asn1_sig(ECDSA_SIG_new()); CHECK(asn1_sig); - BIGNUM* r = BN_new(); - CHECK_NOT_NULL(r); - BIGNUM* s = BN_new(); - CHECK_NOT_NULL(s); - CHECK_EQ(r, BN_bin2bn(sig_data, n, r)); - CHECK_EQ(s, BN_bin2bn(sig_data + n, n, s)); - CHECK_EQ(1, ECDSA_SIG_set0(asn1_sig.get(), r, s)); + BignumPointer r(sig_data, n); + CHECK(r); + BignumPointer s(sig_data + n, n); + CHECK(s); + CHECK_EQ(1, ECDSA_SIG_set0(asn1_sig.get(), r.release(), s.release())); unsigned char* data = nullptr; int len = i2d_ECDSA_SIG(asn1_sig.get(), &data); diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index b164eaadffe705..78656b5ee6f46d 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -596,14 +596,13 @@ MaybeLocal EncodeBignum( const BIGNUM* bn, int size, Local* error) { - std::vector buf = ncrypto::BignumPointer::encodePadded(bn, size); + auto buf = ncrypto::BignumPointer::EncodePadded(bn, size); CHECK_EQ(buf.size(), static_cast(size)); - return StringBytes::Encode( - env->isolate(), - reinterpret_cast(buf.data()), - buf.size(), - BASE64URL, - error); + return StringBytes::Encode(env->isolate(), + reinterpret_cast(buf.get()), + buf.size(), + BASE64URL, + error); } Maybe SetEncodedValue( @@ -615,8 +614,7 @@ Maybe SetEncodedValue( Local value; Local error; CHECK_NOT_NULL(bn); - if (size == 0) - size = BN_num_bytes(bn); + if (size == 0) size = BignumPointer::GetByteCount(bn); if (!EncodeBignum(env, bn, size, &error).ToLocal(&value)) { if (!error.IsEmpty()) env->isolate()->ThrowException(error); diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 0a027aec0a3814..680c30f42ab727 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -262,7 +262,7 @@ class ByteSource { operator bool() const { return data_ != nullptr; } BignumPointer ToBN() const { - return BignumPointer(BN_bin2bn(data(), size(), nullptr)); + return BignumPointer(data(), size()); } // Creates a v8::BackingStore that takes over responsibility for