Remove Channel::m_peer_certs, instead retrieve directly from the state.

This also very happily avoids a race in renegotiation. If you first
negotiated using cert X, then renegotiated with Y, during the period
between the certificate message and the finished message,
Channel::peer_cert_chain would return Y instead of X. Now, it returns
Y only after the finished message has been verified.

6 files changed, 45 insertions, 12 deletions

diff --git a/src/tls/tls_channel.cpp b/src/tls/tls_channel.cpp
index 255d7813a..6d90ed47f 100644
--- a/src/tls/tls_channel.cpp
+++ b/src/tls/tls_channel.cpp
@@ -38,6 +38,13 @@ Channel::~Channel()
    // So unique_ptr destructors run correctly
    }
 
+std::vector<X509_Certificate> Channel::peer_cert_chain() const
+   {
+   if(!m_active_state)
+      return std::vector<X509_Certificate>();
+   return get_peer_cert_chain(*m_active_state);
+   }
+
 Handshake_State& Channel::create_handshake_state()
    {
    if(m_pending_state)
diff --git a/src/tls/tls_channel.h b/src/tls/tls_channel.h
index 1d0629ff9..1ea6b06eb 100644
--- a/src/tls/tls_channel.h
+++ b/src/tls/tls_channel.h
@@ -83,7 +83,7 @@ class BOTAN_DLL Channel
       /**
       * @return certificate chain of the peer (may be empty)
       */
-      std::vector<X509_Certificate> peer_cert_chain() const { return m_peer_certs; }
+      std::vector<X509_Certificate> peer_cert_chain() const;
 
       Channel(std::function<void (const byte[], size_t)> socket_output_fn,
               std::function<void (const byte[], size_t, Alert)> proc_fn,
@@ -106,6 +106,9 @@ class BOTAN_DLL Channel
       virtual void initiate_handshake(Handshake_State& state,
                                       bool force_full_renegotiation) = 0;
 
+      virtual std::vector<X509_Certificate>
+         get_peer_cert_chain(const Handshake_State& state) const = 0;
+
       virtual Handshake_State* new_handshake_state() = 0;
 
       Handshake_State& create_handshake_state();
@@ -148,8 +151,6 @@ class BOTAN_DLL Channel
       RandomNumberGenerator& m_rng;
       Session_Manager& m_session_manager;
 
-      std::vector<X509_Certificate> m_peer_certs;
-
    private:
       void send_record(byte type, const byte input[], size_t length);
 
diff --git a/src/tls/tls_client.cpp b/src/tls/tls_client.cpp
index 62aceda2e..a8c1e90c1 100644
--- a/src/tls/tls_client.cpp
+++ b/src/tls/tls_client.cpp
@@ -83,6 +83,14 @@ Handshake_State* Client::new_handshake_state()
       );
    }
 
+std::vector<X509_Certificate>
+Client::get_peer_cert_chain(const Handshake_State& state) const
+   {
+   if(state.server_certs())
+      return state.server_certs()->cert_chain();
+   return std::vector<X509_Certificate>();
+   }
+
 /*
 * Send a new client hello to renegotiate
 */
