From 00ed1fb41a02471dd7a35500fb68c341b85663ae Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 22 May 2019 13:24:29 -0400 Subject: Add BoGo tests and fix resumption case --- src/bogo_shim/bogo_shim.cpp | 43 ++++++++++++++++++++++++++++++++++------ src/bogo_shim/config.json | 5 +---- src/lib/tls/msg_server_hello.cpp | 4 ---- src/lib/tls/tls_server.cpp | 9 ++++++++- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp index c607ae7c9..47810c4ba 100644 --- a/src/bogo_shim/bogo_shim.cpp +++ b/src/bogo_shim/bogo_shim.cpp @@ -152,6 +152,7 @@ std::string map_to_bogo_error(const std::string& e) { "Server_Hello_Done: Must be empty, and is not", ":DECODE_ERROR:" }, { "Simulated OCSP callback failure", ":OCSP_CB_ERROR:" }, { "Simulating cert verify callback failure", ":CERT_CB_ERROR:" }, + { "Simulating failure from OCSP response callback", ":OCSP_CB_ERROR:" }, { "TLS plaintext record is larger than allowed maximum", ":DATA_LENGTH_TOO_LONG:" }, { "TLS signature extension did not allow for RSA/SHA-256 signature", ":WRONG_SIGNATURE_TYPE:", }, { "Test requires rejecting cert", ":CERTIFICATE_VERIFY_FAILED:" }, @@ -432,6 +433,8 @@ class Shim_Arguments final bool option_used(const std::string& key) const { + if(m_all_options.count(key) == 0) + throw Shim_Exception("Invalid option " + key); if(m_parsed_opts.find(key) != m_parsed_opts.end()) return true; if(m_parsed_int_vec_opts.find(key) != m_parsed_int_vec_opts.end()) @@ -586,7 +589,7 @@ std::unique_ptr parse_options(char* argv[]) "send-alert", "server", "server-preference", - //"set-ocsp-in-callback", + "set-ocsp-in-callback", "shim-shuts-down", "shim-writes-first", //"tls-unique", @@ -639,7 +642,7 @@ std::unique_ptr parse_options(char* argv[]) "expect-ocsp-response", //"expect-quic-transport-params", //"expect-signed-cert-timestamps", - //"ocsp-response", + "ocsp-response", //"quic-transport-params", //"signed-cert-timestamps", //"ticket-key", /* we use a different ticket format from Boring */ @@ -975,7 +978,12 @@ class Shim_Policy final : public Botan::TLS::Policy bool support_cert_status_message() const override { if(m_args.flag_set("server")) - return false; + { + if(!m_args.option_used("ocsp-response")) + return false; + if(m_args.flag_set("decline-ocsp-callback")) + return false; + } return true; } @@ -1226,6 +1234,23 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks } } + std::vector tls_srv_provide_cert_status_response(const std::vector&, + const Botan::TLS::Certificate_Status_Request&) const override + { + if(m_args.flag_set("use-ocsp-callback") && m_args.flag_set("fail-ocsp-callback")) + throw std::runtime_error("Simulating failure from OCSP response callback"); + + if(m_args.flag_set("decline-ocsp-callback")) + return {}; + + if(m_args.option_used("ocsp-response")) + { + return m_args.get_b64_opt("ocsp-response"); + } + + return {}; + } + void tls_record_received(uint64_t /*seq_no*/, const uint8_t data[], size_t size) override { if(size == 0) @@ -1345,10 +1370,16 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks m_policy.incr_session_established(); - if(m_args.option_used("expect-no-session-id")) + if(m_args.flag_set("expect-no-session-id")) + { + // BoGo expects that ticket issuance implies no stateful session... + if(!m_args.flag_set("server") && session.session_id().size() > 0) + shim_exit_with_error("Unexpectedly got a session ID"); + } + else if(m_args.flag_set("expect-session-id")) { - if(session.session_id().size() > 0) - shim_exit_with_error("Unexpected session ID"); + if(session.session_id().empty()) + shim_exit_with_error("Unexpectedly got no session ID"); } if(m_args.option_used("expect-version")) diff --git a/src/bogo_shim/config.json b/src/bogo_shim/config.json index 545b7ce73..b670aa39b 100644 --- a/src/bogo_shim/config.json +++ b/src/bogo_shim/config.json @@ -40,6 +40,7 @@ "*SignedCertificateTimestamp*": "No support for SCT", "*SCT*": "No support for SCT", "Renegotiation-ChangeAuthProperties": "No support for SCT", + "UnsolicitedCertificateExtensions-TLS*": "No support for SCT", "*NULL-SHA*": "No support for NULL ciphers", "*GREASE*": "No support for GREASE", @@ -76,10 +77,6 @@ "CheckLeafCurve": "Botan ignores this", - "OCSPStapling-Server-*": "Server doesn't support OCSP stapling currently", - "UnsolicitedCertificateExtensions-TLS*": "Server doesn't support OCSP stapling currently", - "ServerOCSPCallback*": "Server doesn't support OCSP stapling currently", - "CertificateVerificationDoesNotFailOnResume*": "Botan doesn't support reverify on resume", "CertificateVerificationFailsOnResume*": "Botan doesn't support reverify on resume", "CertificateVerificationPassesOnResume*": "Botan doesn't support reverify on resume", diff --git a/src/lib/tls/msg_server_hello.cpp b/src/lib/tls/msg_server_hello.cpp index b4c47f516..933ee8af7 100644 --- a/src/lib/tls/msg_server_hello.cpp +++ b/src/lib/tls/msg_server_hello.cpp @@ -110,10 +110,6 @@ Server_Hello::Server_Hello(Handshake_IO& io, if(client_hello.supports_extended_master_secret()) m_extensions.add(new Extended_Master_Secret); - // Sending the extension back does not commit us to sending a stapled response - if(client_hello.supports_cert_status_message() && policy.support_cert_status_message()) - m_extensions.add(new Certificate_Status_Request); - if(client_hello.supports_encrypt_then_mac() && policy.negotiate_encrypt_then_mac()) { Ciphersuite c = resumed_session.ciphersuite(); diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp index 153fb05c1..8443c9c8c 100644 --- a/src/lib/tls/tls_server.cpp +++ b/src/lib/tls/tls_server.cpp @@ -37,6 +37,10 @@ class Server_Handshake_State final : public Handshake_State void set_resume_certs(const std::vector& certs) { m_resume_peer_certs = certs; } + void mark_as_resumption() { m_is_a_resumption = true; } + + bool is_a_resumption() const { return m_is_a_resumption; } + private: // Used by the server only, in case of RSA key exchange. Not owned Private_Key* m_server_rsa_kex_key = nullptr; @@ -47,6 +51,8 @@ class Server_Handshake_State final : public Handshake_State */ bool m_allow_session_resumption = true; + bool m_is_a_resumption = false; + std::vector m_resume_peer_certs; }; @@ -740,6 +746,7 @@ void Server::session_resume(Server_Handshake_State& pending_state, secure_renegotiation_check(pending_state.server_hello()); + pending_state.mark_as_resumption(); pending_state.compute_session_keys(session_info.master_secret()); pending_state.set_resume_certs(session_info.peer_certs()); @@ -847,7 +854,7 @@ void Server::session_create(Server_Handshake_State& pending_state, pending_state.hash(), cert_chains[algo_used])); - if(pending_state.client_hello()->supports_cert_status_message()) + if(pending_state.client_hello()->supports_cert_status_message() && pending_state.is_a_resumption() == false) { auto csr = pending_state.client_hello()->extensions().get(); // csr is non-null if client_hello()->supports_cert_status_message() -- cgit v1.2.3