From cc0be38ecff08c170057b59f90185f50a8f371d7 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Thu, 6 Sep 2018 10:36:40 +0000 Subject: OpenSSL version compatibility fixes and better exception safety - On 1.1.0, make TLS version dynamic (but at least v1.2) - On 1.0.1, manually set a P-256 curve for ECDH - Ensure that exception during TLS context construction does not leak SSL_CTX --- .../net/tls/impl/openssl_tls_context_impl.cpp | 52 +++++++++++++--------- .../net/tls/impl/openssl_tls_context_impl.h | 4 +- .../vespa/vespalib/net/tls/impl/openssl_typedefs.h | 14 ++++++ 3 files changed, 48 insertions(+), 22 deletions(-) (limited to 'vespalib/src') diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.cpp b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.cpp index 561c3af0e98..384561d28b9 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.cpp @@ -135,12 +135,17 @@ X509Ptr read_untrusted_x509_from_bio(::BIO& bio) { return X509Ptr(::PEM_read_bio_X509(&bio, nullptr, nullptr, empty_passphrase())); } -::SSL_CTX* new_tls1_2_ctx_with_auto_init() { +SslCtxPtr new_tls1_2_ctx_with_auto_init() { ensure_openssl_initialized_once(); -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - return ::SSL_CTX_new(::TLSv1_2_method()); -#pragma GCC diagnostic pop +#if (OPENSSL_VERSION_NUMBER < 0x10100000L) + return SslCtxPtr(::SSL_CTX_new(::TLSv1_2_method())); +#else + SslCtxPtr ctx(::SSL_CTX_new(::TLS_method())); + if (!::SSL_CTX_set_min_proto_version(ctx.get(), TLS1_2_VERSION)) { + throw CryptoException("SSL_CTX_set_min_proto_version"); + } + return ctx; +#endif } } // anon ns @@ -149,7 +154,7 @@ OpenSslTlsContextImpl::OpenSslTlsContextImpl(const TransportSecurityOptions& ts_ : _ctx(new_tls1_2_ctx_with_auto_init()) { if (!_ctx) { - throw CryptoException("Failed to create new TLSv1.2 context"); + throw CryptoException("Failed to create new TLS context"); } add_certificate_authorities(ts_opts.ca_certs_pem()); add_certificate_chain(ts_opts.cert_chain_pem()); @@ -162,14 +167,12 @@ OpenSslTlsContextImpl::OpenSslTlsContextImpl(const TransportSecurityOptions& ts_ // TODO set peer verification flags! } -OpenSslTlsContextImpl::~OpenSslTlsContextImpl() { - ::SSL_CTX_free(_ctx); -} +OpenSslTlsContextImpl::~OpenSslTlsContextImpl() = default; void OpenSslTlsContextImpl::add_certificate_authorities(vespalib::stringref ca_pem) { // TODO support empty CA set...? Ever useful? auto bio = bio_from_string(ca_pem); - ::X509_STORE* cert_store = ::SSL_CTX_get_cert_store(_ctx); // Internal pointer, not owned by us. + ::X509_STORE* cert_store = ::SSL_CTX_get_cert_store(_ctx.get()); // Internal pointer, not owned by us. while (true) { auto ca_cert = read_untrusted_x509_from_bio(*bio); if (!ca_cert) { @@ -191,7 +194,7 @@ void OpenSslTlsContextImpl::add_certificate_chain(vespalib::stringref chain_pem) } // Ownership of certificate is _not_ transferred, OpenSSL makes internal copy. // This is not well documented, but is mentioned by other impls. - if (::SSL_CTX_use_certificate(_ctx, own_cert.get()) != 1) { + if (::SSL_CTX_use_certificate(_ctx.get(), own_cert.get()) != 1) { throw CryptoException("SSL_CTX_use_certificate"); } // After the node's own certificate comes any intermediate CA-provided certificates. @@ -203,7 +206,7 @@ void OpenSslTlsContextImpl::add_certificate_chain(vespalib::stringref chain_pem) break; } // Ownership of certificate _is_ transferred here! - if (!::SSL_CTX_add_extra_chain_cert(_ctx, ca_cert.release())) { + if (!::SSL_CTX_add_extra_chain_cert(_ctx.get(), ca_cert.release())) { throw CryptoException("SSL_CTX_add_extra_chain_cert"); } } @@ -216,35 +219,44 @@ void OpenSslTlsContextImpl::use_private_key(vespalib::stringref key_pem) { throw CryptoException("Failed to read PEM private key data"); } // Ownership _not_ taken. - if (::SSL_CTX_use_PrivateKey(_ctx, key.get()) != 1) { + if (::SSL_CTX_use_PrivateKey(_ctx.get(), key.get()) != 1) { throw CryptoException("SSL_CTX_use_PrivateKey"); } } void OpenSslTlsContextImpl::verify_private_key() { - if (::SSL_CTX_check_private_key(_ctx) != 1) { + if (::SSL_CTX_check_private_key(_ctx.get()) != 1) { throw CryptoException("SSL_CTX_check_private_key failed; mismatch between public and private key?"); } } void OpenSslTlsContextImpl::enable_ephemeral_key_exchange() { +#if (OPENSSL_VERSION_NUMBER < 0x10100000L) +# if (OPENSSL_VERSION_NUMBER >= 0x10002000L) // Always enabled by default on higher versions. -#if (OPENSSL_VERSION_NUMBER >= 0x10002000L) && (OPENSSL_VERSION_NUMBER < 0x10100000L) // Auto curve selection is preferred over using SSL_CTX_set_ecdh_tmp - if (!::SSL_CTX_set_ecdh_auto(_ctx, 1)) { + if (!::SSL_CTX_set_ecdh_auto(_ctx.get(), 1)) { throw CryptoException("SSL_CTX_set_ecdh_auto"); } // New ECDH key per connection. - ::SSL_CTX_set_options(_ctx, SSL_OP_SINGLE_ECDH_USE); -#else - // TODO make this work on OpenSSL 1.0.1 as well + ::SSL_CTX_set_options(_ctx.get(), SSL_OP_SINGLE_ECDH_USE); +# else + // Set explicit P-256 curve used for ECDH purposes. + EcKeyPtr ec_curve(::EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)); + if (!ec_curve) { + throw CryptoException("EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)"); + } + if (!::SSL_CTX_set_tmp_ecdh(_ctx.get(), ec_curve.get())) { + throw CryptoException("SSL_CTX_set_tmp_ecdh"); + } +# endif #endif } void OpenSslTlsContextImpl::disable_compression() { // TLS stream compression is vulnerable to a host of chosen plaintext // attacks (CRIME, BREACH etc), so disable it. - ::SSL_CTX_set_options(_ctx, SSL_OP_NO_COMPRESSION); + ::SSL_CTX_set_options(_ctx.get(), SSL_OP_NO_COMPRESSION); } } diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.h b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.h index 5fa982ee7ad..208629d913a 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.h +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_tls_context_impl.h @@ -8,12 +8,12 @@ namespace vespalib::net::tls::impl { class OpenSslTlsContextImpl : public TlsContext { - ::SSL_CTX* _ctx; + SslCtxPtr _ctx; public: explicit OpenSslTlsContextImpl(const TransportSecurityOptions&); ~OpenSslTlsContextImpl() override; - ::SSL_CTX* native_context() const noexcept { return _ctx; } + ::SSL_CTX* native_context() const noexcept { return _ctx.get(); } private: // Note: single use per instance; does _not_ clear existing chain! void add_certificate_authorities(stringref ca_pem); diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_typedefs.h b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_typedefs.h index 882ffde5897..afafe556338 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_typedefs.h +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_typedefs.h @@ -22,6 +22,13 @@ struct SslDeleter { }; using SslPtr = std::unique_ptr<::SSL, SslDeleter>; +struct SslCtxDeleter { + void operator()(::SSL_CTX* ssl) const noexcept { + ::SSL_CTX_free(ssl); + } +}; +using SslCtxPtr = std::unique_ptr<::SSL_CTX, SslCtxDeleter>; + struct X509Deleter { void operator()(::X509* cert) const noexcept { ::X509_free(cert); @@ -36,4 +43,11 @@ struct EvpPkeyDeleter { }; using EvpPkeyPtr = std::unique_ptr<::EVP_PKEY, EvpPkeyDeleter>; +struct EcKeyDeleter { + void operator()(::EC_KEY* ec_key) const noexcept { + ::EC_KEY_free(ec_key); + } +}; +using EcKeyPtr = std::unique_ptr<::EC_KEY, EcKeyDeleter>; + } -- cgit v1.2.3