@@ -319,21 +327,23 @@ void Client::process_handshake_msg(const Handshake_State* /*active_state*/,
 
       state.server_certs(new Certificate(contents));
 
-      m_peer_certs = state.server_certs()->cert_chain();
-      if(m_peer_certs.empty())
+      const std::vector<X509_Certificate>& server_certs =
+         state.server_certs()->cert_chain();
+
+      if(server_certs.empty())
          throw TLS_Exception(Alert::HANDSHAKE_FAILURE,
                              "Client: No certificates sent by server");
 
       try
          {
-         m_creds.verify_certificate_chain("tls-client", m_hostname, m_peer_certs);
+         m_creds.verify_certificate_chain("tls-client", m_hostname, server_certs);
          }
       catch(std::exception& e)
          {
          throw TLS_Exception(Alert::BAD_CERTIFICATE, e.what());
          }
 
-      std::unique_ptr<Public_Key> peer_key(m_peer_certs[0].subject_public_key());
+      std::unique_ptr<Public_Key> peer_key(server_certs[0].subject_public_key());
 
       if(peer_key->algo_name() != state.ciphersuite().sig_algo())
          throw TLS_Exception(Alert::ILLEGAL_PARAMETER,
@@ -507,7 +517,7 @@ void Client::process_handshake_msg(const Handshake_State* /*active_state*/,
          CLIENT,
          secure_renegotiation_supported(),
          state.server_hello()->fragment_size(),
-         m_peer_certs,
+         get_peer_cert_chain(state),
          session_ticket,
          m_hostname,
          ""
diff --git a/src/tls/tls_client.h b/src/tls/tls_client.h
index 0baaa6a60..3edcfa495 100644
--- a/src/tls/tls_client.h
+++ b/src/tls/tls_client.h
@@ -66,6 +66,9 @@ class BOTAN_DLL Client : public Channel
              std::function<std::string (std::vector<std::string>)> next_protocol =
                 std::function<std::string (std::vector<std::string>)>());
    private:
+      std::vector<X509_Certificate>
+         get_peer_cert_chain(const Handshake_State& state) const override;
+
       void initiate_handshake(Handshake_State& state,
                               bool force_full_renegotiation) override;
 
diff --git a/src/tls/tls_server.cpp b/src/tls/tls_server.cpp
index d6d9bedc0..67e5a8eda 100644
--- a/src/tls/tls_server.cpp
+++ b/src/tls/tls_server.cpp
@@ -236,6 +236,14 @@ Handshake_State* Server::new_handshake_state()
    return state;
    }
 
+std::vector<X509_Certificate>
+Server::get_peer_cert_chain(const Handshake_State& state) const
+   {
+   if(state.client_certs())
+      return state.client_certs()->cert_chain();
+   return std::vector<X509_Certificate>();
+   }
+
 /*
 * Send a hello request to the client
 */
@@ -589,10 +597,11 @@ void Server::process_handshake_msg(const Handshake_State* active_state,
       {
       state.client_verify(new Certificate_Verify(contents, state.version()));
 
-      m_peer_certs = state.client_certs()->cert_chain();
+      const std::vector<X509_Certificate>& client_certs =
+         state.client_certs()->cert_chain();
 
       const bool sig_valid =
-         state.client_verify()->verify(m_peer_certs[0], state);
+         state.client_verify()->verify(client_certs[0], state);
 
       state.hash().update(state.handshake_io().format(contents, type));
 
@@ -606,7 +615,7 @@ void Server::process_handshake_msg(const Handshake_State* active_state,
 
       try
          {
-         m_creds.verify_certificate_chain("tls-server", "", m_peer_certs);
+         m_creds.verify_certificate_chain("tls-server", "", client_certs);
          }
       catch(std::exception& e)
          {
@@ -658,7 +667,7 @@ void Server::process_handshake_msg(const Handshake_State* active_state,
             SERVER,
             secure_renegotiation_supported(),
             state.server_hello()->fragment_size(),
-            m_peer_certs,
+            get_peer_cert_chain(state),
             std::vector<byte>(),
             m_hostname,
             state.srp_identifier()
diff --git a/src/tls/tls_server.h b/src/tls/tls_server.h
index c7c18eeb7..94127e0d0 100644
--- a/src/tls/tls_server.h
+++ b/src/tls/tls_server.h
@@ -48,6 +48,9 @@ class BOTAN_DLL Server : public Channel
          { return m_next_protocol; }
 
    private:
+      std::vector<X509_Certificate>
+         get_peer_cert_chain(const Handshake_State& state) const override;
+
       void initiate_handshake(Handshake_State& state,
                               bool force_full_renegotiation) override;