From 592a36073991e20cc207c0e69011671d5a262a50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 22 May 2022 00:22:43 +0000 Subject: [PATCH] src: make SecureContext fields private These fields should not be public. Only ctx_ is used outside of the class itself, and should be accessed through the ctx() function instead. --- src/crypto/crypto_common.cc | 4 ++-- src/crypto/crypto_context.h | 27 ++++++++++++++------------- src/crypto/crypto_tls.cc | 12 ++++++------ 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 09b93d40c277ed..0aaf00436666d1 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -154,7 +154,7 @@ long VerifyPeerCertificate( // NOLINT(runtime/int) bool UseSNIContext( const SSLPointer& ssl, BaseObjectPtr context) { - SSL_CTX* ctx = context->ctx_.get(); + SSL_CTX* ctx = context->ctx().get(); X509* x509 = SSL_CTX_get0_certificate(ctx); EVP_PKEY* pkey = SSL_CTX_get0_privatekey(ctx); STACK_OF(X509)* chain; @@ -218,7 +218,7 @@ const char* GetServerName(SSL* ssl) { } bool SetGroups(SecureContext* sc, const char* groups) { - return SSL_CTX_set1_groups_list(sc->ssl_ctx(), groups) == 1; + return SSL_CTX_set1_groups_list(sc->ctx().get(), groups) == 1; } const char* X509ErrorCode(long err) { // NOLINT(runtime/int) diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h index 0d290eb8368f35..ee2df97ac21411 100644 --- a/src/crypto/crypto_context.h +++ b/src/crypto/crypto_context.h @@ -41,7 +41,7 @@ class SecureContext final : public BaseObject { static void RegisterExternalReferences(ExternalReferenceRegistry* registry); static SecureContext* Create(Environment* env); - SSL_CTX* ssl_ctx() const { return ctx_.get(); } + const SSLCtxPointer& ctx() const { return ctx_; } SSLPointer CreateSSL(); @@ -55,14 +55,6 @@ class SecureContext final : public BaseObject { SET_MEMORY_INFO_NAME(SecureContext) SET_SELF_SIZE(SecureContext) - SSLCtxPointer ctx_; - X509Pointer cert_; - X509Pointer issuer_; -#ifndef OPENSSL_NO_ENGINE - bool client_cert_engine_provided_ = false; - EnginePointer private_key_engine_; -#endif // !OPENSSL_NO_ENGINE - static const int kMaxSessionSize = 10 * 1024; // See TicketKeyCallback @@ -72,10 +64,6 @@ class SecureContext final : public BaseObject { static const int kTicketKeyNameIndex = 3; static const int kTicketKeyIVIndex = 4; - unsigned char ticket_key_name_[16]; - unsigned char ticket_key_aes_[16]; - unsigned char ticket_key_hmac_[16]; - protected: // OpenSSL structures are opaque. This is sizeof(SSL_CTX) for OpenSSL 1.1.1b: static const int64_t kExternalSize = 1024; @@ -137,6 +125,19 @@ class SecureContext final : public BaseObject { SecureContext(Environment* env, v8::Local wrap); void Reset(); + + private: + SSLCtxPointer ctx_; + X509Pointer cert_; + X509Pointer issuer_; +#ifndef OPENSSL_NO_ENGINE + bool client_cert_engine_provided_ = false; + EnginePointer private_key_engine_; +#endif // !OPENSSL_NO_ENGINE + + unsigned char ticket_key_name_[16]; + unsigned char ticket_key_aes_[16]; + unsigned char ticket_key_hmac_[16]; }; } // namespace crypto diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 83dee0be121d40..a192956f0f7cfe 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -295,8 +295,8 @@ int TLSExtStatusCallback(SSL* s, void* arg) { void ConfigureSecureContext(SecureContext* sc) { // OCSP stapling - SSL_CTX_set_tlsext_status_cb(sc->ctx_.get(), TLSExtStatusCallback); - SSL_CTX_set_tlsext_status_arg(sc->ctx_.get(), nullptr); + SSL_CTX_set_tlsext_status_cb(sc->ctx().get(), TLSExtStatusCallback); + SSL_CTX_set_tlsext_status_arg(sc->ctx().get(), nullptr); } inline bool Set( @@ -1303,20 +1303,20 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) { p->sni_context_ = BaseObjectPtr(sc); ConfigureSecureContext(sc); - CHECK_EQ(SSL_set_SSL_CTX(p->ssl_.get(), sc->ctx_.get()), sc->ctx_.get()); + CHECK_EQ(SSL_set_SSL_CTX(p->ssl_.get(), sc->ctx().get()), sc->ctx().get()); p->SetCACerts(sc); return SSL_TLSEXT_ERR_OK; } int TLSWrap::SetCACerts(SecureContext* sc) { - int err = SSL_set1_verify_cert_store( - ssl_.get(), SSL_CTX_get_cert_store(sc->ctx_.get())); + int err = SSL_set1_verify_cert_store(ssl_.get(), + SSL_CTX_get_cert_store(sc->ctx().get())); if (err != 1) return err; STACK_OF(X509_NAME)* list = - SSL_dup_CA_list(SSL_CTX_get_client_CA_list(sc->ctx_.get())); + SSL_dup_CA_list(SSL_CTX_get_client_CA_list(sc->ctx().get())); // NOTE: `SSL_set_client_CA_list` takes the ownership of `list` SSL_set_client_CA_list(ssl_.get(), list);