Repository: randombit/botan
Pull request #2976 (2022-05-18 08:11:09)

Introduce TLS::Callbacks::tls_current_timestamp()
This extracts the TLS 1.2 portions of [yesterday's pull request](https://github.com/randombit/botan/pull/2975). 

Note that I removed the methods `Connection_Cipher_State::age()` (not used anymore) and `Session::session_age()` (users will need to use `::now() - Session::start_time()`). The latter is a  public API change, though...

Furthermore, there are calls to `system_clock::now()` in the implementations of the `Session_Manager` base class (e.g. [in the `Session_Manager_In_Memory`](https://github.com/randombit/botan/blob/2fb7ba81b0304f0864a557e3f3a010f4c2ce08d3/src/lib/tls/tls_session_manager_memory.cpp#L45)). I didn't change those because the `Callbacks&` aren't available in their context without an API change either. I guess it is worth discussing how to deal with those locations.


Patch:
diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp
index 4cc7bca2d..351214103 100644
--- a/src/bogo_shim/bogo_shim.cpp
+++ b/src/bogo_shim/bogo_shim.cpp
@@ -1209,7 +1209,8 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks
          m_warning_alerts(0),
          m_empty_records(0),
          m_sessions_established(0),
-         m_got_close(false)
+         m_got_close(false),
+         m_clock_skew(0)
          {}
 
       size_t sessions_established() const { return m_sessions_established; }
@@ -1219,6 +1220,8 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks
          m_channel = channel;
          }
 
+      void set_clock_skew(std::chrono::seconds clock_skew) { m_clock_skew = clock_skew; }
+
       bool saw_close_notify() const { return m_got_close; }
 
       void tls_emit_data(const uint8_t data[], size_t size) override
@@ -1493,6 +1496,16 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks
             }
          }
 
+      std::chrono::system_clock::time_point tls_current_timestamp() override
+         {
+         // Some tests require precise timings. Hence, the TLS 'now' timestamp
+         // is frozen on first access and rounded to the last full second. E.g.
+         // storage of sessions does store the timestamp with second-resolution.
+         using sec = std::chrono::seconds;
+         static auto g_now = std::chrono::floor<sec>(std::chrono::system_clock::now());
+         return g_now + m_clock_skew;
+         }
+
    private:
       Botan::TLS::Channel* m_channel;
       const Shim_Arguments& m_args;
@@ -1503,6 +1516,7 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks
       size_t m_empty_records;
       size_t m_sessions_established;
       bool m_got_close;
+      std::chrono::seconds m_clock_skew;
    };
 
 }
@@ -1538,6 +1552,12 @@ int main(int /*argc*/, char* argv[])
          Shim_Policy policy(*args);
          Shim_Callbacks callbacks(*args, socket, policy);
 
+         if(args->option_used("resumption-delay") && i > 0)
+            {
+            shim_log("skewing the clock by " + std::to_string(args->get_int_opt("resumption-delay")) + " seconds");
+            callbacks.set_clock_skew(std::chrono::seconds(args->get_int_opt("resumption-delay")));
+            }
+
          std::unique_ptr<Botan::TLS::Channel> chan;
 
          if(is_server)
diff --git a/src/cli/tls_client.cpp b/src/cli/tls_client.cpp
index 177b6d522..9d18c7a16 100644
--- a/src/cli/tls_client.cpp
+++ b/src/cli/tls_client.cpp
@@ -294,7 +294,7 @@ class TLS_Client final : public Command, public Botan::TLS::Callbacks
                trusted_roots,
                hostname,
                usage,
