Skip to content

Commit 9ada536

Browse files
jasnellnpaun
authored andcommitted
src: cleanup crypto more
* Use ncrypto APIs where appropriate * Remove obsolete no-longer used functions * Improve error handling a bit * move secure heap handling to ncrypto To simplify handling of boringssl/openssl, move secure heap impl to ncrypto. Overall the reduces the complexity of the code in crypto_util by eliminating additional ifdef branches. * simplify CryptoErrorStore::ToException a bit * simplify error handling in crypto_common * move curve utility methods to ncrypto * verify that released DataPointers aren't on secure heap The ByteSource does not currently know how to free a DataPointer allocated on the secure heap, so just verify. DataPointers on the secure heap are not something that users can allocate on their own. Their use is rare. Eventually ByteSource is going to be refactored around ncrypto APIs so these additional checks should be temporary. * simplify some ifdefs that are covered by ncrypto * cleanup some obsolete includes in crypto_util PR-URL: nodejs/node#57323 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent e25bd01 commit 9ada536

File tree

2 files changed

+137
-16
lines changed

2 files changed

+137
-16
lines changed

ncrypto.cc

Lines changed: 103 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,20 +111,64 @@ DataPointer DataPointer::Alloc(size_t len) {
111111
#endif
112112
}
113113

114+
DataPointer DataPointer::SecureAlloc(size_t len) {
115+
#ifndef OPENSSL_IS_BORINGSSL
116+
auto ptr = OPENSSL_secure_zalloc(len);
117+
if (ptr == nullptr) return {};
118+
return DataPointer(ptr, len, true);
119+
#else
120+
// BoringSSL does not implement the OPENSSL_secure_zalloc API.
121+
auto ptr = OPENSSL_malloc(len);
122+
if (ptr == nullptr) return {};
123+
memset(ptr, 0, len);
124+
return DataPointer(ptr, len);
125+
#endif
126+
}
127+
128+
size_t DataPointer::GetSecureHeapUsed() {
129+
#ifndef OPENSSL_IS_BORINGSSL
130+
return CRYPTO_secure_malloc_initialized() ? CRYPTO_secure_used() : 0;
131+
#else
132+
// BoringSSL does not have the secure heap and therefore
133+
// will always return 0.
134+
return 0;
135+
#endif
136+
}
137+
138+
DataPointer::InitSecureHeapResult DataPointer::TryInitSecureHeap(size_t amount,
139+
size_t min) {
140+
#ifndef OPENSSL_IS_BORINGSSL
141+
switch (CRYPTO_secure_malloc_init(amount, min)) {
142+
case 0:
143+
return InitSecureHeapResult::FAILED;
144+
case 2:
145+
return InitSecureHeapResult::UNABLE_TO_MEMORY_MAP;
146+
case 1:
147+
return InitSecureHeapResult::OK;
148+
default:
149+
return InitSecureHeapResult::FAILED;
150+
}
151+
#else
152+
// BoringSSL does not actually support the secure heap
153+
return InitSecureHeapResult::FAILED;
154+
#endif
155+
}
156+
114157
DataPointer DataPointer::Copy(const Buffer<const void>& buffer) {
115158
return DataPointer(OPENSSL_memdup(buffer.data, buffer.len), buffer.len);
116159
}
117160

118-
DataPointer::DataPointer(void* data, size_t length)
119-
: data_(data), len_(length) {}
161+
DataPointer::DataPointer(void* data, size_t length, bool secure)
162+
: data_(data), len_(length), secure_(secure) {}
120163

121-
DataPointer::DataPointer(const Buffer<void>& buffer)
122-
: data_(buffer.data), len_(buffer.len) {}
164+
DataPointer::DataPointer(const Buffer<void>& buffer, bool secure)
165+
: data_(buffer.data), len_(buffer.len), secure_(secure) {}
123166

