Merge GH #1967 #1703 Add OCSP stapling support for TLS servers

9 files changed, 122 insertions, 25 deletions

diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp
index c607ae7c9..29331d3cd 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<Shim_Arguments> 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<Shim_Arguments> 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<uint8_t> tls_provide_cert_status(const std::vector<Botan::X509_Certificate>&,
+                                                   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_cert_status.cpp b/src/lib/tls/msg_cert_status.cpp
index c0cd82a28..ecc649a13 100644
--- a/src/lib/tls/msg_cert_status.cpp
+++ b/src/lib/tls/msg_cert_status.cpp
@@ -42,17 +42,25 @@ Certificate_Status::Certificate_Status(Handshake_IO& io,
    hash.update(io.send(*this));
    }
 
+Certificate_Status::Certificate_Status(Handshake_IO& io,
+                                       Handshake_Hash& hash,
+                                       const std::vector<uint8_t>& raw_response_bytes) :
+   m_response(raw_response_bytes)
+   {
+   hash.update(io.send(*this));
+   }
+
 std::vector<uint8_t> Certificate_Status::serialize() const
    {
    if(m_response.size() > 0xFFFFFF) // unlikely
       throw Encoding_Error("OCSP response too long to encode in TLS");
 
-   const uint32_t m_response_len = static_cast<uint32_t>(m_response.size());
+   const uint32_t response_len = static_cast<uint32_t>(m_response.size());
 
    std::vector<uint8_t> buf;
    buf.push_back(1); // type OCSP
    for(size_t i = 1; i < 4; ++i)
-      buf[i] = get_byte(i, m_response_len);
+      buf.push_back(get_byte(i, response_len));
 
    buf += m_response;
    return buf;
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_callbacks.h b/src/lib/tls/tls_callbacks.h
index b35cf0051..325ed884b 100644
--- a/src/lib/tls/tls_callbacks.h
+++ b/src/lib/tls/tls_callbacks.h
@@ -31,6 +31,7 @@ namespace TLS {
 class Handshake_Message;
 class Policy;
 class Extensions;
+class Certificate_Status_Request;
 
 /**
 * Encapsulates the callbacks that a TLS channel will make which are due to
@@ -142,6 +143,24 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
           return std::chrono::milliseconds(0);
           }
 
+      /**
+       * Called by the TLS server whenever the client included the
+       * status_request extension (see RFC 6066, a.k.a OCSP stapling)
+       * in the ClientHello.
+       *
+       * @return the encoded OCSP response to be sent to the client which
+       * indicates the revocation status of the server certificate. Return an
+       * empty vector to indicate that no response is available, and thus
+       * suppress the Certificate_Status message.
+       */
+       virtual std::vector<uint8_t> tls_provide_cert_status(const std::vector<X509_Certificate>& chain,
+                                                            const Certificate_Status_Request& csr) const
+          {
+          BOTAN_UNUSED(chain);
+          BOTAN_UNUSED(csr);
+          return std::vector<uint8_t>();
+          }
+
        /**
        * Optional callback with default impl: sign a message
        *
diff --git a/src/lib/tls/tls_extensions.cpp b/src/lib/tls/tls_extensions.cpp
index a673f867b..ca4e1200f 100644
--- a/src/lib/tls/tls_extensions.cpp
+++ b/src/lib/tls/tls_extensions.cpp
@@ -34,7 +34,7 @@ Extension* make_extension(TLS_Data_Reader& reader, uint16_t code, uint16_t size,
          return new Supported_Groups(reader, size);
 
       case TLSEXT_CERT_STATUS_REQUEST:
-         return new Certificate_Status_Request(reader, size);
+         return new Certificate_Status_Request(reader, size, side);
 
       case TLSEXT_EC_POINT_FORMATS:
          return new Supported_Point_Formats(reader, size);
@@ -538,15 +538,19 @@ std::vector<uint8_t> Certificate_Status_Request::serialize() const
    }
 
 Certificate_Status_Request::Certificate_Status_Request(TLS_Data_Reader& reader,
-                                                       uint16_t extension_size) :
-   m_server_side(false)
+                                                       uint16_t extension_size,
+                                                       Connection_Side side) :
+   m_server_side(side == SERVER)
    {
    if(extension_size > 0)
       {
       const uint8_t type = reader.get_byte();
       if(type == 1)
          {
-         reader.discard_next(extension_size - 1); // fixme
+         size_t len_resp_id_list = reader.get_uint16_t();
+         m_ocsp_names = reader.get_fixed<uint8_t>(len_resp_id_list);
+         size_t len_requ_ext = reader.get_uint16_t();
+         m_extension_bytes = reader.get_fixed<uint8_t>(len_requ_ext );
          }
       else
          {
@@ -555,7 +559,7 @@ Certificate_Status_Request::Certificate_Status_Request(TLS_Data_Reader& reader,
       }
    }
 
-Certificate_Status_Request::Certificate_Status_Request(const std::vector<X509_DN>& ocsp_responder_ids,
+Certificate_Status_Request::Certificate_Status_Request(const std::vector<uint8_t>& ocsp_responder_ids,
                                                        const std::vector<std::vector<uint8_t>>& ocsp_key_ids) :
    m_ocsp_names(ocsp_responder_ids),
    m_ocsp_keys(ocsp_key_ids),
diff --git a/src/lib/tls/tls_extensions.h b/src/lib/tls/tls_extensions.h
index 3ecfb7c0f..5920a1576 100644
--- a/src/lib/tls/tls_extensions.h
+++ b/src/lib/tls/tls_extensions.h
@@ -397,17 +397,29 @@ class BOTAN_UNSTABLE_API Certificate_Status_Request final : public Extension
 
       bool empty() const override { return false; }
 
+      const std::vector<uint8_t>& get_responder_id_list() const
+         {
+         return m_ocsp_names;
+         }
+
+      const std::vector<uint8_t>& get_request_extensions() const
+         {
+         return m_extension_bytes;
+         }
+
       // Server generated version: empty
       Certificate_Status_Request();
 
       // Client version, both lists can be empty
-      Certificate_Status_Request(const std::vector<X509_DN>& ocsp_responder_ids,
+      Certificate_Status_Request(const std::vector<uint8_t>& ocsp_responder_ids,
                                  const std::vector<std::vector<uint8_t>>& ocsp_key_ids);
 
-      Certificate_Status_Request(TLS_Data_Reader& reader, uint16_t extension_size);
+      Certificate_Status_Request(TLS_Data_Reader& reader,
+                                 uint16_t extension_size,
+                                 Connection_Side side);
    private:
-      std::vector<X509_DN> m_ocsp_names;
-      std::vector<std::vector<uint8_t>> m_ocsp_keys;
+      std::vector<uint8_t> m_ocsp_names;
+      std::vector<std::vector<uint8_t>> m_ocsp_keys; // is this field really needed
       std::vector<uint8_t> m_extension_bytes;
       bool m_server_side;
    };
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index 0549a66a6..31c067696 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -399,6 +399,13 @@ class BOTAN_UNSTABLE_API Certificate_Status final : public Handshake_Message
                          Handshake_Hash& hash,
                          std::shared_ptr<const OCSP::Response> response);
 
+      /*
+       * Create a Certificate_Status message using an already DER encoded OCSP response.
+       */
+      Certificate_Status(Handshake_IO& io,
+                         Handshake_Hash& hash,
+                         std::vector<uint8_t> const& raw_response_bytes );
+
    private:
       std::vector<uint8_t> serialize() const override;
       std::vector<uint8_t> m_response;
diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp
index d0c3baa2e..63e04a568 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<X509_Certificate>& 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<X509_Certificate> 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,6 +854,22 @@ 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() && pending_state.is_a_resumption() == false)
+         {
+         auto csr = pending_state.client_hello()->extensions().get<Certificate_Status_Request>();
+         // csr is non-null if client_hello()->supports_cert_status_message()
+         BOTAN_ASSERT_NOMSG(csr != nullptr);
+         const auto resp_bytes = callbacks().tls_provide_cert_status(cert_chains[algo_used], *csr);
+         if(resp_bytes.size() > 0)
+            {
+            pending_state.server_cert_status(new Certificate_Status(
+                                                pending_state.handshake_io(),
+                                                pending_state.hash(),
+                                                resp_bytes
+                                                ));
+            }
+         }
+
       private_key = m_creds.private_key_for(
          pending_state.server_certs()->cert_chain()[0],
          "tls-server",