OpenSSL: don't allow the server certificate to change during renegotiation.

net/socket/ssl_client_socket_openssl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <openssl/err.h>
@@ skipping 165 matching lines @@
     case SSL_R_SSLV3_ALERT_NO_CERTIFICATE:
     case SSL_R_SSLV3_ALERT_ILLEGAL_PARAMETER:
     case SSL_R_TLSV1_ALERT_DECODE_ERROR:
     case SSL_R_TLSV1_ALERT_DECRYPTION_FAILED:
     case SSL_R_TLSV1_ALERT_EXPORT_RESTRICTION:
     case SSL_R_TLSV1_ALERT_INTERNAL_ERROR:
     case SSL_R_TLSV1_ALERT_NO_RENEGOTIATION:
     case SSL_R_TLSV1_ALERT_RECORD_OVERFLOW:
     case SSL_R_TLSV1_ALERT_USER_CANCELLED:
       return ERR_SSL_PROTOCOL_ERROR;
+    case SSL_R_CERTIFICATE_VERIFY_FAILED:
+      // The only way that the certificate verify callback can fail is if
+      // the leaf certificate changed during a renegotiation.
+      return ERR_SSL_SERVER_CERT_CHANGED;
     default:
       LOG(WARNING) << "Unmapped error reason: " << ERR_GET_REASON(error_code);
       return ERR_FAILED;
   }
 }
 
 // Converts an OpenSSL error code into a net error code, walking the OpenSSL
 // error stack if needed. Note that |tracer| is not currently used in the
 // implementation, but is passed in anyway as this ensures the caller will clear
 // any residual codes left on the error stack.
 int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
   switch (err) {
     case SSL_ERROR_WANT_READ:
     case SSL_ERROR_WANT_WRITE:
       return ERR_IO_PENDING;
     case SSL_ERROR_SYSCALL:
       LOG(ERROR) << "OpenSSL SYSCALL error, earliest error code in "
                     "error queue: " << ERR_peek_error() << ", errno: "
                  << errno;
       return ERR_SSL_PROTOCOL_ERROR;
     case SSL_ERROR_SSL:
       return MapOpenSSLErrorSSL();
     default:
       // TODO(joth): Implement full mapping.
       LOG(WARNING) << "Unknown OpenSSL error " << err;
       return ERR_SSL_PROTOCOL_ERROR;
   }
 }
 
-// We do certificate verification after handshake, so we disable the default
-// by registering a no-op verify function.
-int NoOpVerifyCallback(X509_STORE_CTX*, void *) {
-  DVLOG(3) << "skipping cert verify";
-  return 1;
-}
-
 // Utility to construct the appropriate set & clear masks for use the OpenSSL
 // options and mode configuration functions. (SSL_set_options etc)
 struct SslSetClearMask {
   SslSetClearMask() : set_mask(0), clear_mask(0) {}
   void ConfigureFlag(long flag, bool state) {
     (state ? set_mask : clear_mask) |= flag;
     // Make sure we haven't got any intersection in the set & clear options.
     DCHECK_EQ(0, set_mask & clear_mask) << flag << ":" << state;
   }
   long set_mask;
@@ skipping 31 matching lines @@
 
  private:
   friend struct DefaultSingletonTraits<SSLContext>;
 
   SSLContext() {
     crypto::EnsureOpenSSLInit();
     ssl_socket_data_index_ = SSL_get_ex_new_index(0, 0, 0, 0, 0);
     DCHECK_NE(ssl_socket_data_index_, -1);
     ssl_ctx_.reset(SSL_CTX_new(SSLv23_client_method()));
     session_cache_.Reset(ssl_ctx_.get(), kDefaultSessionCacheConfig);
-    SSL_CTX_set_cert_verify_callback(ssl_ctx_.get(), NoOpVerifyCallback, NULL);
+    SSL_CTX_set_cert_verify_callback(ssl_ctx_.get(), CertVerifyCallback, NULL);
     SSL_CTX_set_client_cert_cb(ssl_ctx_.get(), ClientCertCallback);
     SSL_CTX_set_channel_id_cb(ssl_ctx_.get(), ChannelIDCallback);
+    SSL_CTX_set_verify(ssl_ctx_.get(), SSL_VERIFY_PEER, NULL);
 #if defined(OPENSSL_NPN_NEGOTIATED)
     // TODO(kristianm): Only select this if ssl_config_.next_proto is not empty.
     // It would be better if the callback were not a global setting,
     // but that is an OpenSSL issue.
     SSL_CTX_set_next_proto_select_cb(ssl_ctx_.get(), SelectNextProtoCallback,
                                      NULL);
 #endif
   }
 
   static std::string GetSessionCacheKey(const SSL* ssl) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
     return GetSocketSessionCacheKey(*socket);
   }
 
   static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
 
   static int ClientCertCallback(SSL* ssl, X509** x509, EVP_PKEY** pkey) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     CHECK(socket);
     return socket->ClientCertRequestCallback(ssl, x509, pkey);
   }
 
   static void ChannelIDCallback(SSL* ssl, EVP_PKEY** pkey) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     CHECK(socket);
     socket->ChannelIDRequestCallback(ssl, pkey);
   }
 
