Refactor: basic session sanity checks

randombit · Feb 21, 2023 · c1a086e · c1a086e
1 parent 04f0d8b
commit c1a086e
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 15 deletions.
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
@@ -840,18 +840,7 @@ Client_Hello_13::Client_Hello_13(const Policy& policy,
 
    if(session_and_handle.has_value())
       {
-      // RFC 8446 4.6.1
-      //    Clients MUST only resume if the new SNI value is valid for the
-      //    server certificate presented in the original session and SHOULD only
-      //    resume if the SNI value matches the one used in the original session.
-      //
-      // If the provided session posesses a different host name we don't attempt
-      // a resumption. As a result, a normal handshake will be performed.
-      const auto& [session, _] = session_and_handle.value();
-      if(session.server_info().empty() || session.server_info().hostname() == hostname)
-         {
-         m_data->extensions.add(new PSK(session_and_handle.value(), cb));
-         }
+      m_data->extensions.add(new PSK(session_and_handle.value(), cb));
       }
 
    cb.tls_modify_extensions(m_data->extensions, Connection_Side::Client, type());

diff --git a/src/lib/tls/tls13/tls_client_impl_13.cpp b/src/lib/tls/tls13/tls_client_impl_13.cpp
@@ -131,12 +131,37 @@ bool Client_Impl_13::handshake_finished() const
 
 std::optional<std::pair<Session, Session_Handle>> Client_Impl_13::find_session_for_resumption()
    {
+   auto sessions = session_manager().find(m_info, callbacks(), policy());
+   if(sessions.empty())
+      { return std::nullopt; }
+
    // TODO: TLS 1.3 allows sending more than one ticket (for resumption) in a
    //       Client Hello. Currently, we do not support that. The Session_Manager
    //       does imply otherwise with its API, though.
-   if(auto sessions = session_manager().find(m_info, callbacks(), policy()); !sessions.empty())
-      return sessions.front();
-   return std::nullopt;
+   auto& session_to_resume = sessions.front();
+
+   // RFC 8446 4.6.1
+   //    Clients MUST only resume if the new SNI value is valid for the
+   //    server certificate presented in the original session and SHOULD only
+   //    resume if the SNI value matches the one used in the original session.
+   //
+   // ... i.e. we do not attempt a resumption if the session's certificate does
+   // not withstand basic validity checks or -- if no server certificate is
+   // available -- at least the session's host name matches the expectations.
+   const auto& cert_chain = session_to_resume.first.peer_certs();
+   if(!cert_chain.empty())
+      {
+      const auto& server_cert = cert_chain.front();
+      if(!server_cert.is_self_signed() &&
+         (!server_cert.matches_dns_name(m_info.hostname()) ||
+          X509_Time(callbacks().tls_current_timestamp()) > server_cert.not_after()))
+         { return std::nullopt; }
+      }
+   else if(!session_to_resume.first.server_info().empty() &&
+           session_to_resume.first.server_info().hostname() != m_info.hostname())
+      { return std::nullopt; }
+
+   return std::move(session_to_resume);
    }
 
 void Client_Impl_13::handle(const Server_Hello_12& server_hello_msg)