diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 8ef74aee2d0e..3247145e77a9 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -134,7 +134,8 @@ int EarlyClientHelloCallback(SSL* s, int* al, void* arg) { s, TLSEXT_TYPE_session_ticket, &ext, &ext_len) == 1 && ext_len > 0; - return w->OnEarlyClientHello(session_id, session_id_len, has_ticket) + return w->OnEarlyClientHello( + session_id, session_id_len, HAS_TICKET(has_ticket)) ? SSL_CLIENT_HELLO_SUCCESS : SSL_CLIENT_HELLO_RETRY; } @@ -199,8 +200,7 @@ int NewSessionCallback(SSL* s, SSL_SESSION* sess) { // On servers, we pause the handshake until callback of 'newSession', which // calls NewSessionDoneCb(). On clients, there is no callback to wait for. - if (w->is_server()) - w->set_awaiting_new_session(true); + if (w->is_server()) w->set_awaiting_new_session(ON::YES); w->MakeCallback(env->onnewsession_string(), arraysize(argv), argv); @@ -224,8 +224,9 @@ int SSLCertCallback(SSL* s, void* arg) { std::string servername; if (auto name = SSLPointer::GetServerName(s)) servername = *name; - w->ScheduleCertCb(std::move(servername), - SSL_get_tlsext_status_type(s) == TLSEXT_STATUSTYPE_ocsp); + w->ScheduleCertCb( + std::move(servername), + OCSP(SSL_get_tlsext_status_type(s) == TLSEXT_STATUSTYPE_ocsp)); // Suspend handshake with SSL_ERROR_WANT_X509_LOOKUP, and handshake will // continue after certcb is done. @@ -352,14 +353,14 @@ void ConfigureSecureContext(SecureContext* sc) { sc->ctx().setStatusCallback(TLSExtStatusCallback); } -inline bool Set( - Environment* env, - Local target, - Local name, - const char* value, - bool ignore_null = true) { - if (value == nullptr) - return ignore_null; +STRONG_BOOL(IGNORE_NULL); + +inline bool Set(Environment* env, + Local target, + Local name, + const char* value, + IGNORE_NULL ignore_null = IGNORE_NULL::YES) { + if (value == nullptr) return ignore_null.toBool(); return !target->Set( env->context(), name, @@ -371,8 +372,8 @@ inline bool Set(Environment* env, Local target, Local name, const std::string_view& value, - bool ignore_null = true) { - if (value.empty()) return ignore_null; + IGNORE_NULL ignore_null = IGNORE_NULL::YES) { + if (value.empty()) return ignore_null.toBool(); return !target ->Set(env->context(), name, @@ -464,7 +465,7 @@ void TLSWrap::NewSessionDoneCb() { // and so emit spurious 'resumeSession'/'newSession' events here. bool TLSWrap::OnEarlyClientHello(const unsigned char* session_id, size_t session_id_len, - bool has_ticket) { + HAS_TICKET has_ticket) { if (!hello_emitted_) { hello_emitted_ = true; Debug(this, "Scheduling onclienthello"); @@ -484,7 +485,7 @@ bool TLSWrap::OnEarlyClientHello(const unsigned char* session_id, } void TLSWrap::EmitClientHello(const std::vector& session_id, - bool has_ticket) { + HAS_TICKET has_ticket) { Debug(this, "Emitting onclienthello"); Environment* env = this->env(); HandleScope handle_scope(env->isolate()); @@ -503,7 +504,7 @@ void TLSWrap::EmitClientHello(const std::vector& session_id, hello_obj ->Set(env->context(), env->tls_ticket_string(), - Boolean::New(env->isolate(), has_ticket)) + has_ticket.ToJs(env->isolate())) .IsNothing()) { // An exception is pending, so don't re-enter SSL or JS to resume. return; @@ -515,7 +516,7 @@ void TLSWrap::EmitClientHello(const std::vector& session_id, // As with the ClientHello, JS must not run on the library's stack: 'oncertcb' // handlers synchronously call back into the handle to resume the handshake. -void TLSWrap::ScheduleCertCb(std::string servername, bool ocsp) { +void TLSWrap::ScheduleCertCb(std::string servername, OCSP ocsp) { Debug(this, "Scheduling oncertcb"); BaseObjectPtr strong_ref{this}; env()->SetImmediate( @@ -525,7 +526,7 @@ void TLSWrap::ScheduleCertCb(std::string servername, bool ocsp) { }); } -void TLSWrap::EmitCertCb(const std::string& servername, bool ocsp) { +void TLSWrap::EmitCertCb(const std::string& servername, OCSP ocsp) { Debug(this, "Emitting oncertcb"); Environment* env = this->env(); HandleScope handle_scope(env->isolate()); @@ -538,7 +539,7 @@ void TLSWrap::EmitCertCb(const std::string& servername, bool ocsp) { .IsNothing() || info->Set(env->context(), env->ocsp_request_string(), - Boolean::New(env->isolate(), ocsp)) + ocsp.ToJs(env->isolate())) .IsNothing()) { return; } @@ -945,7 +946,8 @@ void TLSWrap::ClearOut() { const char* rs = ERR_reason_error_string(ssl_err); if (!Set(env(), obj, env()->library_string(), ls) || !Set(env(), obj, env()->function_string(), fs) || - !Set(env(), obj, env()->reason_string(), rs, false)) return; + !Set(env(), obj, env()->reason_string(), rs, IGNORE_NULL::NO)) + return; // SSL has no API to recover the error name from the number, so we // transform reason strings like "this error" to "ERR_SSL_THIS_ERROR", // which ends up being close to the original error macro name. diff --git a/src/crypto/crypto_tls.h b/src/crypto/crypto_tls.h index a5ded3392915..dafc4a2cca43 100644 --- a/src/crypto/crypto_tls.h +++ b/src/crypto/crypto_tls.h @@ -38,6 +38,9 @@ namespace node { namespace crypto { +STRONG_BOOL(OCSP); +STRONG_BOOL(HAS_TICKET); + class TLSWrap : public AsyncWrap, public StreamBase, public StreamListener { @@ -70,9 +73,11 @@ class TLSWrap : public AsyncWrap, inline bool should_suspend_for_client_hello() const { return is_server() && session_callbacks_ && !hello_answered_; } - inline void set_cert_cb_running(bool on = true) { cert_cb_running_ = on; } - inline void set_awaiting_new_session(bool on = true) { - awaiting_new_session_ = on; + inline void set_cert_cb_running(ON on = ON::YES) { + cert_cb_running_ = on.toBool(); + } + inline void set_awaiting_new_session(ON on = ON::YES) { + awaiting_new_session_ = on.toBool(); } inline void enable_session_callbacks() { session_callbacks_ = true; } inline bool is_server() const { return kind_ == Kind::kServer; } @@ -113,10 +118,10 @@ class TLSWrap : public AsyncWrap, // clientHelloDone(). The emit itself must not run on the library's stack. bool OnEarlyClientHello(const unsigned char* session_id, size_t session_id_len, - bool has_ticket); + HAS_TICKET has_ticket); // Schedules 'oncertcb'. The handshake stays suspended until certCbDone(). - void ScheduleCertCb(std::string servername, bool ocsp); + void ScheduleCertCb(std::string servername, OCSP ocsp); // Implement MemoryRetainer: void MemoryInfo(MemoryTracker* tracker) const override; @@ -151,8 +156,8 @@ class TLSWrap : public AsyncWrap, void WaitForCertCb(CertCb cb, void* arg); void EmitClientHello(const std::vector& session_id, - bool has_ticket); - void EmitCertCb(const std::string& servername, bool ocsp); + HAS_TICKET has_ticket); + void EmitCertCb(const std::string& servername, OCSP ocsp); TLSWrap(Environment* env, v8::Local obj, diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 166fea15da59..df443d0d27ac 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -515,8 +515,9 @@ ByteSource ByteSource::FromStringOrBuffer(Environment* env, : FromString(env, value.As()); } -ByteSource ByteSource::FromString(Environment* env, Local str, - bool ntc) { +ByteSource ByteSource::FromString(Environment* env, + Local str, + NULL_TERMINATE ntc) { CHECK(str->IsString()); size_t size = str->Utf8LengthV2(env->isolate()); size_t alloc_size = ntc ? size + 1 : size; @@ -528,7 +529,7 @@ ByteSource ByteSource::FromString(Environment* env, Local str, return ByteSource::Allocated(out.release()); } -ByteSource ByteSource::FromBuffer(Local buffer, bool ntc) { +ByteSource ByteSource::FromBuffer(Local buffer, NULL_TERMINATE ntc) { ArrayBufferOrViewContents buf(buffer); return ntc ? buf.ToNullTerminatedCopy() : buf.ToByteSource(); } @@ -546,8 +547,9 @@ ByteSource ByteSource::FromSecretKeyBytes( ByteSource ByteSource::NullTerminatedCopy(Environment* env, Local value) { - return Buffer::HasInstance(value) ? FromBuffer(value, true) - : FromString(env, value.As(), true); + return Buffer::HasInstance(value) + ? FromBuffer(value, NULL_TERMINATE::YES) + : FromString(env, value.As(), NULL_TERMINATE::YES); } ByteSource ByteSource::FromSymmetricKeyObjectHandle(Local handle) { diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 5c9dad13196b..30f25e9a23b1 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -188,6 +188,8 @@ T* MallocOpenSSL(size_t count) { return static_cast(mem); } +STRONG_BOOL(NULL_TERMINATE); + // A helper class representing a read-only byte array. When deallocated, its // contents are zeroed. class ByteSource final { @@ -251,10 +253,10 @@ class ByteSource final { static ByteSource FromString(Environment* env, v8::Local str, - bool ntc = false); + NULL_TERMINATE ntc = NULL_TERMINATE::NO); static ByteSource FromBuffer(v8::Local buffer, - bool ntc = false); + NULL_TERMINATE ntc = NULL_TERMINATE::NO); static ByteSource FromBIO(const ncrypto::BIOPointer& bio); diff --git a/src/util.h b/src/util.h index 48305bfdc131..16302da5fa32 100644 --- a/src/util.h +++ b/src/util.h @@ -1078,6 +1078,45 @@ inline v8::Local Uint32ToString(v8::Local context, ->ToString(context) .ToLocalChecked(); } + +// A type that can be used to represent a boolean value in a way that is +// distinct from the built-in bool type... Specifically, it is used to +// avoid bool arguments where the callsite is not clear about what the +// boolean value represents. +// Adopted from cloudflare/workerd +// https://github.com/cloudflare/workerd/blob/main/src/workerd/util/strong-bool.h +// where it is used for exactly the same purpose. +#define STRONG_BOOL(Type) \ + class Type final { \ + public: \ + static const Type NO; \ + static const Type YES; \ + constexpr explicit Type(bool booleanValue) \ + : value(booleanValue ? Value::YES : Value::NO) {} \ + constexpr explicit operator bool() const { return toBool(); } \ + constexpr bool toBool() const { return value == YES; } \ + v8::Local ToJs(v8::Isolate* isolate) const { \ + return v8::Boolean::New(isolate, toBool()); \ + } \ + constexpr auto operator<=>(const Type&) const = default; \ + constexpr Type operator&&(const Type& other) const { \ + return Type(value == YES && other.value == YES); \ + } \ + constexpr Type operator||(const Type& other) const { \ + return Type(value == YES || other.value == YES); \ + } \ + \ + private: \ + enum class Value : std::uint8_t { NO, YES }; \ + constexpr Type(Value value) : value(value) {} \ + Value value; \ + }; \ + inline constexpr Type Type::NO{Type::Value::NO}; \ + inline constexpr Type Type::YES { Type::Value::YES } + +// A generic strong bool type that can be used for ON/OFF flags +STRONG_BOOL(ON); + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS