[TLS 1.3] Support Session Resumption for TLS Server

src/bogo_shim/bogo_shim.cpp

@@ -110,7 +110,9 @@ std::string map_to_bogo_error(const std::string& e)
          { "Client certificate verification failed", ":BAD_SIGNATURE:" },
          { "Client did not comply with the requested key exchange group", ":WRONG_CURVE:" },
          { "Client did not offer NULL compression", ":INVALID_COMPRESSION_LIST:" },
+         { "Client did not comply with the requested key exchange group", ":WRONG_CURVE:" },
          { "Client Hello must either contain both key_share and supported_groups extensions or neither", ":MISSING_KEY_SHARE:" },
+         { "Client Hello offered a PSK without a psk_key_exchange_modes extension", ":MISSING_EXTENSION:" },
          { "Client offered DTLS version with major version 0xFF",  ":UNSUPPORTED_PROTOCOL:" },
          { "Client offered SSLv3 which is not supported", ":UNSUPPORTED_PROTOCOL:" },
          { "Client offered TLS version with major version under 3", ":UNSUPPORTED_PROTOCOL:" },
@@ -128,13 +130,18 @@ std::string map_to_bogo_error(const std::string& e)
          { "Non-PSK Client Hello did not contain supported_groups and signature_algorithms extensions", ":NO_SHARED_GROUP:" },
          { "No certificates sent by server", ":PEER_DID_NOT_RETURN_A_CERTIFICATE:" },
          { "Not enough data to read another KeyShareEntry", ":DECODE_ERROR:" },
+         { "Not enough PSK binders", ":PSK_IDENTITY_BINDER_COUNT_MISMATCH:" },
          { "Counterparty sent inconsistent key and sig types", ":WRONG_SIGNATURE_TYPE:" },
          { "Downgrade attack detected", ":TLS13_DOWNGRADE:" },
          { "Empty ALPN protocol not allowed", ":PARSE_TLSEXT:" },