124167
DataPointer::DataPointer(DataPointer&& other) noexcept
125-
: data_(other.data_), len_(other.len_) {
168+
: data_(other.data_), len_(other.len_), secure_(other.secure_) {
126169
other.data_ = nullptr;
127170
other.len_ = 0;
171+
other.secure_ = false;
128172
}
129173

130174
DataPointer& DataPointer::operator=(DataPointer&& other) noexcept {
@@ -144,7 +188,11 @@ void DataPointer::zero() {
144188

145189
void DataPointer::reset(void* data, size_t length) {
146190
if (data_ != nullptr) {
147-
OPENSSL_clear_free(data_, len_);
191+
if (secure_) {
192+
OPENSSL_secure_clear_free(data_, len_);
193+
} else {
194+
OPENSSL_clear_free(data_, len_);
195+
}
148196
}
149197
data_ = data;
150198
len_ = length;
@@ -175,6 +223,7 @@ DataPointer DataPointer::resize(size_t len) {
175223

176224
// ============================================================================
177225
bool isFipsEnabled() {
226+
ClearErrorOnReturn clear_error_on_return;
178227
#if OPENSSL_VERSION_MAJOR >= 3
179228
return EVP_default_properties_is_fips_enabled(nullptr) == 1;
180229
#else
@@ -186,30 +235,31 @@ bool setFipsEnabled(bool enable, CryptoErrorList* errors) {
186235
if (isFipsEnabled() == enable) return true;
187236
ClearErrorOnReturn clearErrorOnReturn(errors);
188237
#if OPENSSL_VERSION_MAJOR >= 3
189-
return EVP_default_properties_enable_fips(nullptr, enable ? 1 : 0) == 1;
238+
return EVP_default_properties_enable_fips(nullptr, enable ? 1 : 0) == 1 &&
239+
EVP_default_properties_is_fips_enabled(nullptr);
190240
#else
191241
return FIPS_mode_set(enable ? 1 : 0) == 1;
192242
#endif
193243
}
194244

195245
bool testFipsEnabled() {
246+
ClearErrorOnReturn clear_error_on_return;
196247
#if OPENSSL_VERSION_MAJOR >= 3
197248
OSSL_PROVIDER* fips_provider = nullptr;
198249
if (OSSL_PROVIDER_available(nullptr, "fips")) {
199250
fips_provider = OSSL_PROVIDER_load(nullptr, "fips");
200251
}
201-
const auto enabled = fips_provider == nullptr ? 0
202-
: OSSL_PROVIDER_self_test(fips_provider) ? 1
203-
: 0;
252+
if (fips_provider == nullptr) return false;
253+
int result = OSSL_PROVIDER_self_test(fips_provider);
254+
OSSL_PROVIDER_unload(fips_provider);
255+
return result;
204256
#else
205257
#ifdef OPENSSL_FIPS
206-
const auto enabled = FIPS_selftest() ? 1 : 0;
258+
return FIPS_selftest();
207259
#else // OPENSSL_FIPS
208-
const auto enabled = 0;
260+
return false;
209261
#endif // OPENSSL_FIPS
210262
#endif
211-
212-
return enabled;
213263
}
214264

215265
// ============================================================================
@@ -2689,6 +2739,21 @@ std::optional<std::string_view> SSLPointer::getCipherVersion() const {
26892739
return SSL_CIPHER_get_version(cipher);
26902740
}
26912741

2742+
std::optional<int> SSLPointer::getSecurityLevel() {
2743+
#ifndef OPENSSL_IS_BORINGSSL
2744+
auto ctx = SSLCtxPointer::New();
2745+
if (!ctx) return std::nullopt;
2746+
2747+
auto ssl = SSLPointer::New(ctx);
2748+
if (!ssl) return std::nullopt;
2749+
2750+
return SSL_get_security_level(ssl);
2751+
#else
2752+
// for BoringSSL assume the same as the default
2753+
return OPENSSL_TLS_SECURITY_LEVEL;
2754+
#endif // OPENSSL_IS_BORINGSSL
2755+
}
2756+
26922757
SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {}
26932758

26942759
SSLCtxPointer::SSLCtxPointer(SSLCtxPointer&& other) noexcept
@@ -3290,6 +3355,10 @@ EVPKeyCtxPointer EVPKeyCtxPointer::New(const EVPKeyPointer& key) {
32903355
}
32913356

32923357
EVPKeyCtxPointer EVPKeyCtxPointer::NewFromID(int id) {
3358+
#ifdef OPENSSL_IS_BORINGSSL
3359+
// DSA keys are not supported with BoringSSL
3360+
if (id == EVP_PKEY_DSA) return {};
3361+
#endif
32933362
return EVPKeyCtxPointer(EVP_PKEY_CTX_new_id(id, nullptr));
32943363
}
32953364

@@ -3852,6 +3921,26 @@ int Ec::getCurve() const {
38523921
return EC_GROUP_get_curve_name(getGroup());
38533922
}
38543923

3924+
int Ec::GetCurveIdFromName(std::string_view name) {
3925+
int nid = EC_curve_nist2nid(name.data());
3926+
if (nid == NID_undef) {
3927+
nid = OBJ_sn2nid(name.data());
3928+
}
3929+
return nid;
3930+
}
3931+
3932+
bool Ec::GetCurves(Ec::GetCurveCallback callback) {
3933+
const size_t count = EC_get_builtin_curves(nullptr, 0);
3934+
std::vector<EC_builtin_curve> curves(count);
3935+
if (EC_get_builtin_curves(curves.data(), count) != count) {
3936+
return false;
3937+
}
3938+
for (auto curve : curves) {
3939+
if (!callback(OBJ_nid2sn(curve.nid))) return false;
3940+
}
3941+
return true;
3942+
}
3943+
38553944
// ============================================================================
38563945

38573946
EVPMDCtxPointer::EVPMDCtxPointer() : ctx_(nullptr) {}

ncrypto.h

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,11 @@ class Ec final {
469469
inline operator bool() const { return ec_ != nullptr; }
470470
inline operator OSSL3_CONST EC_KEY*() const { return ec_; }
471471

472+
static int GetCurveIdFromName(std::string_view name);
473+
474+
using GetCurveCallback = std::function<bool(std::string_view)>;
475+
static bool GetCurves(GetCurveCallback callback);
476+
472477
private:
473478
OSSL3_CONST EC_KEY* ec_ = nullptr;
474479
};
@@ -480,9 +485,31 @@ class DataPointer final {
480485
static DataPointer Alloc(size_t len);
481486
static DataPointer Copy(const Buffer<const void>& buffer);
482487

488+
// Attempts to allocate the buffer space using the secure heap, if
489+
// supported/enabled. If the secure heap is disabled, then this
490+
// ends up being equivalent to Alloc(len). Note that allocation
491+
// will fail if there is not enough free space remaining in the
492+
// secure heap space.
493+
static DataPointer SecureAlloc(size_t len);
494+
495+
// If the secure heap is enabled, returns the amount of data that
496+
// has been allocated from the heap.
497+
static size_t GetSecureHeapUsed();
498+
499+
enum class InitSecureHeapResult {
500+
FAILED,
501+
UNABLE_TO_MEMORY_MAP,
502+
OK,
503+
};
504+
505+
// Attempt to initialize the secure heap. The secure heap is not
506+
// supported on all operating systems and whenever boringssl is
507+
// used.
508+
static InitSecureHeapResult TryInitSecureHeap(size_t amount, size_t min);
509+
483510
DataPointer() = default;
484-
explicit DataPointer(void* data, size_t len);
485-
explicit DataPointer(const Buffer<void>& buffer);
511+
explicit DataPointer(void* data, size_t len, bool secure = false);
512+
explicit DataPointer(const Buffer<void>& buffer, bool secure = false);
486513
DataPointer(DataPointer&& other) noexcept;
487514
DataPointer& operator=(DataPointer&& other) noexcept;
488515
NCRYPTO_DISALLOW_COPY(DataPointer)
@@ -518,9 +545,12 @@ class DataPointer final {
518545
};
519546
}
520547

548+
bool isSecure() const { return secure_; }
549+
521550
private:
522551
void* data_ = nullptr;
523552
size_t len_ = 0;
553+
bool secure_ = false;
524554
};
525555

526556
class BIOPointer final {
@@ -1048,6 +1078,8 @@ class SSLPointer final {
10481078

10491079
std::optional<uint32_t> verifyPeerCertificate() const;
10501080

1081+
static std::optional<int> getSecurityLevel();
1082+
10511083
void getCiphers(std::function<void(const std::string_view)> cb) const;
10521084

10531085
static SSLPointer New(const SSLCtxPointer& ctx);

0 commit comments

Comments
 (0)