Make OpenSSL UpdateServerCert() OS independent.

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index b253dfeae50884859e2dceb3657363d2a6834b29..cceacebb67f7cf46d0a4a13be90e309c6973fb73 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -319,6 +319,88 @@ class SSLClientSocketOpenSSL::SSLContext {
   SSLSessionCacheOpenSSL session_cache_;
 };
 
+// PeerCertificateChain is a helper object which extracts the certificate
+// chain, as given by the server, from an OpenSSL socket and performs the needed
+// resource management. The first element of the chain is the leaf certificate
+// and the other elements are in the order given by the server.
+class SSLClientSocketOpenSSL::PeerCertificateChain {
+ public:
+  virtual ~PeerCertificateChain() {};
+
+  // Resets the current chain, freeing any resources, and updates the current
+  // chain to be a referenced copy of the chain stored in |ssl|.
+  // If |ssl| is NULL, then the current certificate chain will be freed.
+  void Reset(SSL* ssl) {
+    x509_certs_.clear();
+    der_certs_chain_.clear();
+    der_certs_mem_handler_.clear();
+
+    if (ssl == NULL)
+      return;
+
+    STACK_OF(X509)* intermediates = SSL_get_peer_cert_chain(ssl);
+    if (intermediates) {
+      for (int i = 0; i < sk_X509_num(intermediates); ++i) {
+        if (!AddCert(sk_X509_value(intermediates, i))) {
+          Reset(NULL);
+          return;
+        }
+      }
+    }
+  }
+
+  // Returns the current certificate chain as a vector of DER-encoded
+  // base::StringPieces. The returned vector remains valid until Reset is
+  // called.
+  std::vector<base::StringPiece> AsStringPieceVector() const {
+    return der_certs_chain_;
+  }
+
+  bool empty() const { return x509_certs_.empty(); }
+  size_t size() const { return x509_certs_.size(); }
+
+  X509* operator[](size_t index) const {
+    DCHECK_LT(index, x509_certs_.size());
+    return x509_certs_[index].get();
+  }
+
+ private:
+  struct OpenSSLFreeDeleter {
+    inline void operator()(void* ptr) const { OPENSSL_free(ptr); }
+  };
+
+  struct X509Deleter {
+    inline void operator()(X509* ptr) const { X509_free(ptr); }
+  };
+
+  typedef scoped_ptr<unsigned char, OpenSSLFreeDeleter> ScopedOpenSSLArray;
+  typedef scoped_ptr<X509, X509Deleter> ScopedX509;
+
+  bool AddCert(X509* cert) {
+    unsigned char* cert_data = NULL;
+    int cert_data_length = i2d_X509(cert, &cert_data);
+    if (cert_data_length <= 0 || !cert_data)
+      return false;
+
+    der_certs_mem_handler_.push_back(ScopedOpenSSLArray(cert_data));
+    der_certs_chain_.push_back(base::StringPiece(
+        reinterpret_cast<char*>(cert_data), cert_data_length));
+
+    x509_certs_.push_back(ScopedX509(cert));
+    // Increase reference count for cert
+    CRYPTO_add(&cert->references, 1, CRYPTO_LOCK_X509);
+
+    return true;
+  }
+
+  std::vector<base::StringPiece> der_certs_chain_;
+
+  // Make sure the DER data strings are free'd with OPENSSL_free
+  std::vector<PeerCertificateChain::ScopedOpenSSLArray> der_certs_mem_handler_;
+
+  std::vector<ScopedX509> x509_certs_;

[Ryan Sleevi, 2014/02/25 20:27:09]

Storing a vector of ScopedX509 doesn't work before C++11.

Rather than having to worry about this bookkeeping - especially with
AsStringPieceVector - why not simplify?

void sk_X509_free_helper(STACK_OF(X509)* stack) {
  sk_X509_pop_free(stack, X509_free);
}

class SSLClientSocketOpenSSL::PeerCertificateChain {
 public:
  explicit PeerCertificateChain(SSL* ssl);
  ~PeerCertificateChain();

  void Reset(SSL* ssl);
  const scoped_ptr<X509Certificate>& AsNativeChain() const {
    return native_chain_;
  }

  size_t size() const {
    if (!openssl_chain_.get())
      return 0;
    return sk_X509_num(openssl_chain_.get());
  }

  X509* operator[](size_t index) const {
    DCHECK_LT(index, size());
    return sk_X509_value(openssl_chain_.get(), index);
  }

 private:
  crypto::ScopedOpenSSL<STACK_OF(X509), sk_X509_free_helper> openssl_chain_;
  scoped_refptr<X509Certificate> native_chain_;
};

PeerCertificateChain::PeerCertificateChain(SSL* ssl) {
  Reset(ssl);
}

PeerCertificateChain::~PeerCertificateChain() {}

void PeerCertificateChain::Reset(SSL* ssl) {
  openssl_chain_.reset();
  native_chain_ = NULL;

  if (ssl == NULL)
    return;

  STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
  if (!chain)
    return;

  openssl_chain_.reset(sk_X509_dup(chain));

  std::vector<base::StringPiece> der_chain;
  for (int i = 0; i < sk_X509_num(openssl_chain_.get()); ++i) {
    X509* x = sk_X509_value(openssl_chain_.get(), i);
    CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509);

    unsigned char* cert_data = NULL;
    int cert_data_length = i2d_X509(x, &cert_data);
    if (cert_data_length && cert_data) {
      der_chain.push_back(base::StringPiece(reinterpret_cast<char*>(cert_data),
cert_data_length);
    }
  }

  native_chain_ = X509Certificate::CreateFromDERCertChain(der_chain);

  for (size_t i = 0; i < der_chain.size(); ++i) {
    OPENSSL_free(der_chain[i].data());
  }

  if (native_chain_.size() !=
static_cast<size_t>(sk_X509_num(openssl_chain_.get()))) {
    openssl_chain_.reset(NULL);
    native_chain_ = NULL;
  }
}


This also allows for support for the operator= / Equals support to be added in.

[haavardm, 2014/02/25 20:53:52]

Seems good. I guess I got a little too focused on getting the scoped_ptr in
there :)


