Skip to content

Commit

Permalink
src: make SecureContext fields private
Browse files Browse the repository at this point in the history
These fields should not be public. Only ctx_ is used outside of the
class itself, and should be accessed through the ctx() function
instead.

PR-URL: #43173
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
  • Loading branch information
tniessen authored and targos committed Jul 12, 2022
1 parent 78d4a11 commit 6c4ce46
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ long VerifyPeerCertificate( // NOLINT(runtime/int)

bool UseSNIContext(
const SSLPointer& ssl, BaseObjectPtr<SecureContext> 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;
Expand Down Expand Up @@ -209,7 +209,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)
Expand Down
27 changes: 14 additions & 13 deletions src/crypto/crypto_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -137,6 +125,19 @@ class SecureContext final : public BaseObject {

SecureContext(Environment* env, v8::Local<v8::Object> 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
Expand Down
12 changes: 6 additions & 6 deletions src/crypto/crypto_tls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1303,20 +1303,20 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
p->sni_context_ = BaseObjectPtr<SecureContext>(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);
Expand Down

0 comments on commit 6c4ce46

Please sign in to comment.