Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(527)

Unified Diff: net/socket/ssl_client_socket_openssl.cc

Issue 173853014: Make OpenSSL UpdateServerCert() OS independent. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: More cleanup and fixes Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..37e52a51f6185d1933364a9eb960947778980d89 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -319,6 +319,129 @@ 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:
+ explicit PeerCertificateChain(SSL* ssl) { Reset(ssl); }
+ PeerCertificateChain(const PeerCertificateChain& other) { *this = other; }
+ ~PeerCertificateChain() {}
+
+ PeerCertificateChain& operator=(const PeerCertificateChain& other) {
+ if (this == &other)
+ return *this;
+
+ // os_chain_ is reference counted by scoped_refptr;
+ os_chain_ = other.os_chain_;
+
+ // Must increase the reference count manually for sk_X509_dup
+ openssl_chain_.reset(sk_X509_dup(other.openssl_chain_.get()));
+ 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);
+ }
+ return *this;
+ }
+
+#if defined(USE_OPENSSL)
Ryan Sleevi 2014/03/12 23:12:03 STYLE: Rather than putting this all inline - espec
haavardm 2014/03/13 10:32:02 Done.
+ // When OSCertHandle is typedef'ed to X509, we can do a short cut
+ // to avoid converting back and forth between der and X509 struct.
+ void Reset(SSL* ssl) {
+ openssl_chain_.reset(NULL);
+ os_chain_ = NULL;
+
+ if (ssl == NULL)
+ return;
+
+ STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
+ if (!chain)
+ return;
+
+ X509Certificate::OSCertHandles intermediates;
+ for (int i = 1; i < sk_X509_num(chain); ++i)
+ intermediates.push_back(sk_X509_value(chain, i));
+
+ os_chain_ = X509Certificate::CreateFromHandle(sk_X509_value(chain, 0),
+ intermediates);
+
+ // sk_X509_dup does not increase reference count on the certs in the stack.
+ 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);
+ // We increase reference count for the certs in openssl_chain_.
+ CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509);
+ }
+ }
+#else // !defined(USE_OPENSSL)
+ void Reset(SSL* ssl) {
+ openssl_chain_.reset(NULL);
+ os_chain_ = NULL;
+
+ if (ssl == NULL)
+ return;
+
+ STACK_OF(X509)* chain = SSL_get_peer_cert_chain(ssl);
+ if (!chain)
+ return;
+
+ // sk_X509_dup does not increase reference count on the certs in the stack.
+ 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);
+
+ // We increase reference count for the certs in openssl_chain_.
+ 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));
+ }
+
+ os_chain_ = X509Certificate::CreateFromDERCertChain(der_chain);
+
+ for (size_t i = 0; i < der_chain.size(); ++i) {
+ OPENSSL_free(const_cast<char*>(der_chain[i].data()));
+ }
+
+ if (der_chain.size() !=
+ static_cast<size_t>(sk_X509_num(openssl_chain_.get()))) {
+ openssl_chain_.reset(NULL);
+ os_chain_ = NULL;
+ }
+ }
+#endif // USE_OPENSSL
+
+ // Note that when USE_OPENSSL defined OSCertHandle is X509*
+ const scoped_refptr<X509Certificate>& AsOSChain() const { return os_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:
+ static void FreeX509Stack(STACK_OF(X509) * chain) {
+ sk_X509_pop_free(chain, X509_free);
+ }
+ crypto::ScopedOpenSSL<STACK_OF(X509), FreeX509Stack> openssl_chain_;
+
+ scoped_refptr<X509Certificate> os_chain_;
+};
+
// static
SSLSessionCacheOpenSSL::Config
SSLClientSocketOpenSSL::SSLContext::kDefaultSessionCacheConfig = {
@@ -347,6 +470,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
weak_factory_(this),
pending_read_error_(kNoPendingReadResult),
transport_write_error_(OK),
+ server_cert_chain_(new PeerCertificateChain(NULL)),
completed_handshake_(false),
client_auth_cert_needed_(false),
cert_verifier_(context.cert_verifier),
@@ -362,8 +486,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
npn_status_(kNextProtoUnsupported),
channel_id_request_return_value_(ERR_UNEXPECTED),
channel_id_xtn_negotiated_(false),
- net_log_(transport_->socket()->NetLog()) {
-}
+ net_log_(transport_->socket()->NetLog()) {}
SSLClientSocketOpenSSL::~SSLClientSocketOpenSSL() {
Disconnect();
@@ -917,26 +1040,8 @@ 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_));
- if (!cert.get()) {
- LOG(WARNING) << "SSL_get_peer_certificate returned NULL";
- return NULL;
- }
-
- // 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)
- intermediates.push_back(sk_X509_value(chain, i));
- }
- server_cert_ = X509Certificate::CreateFromHandle(cert.get(), intermediates);
- DCHECK(server_cert_.get());
-
+ server_cert_chain_->Reset(ssl_);
+ server_cert_ = server_cert_chain_->AsOSChain();
return server_cert_.get();
}
@@ -1424,4 +1529,9 @@ int SSLClientSocketOpenSSL::SelectNextProtoCallback(unsigned char** out,
return SSL_TLSEXT_ERR_OK;
}
+const X509Certificate*
+SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
+ return server_cert_;
+}
+
} // namespace net

Powered by Google App Engine
This is Rietveld 408576698