On 2014/02/25 20:27:09, Ryan Sleevi wrote:

[haavardm, 2014/03/11 18:43:20]

Done.

+};
+
 // static
 SSLSessionCacheOpenSSL::Config
     SSLClientSocketOpenSSL::SSLContext::kDefaultSessionCacheConfig = {
@@ -347,6 +429,8 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       weak_factory_(this),
       pending_read_error_(kNoPendingReadResult),
       transport_write_error_(OK),
+      server_cert_chain_(new PeerCertificateChain()),
+      server_cert_(NULL),
       completed_handshake_(false),
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
@@ -917,25 +1001,22 @@ void SSLClientSocketOpenSSL::DoConnectCallback(int rv) {
 }
 
 X509Certificate* SSLClientSocketOpenSSL::UpdateServerCert() {
-  if (server_cert_.get())
-    return server_cert_.get();
-
-  crypto::ScopedOpenSSL<X509, X509_free> cert(SSL_get_peer_certificate(ssl_));

[haavardm, 2014/02/25 19:37:37]

I removed this, as it seems that server certificate is the first certificate in
the stack returned by by SSL_get_peer_cert_chain().

https://www.openssl.org/docs/ssl/SSL_get_peer_cert_chain.html:
"If called on the client side, the stack also contains the peer's
certificate.

Some basic testing confirmed that SSL_get_peer_certificate(ssl_) ==
sk_X509_value(SSL_get_peer_cert_chain(ssl_), 0))

-  if (!cert.get()) {
-    LOG(WARNING) << "SSL_get_peer_certificate returned NULL";
-    return NULL;
-  }
-
+#ifdef USE_OPENSSL
   // Unlike SSL_get_peer_certificate, SSL_get_peer_cert_chain does not
   // increment the reference so sk_X509_free does not need to be called.
   STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl_);
   X509Certificate::OSCertHandles intermediates;
   if (chain) {
-    for (int i = 0; i < sk_X509_num(chain); ++i)
+    for (int i = 1; i < sk_X509_num(chain); ++i)
       intermediates.push_back(sk_X509_value(chain, i));
   }
-  server_cert_ = X509Certificate::CreateFromHandle(cert.get(), intermediates);
-  DCHECK(server_cert_.get());
+  server_cert_ =
+      X509Certificate::CreateFromHandle(sk_X509_value(chain, 0), intermediates);

[Ryan Sleevi, 2014/02/25 20:27:09]

if |chain| is false, or sk_X509_num(chain) is 0, this will crash (eg: compare
line 1009)

[haavardm, 2014/03/11 18:43:20]

Done.

+#else
+  server_cert_chain_->Reset(ssl_);
+  server_cert_ = X509Certificate::CreateFromDERCertChain(
+      server_cert_chain_->AsStringPieceVector());
+#endif
 
   return server_cert_.get();
 }