diff options
| -rw-r--r-- | net/base/ssl_client_auth_cache.cc | 15 | ||||
| -rw-r--r-- | net/base/ssl_client_auth_cache.h | 18 | ||||
| -rw-r--r-- | net/base/ssl_client_auth_cache_unittest.cc | 89 | ||||
| -rw-r--r-- | net/http/http_network_transaction.cc | 53 |
4 files changed, 131 insertions, 44 deletions
diff --git a/net/base/ssl_client_auth_cache.cc b/net/base/ssl_client_auth_cache.cc index d2f47cc..355073f 100644 --- a/net/base/ssl_client_auth_cache.cc +++ b/net/base/ssl_client_auth_cache.cc @@ -4,15 +4,26 @@ #include "net/base/ssl_client_auth_cache.h" +#include "base/logging.h" +#include "net/base/x509_certificate.h" + namespace net { SSLClientAuthCache::SSLClientAuthCache() {} SSLClientAuthCache::~SSLClientAuthCache() {} -X509Certificate* SSLClientAuthCache::Lookup(const std::string& server) { +bool SSLClientAuthCache::Lookup( + const std::string& server, + scoped_refptr<X509Certificate>* certificate) { + DCHECK(certificate); + AuthCacheMap::iterator iter = cache_.find(server); - return (iter == cache_.end()) ? NULL : iter->second; + if (iter == cache_.end()) + return false; + + *certificate = iter->second; + return true; } void SSLClientAuthCache::Add(const std::string& server, diff --git a/net/base/ssl_client_auth_cache.h b/net/base/ssl_client_auth_cache.h index 023480b..2b276a2 100644 --- a/net/base/ssl_client_auth_cache.h +++ b/net/base/ssl_client_auth_cache.h @@ -10,10 +10,11 @@ #include <map> #include "base/ref_counted.h" -#include "net/base/x509_certificate.h" namespace net { +class X509Certificate; + // The SSLClientAuthCache class is a simple cache structure to store SSL // client certificates. Provides lookup, insertion, and deletion of entries. // The parameter for doing lookups, insertions, and deletions is the server's @@ -26,13 +27,18 @@ class SSLClientAuthCache { SSLClientAuthCache(); ~SSLClientAuthCache(); - // Check if we have a client certificate for SSL server at |server|. - // Returns the client certificate (if found) or NULL (if not found). - X509Certificate* Lookup(const std::string& server); + // Checks for a client certificate preference for SSL server at |server|. + // Returns true if a preference is found, and sets |*certificate| to the + // desired client certificate. The desired certificate may be NULL, which + // indicates a preference to not send any certificate to |server|. + // If a certificate preference is not found, returns false. + bool Lookup(const std::string& server, + scoped_refptr<X509Certificate>* certificate); // Add a client certificate for |server| to the cache. If there is already - // a client certificate for |server|, it will be overwritten. Both parameters - // are IN only. + // a client certificate for |server|, it will be overwritten. A NULL + // |client_cert| indicates a preference that no client certificate should + // be sent to |server|. void Add(const std::string& server, X509Certificate* client_cert); // Remove the client certificate for |server| from the cache, if one exists. diff --git a/net/base/ssl_client_auth_cache_unittest.cc b/net/base/ssl_client_auth_cache_unittest.cc index 85b3d5e..f528d58 100644 --- a/net/base/ssl_client_auth_cache_unittest.cc +++ b/net/base/ssl_client_auth_cache_unittest.cc @@ -5,6 +5,7 @@ #include "net/base/ssl_client_auth_cache.h" #include "base/time.h" +#include "net/base/x509_certificate.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { @@ -27,32 +28,50 @@ TEST(SSLClientAuthCacheTest, LookupAddRemove) { scoped_refptr<X509Certificate> cert3( new X509Certificate("foo3", "CA", start_date, expiration_date)); + scoped_refptr<X509Certificate> cached_cert; // Lookup non-existent client certificate. - EXPECT_TRUE(cache.Lookup(server1) == NULL); + cached_cert = NULL; + EXPECT_FALSE(cache.Lookup(server1, &cached_cert)); // Add client certificate for server1. - cache.Add(server1, cert1.get()); - EXPECT_EQ(cert1.get(), cache.Lookup(server1)); + cache.Add(server1, cert1); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(cert1, cached_cert); // Add client certificate for server2. - cache.Add(server2, cert2.get()); - EXPECT_EQ(cert1.get(), cache.Lookup(server1)); - EXPECT_EQ(cert2.get(), cache.Lookup(server2)); + cache.Add(server2, cert2); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(cert1, cached_cert.get()); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server2, &cached_cert)); + EXPECT_EQ(cert2, cached_cert); // Overwrite the client certificate for server1. - cache.Add(server1, cert3.get()); - EXPECT_EQ(cert3.get(), cache.Lookup(server1)); - EXPECT_EQ(cert2.get(), cache.Lookup(server2)); + cache.Add(server1, cert3); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(cert3, cached_cert); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server2, &cached_cert)); + EXPECT_EQ(cert2, cached_cert); // Remove client certificate of server1. cache.Remove(server1); - EXPECT_TRUE(cache.Lookup(server1) == NULL); - EXPECT_EQ(cert2.get(), cache.Lookup(server2)); + cached_cert = NULL; + EXPECT_FALSE(cache.Lookup(server1, &cached_cert)); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server2, &cached_cert)); + EXPECT_EQ(cert2, cached_cert); // Remove non-existent client certificate. cache.Remove(server1); - EXPECT_TRUE(cache.Lookup(server1) == NULL); - EXPECT_EQ(cert2.get(), cache.Lookup(server2)); + cached_cert = NULL; + EXPECT_FALSE(cache.Lookup(server1, &cached_cert)); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server2, &cached_cert)); + EXPECT_EQ(cert2, cached_cert); } // Check that if the server differs only by port number, it is considered @@ -74,8 +93,48 @@ TEST(SSLClientAuthCacheTest, LookupWithPort) { cache.Add(server1, cert1.get()); cache.Add(server2, cert2.get()); - EXPECT_EQ(cert1.get(), cache.Lookup(server1)); - EXPECT_EQ(cert2.get(), cache.Lookup(server2)); + scoped_refptr<X509Certificate> cached_cert; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(cert1.get(), cached_cert); + EXPECT_TRUE(cache.Lookup(server2, &cached_cert)); + EXPECT_EQ(cert2.get(), cached_cert); +} + +// Check that the a NULL certificate, indicating the user has declined to send +// a certificate, is properly cached. +TEST(SSLClientAuthCacheTest, LookupNullPreference) { + SSLClientAuthCache cache; + base::Time start_date = base::Time::Now(); + base::Time expiration_date = start_date + base::TimeDelta::FromDays(1); + + std::string server1("foo:443"); + scoped_refptr<X509Certificate> cert1( + new X509Certificate("foo", "CA", start_date, expiration_date)); + + cache.Add(server1, NULL); + + scoped_refptr<X509Certificate> cached_cert(cert1); + // Make sure that |cached_cert| is updated to NULL, indicating the user + // declined to send a certificate to |server1|. + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(NULL, cached_cert.get()); + + // Remove the existing cached certificate. + cache.Remove(server1); + cached_cert = NULL; + EXPECT_FALSE(cache.Lookup(server1, &cached_cert)); + + // Add a new preference for a specific certificate. + cache.Add(server1, cert1); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(cert1, cached_cert); + + // Replace the specific preference with a NULL certificate. + cache.Add(server1, NULL); + cached_cert = NULL; + EXPECT_TRUE(cache.Lookup(server1, &cached_cert)); + EXPECT_EQ(NULL, cached_cert.get()); } } // namespace net diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 3e1d8d1..c84dfa4 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -96,7 +96,6 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session) session->ssl_config_service()->GetSSLConfig(&ssl_config_); if (session->http_stream_factory()->next_protos()) ssl_config_.next_protos = *session->http_stream_factory()->next_protos(); - } HttpNetworkTransaction::~HttpNetworkTransaction() { @@ -171,10 +170,8 @@ int HttpNetworkTransaction::RestartWithCertificate( DCHECK_EQ(STATE_NONE, next_state_); ssl_config_.client_cert = client_cert; - if (client_cert) { - session_->ssl_client_auth_cache()->Add( - response_.cert_request_info->host_and_port, client_cert); - } + session_->ssl_client_auth_cache()->Add( + response_.cert_request_info->host_and_port, client_cert); ssl_config_.send_client_cert = true; // Reset the other member variables. // Note: this is necessary only with SSL renegotiation. @@ -971,29 +968,43 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) { // handshake. stream_request_.reset(); - // If the user selected one of the certificate in client_certs for this - // server before, use it automatically. - X509Certificate* client_cert = session_->ssl_client_auth_cache()->Lookup( - response_.cert_request_info->host_and_port); + // If the user selected one of the certificates in client_certs or declined + // to provide one for this server before, use the past decision + // automatically. + scoped_refptr<X509Certificate> client_cert; + bool found_cached_cert = session_->ssl_client_auth_cache()->Lookup( + response_.cert_request_info->host_and_port, &client_cert); + if (!found_cached_cert) + return error; + + // Check that the certificate selected is still a certificate the server + // is likely to accept, based on the criteria supplied in the + // CertificateRequest message. if (client_cert) { const std::vector<scoped_refptr<X509Certificate> >& client_certs = response_.cert_request_info->client_certs; + bool cert_still_valid = false; for (size_t i = 0; i < client_certs.size(); ++i) { - if (client_cert->fingerprint().Equals(client_certs[i]->fingerprint())) { - // TODO(davidben): Add a unit test which covers this path; we need to be - // able to send a legitimate certificate and also bypass/clear the - // SSL session cache. - ssl_config_.client_cert = client_cert; - ssl_config_.send_client_cert = true; - next_state_ = STATE_CREATE_STREAM; - // Reset the other member variables. - // Note: this is necessary only with SSL renegotiation. - ResetStateForRestart(); - return OK; + if (client_cert->Equals(client_certs[i])) { + cert_still_valid = true; + break; } } + + if (!cert_still_valid) + return error; } - return error; + + // TODO(davidben): Add a unit test which covers this path; we need to be + // able to send a legitimate certificate and also bypass/clear the + // SSL session cache. + ssl_config_.client_cert = client_cert; + ssl_config_.send_client_cert = true; + next_state_ = STATE_CREATE_STREAM; + // Reset the other member variables. + // Note: this is necessary only with SSL renegotiation. + ResetStateForRestart(); + return OK; } // This method determines whether it is safe to resend the request after an |