+  static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) {
+    SSL* ssl = reinterpret_cast<SSL*>(X509_STORE_CTX_get_ex_data(
+        store_ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
+    SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
+    CHECK(socket);
+
+    return socket->CertVerifyCallback(store_ctx);
+  }
+
   static int SelectNextProtoCallback(SSL* ssl,
                                      unsigned char** out, unsigned char* outlen,
                                      const unsigned char* in,
                                      unsigned int inlen, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     return socket->SelectNextProtoCallback(out, outlen, in, inlen);
   }
 
   // This is the index used with SSL_get_ex_data to retrieve the owner
   // SSLClientSocketOpenSSL object from an SSL instance.
@@ skipping 1050 matching lines @@
       crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
           ServerBoundCertService::kEPKIPassword,
           encrypted_private_key_info,
           subject_public_key_info));
   if (!ec_private_key)
     return;
   set_channel_id_sent(true);
   *pkey = EVP_PKEY_dup(ec_private_key->key());
 }
 
+int SSLClientSocketOpenSSL::CertVerifyCallback(X509_STORE_CTX* store_ctx) {
+  if (!completed_handshake_) {
+    // If the first handshake hasn't completed then we accept any certificates
+    // because we verify after the handshake.
+    return 1;
+  }
+
+  std::string der_current_cert;
+  if (!X509Certificate::GetDEREncoded(server_cert_->os_cert_handle(),
+                                      &der_current_cert)) {
+    LOG(ERROR) << "Failed to get current certificate in DER form";
+    return 0;
+  }
+
+  X509* leaf_cert = sk_X509_value(store_ctx->chain, 0);
+  int len = i2d_X509(leaf_cert, NULL);
+  if (len < 0) {
+    LOG(ERROR) << "Failed to marshal certificate from renegotiation";
+    return 0;
+  }
+
+  scoped_ptr<uint8[]> der_leaf_cert(new uint8[len]);
+  uint8 *outp = der_leaf_cert.get();
+  len = i2d_X509(leaf_cert, &outp);
+
+  if (static_cast<size_t>(len) == der_current_cert.size() &&
+      memcmp(der_leaf_cert.get(),
+             der_current_cert.data(),
+             der_current_cert.size()) == 0) {
+    // The certificates match so the renegotiation can continue.
+    return 1;
+  }

[Ryan Sleevi, 2014/02/27 22:43:40]

Why do this? Why not simply use

if (!completed_handshake_)
  return 1;

if (X509Certificate::IsSameOSCert(server_cert_->os_cert_handle(),
sk_X509_value(store_ctx->chain, 0)))
  return 1;

return 0;

[agl, 2014/02/27 22:57:10]

Yep, that works. Thanks!

+
+  LOG(ERROR) << "Server certificate changed between handshakes";
+  return 0;
+}
+
 // SelectNextProtoCallback is called by OpenSSL during the handshake. If the
 // server supports NPN, selects a protocol from the list that the server
 // provides. According to third_party/openssl/openssl/ssl/ssl_lib.c, the
 // callback can assume that |in| is syntactically valid.
 int SSLClientSocketOpenSSL::SelectNextProtoCallback(unsigned char** out,
                                                     unsigned char* outlen,
                                                     const unsigned char* in,
                                                     unsigned int inlen) {
 #if defined(OPENSSL_NPN_NEGOTIATED)
   if (ssl_config_.next_protos.empty()) {
@@ skipping 33 matching lines @@
   }
 
   npn_proto_.assign(reinterpret_cast<const char*>(*out), *outlen);
   server_protos_.assign(reinterpret_cast<const char*>(in), inlen);
   DVLOG(2) << "next protocol: '" << npn_proto_ << "' status: " << npn_status_;
 #endif
   return SSL_TLSEXT_ERR_OK;
 }
 
 }  // namespace net