diff options
16 files changed, 360 insertions, 91 deletions
diff --git a/fbench/src/fbench/fbench.cpp b/fbench/src/fbench/fbench.cpp index 91475ce2125..593ae30a0e5 100644 --- a/fbench/src/fbench/fbench.cpp +++ b/fbench/src/fbench/fbench.cpp @@ -86,10 +86,13 @@ FBench::init_crypto_engine(const std::string &ca_certs_file_name, return false; } bool load_failed = false; - vespalib::net::tls::TransportSecurityOptions - tls_opts(maybe_load(ca_certs_file_name, load_failed), - maybe_load(cert_chain_file_name, load_failed), - maybe_load(private_key_file_name, load_failed)); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(maybe_load(ca_certs_file_name, load_failed)). + cert_chain_pem(maybe_load(cert_chain_file_name, load_failed)). + private_key_pem(maybe_load(private_key_file_name, load_failed)). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // TODO configurable or default false! + vespalib::net::tls::TransportSecurityOptions tls_opts(std::move(ts_builder)); if (load_failed) { fprintf(stderr, "failed to load transport security options\n"); return false; diff --git a/vbench/src/vbench/vbench/vbench.cpp b/vbench/src/vbench/vbench/vbench.cpp index 4f6efadfbdd..58854af705e 100644 --- a/vbench/src/vbench/vbench/vbench.cpp +++ b/vbench/src/vbench/vbench/vbench.cpp @@ -29,11 +29,13 @@ CryptoEngine::SP setup_crypto(const vespalib::slime::Inspector &tls) { if (!tls.valid()) { return std::make_shared<vespalib::NullCryptoEngine>(); } - vespalib::net::tls::TransportSecurityOptions - tls_opts(maybe_load(tls["ca-certificates"]), - maybe_load(tls["certificates"]), - maybe_load(tls["private-key"])); - return std::make_shared<vespalib::TlsCryptoEngine>(tls_opts); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(maybe_load(tls["ca-certificates"])). + cert_chain_pem(maybe_load(tls["certificates"])). + private_key_pem(maybe_load(tls["private-key"])). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // TODO configurable or default false! + return std::make_shared<vespalib::TlsCryptoEngine>(vespalib::net::tls::TransportSecurityOptions(std::move(ts_builder))); } } // namespace vbench::<unnamed> diff --git a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp index 78838ce2cd2..54c8c19fc64 100644 --- a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp +++ b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp @@ -57,11 +57,23 @@ void print_decode_result(const char* mode, const DecodeResult& res) { decode_state_to_str(res.state)); } +TransportSecurityOptions ts_from_pems(vespalib::stringref ca_certs_pem, + vespalib::stringref cert_chain_pem, + vespalib::stringref private_key_pem) +{ + auto ts_builder = TransportSecurityOptions::Params(). + ca_certs_pem(ca_certs_pem). + cert_chain_pem(cert_chain_pem). + private_key_pem(private_key_pem). + authorized_peers(AuthorizedPeers::allow_all_authenticated()); + return TransportSecurityOptions(std::move(ts_builder)); +} + struct Fixture { TransportSecurityOptions tls_opts; std::shared_ptr<TlsContext> tls_ctx; - std::unique_ptr<CryptoCodec> client; - std::unique_ptr<CryptoCodec> server; + std::unique_ptr<OpenSslCryptoCodecImpl> client; + std::unique_ptr<OpenSslCryptoCodecImpl> server; SmartBuffer client_to_server; SmartBuffer server_to_client; @@ -77,16 +89,21 @@ struct Fixture { static TransportSecurityOptions create_options_without_own_peer_cert() { auto source_opts = vespalib::test::make_tls_options_for_testing(); - return TransportSecurityOptions(source_opts.ca_certs_pem(), "", ""); + return ts_from_pems(source_opts.ca_certs_pem(), "", ""); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( - const TransportSecurityOptions& opts, CryptoCodec::Mode mode) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const TransportSecurityOptions& opts, CryptoCodec::Mode mode, const SocketSpec& peer_spec) { auto ctx = TlsContext::create_default_context(opts, AuthorizationMode::Enforce); - return create_openssl_codec(ctx, mode); + return create_openssl_codec(ctx, mode, peer_spec); + } + + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const TransportSecurityOptions& opts, CryptoCodec::Mode mode) { + return create_openssl_codec(opts, mode, SocketSpec::invalid); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( const TransportSecurityOptions& opts, std::shared_ptr<CertificateVerificationCallback> cert_verify_callback, CryptoCodec::Mode mode) { @@ -94,21 +111,30 @@ struct Fixture { return create_openssl_codec(ctx, mode); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( - const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode, const SocketSpec& peer_spec) { auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); - return std::make_unique<impl::OpenSslCryptoCodecImpl>(std::move(ctx_impl), SocketAddress(), mode); + if (mode == CryptoCodec::Mode::Client) { + return OpenSslCryptoCodecImpl::make_client_codec(std::move(ctx_impl), peer_spec, SocketAddress()); + } else { + return OpenSslCryptoCodecImpl::make_server_codec(std::move(ctx_impl), SocketAddress()); + } } - EncodeResult do_encode(CryptoCodec& codec, Output& buffer, vespalib::stringref plaintext) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode) { + return create_openssl_codec(ctx, mode, SocketSpec::invalid); + } + + static EncodeResult do_encode(CryptoCodec& codec, Output& buffer, vespalib::stringref plaintext) { auto out = buffer.reserve(codec.min_encode_buffer_size()); auto enc_res = codec.encode(plaintext.data(), plaintext.size(), out.data, out.size); buffer.commit(enc_res.bytes_produced); return enc_res; } - DecodeResult do_decode(CryptoCodec& codec, Input& buffer, vespalib::string& out, - size_t max_bytes_produced, size_t max_bytes_consumed) { + static DecodeResult do_decode(CryptoCodec& codec, Input& buffer, vespalib::string& out, + size_t max_bytes_produced, size_t max_bytes_consumed) { auto in = buffer.obtain(); out.resize(max_bytes_produced); auto to_consume = std::min(in.size, max_bytes_consumed); @@ -382,13 +408,13 @@ l9pLv1vrujrPEC78cyIQe2x55wf3pRoaDg== -----END EC PRIVATE KEY-----)"; TEST_F("client with certificate signed by untrusted CA is rejected by server", Fixture) { - TransportSecurityOptions client_opts(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto client_opts = ts_from_pems(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); f.client = f.create_openssl_codec(client_opts, CryptoCodec::Mode::Client); EXPECT_FALSE(f.handshake()); } TEST_F("server with certificate signed by untrusted CA is rejected by client", Fixture) { - TransportSecurityOptions server_opts(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto server_opts = ts_from_pems(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); f.server = f.create_openssl_codec(server_opts, CryptoCodec::Mode::Server); EXPECT_FALSE(f.handshake()); } @@ -396,8 +422,8 @@ TEST_F("server with certificate signed by untrusted CA is rejected by client", F TEST_F("Can specify multiple trusted CA certs in transport options", Fixture) { auto& base_opts = f.tls_opts; auto multi_ca_pem = base_opts.ca_certs_pem() + "\n" + unknown_ca_pem; - TransportSecurityOptions multi_ca_using_ca_1(multi_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); - TransportSecurityOptions multi_ca_using_ca_2(multi_ca_pem, base_opts.cert_chain_pem(), base_opts.private_key_pem()); + auto multi_ca_using_ca_1 = ts_from_pems(multi_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto multi_ca_using_ca_2 = ts_from_pems(multi_ca_pem, base_opts.cert_chain_pem(), base_opts.private_key_pem()); // Let client be signed by CA 1, server by CA 2. Both have the two CAs in their trust store // so this should allow for a successful handshake. f.client = f.create_openssl_codec(multi_ca_using_ca_1, CryptoCodec::Mode::Client); @@ -446,7 +472,7 @@ struct CertFixture : Fixture { return {std::move(cert), std::move(key)}; } - static std::unique_ptr<CryptoCodec> create_openssl_codec_with_authz_mode( + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec_with_authz_mode( const TransportSecurityOptions& opts, std::shared_ptr<CertificateVerificationCallback> cert_verify_callback, CryptoCodec::Mode codec_mode, @@ -455,33 +481,52 @@ struct CertFixture : Fixture { return create_openssl_codec(ctx, codec_mode); } + TransportSecurityOptions::Params ts_builder_from(const CertKeyWrapper& ck) const { + return TransportSecurityOptions::Params(). + ca_certs_pem(root_ca.cert->to_pem()). + cert_chain_pem(ck.cert->to_pem()). + private_key_pem(ck.key->private_to_pem()); + } + void reset_client_with_cert_opts(const CertKeyWrapper& ck, AuthorizedPeers authorized) { - TransportSecurityOptions client_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), - ck.key->private_to_pem(), std::move(authorized)); - client = create_openssl_codec(client_opts, CryptoCodec::Mode::Client); + auto ts_params = ts_builder_from(ck).authorized_peers(std::move(authorized)); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), CryptoCodec::Mode::Client); } void reset_client_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb) { - TransportSecurityOptions client_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - client = create_openssl_codec(client_opts, std::move(cert_cb), CryptoCodec::Mode::Client); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Client); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, AuthorizedPeers authorized) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), - ck.key->private_to_pem(), std::move(authorized)); - server = create_openssl_codec(server_opts, CryptoCodec::Mode::Server); + auto ts_params = ts_builder_from(ck).authorized_peers(std::move(authorized)); + server = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), CryptoCodec::Mode::Server); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - server = create_openssl_codec(server_opts, std::move(cert_cb), CryptoCodec::Mode::Server); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + server = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Server); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb, AuthorizationMode authz_mode) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - server = create_openssl_codec_with_authz_mode(server_opts, std::move(cert_cb), CryptoCodec::Mode::Server, authz_mode); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + server = create_openssl_codec_with_authz_mode(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Server, authz_mode); + } + + void reset_client_with_peer_spec(const CertKeyWrapper& ck, + const SocketSpec& peer_spec, + bool disable_hostname_validation = false) + { + auto ts_params = ts_builder_from(ck). + authorized_peers(AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(disable_hostname_validation); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + CryptoCodec::Mode::Client, peer_spec); } }; @@ -537,7 +582,7 @@ TEST_F("Exception during verification callback processing breaks handshake", Cer EXPECT_FALSE(f.handshake()); } -TEST_F("certificate verification callback observes CN and DNS SANs", CertFixture) { +TEST_F("Certificate verification callback observes CN and DNS SANs", CertFixture) { auto ck = f.create_ca_issued_peer_cert( {{"rockets.wile.example.com"}}, {{"DNS:crash.wile.example.com"}, {"DNS:burn.wile.example.com"}}); @@ -556,7 +601,7 @@ TEST_F("certificate verification callback observes CN and DNS SANs", CertFixture EXPECT_EQUAL("burn.wile.example.com", creds.dns_sans[1]); } -TEST_F("last occurring CN is given to verification callback if multiple CNs are present", CertFixture) { +TEST_F("Last occurring CN is given to verification callback if multiple CNs are present", CertFixture) { auto ck = f.create_ca_issued_peer_cert( {{"foo.wile.example.com"}, {"bar.wile.example.com"}, {"baz.wile.example.com"}}, {}); @@ -646,6 +691,51 @@ TEST_F("Disabled insecure authorization mode ignores verification result", CertF EXPECT_TRUE(f.handshake()); } +void reset_peers_with_client_peer_spec(CertFixture& f, + const SocketSpec& peer_spec, + bool disable_hostname_validation = false) +{ + auto client_ck = f.create_ca_issued_peer_cert({"hello.world.example.com"}, {}); + f.reset_client_with_peer_spec(client_ck, peer_spec, disable_hostname_validation); + // Since hostname validation is enabled by default, providing a peer spec also + // means that we must have a valid server name to present back (or the handshake fails). + auto server_ck = f.create_ca_issued_peer_cert({}, {{"DNS:*.example.com"}}); + f.reset_server_with_cert_opts(server_ck, AuthorizedPeers::allow_all_authenticated()); +} + +TEST_F("Client does not send SNI extension if hostname not provided in spec", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::invalid); + + ASSERT_TRUE(f.handshake()); + auto maybe_sni = f.server->client_provided_sni_extension(); + EXPECT_FALSE(maybe_sni.has_value()); +} + +TEST_F("Client sends SNI extension with hostname provided in spec", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("sni-test.example.com", 12345)); + + ASSERT_TRUE(f.handshake()); + auto maybe_sni = f.server->client_provided_sni_extension(); + ASSERT_TRUE(maybe_sni.has_value()); + EXPECT_EQUAL("sni-test.example.com", *maybe_sni); +} + +TEST_F("Client hostname validation passes handshake if server hostname matches certificate", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("server-must-be-under.example.com", 12345), false); + EXPECT_TRUE(f.handshake()); +} + +TEST_F("Client hostname validation fails handshake if server hostname mismatches certificate", CertFixture) { + // Wildcards only apply to a single level, so this should fail as the server only has a cert for *.example.com + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("nested.name.example.com", 12345), false); + EXPECT_FALSE(f.handshake()); +} + +TEST_F("Mismatching server cert vs hostname does not fail if hostname validation is disabled", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("a.very.nested.name.example.com", 12345), true); + EXPECT_TRUE(f.handshake()); +} + TEST_F("Failure statistics are incremented on authorization failures", CertFixture) { reset_peers_with_server_authz_mode(f, AuthorizationMode::Enforce); auto server_before = ConnectionStatistics::get(true).snapshot(); diff --git a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp index a54e2f29aa1..00459a4e69c 100644 --- a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp +++ b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp @@ -155,6 +155,47 @@ TEST("accepted cipher list is populated if specified") { EXPECT_EQUAL("bar", ciphers[1]); } +// FIXME this is temporary until we know enabling it by default won't break the world! +TEST("hostname validation is DISABLED by default when creating options from config file") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}})"; + EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("TransportSecurityOptions builder does not disable hostname validation by default") { + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem("foo"). + cert_chain_pem("bar"). + private_key_pem("fantonald"); + TransportSecurityOptions ts_opts(std::move(ts_builder)); + EXPECT_FALSE(ts_opts.disable_hostname_validation()); +} + +TEST("hostname validation can be explicitly disabled") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "disable-hostname-validation": true})"; + EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("hostname validation can be explicitly enabled") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "disable-hostname-validation": false})"; + EXPECT_FALSE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("unknown fields are ignored at parse-time") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "flipper-the-dolphin": "*weird dolphin noises*"})"; + EXPECT_TRUE(read_options_from_json_string(json).get() != nullptr); // And no exception thrown. +} + // TODO test parsing of multiple policies TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/net/socket_spec.cpp b/vespalib/src/vespa/vespalib/net/socket_spec.cpp index 06682086670..d1dd81a454a 100644 --- a/vespalib/src/vespa/vespalib/net/socket_spec.cpp +++ b/vespalib/src/vespa/vespalib/net/socket_spec.cpp @@ -41,7 +41,7 @@ SocketSpec::address(bool server) const return SocketAddress(); } -SocketSpec SocketSpec::invalid; +const SocketSpec SocketSpec::invalid; SocketSpec::SocketSpec(const vespalib::string &spec) : SocketSpec() diff --git a/vespalib/src/vespa/vespalib/net/socket_spec.h b/vespalib/src/vespa/vespalib/net/socket_spec.h index 01af382d638..4e3dddf6814 100644 --- a/vespalib/src/vespa/vespalib/net/socket_spec.h +++ b/vespalib/src/vespa/vespalib/net/socket_spec.h @@ -24,7 +24,7 @@ private: : _type(type), _node(node), _port(port) {} SocketAddress address(bool server) const; public: - static SocketSpec invalid; + static const SocketSpec invalid; explicit SocketSpec(const vespalib::string &spec); vespalib::string spec() const; SocketSpec replace_host(const vespalib::string &new_host) const; diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp index c54990b3782..d3ac975d90a 100644 --- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp @@ -6,12 +6,23 @@ namespace vespalib::net::tls { -std::unique_ptr<CryptoCodec> CryptoCodec::create_default_codec( - std::shared_ptr<TlsContext> ctx, const SocketAddress& peer_address, Mode mode) +std::unique_ptr<CryptoCodec> +CryptoCodec::create_default_client_codec(std::shared_ptr<TlsContext> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address) { auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); // only takes by const ref assert(ctx_impl); - return std::make_unique<impl::OpenSslCryptoCodecImpl>(std::move(ctx_impl), peer_address, mode); + return impl::OpenSslCryptoCodecImpl::make_client_codec(std::move(ctx_impl), peer_spec, peer_address); +} + +std::unique_ptr<CryptoCodec> +CryptoCodec::create_default_server_codec(std::shared_ptr<TlsContext> ctx, + const SocketAddress& peer_address) +{ + auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); // only takes by const ref + assert(ctx_impl); + return impl::OpenSslCryptoCodecImpl::make_server_codec(std::move(ctx_impl), peer_address); } } diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h index 5d9684461d7..787485b47be 100644 --- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h +++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h @@ -4,6 +4,8 @@ #include <vespa/vespalib/net/socket_address.h> #include <memory> +namespace vespalib { class SocketSpec; } + namespace vespalib::net::tls { struct HandshakeResult { @@ -179,9 +181,13 @@ public: * * Throws CryptoException if resources cannot be allocated for the codec. */ - static std::unique_ptr<CryptoCodec> create_default_codec(std::shared_ptr<TlsContext> ctx, - const SocketAddress& peer_address, - Mode mode); + static std::unique_ptr<CryptoCodec> + create_default_client_codec(std::shared_ptr<TlsContext> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address); + static std::unique_ptr<CryptoCodec> + create_default_server_codec(std::shared_ptr<TlsContext> ctx, + const SocketAddress& peer_address); }; } diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp index 5315754d53a..6a79caa8264 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp @@ -172,9 +172,11 @@ void log_ssl_error(const char* source, const SocketAddress& peer_address, int ss } // anon ns OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, const SocketAddress& peer_address, Mode mode) : _ctx(std::move(ctx)), + _peer_spec(peer_spec), _peer_address(peer_address), _ssl(::SSL_new(_ctx->native_context())), _mode(mode), @@ -219,6 +221,8 @@ OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContext _output_bio = tmp_output_bio.release(); if (_mode == Mode::Client) { ::SSL_set_connect_state(_ssl.get()); + enable_hostname_validation_if_requested(); + set_server_name_indication_extension(); } else { ::SSL_set_accept_state(_ssl.get()); } @@ -230,6 +234,59 @@ OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContext OpenSslCryptoCodecImpl::~OpenSslCryptoCodecImpl() = default; +std::unique_ptr<OpenSslCryptoCodecImpl> +OpenSslCryptoCodecImpl::make_client_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address) +{ + // Naked new due to private ctor + return std::unique_ptr<OpenSslCryptoCodecImpl>( + new OpenSslCryptoCodecImpl(std::move(ctx), peer_spec, peer_address, Mode::Client)); +} +std::unique_ptr<OpenSslCryptoCodecImpl> +OpenSslCryptoCodecImpl::make_server_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketAddress& peer_address) +{ + // Naked new due to private ctor + return std::unique_ptr<OpenSslCryptoCodecImpl>( + new OpenSslCryptoCodecImpl(std::move(ctx), SocketSpec::invalid, peer_address, Mode::Server)); +} + +void OpenSslCryptoCodecImpl::enable_hostname_validation_if_requested() { + if (_peer_spec.valid() && !_ctx->transport_security_options().disable_hostname_validation()) { + auto* verify_param = SSL_get0_param(_ssl.get()); // Internal ptr, no refcount bump or alloc. We must not free. + LOG_ASSERT(verify_param != nullptr); + vespalib::string host = _peer_spec.host(); + if (X509_VERIFY_PARAM_set1_host(verify_param, host.c_str(), host.size()) != 1) { + throw CryptoException("X509_VERIFY_PARAM_set1_host() failed"); + } + // TODO should we set expected IP based on peer address as well? + } +} + +void OpenSslCryptoCodecImpl::set_server_name_indication_extension() { + if (_peer_spec.valid()) { + vespalib::string host = _peer_spec.host(); + // OpenSSL tries to cast const char* to void* in a macro, even on 1.1.1. GCC is not overly impressed, + // so to satiate OpenSSL's quirks we pre-cast away the constness. + auto* host_cstr_that_trusts_openssl_not_to_mess_up = const_cast<char*>(host.c_str()); + if (SSL_set_tlsext_host_name(_ssl.get(), host_cstr_that_trusts_openssl_not_to_mess_up) != 1) { + throw CryptoException("SSL_set_tlsext_host_name() failed"); + } + } +} + +std::optional<vespalib::string> OpenSslCryptoCodecImpl::client_provided_sni_extension() const { + if ((_mode != Mode::Server) || (SSL_get_servername_type(_ssl.get()) != TLSEXT_NAMETYPE_host_name)) { + return {}; + } + const char* sni_host_raw = SSL_get_servername(_ssl.get(), TLSEXT_NAMETYPE_host_name); + if (sni_host_raw == nullptr) { + return {}; + } + return vespalib::string(sni_host_raw); +} + HandshakeResult OpenSslCryptoCodecImpl::handshake(const char* from_peer, size_t from_peer_buf_size, char* to_peer, size_t to_peer_buf_size) noexcept { @@ -428,3 +485,5 @@ EncodeResult OpenSslCryptoCodecImpl::half_close(char* ciphertext, size_t ciphert // External references: // [0] http://openssl.6102.n7.nabble.com/nonblocking-implementation-question-tp1728p1732.html // [1] https://github.com/grpc/grpc/blob/master/src/core/tsi/ssl_transport_security.cc +// [2] https://wiki.openssl.org/index.php/Hostname_validation +// [3] https://wiki.openssl.org/index.php/SSL/TLS_Client diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h index 14200de449a..ec8df853c16 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h @@ -3,6 +3,7 @@ #include "openssl_typedefs.h" #include <vespa/vespalib/net/socket_address.h> +#include <vespa/vespalib/net/socket_spec.h> #include <vespa/vespalib/net/tls/transport_security_options.h> #include <vespa/vespalib/net/tls/crypto_codec.h> #include <memory> @@ -46,17 +47,23 @@ class OpenSslCryptoCodecImpl : public CryptoCodec { // The context maintains shared verification callback state, so it must be // kept alive explictly for at least as long as any codecs. std::shared_ptr<OpenSslTlsContextImpl> _ctx; + SocketSpec _peer_spec; SocketAddress _peer_address; - SslPtr _ssl; - ::BIO* _input_bio; // Owned by _ssl - ::BIO* _output_bio; // Owned by _ssl - Mode _mode; + SslPtr _ssl; + ::BIO* _input_bio; // Owned by _ssl + ::BIO* _output_bio; // Owned by _ssl + Mode _mode; std::optional<DeferredHandshakeParams> _deferred_handshake_params; - std::optional<HandshakeResult> _deferred_handshake_result; + std::optional<HandshakeResult> _deferred_handshake_result; public: - OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, const SocketAddress& peer_address, Mode mode); ~OpenSslCryptoCodecImpl() override; + static std::unique_ptr<OpenSslCryptoCodecImpl> make_client_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address); + static std::unique_ptr<OpenSslCryptoCodecImpl> make_server_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketAddress& peer_address); + /* * From RFC 8449 (Record Size Limit Extension for TLS), section 1: * "TLS versions 1.2 [RFC5246] and earlier permit senders to @@ -89,7 +96,20 @@ public: EncodeResult half_close(char* ciphertext, size_t ciphertext_size) noexcept override; const SocketAddress& peer_address() const noexcept { return _peer_address; } + /* + * If a client has sent a SNI extension field as part of the handshake, + * returns the raw string representation of this. It only makes sense to + * call this for codecs in server mode. + */ + std::optional<vespalib::string> client_provided_sni_extension() const; private: + OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address, + Mode mode); + + void enable_hostname_validation_if_requested(); + void set_server_name_indication_extension(); HandshakeResult do_handshake_and_consume_peer_input_bytes() noexcept; DecodeResult drain_and_produce_plaintext_from_ssl(char* plaintext, size_t plaintext_size) noexcept; // Precondition: read_result < 0 diff --git a/vespalib/src/vespa/vespalib/net/tls/peer_policies.h b/vespalib/src/vespa/vespalib/net/tls/peer_policies.h index 44cbe70fd92..c558708de8f 100644 --- a/vespalib/src/vespa/vespalib/net/tls/peer_policies.h +++ b/vespalib/src/vespa/vespalib/net/tls/peer_policies.h @@ -64,19 +64,24 @@ public: class AuthorizedPeers { // A peer will be authorized iff it matches _one or more_ policies. std::vector<PeerPolicy> _peer_policies; - bool _allow_all_if_empty = false; + bool _allow_all_if_empty; explicit AuthorizedPeers(bool allow_all_if_empty) : _peer_policies(), _allow_all_if_empty(allow_all_if_empty) {} public: - AuthorizedPeers() = default; + AuthorizedPeers() : _peer_policies(), _allow_all_if_empty(false) {} explicit AuthorizedPeers(std::vector<PeerPolicy> peer_policies_) : _peer_policies(std::move(peer_policies_)), _allow_all_if_empty(false) {} + AuthorizedPeers(const AuthorizedPeers&) = default; + AuthorizedPeers& operator=(const AuthorizedPeers&) = default; + AuthorizedPeers(AuthorizedPeers&&) noexcept = default; + AuthorizedPeers& operator=(AuthorizedPeers&&) noexcept = default; + static AuthorizedPeers allow_all_authenticated() { return AuthorizedPeers(true); } diff --git a/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp b/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp index d0475f3e88d..99862e83dbf 100644 --- a/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp @@ -12,18 +12,16 @@ TlsCryptoEngine::TlsCryptoEngine(net::tls::TransportSecurityOptions tls_opts, ne } std::unique_ptr<TlsCryptoSocket> -TlsCryptoEngine::create_tls_client_crypto_socket(SocketHandle socket, const SocketSpec &) +TlsCryptoEngine::create_tls_client_crypto_socket(SocketHandle socket, const SocketSpec &peer_spec) { - auto mode = net::tls::CryptoCodec::Mode::Client; - auto codec = net::tls::CryptoCodec::create_default_codec(_tls_ctx, SocketAddress::peer_address(socket.get()), mode); + auto codec = net::tls::CryptoCodec::create_default_client_codec(_tls_ctx, peer_spec, SocketAddress::peer_address(socket.get())); return std::make_unique<net::tls::CryptoCodecAdapter>(std::move(socket), std::move(codec)); } std::unique_ptr<TlsCryptoSocket> TlsCryptoEngine::create_tls_server_crypto_socket(SocketHandle socket) { - auto mode = net::tls::CryptoCodec::Mode::Server; - auto codec = net::tls::CryptoCodec::create_default_codec(_tls_ctx, SocketAddress::peer_address(socket.get()), mode); + auto codec = net::tls::CryptoCodec::create_default_server_codec(_tls_ctx, SocketAddress::peer_address(socket.get())); return std::make_unique<net::tls::CryptoCodecAdapter>(std::move(socket), std::move(codec)); } diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp index c9a5fa5a6e9..47b0e1e0a43 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp @@ -11,41 +11,57 @@ TransportSecurityOptions::TransportSecurityOptions(Params params) _cert_chain_pem(std::move(params._cert_chain_pem)), _private_key_pem(std::move(params._private_key_pem)), _authorized_peers(std::move(params._authorized_peers)), - _accepted_ciphers(std::move(params._accepted_ciphers)) + _accepted_ciphers(std::move(params._accepted_ciphers)), + _disable_hostname_validation(params._disable_hostname_validation) { } TransportSecurityOptions::TransportSecurityOptions(vespalib::string ca_certs_pem, vespalib::string cert_chain_pem, - vespalib::string private_key_pem) + vespalib::string private_key_pem, + AuthorizedPeers authorized_peers, + bool disable_hostname_validation) : _ca_certs_pem(std::move(ca_certs_pem)), _cert_chain_pem(std::move(cert_chain_pem)), _private_key_pem(std::move(private_key_pem)), - _authorized_peers(AuthorizedPeers::allow_all_authenticated()) + _authorized_peers(std::move(authorized_peers)), + _disable_hostname_validation(disable_hostname_validation) { } -TransportSecurityOptions::TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem, - AuthorizedPeers authorized_peers) - : _ca_certs_pem(std::move(ca_certs_pem)), - _cert_chain_pem(std::move(cert_chain_pem)), - _private_key_pem(std::move(private_key_pem)), - _authorized_peers(std::move(authorized_peers)) -{ +TransportSecurityOptions TransportSecurityOptions::copy_without_private_key() const { + return TransportSecurityOptions(_ca_certs_pem, _cert_chain_pem, "", + _authorized_peers, _disable_hostname_validation); } void secure_memzero(void* buf, size_t size) noexcept { OPENSSL_cleanse(buf, size); } -TransportSecurityOptions::Params::Params() = default; +TransportSecurityOptions::Params::Params() + : _ca_certs_pem(), + _cert_chain_pem(), + _private_key_pem(), + _authorized_peers(), + _accepted_ciphers(), + _disable_hostname_validation(false) +{ +} TransportSecurityOptions::Params::~Params() { secure_memzero(&_private_key_pem[0], _private_key_pem.size()); } +TransportSecurityOptions::Params::Params(const Params&) = default; + +TransportSecurityOptions::Params& +TransportSecurityOptions::Params::operator=(const TransportSecurityOptions::Params&) = default; + +TransportSecurityOptions::Params::Params(Params&&) noexcept = default; + +TransportSecurityOptions::Params& +TransportSecurityOptions::Params::operator=(TransportSecurityOptions::Params&&) noexcept = default; + TransportSecurityOptions::~TransportSecurityOptions() { secure_memzero(&_private_key_pem[0], _private_key_pem.size()); } diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h index 39abb1ce4de..e0a48fc6cf5 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h @@ -14,18 +14,22 @@ class TransportSecurityOptions { vespalib::string _private_key_pem; AuthorizedPeers _authorized_peers; std::vector<vespalib::string> _accepted_ciphers; + bool _disable_hostname_validation; public: - TransportSecurityOptions() = default; - struct Params { vespalib::string _ca_certs_pem; vespalib::string _cert_chain_pem; vespalib::string _private_key_pem; AuthorizedPeers _authorized_peers; std::vector<vespalib::string> _accepted_ciphers; + bool _disable_hostname_validation; Params(); ~Params(); + Params(const Params&); + Params& operator=(const Params&); + Params(Params&&) noexcept; + Params& operator=(Params&&) noexcept; Params& ca_certs_pem(vespalib::stringref pem) { _ca_certs_pem = pem; return *this; } Params& cert_chain_pem(vespalib::stringref pem) { _cert_chain_pem = pem; return *this; } @@ -35,19 +39,14 @@ public: _accepted_ciphers = std::move(ciphers); return *this; } + Params& disable_hostname_validation(bool disable) { + _disable_hostname_validation = disable; + return *this; + } }; explicit TransportSecurityOptions(Params params); - TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem); - - TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem, - AuthorizedPeers authorized_peers); - ~TransportSecurityOptions(); const vespalib::string& ca_certs_pem() const noexcept { return _ca_certs_pem; } @@ -55,10 +54,16 @@ public: const vespalib::string& private_key_pem() const noexcept { return _private_key_pem; } const AuthorizedPeers& authorized_peers() const noexcept { return _authorized_peers; } - TransportSecurityOptions copy_without_private_key() const { - return TransportSecurityOptions(_ca_certs_pem, _cert_chain_pem, "", _authorized_peers); - } + TransportSecurityOptions copy_without_private_key() const; const std::vector<vespalib::string>& accepted_ciphers() const noexcept { return _accepted_ciphers; } + bool disable_hostname_validation() const noexcept { return _disable_hostname_validation; } + +private: + TransportSecurityOptions(vespalib::string ca_certs_pem, + vespalib::string cert_chain_pem, + vespalib::string private_key_pem, + AuthorizedPeers authorized_peers, + bool disable_hostname_validation); }; // Zeroes out `size` bytes in `buf` in a way that shall never be optimized diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp b/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp index 6b6e65bef8a..80caa15e8b2 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp @@ -123,6 +123,12 @@ std::unique_ptr<TransportSecurityOptions> load_from_input(Input& input) { auto priv_key = load_file_referenced_by_field(files, "private-key"); auto authorized_peers = parse_authorized_peers(root["authorized-peers"]); auto accepted_ciphers = parse_accepted_ciphers(root["accepted-ciphers"]); + // FIXME this is temporary until we know it won't break a bunch of things! + // It's still possible to explicitly enable hostname validation by setting this to false. + bool disable_hostname_validation = true; + if (root["disable-hostname-validation"].valid()) { + disable_hostname_validation = root["disable-hostname-validation"].asBool(); + } auto options = std::make_unique<TransportSecurityOptions>( TransportSecurityOptions::Params() @@ -130,7 +136,8 @@ std::unique_ptr<TransportSecurityOptions> load_from_input(Input& input) { .cert_chain_pem(certs) .private_key_pem(priv_key) .authorized_peers(std::move(authorized_peers)) - .accepted_ciphers(std::move(accepted_ciphers))); + .accepted_ciphers(std::move(accepted_ciphers)) + .disable_hostname_validation(disable_hostname_validation)); secure_memzero(&priv_key[0], priv_key.size()); return options; } diff --git a/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp b/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp index dcd2ced8036..b31f2830976 100644 --- a/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp +++ b/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp @@ -70,7 +70,13 @@ namespace vespalib::test { SocketSpec local_spec("tcp/localhost:123"); vespalib::net::tls::TransportSecurityOptions make_tls_options_for_testing() { - return vespalib::net::tls::TransportSecurityOptions(ca_pem, cert_pem, key_pem); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(ca_pem). + cert_chain_pem(cert_pem). + private_key_pem(key_pem). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // FIXME this is to avoid mass breakage of TLS'd networking tests. + return vespalib::net::tls::TransportSecurityOptions(std::move(ts_builder)); } } // namespace vespalib::test |