+         { "Empty PSK binders list", ":DECODE_ERROR: "},
          { "Encoding error: Cannot encode PSS string, output length too small", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
          { "Expected TLS but got a record with DTLS version", ":WRONG_VERSION_NUMBER:" },
+         { "Extension removed in updated Client Hello", ":INCONSISTENT_CLIENT_HELLO:" },
          { "Failed to agree on a signature algorithm", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
          { "Failed to agree on any signature algorithm", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
+         { "Failed to negotiate a common signature algorithm for client authentication", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
+         { "PSK extension was not at the very end of the Client Hello", ":PRE_SHARED_KEY_MUST_BE_LAST:" },
          { "Finished message didn't verify", ":DIGEST_CHECK_FAILED:" },
          { "Have data remaining in buffer after ClientHello", ":EXCESS_HANDSHAKE_DATA:" },
          { "Have data remaining in buffer after Finished", ":EXCESS_HANDSHAKE_DATA:" },
@@ -165,6 +172,7 @@ std::string map_to_bogo_error(const std::string& e)
          { "Policy forbids all available TLS version", ":NO_SUPPORTED_VERSIONS_ENABLED:" },
          { "Policy refuses to accept signing with any hash supported by peer", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
          { "Policy requires client send a certificate, but it did not", ":PEER_DID_NOT_RETURN_A_CERTIFICATE:" },
+         { "PSK binder does not check out", ":DIGEST_CHECK_FAILED:" },
          { "PSK identity selected by server is out of bounds", ":PSK_IDENTITY_NOT_FOUND:" },
          { "PSK and ciphersuite selected by server are not compatible", ":OLD_SESSION_PRF_HASH_MISMATCH:" },
          { "Received a record that exceeds maximum size", ":ENCRYPTED_LENGTH_TOO_LONG:" },
@@ -229,6 +237,7 @@ std::string map_to_bogo_error(const std::string& e)
          { "TLS record type had unexpected value", ":UNEXPECTED_RECORD:" },
          { "TLS record version had unexpected value", ":WRONG_VERSION_NUMBER:" },
          { "Test requires rejecting cert", ":CERTIFICATE_VERIFY_FAILED:" },
+         { "Too many PSK binders", ":PSK_IDENTITY_BINDER_COUNT_MISMATCH:" },
          { "Unexpected ALPN protocol", ":INVALID_ALPN_PROTOCOL:" },
          { "Unexpected record type 42 from counterparty", ":UNEXPECTED_RECORD:" },
          { "Unexpected state transition in handshake got a certificate_request expected server_hello_done seen server_hello+server_key_exchange", ":UNEXPECTED_MESSAGE:" },
@@ -737,6 +746,7 @@ std::unique_ptr<Shim_Arguments> parse_options(char* argv[])
       "expect-advertised-alpn",
       "expect-alpn",
       "expect-client-ca-list",
+      "expect-early-data-reason",
       "expect-late-alpn",
       "expect-msg-callback",
       //"expect-next-proto",
@@ -1071,6 +1081,11 @@ class Shim_Policy final : public Botan::TLS::Policy
 
       //std::chrono::seconds session_ticket_lifetime() const override;
 
+      size_t new_session_tickets_upon_handshake_success() const override
+         {
+         return m_args.flag_set("no-ticket") ? 0 : 1;
+         }
+
       std::vector<uint16_t> srtp_profiles() const override
          {
          if(m_args.option_used("srtp-profiles"))

src/bogo_shim/config.json

@@ -31,6 +31,7 @@
         "TLS11-*": "No TLS 1.1",
 
         "CBCRecordSplitting*": "No need to split CBC records in TLS 1.2",
+        "DelegatedCredentials*": "No support of -delegated-cerdential",
 
         "*SCSV*": "SCSV is meaningless without TLS 1.0/1.1 support",
 
@@ -49,16 +50,9 @@
         "Ticket-Forbidden-TLS13": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
         "Resume-Client-NoResume-TLS12-TLS13-TLS": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
         "Resume-Client-Mismatch-TLS12-TLS13-TLS": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
-
-        "DelegatedCredentials*": "No TLS 1.3 server, yet",
-        "Resume-Server-OmitPSKsOnSecondClientHello": "No TLS 1.3 server, yet",
-        "PartialClientFinishedWithSecondClientHello": "No TLS 1.3 server, yet",
-
-        "Resume-Server*TLS13*": "No TLS 1.3 server, yet",
-        "Resume-Server-*": "No TLS 1.3 server, yet",
+        "Resume-Server-UnofferedCipher-TLS13": "BoringSSL will not allow switching ciphers during TLS 1.3 resumption, we do, though.",
 
         "ExportKeyingMaterial-Server-HalfRTT-TLS13": "No TLS 1.3 server, yet",
-        "TLS13-TicketAgeSkew-*": "No TLS 1.3 server, yet",
 
         "HttpGET": "TLS 1.3 server does not detect HTTP",
         "HttpPOST": "TLS 1.3 server does not detect HTTP",
@@ -68,30 +62,13 @@
 
         "ExtraClientEncryptedExtension-TLS-TLS13": "TLS 1.3 server UNIMPLEMENTED",
 
+        "Server-Verify-*-TLS13": "TODO: FIXME - missing deny-list for outdated signature schemes",
+        "Server-VerifyDefault-*-TLS13": "TODO: FIXME - missing deny-list for outdated signature schemes",
+
         "ALPNServer-Decline-TLS-TLS13": "TLS 1.3 server session resumption NYI",
-        "ALPNServer-TLS-TLS13": "TLS 1.3 server session resumption NYI",
-        "ALPNServer-Async-TLS-TLS13": "TLS 1.3 server session resumption NYI",
-        "CertificateVerificationSucceed-Server-TLS13-*": "TLS 1.3 server session resumption NYI",
-        "CurveID-Resume-Server-TLS13": "TLS 1.3 server session resumption NYI",
-        "ExtraPSKIdentity-TLS13": "TLS 1.3 server session resumption NYI",
-        "NoClientCertificate-Server-TLS13": "TLS 1.3 server session resumption NYI",
-        "NoClientCertificateRequested-Server-TLS13": "TLS 1.3 server session resumption NYI",
-        "Server-Verify-*-TLS13": "TLS 1.3 server session resumption NYI",
-        "Server-VerifyDefault-*-TLS13": "TLS 1.3 server session resumption NYI",
-        "ServerNameExtensionServer-TLS-TLS13": "TLS 1.3 server session resumption NYI",
-        "TLS-TLS13-*-server": "TLS 1.3 server session resumption NYI",
-        "TLS13-SendNoKEMModesWithPSK-Server": "TLS 1.3 server session resumption NYI",
-        "TrailingDataWithFinished-Resume-Server-TLS13": "TLS 1.3 server session resumption NYI",
-        "TLS13-ExpectNoSessionTicketOnBadKEMode-Server": "TLS 1.3 server session resumption NYI",
-        "TLS13-SendUnknownModeSessionTicket-Server": "TLS 1.3 server session resumption NYI",
-        "TLS13-SendBadKEModeSessionTicket-Server": "TLS 1.3 server session resumption NYI",
-        "TLS13-HelloRetryRequest-Server-*": "TLS 1.3 server session resumption NYI",
-        "OCSPStapling-Server-TLS13-*":  "TLS 1.3 server session resumption NYI",
-        "ServerOCSPCallback*-TLS13-*":  "TLS 1.3 server session resumption NYI",
-        "TLS13-1RTT-Server-*":  "TLS 1.3 server session resumption NYI",
 
         "*EarlyData*": "No TLS 1.3 Early Data, yet",
-        "TLS13-1RTT-Client-*": "No TLS 1.3 Early Data, yet",
+        "TLS13-TicketAgeSkew-*": "No TLS 1.3 Early Data, yet",
 
         "SendNoClientCertificateExtensions-TLS13": "-signed-cert-timestamps currently not supported in the shim",
         "KeyUpdate-RequestACK-UnfinishedWrite": "-read-with-unfinished-write currently not supported in the shim",

src/lib/tls/msg_client_hello.cpp

@@ -654,6 +654,16 @@ Client_Hello_13::Client_Hello_13(std::unique_ptr<Client_Hello_Internal> data)
          throw TLS_Exception(Alert::MissingExtension,
                              "Client Hello offered a PSK without a psk_key_exchange_modes extension");
          }
+
+      // RFC 8446 4.2.11
+      //     The "pre_shared_key" extension MUST be the last extension in the
+      //     ClientHello [...]. Servers MUST check that it is the last extension
+      //     and otherwise fail the handshake with an "illegal_parameter" alert.
+      if(exts.all().back()->type() != Extension_Code::PresharedKey)
+         {
+         throw TLS_Exception(Alert::IllegalParameter,
+                             "PSK extension was not at the very end of the Client Hello");
+         }
       }
 
    // RFC 8446 9.2
@@ -747,7 +757,7 @@ Client_Hello_13::Client_Hello_13(const Policy& policy,
                                  RandomNumberGenerator& rng,
                                  const std::string& hostname,
                                  const std::vector<std::string>& next_protocols,
-                                 const std::optional<Session_with_Handle>& session)
+                                 std::optional<Session_with_Handle>& session)
    {
    // RFC 8446 4.1.2
    //    In TLS 1.3, the client indicates its version preferences in the
@@ -826,20 +836,23 @@ Client_Hello_13::Client_Hello_13(const Policy& policy,
          m_data->extensions.add(new Supported_Point_Formats(policy.use_ecc_point_compression()));
       }
 
-   // TODO: Some extensions require a certain order or pose other assumptions.
-   //       We should check those after the user was allowed to make changes to
-   //       the extensions.
-   cb.tls_modify_extensions(m_data->extensions, Connection_Side::Client, type());
-
-   // RFC 8446 4.2.11
-   //    The "pre_shared_key" extension MUST be the last extension in the
-   //    ClientHello (this facilitates implementation [...]).
-   //
-   // The PSK extension takes the partial transcript hash into account. Passing
-   // into Callbacks::tls_modify_extensions() does not make sense therefore.
    if(session.has_value())
       {
       m_data->extensions.add(new PSK(session.value(), cb));
+      }
+
+   cb.tls_modify_extensions(m_data->extensions, Connection_Side::Client, type());
+
+   if(m_data->extensions.has<PSK>())
+      {
+      // RFC 8446 4.2.11
+      //    The "pre_shared_key" extension MUST be the last extension in the
+      //    ClientHello (this facilitates implementation [...]).
+      if(m_data->extensions.all().back()->type() != Extension_Code::PresharedKey)
+         {
+         throw TLS_Exception(Alert::InternalError,
+                             "Application modified extensions of Client Hello, PSK is not last anymore");
+         }
       calculate_psk_binders({});
       }
    }
@@ -954,7 +967,7 @@ void Client_Hello_13::validate_updates(const Client_Hello_13& new_ch)
          // if(oldext == Padding::static_type())
          //    continue;
 
-         throw TLS_Exception(Alert::MissingExtension, "Extension removed in updated Client Hello");
+         throw TLS_Exception(Alert::IllegalParameter, "Extension removed in updated Client Hello");
          }
       }
 

src/lib/tls/msg_server_hello.cpp

@@ -14,6 +14,7 @@
 #include <botan/tls_extensions.h>
 #include <botan/tls_exceptn.h>
 #include <botan/tls_callbacks.h>
+#include <botan/tls_session_manager.h>
 #include <botan/internal/tls_reader.h>
 #include <botan/mem_ops.h>
 #include <botan/internal/tls_session_key.h>
@@ -501,9 +502,18 @@ Server_Hello_13::Hello_Retry_Request_Creation_Tag Server_Hello_13::as_new_hello_
 
 std::variant<Hello_Retry_Request, Server_Hello_13> Server_Hello_13::create(const Client_Hello_13& ch,
       bool hello_retry_request_allowed,
+      Session_Manager& session_mgr,
       RandomNumberGenerator& rng, const Policy& policy, Callbacks& cb)
    {
    const auto& exts = ch.extensions();
+
+   // RFC 8446 4.2.9
+   //    [With PSK with (EC)DHE key establishment], the client and server MUST
+   //    supply "key_share" values [...].
+   //
+   // Note: We currently do not support PSK without (EC)DHE, hence, we can
+   //       assume that those extensions are available.
+   BOTAN_ASSERT_NOMSG(exts.has<Supported_Groups>() && exts.has<Key_Share>());
    const auto& supported_by_client = exts.get<Supported_Groups>()->groups();
    const auto& offered_by_client = exts.get<Key_Share>()->offered_groups();
    const auto selected_group = policy.choose_key_exchange_group(supported_by_client, offered_by_client);
@@ -529,7 +539,7 @@ std::variant<Hello_Retry_Request, Server_Hello_13> Server_Hello_13::create(const
       }
    else
       {
-      return Server_Hello_13(ch, selected_group, rng, cb, policy);
+      return Server_Hello_13(ch, selected_group, session_mgr, rng, cb, policy);
       }
    }
 
@@ -631,7 +641,6 @@ Server_Hello_13::Server_Hello_13(std::unique_ptr<Server_Hello_Internal> data,
    const std::set<Extension_Code> allowed =
       {
       Extension_Code::KeyShare,
-      Extension_Code::PskKeyExchangeModes,
       Extension_Code::SupportedVersions,
       Extension_Code::PresharedKey,
       };
@@ -711,6 +720,27 @@ uint16_t choose_ciphersuite(const Client_Hello_13& ch, const Policy& policy)
 
    for(auto suite_id : pref_list)
       {
+      // TODO: take potentially available PSKs into account to select a
+      //       compatible ciphersuite.
+      //
+      // Assuming the client sent one or more PSKs, we would first need to find
+      // the hash functions they are associated to. For session tickets, that
+      // would mean decrypting the ticket and comparing the cipher suite used in
+      // those tickets. For (currently not yet supported) pre-assigned PSKs, the
+      // hash function needs to be specified along with them.
+      //
+      // Then we could refine the ciphersuite selection using the required hash
+      // function for the PSK(s) we are wishing to use down the road.
+      //
+      // For now, we just negotiate the cipher suite blindly and hope for the
+      // best. As long as PSKs are used for session resumption only, this has a
+      // high chance of success. Previous handshakes with this client have very
+      // likely selected the same ciphersuite anyway.
+      //
+      // See also RFC 8446 4.2.11
+      //    When session resumption is the primary use case of PSKs, the most
+      //    straightforward way to implement the PSK/cipher suite matching
+      //    requirements is to negotiate the cipher suite first [...].
       if(value_exists(other_list, suite_id))
          { return suite_id; }
       }
@@ -726,6 +756,7 @@ uint16_t choose_ciphersuite(const Client_Hello_13& ch, const Policy& policy)
 
 Server_Hello_13::Server_Hello_13(const Client_Hello_13& ch,
                                  std::optional<Named_Group> key_exchange_group,
+                                 Session_Manager& session_mgr,
                                  RandomNumberGenerator& rng,
                                  Callbacks& cb,
                                  const Policy& policy)
@@ -750,6 +781,35 @@ Server_Hello_13::Server_Hello_13(const Client_Hello_13& ch,
    if(key_exchange_group.has_value())
       { m_data->extensions.add(new Key_Share(key_exchange_group.value(), cb, rng)); }
 
+   auto& ch_exts = ch.extensions();
+
+   if(ch_exts.has<PSK>())
+      {
+      const auto cs = Ciphersuite::by_id(m_data->ciphersuite);
+      BOTAN_ASSERT_NOMSG(cs);
+
+      // RFC 8446 4.2.9
+      //    A client MUST provide a "psk_key_exchange_modes" extension if it
+      //    offers a "pre_shared_key" extension.
+      //
+      // Note: Client_Hello_13 constructor already performed a graceful check.
+      const auto psk_modes = ch_exts.get<PSK_Key_Exchange_Modes>();
+      BOTAN_ASSERT_NONNULL(psk_modes);
+
+      // TODO: also support PSK_Key_Exchange_Mode::PSK_KE
+      //       (PSK-based handshake without an additional ephemeral key exchange)
+      if(value_exists(psk_modes->modes(), PSK_Key_Exchange_Mode::PSK_DHE_KE))
+         {
+         if(auto server_psk = ch_exts.get<PSK>()->select_offered_psk(cs.value(), session_mgr, cb, policy))
+            {
+            // RFC 8446 4.2.11
+            //    In order to accept PSK key establishment, the server sends a
+            //    "pre_shared_key" extension indicating the selected identity.
+            m_data->extensions.add(std::move(server_psk));
+            }
+         }
+      }
+
    cb.tls_modify_extensions(m_data->extensions, Connection_Side::Server, type());
    }