-               std::chrono::system_clock::now(),
+               tls_current_timestamp(),
                ocsp_timeout,
                ocsp);
 
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
index c164f23f6..6b56056b9 100644
--- a/src/lib/tls/msg_client_hello.cpp
+++ b/src/lib/tls/msg_client_hello.cpp
@@ -33,6 +33,7 @@ enum
    };
 
 std::vector<uint8_t> make_hello_random(RandomNumberGenerator& rng,
+                                       Callbacks& cb,
                                        const Policy& policy)
    {
    std::vector<uint8_t> buf(32);
@@ -48,7 +49,7 @@ std::vector<uint8_t> make_hello_random(RandomNumberGenerator& rng,
    if(policy.include_time_in_hello_random())
       {
       const uint32_t time32 = static_cast<uint32_t>(
-                                 std::chrono::system_clock::to_time_t(std::chrono::system_clock::now()));
+                                 std::chrono::system_clock::to_time_t(cb.tls_current_timestamp()));
 
       store_be(time32, buf.data());
       }
@@ -355,7 +356,7 @@ Client_Hello_12::Client_Hello_12(Handshake_IO& io,
                                  const std::vector<std::string>& next_protocols)
    {
    m_legacy_version = client_settings.protocol_version();
-   m_random = make_hello_random(rng, policy);
+   m_random = make_hello_random(rng, cb, policy);
    m_suites = policy.ciphersuite_list(client_settings.protocol_version());
 
    if(!policy.acceptable_protocol_version(m_legacy_version))
@@ -418,7 +419,7 @@ Client_Hello_12::Client_Hello_12(Handshake_IO& io,
                                  const std::vector<std::string>& next_protocols)
    {
    m_legacy_version = session.version();
-   m_random = make_hello_random(rng, policy);
+   m_random = make_hello_random(rng, cb, policy);
    m_session_id = session.session_id();
    m_suites = policy.ciphersuite_list(m_legacy_version);
 
diff --git a/src/lib/tls/msg_server_hello.cpp b/src/lib/tls/msg_server_hello.cpp
index 5c5ad902f..b5f996f3d 100644
--- a/src/lib/tls/msg_server_hello.cpp
+++ b/src/lib/tls/msg_server_hello.cpp
@@ -32,10 +32,11 @@ const uint64_t DOWNGRADE_TLS12 = 0x444F574E47524401;
 std::vector<uint8_t>
 make_server_hello_random(RandomNumberGenerator& rng,
                          Protocol_Version offered_version,
+                         Callbacks& cb,
                          const Policy& policy)
    {
    BOTAN_UNUSED(offered_version);
-   auto random = make_hello_random(rng, policy);
+   auto random = make_hello_random(rng, cb, policy);
    return random;
    }
 
@@ -164,7 +165,7 @@ Server_Hello_12::Server_Hello_12(Handshake_IO& io,
    Server_Hello(std::make_unique<Server_Hello::Internal>(
                    server_settings.protocol_version(),
                    server_settings.session_id(),
-                   make_server_hello_random(rng, server_settings.protocol_version(), policy),
+                   make_server_hello_random(rng, server_settings.protocol_version(), cb, policy),
                    server_settings.ciphersuite(),
                    uint8_t(0)))
    {
@@ -248,7 +249,7 @@ Server_Hello_12::Server_Hello_12(Handshake_IO& io,
    Server_Hello(std::make_unique<Server_Hello::Internal>(
                    resumed_session.version(),
                    client_hello.session_id(),
-                   make_hello_random(rng, policy),
+                   make_hello_random(rng, cb, policy),
                    resumed_session.ciphersuite_code(),
                    uint8_t(0)))
    {
diff --git a/src/lib/tls/tls12/tls_client_impl_12.cpp b/src/lib/tls/tls12/tls_client_impl_12.cpp
index e859b18e1..280601abb 100644
--- a/src/lib/tls/tls12/tls_client_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_client_impl_12.cpp
@@ -711,7 +711,7 @@ void Client_Impl_12::process_handshake_msg(const Handshake_State* active_state,
       const std::vector<uint8_t>& session_ticket = state.session_ticket();
 
       if(session_id.empty() && !session_ticket.empty())
-         session_id = make_hello_random(rng(), policy());
+         session_id = make_hello_random(rng(), callbacks(), policy());
 
       Session session_info(
          session_id,
@@ -724,7 +724,8 @@ void Client_Impl_12::process_handshake_msg(const Handshake_State* active_state,
          get_peer_cert_chain(state),
          session_ticket,
          m_info,
-         state.server_hello()->srtp_profile()
+         state.server_hello()->srtp_profile(),
+         callbacks().tls_current_timestamp()
          );
 
       const bool should_save = save_session(session_info);
diff --git a/src/lib/tls/tls12/tls_record.cpp b/src/lib/tls/tls12/tls_record.cpp
index 3dde0caa4..06ebd31ec 100644
--- a/src/lib/tls/tls12/tls_record.cpp
+++ b/src/lib/tls/tls12/tls_record.cpp
@@ -9,6 +9,7 @@
 
 #include <botan/internal/tls_record.h>
 #include <botan/tls_ciphersuite.h>
+#include <botan/tls_callbacks.h>
 #include <botan/tls_exceptn.h>
 #include <botan/internal/loadstor.h>
 #include <botan/internal/tls_seq_numbers.h>
@@ -28,8 +29,7 @@ Connection_Cipher_State::Connection_Cipher_State(Protocol_Version version,
                                                  bool our_side,
                                                  const Ciphersuite& suite,
                                                  const Session_Keys& keys,
-                                                 bool uses_encrypt_then_mac) :
-   m_start_time(std::chrono::system_clock::now())
+                                                 bool uses_encrypt_then_mac)
    {
    m_nonce_format = suite.nonce_format();
    m_nonce_bytes_from_record = suite.nonce_bytes_from_record(version);
diff --git a/src/lib/tls/tls12/tls_record.h b/src/lib/tls/tls12/tls_record.h
index 08ad14784..4ba672bcd 100644
--- a/src/lib/tls/tls12/tls_record.h
+++ b/src/lib/tls/tls12/tls_record.h
@@ -21,6 +21,7 @@ namespace Botan {
 
 namespace TLS {
 
+class Callbacks;
 class Ciphersuite;
 class Session_Keys;
 
@@ -61,12 +62,6 @@ class Connection_Cipher_State final
 
       Nonce_Format nonce_format() const { return m_nonce_format; }
 
-      std::chrono::seconds age() const
-         {
-         return std::chrono::duration_cast<std::chrono::seconds>(
-            std::chrono::system_clock::now() - m_start_time);
-         }
-
    private:
       std::chrono::system_clock::time_point m_start_time;
       std::unique_ptr<AEAD_Mode> m_aead;
diff --git a/src/lib/tls/tls12/tls_server_impl_12.cpp b/src/lib/tls/tls12/tls_server_impl_12.cpp
index 26bfce45d..06fc7dac1 100644
--- a/src/lib/tls/tls12/tls_server_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_server_impl_12.cpp
@@ -62,6 +62,7 @@ namespace {
 bool check_for_resume(Session& session_info,
                       Session_Manager& session_manager,
                       Credentials_Manager& credentials,
+                      Callbacks& cb,
                       const Client_Hello_12* client_hello,
                       std::chrono::seconds session_ticket_lifetime)
    {
@@ -87,7 +88,7 @@ bool check_for_resume(Session& session_info,
             credentials.psk("tls-server", "session-ticket", ""));
 
          if(session_ticket_lifetime != std::chrono::seconds(0) &&
-            session_info.session_age() > session_ticket_lifetime)
+            cb.tls_current_timestamp() - session_info.start_time() > session_ticket_lifetime)
             return false; // ticket has expired
          }
       catch(...)
@@ -489,6 +490,7 @@ void Server_Impl_12::process_client_hello_msg(const Handshake_State* active_stat
       check_for_resume(session_info,
                        session_manager(),
                        m_creds,
+                       callbacks(),
                        pending_state.client_hello(),
                        std::chrono::seconds(policy().session_ticket_lifetime()));
 
@@ -637,7 +639,8 @@ void Server_Impl_12::process_finished_msg(Server_Handshake_State& pending_state,
          get_peer_cert_chain(pending_state),
          std::vector<uint8_t>(),
          Server_Information(pending_state.client_hello()->sni_hostname()),
-         pending_state.server_hello()->srtp_profile());
+         pending_state.server_hello()->srtp_profile(),
+         callbacks().tls_current_timestamp());
 
       if(save_session(session_info))
          {
@@ -825,7 +828,7 @@ void Server_Impl_12::session_create(Server_Handshake_State& pending_state,
                                                    *pending_state.client_hello());
 
    Server_Hello_12::Settings srv_settings(
-      make_hello_random(rng(), policy()), // new session ID
+      make_hello_random(rng(), callbacks(), policy()), // new session ID
       pending_state.version(),
       ciphersuite,
       have_session_ticket_key);
diff --git a/src/lib/tls/tls_callbacks.cpp b/src/lib/tls/tls_callbacks.cpp
index 94d25e273..6edb0f1ff 100644
--- a/src/lib/tls/tls_callbacks.cpp
+++ b/src/lib/tls/tls_callbacks.cpp
@@ -37,6 +37,11 @@ std::string TLS::Callbacks::tls_peer_network_identity()
    return "";
    }
 
+std::chrono::system_clock::time_point TLS::Callbacks::tls_current_timestamp()
+   {
+   return std::chrono::system_clock::now();
+   }
+
 void TLS::Callbacks::tls_modify_extensions(Extensions& /*unused*/, Connection_Side /*unused*/)
    {
    }
@@ -70,7 +75,7 @@ void TLS::Callbacks::tls_verify_cert_chain(
                          trusted_roots,
                          (usage == Usage_Type::TLS_SERVER_AUTH ? hostname : ""),
                          usage,
-                         std::chrono::system_clock::now(),
+                         tls_current_timestamp(),
                          tls_verify_cert_chain_ocsp_timeout(),
                          ocsp_responses);
 
diff --git a/src/lib/tls/tls_callbacks.h b/src/lib/tls/tls_callbacks.h
index 67da7c5a7..dd52f848e 100644
--- a/src/lib/tls/tls_callbacks.h
+++ b/src/lib/tls/tls_callbacks.h
@@ -15,6 +15,7 @@
 #include <botan/pubkey.h>
 #include <botan/ocsp.h>
 #include <optional>
+#include <chrono>
 
 namespace Botan {
 
@@ -357,6 +358,17 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
        */
        virtual std::string tls_peer_network_identity();
 
+       /**
+       * Optional callback: return a custom time stamp value
+       *
+       * This allows the library user to specify a custom "now" timestamp when
+       * needed. By default it will use the current system clock time.
+       *
+       * Note that typical usages will not need to override this callback but it
+       * is useful for testing purposes to allow for deterministic test outcomes.
+       */
+       virtual std::chrono::system_clock::time_point tls_current_timestamp();
+
        /**
        * Optional callback: error logging. (not currently called)
        * @param err An error message related to this connection.
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index c502961ba..4a69cf428 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -47,6 +47,7 @@ class Callbacks;
 class Cipher_State;
 
 std::vector<uint8_t> make_hello_random(RandomNumberGenerator& rng,
+                                       Callbacks& cb,
                                        const Policy& policy);
 
 /**
diff --git a/src/lib/tls/tls_session.cpp b/src/lib/tls/tls_session.cpp
index 9dd6c7f8f..9f3c8787e 100644
--- a/src/lib/tls/tls_session.cpp
+++ b/src/lib/tls/tls_session.cpp
@@ -27,8 +27,9 @@ Session::Session(const std::vector<uint8_t>& session_identifier,
                  const std::vector<X509_Certificate>& certs,
                  const std::vector<uint8_t>& ticket,
                  const Server_Information& server_info,
-                 uint16_t srtp_profile) :
-   m_start_time(std::chrono::system_clock::now()),
+                 uint16_t srtp_profile,
+                 std::chrono::system_clock::time_point current_timestamp) :
+   m_start_time(current_timestamp),
    m_identifier(session_identifier),
    m_session_ticket(ticket),
    m_master_secret(master_secret),
@@ -188,12 +189,6 @@ Ciphersuite Session::ciphersuite() const
    return suite.value();
    }
 
-std::chrono::seconds Session::session_age() const
-   {
-   return std::chrono::duration_cast<std::chrono::seconds>(
-      std::chrono::system_clock::now() - m_start_time);
-   }
-
 namespace {
 
 // The output length of the HMAC must be a valid keylength for the AEAD
diff --git a/src/lib/tls/tls_session.h b/src/lib/tls/tls_session.h
index 157b3d92d..3f5b2a121 100644
--- a/src/lib/tls/tls_session.h
+++ b/src/lib/tls/tls_session.h
@@ -54,7 +54,8 @@ class BOTAN_PUBLIC_API(2,0) Session final
               const std::vector<X509_Certificate>& peer_certs,
               const std::vector<uint8_t>& session_ticket,
               const Server_Information& server_info,
-              uint16_t srtp_profile);
+              uint16_t srtp_profile,
+              std::chrono::system_clock::time_point current_timestamp);
 
       /**
       * Load a session from DER representation (created by DER_encode)
@@ -161,11 +162,6 @@ class BOTAN_PUBLIC_API(2,0) Session final
       */
       std::chrono::system_clock::time_point start_time() const { return m_start_time; }
 
-      /**
-      * Return how long this session has existed (in seconds)
-      */
-      std::chrono::seconds session_age() const;
-
       /**
       * Return the session ticket the server gave us
       */


Test patch:
diff --git a/src/tests/test_tls.cpp b/src/tests/test_tls.cpp
index 6d0fb04ed..8c92913ad 100644
--- a/src/tests/test_tls.cpp
+++ b/src/tests/test_tls.cpp
@@ -52,7 +52,8 @@ class TLS_Session_Tests final : public Test
                                      std::vector<Botan::X509_Certificate>(),
                                      std::vector<uint8_t>(),
                                      Botan::TLS::Server_Information("server"),
-                                     0x0000);
+                                     0x0000,
+                                     std::chrono::system_clock::now());
 
          const std::string pem = session.PEM_encode();
          Botan::TLS::Session session_from_pem(pem);
@@ -96,7 +97,8 @@ class TLS_Session_Tests final : public Test
                                      std::vector<Botan::X509_Certificate>(),
                                      std::vector<uint8_t>(),
                                      Botan::TLS::Server_Information("server"),
-                                     0x0000);
+                                     0x0000,
+                                     std::chrono::system_clock::now());
          const std::string pem_with_unknown_ciphersuite = session2.PEM_encode();
 
          result.test_throws("unknown ciphersuite during session parsing",