Skip to content

Commit

Permalink
net: assorted changes needed for Snap Start tests.
Browse files Browse the repository at this point in the history
In IsAllowedBadCert, the pointer compare doesn't always work. Creating a
certificate by X509Certificate::CreateFromBytes and putting it in the
allowed bad list doesn't match the same certificate from a server.

InvalidateSessionIfBadCertificate: I'm sure that there's history here
that I don't know, so please review carefully. In order to test
resumption against a server with a test certificate we need to avoid
destroying the session.

ClearSessionCache, AdoptSocket: only for testing

BUG=none
TEST=none

http://codereview.chromium.org/4558004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65713 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
agl@chromium.org committed Nov 10, 2010
1 parent bb339ae commit 2d6e778
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 19 deletions.
2 changes: 1 addition & 1 deletion net/base/ssl_config_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ SSLConfig::~SSLConfig() {

bool SSLConfig::IsAllowedBadCert(X509Certificate* cert) const {
for (size_t i = 0; i < allowed_bad_certs.size(); ++i) {
if (cert == allowed_bad_certs[i].cert)
if (cert->Equals(allowed_bad_certs[i].cert))
return true;
}
return false;
Expand Down
4 changes: 4 additions & 0 deletions net/base/x509_certificate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ bool X509Certificate::HasExpired() const {
return base::Time::Now() > valid_expiry();
}

bool X509Certificate::Equals(const X509Certificate* other) const {
return IsSameOSCert(cert_handle_, other->cert_handle_);
}

bool X509Certificate::HasIntermediateCertificate(OSCertHandle cert) {
#if defined(OS_MACOSX) || defined(OS_WIN) || defined(USE_OPENSSL)
for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i) {
Expand Down
3 changes: 3 additions & 0 deletions net/base/x509_certificate.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
// now.
bool HasExpired() const;

// Returns true if this object and |other| represent the same certificate.
bool Equals(const X509Certificate* other) const;

// Returns intermediate certificates added via AddIntermediateCertificate().
// Ownership follows the "get" rule: it is the caller's responsibility to
// retain the elements of the result.
Expand Down
16 changes: 5 additions & 11 deletions net/socket/ssl_client_socket_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -883,19 +883,11 @@ int SSLClientSocketNSS::InitializeSSLPeerName() {
return OK;
}

void SSLClientSocketNSS::InvalidateSessionIfBadCertificate() {
if (UpdateServerCert() != NULL &&
ssl_config_.IsAllowedBadCert(server_cert_)) {
SSL_InvalidateSession(nss_fd_);
}
}

void SSLClientSocketNSS::Disconnect() {
EnterFunction("");

// TODO(wtc): Send SSL close_notify alert.
if (nss_fd_ != NULL) {
InvalidateSessionIfBadCertificate();
PR_Close(nss_fd_);
nss_fd_ = NULL;
}
Expand Down Expand Up @@ -1090,6 +1082,11 @@ bool SSLClientSocketNSS::SetSendBufferSize(int32 size) {
return transport_->socket()->SetSendBufferSize(size);
}

// static
void SSLClientSocketNSS::ClearSessionCache() {
SSL_ClearSessionCache();
}

// Sets server_cert_ and server_cert_nss_ if not yet set.
// Returns server_cert_.
X509Certificate *SSLClientSocketNSS::UpdateServerCert() {
Expand Down Expand Up @@ -2456,9 +2453,6 @@ int SSLClientSocketNSS::DoVerifyCertComplete(int result) {
LogConnectionTypeMetrics();

completed_handshake_ = true;
// TODO(ukai): we may not need this call because it is now harmless to have a
// session with a bad cert.
InvalidateSessionIfBadCertificate();

// If we merged a Write call into the handshake we need to make the
// callback now.
Expand Down
4 changes: 3 additions & 1 deletion net/socket/ssl_client_socket_nss.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,16 @@ class SSLClientSocketNSS : public SSLClientSocket {
virtual bool SetReceiveBufferSize(int32 size);
virtual bool SetSendBufferSize(int32 size);

// For tests
static void ClearSessionCache();

private:
// Initializes NSS SSL options. Returns a net error code.
int InitializeSSLOptions();

// Initializes the socket peer name in SSL. Returns a net error code.
int InitializeSSLPeerName();

void InvalidateSessionIfBadCertificate();
#if defined(OS_MACOSX) || defined(OS_WIN)
// Creates an OS certificate from a DER-encoded certificate.
static X509Certificate::OSCertHandle CreateOSCert(const SECItem& der_cert);
Expand Down
9 changes: 5 additions & 4 deletions net/socket/ssl_host_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ SSLHostInfo::State::~State() {}
SSLHostInfo::SSLHostInfo(
const std::string& hostname,
const SSLConfig& ssl_config)
: hostname_(hostname),
cert_verification_complete_(false),
: cert_verification_complete_(false),
cert_verification_error_(ERR_CERT_INVALID),
hostname_(hostname),
cert_parsing_failed_(false),
cert_verification_callback_(NULL),
rev_checking_enabled_(ssl_config.rev_checking_enabled),
Expand Down Expand Up @@ -149,7 +150,7 @@ const CertVerifyResult& SSLHostInfo::cert_verify_result() const {

int SSLHostInfo::WaitForCertVerification(CompletionCallback* callback) {
if (cert_verification_complete_)
return cert_verification_result_;
return cert_verification_error_;
DCHECK(!cert_parsing_failed_);
DCHECK(!cert_verification_callback_);
DCHECK(!state_.certs.empty());
Expand All @@ -164,7 +165,7 @@ void SSLHostInfo::VerifyCallback(int rv) {
UMA_HISTOGRAM_TIMES("Net.SSLHostInfoVerificationTimeMs", duration);
VLOG(1) << "Verification took " << duration.InMilliseconds() << "ms";
cert_verification_complete_ = true;
cert_verification_result_ = rv;
cert_verification_error_ = rv;
if (cert_verification_callback_) {
CompletionCallback* callback = cert_verification_callback_;
cert_verification_callback_ = NULL;
Expand Down
4 changes: 2 additions & 2 deletions net/socket/ssl_host_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,16 @@ class SSLHostInfo {
bool Parse(const std::string& data);
std::string Serialize() const;
State state_;
bool cert_verification_complete_;
int cert_verification_error_;

private:
// This is the callback function which the CertVerifier calls via |callback_|.
void VerifyCallback(int rv);

// This is the hostname that we'll validate the certificates against.
const std::string hostname_;
bool cert_verification_complete_;
bool cert_parsing_failed_;
int cert_verification_result_;
CompletionCallback* cert_verification_callback_;
// These two members are taken from the SSLConfig.
bool rev_checking_enabled_;
Expand Down
15 changes: 15 additions & 0 deletions net/socket/tcp_client_socket_libevent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@ TCPClientSocketLibevent::~TCPClientSocketLibevent() {
net_log_.EndEvent(NetLog::TYPE_SOCKET_ALIVE, NULL);
}

void TCPClientSocketLibevent::AdoptSocket(int socket) {
DCHECK_EQ(socket_, kInvalidSocket);
socket_ = socket;
int error = SetupSocket();
DCHECK_EQ(0, error);
// This is to make GetPeerAddress work. It's up to the test that is calling
// this function to ensure that address_ contains a reasonable address for
// this socket. (i.e. at least match IPv4 vs IPv6!).
current_ai_ = addresses_.head();
use_history_.set_was_ever_connected();
}

int TCPClientSocketLibevent::Connect(CompletionCallback* callback) {
DCHECK(CalledOnValidThread());

Expand Down Expand Up @@ -467,7 +479,10 @@ int TCPClientSocketLibevent::CreateSocket(const addrinfo* ai) {
socket_ = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
if (socket_ == kInvalidSocket)
return errno;
return SetupSocket();
}

int TCPClientSocketLibevent::SetupSocket() {
if (SetNonBlocking(socket_)) {
const int err = errno;
close(socket_);
Expand Down
9 changes: 9 additions & 0 deletions net/socket/tcp_client_socket_libevent.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ class TCPClientSocketLibevent : public ClientSocket, NonThreadSafe {

virtual ~TCPClientSocketLibevent();

// AdoptSocket causes the given, connected socket to be adopted as a TCP
// socket. This object must not be connected. This object takes ownership of
// the given socket and then acts as if Connect() had been called. This
// function is intended for testing only.
void AdoptSocket(int socket);

// ClientSocket methods:
virtual int Connect(CompletionCallback* callback);
virtual void Disconnect();
Expand Down Expand Up @@ -125,6 +131,9 @@ class TCPClientSocketLibevent : public ClientSocket, NonThreadSafe {
// Returns the OS error code (or 0 on success).
int CreateSocket(const struct addrinfo* ai);

// Returns the OS error code (or 0 on success).
int SetupSocket();

// Helper to add a TCP_CONNECT (end) event to the NetLog.
void LogConnectCompletion(int net_error);

Expand Down
12 changes: 12 additions & 0 deletions net/socket/tcp_client_socket_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,15 @@ TCPClientSocketWin::~TCPClientSocketWin() {
net_log_.EndEvent(NetLog::TYPE_SOCKET_ALIVE, NULL);
}

void TCPClientSocketWin::AdoptSocket(SOCKET socket) {
DCHECK_EQ(socket_, INVALID_SOCKET);
socket_ = socket;
int error = SetupSocket();
DCHECK_EQ(0, error);
current_ai_ = addresses_.head();
use_history_.set_was_ever_connected();
}

int TCPClientSocketWin::Connect(CompletionCallback* callback) {
DCHECK(CalledOnValidThread());

Expand Down Expand Up @@ -674,7 +683,10 @@ int TCPClientSocketWin::CreateSocket(const struct addrinfo* ai) {
LOG(ERROR) << "WSASocket failed: " << os_error;
return os_error;
}
return SetupSocket();
}

int TCPClientSocketWin::SetupSocket() {
// Increase the socket buffer sizes from the default sizes for WinXP. In
// performance testing, there is substantial benefit by increasing from 8KB
// to 64KB.
Expand Down
9 changes: 9 additions & 0 deletions net/socket/tcp_client_socket_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ class TCPClientSocketWin : public ClientSocket, NonThreadSafe {

virtual ~TCPClientSocketWin();

// AdoptSocket causes the given, connected socket to be adopted as a TCP
// socket. This object must not be connected. This object takes ownership of
// the given socket and then acts as if Connect() had been called. This
// function is intended for testing only.
void AdoptSocket(SOCKET socket);

// ClientSocket methods:
virtual int Connect(CompletionCallback* callback);
virtual void Disconnect();
Expand Down Expand Up @@ -78,6 +84,9 @@ class TCPClientSocketWin : public ClientSocket, NonThreadSafe {
// Returns the OS error code (or 0 on success).
int CreateSocket(const struct addrinfo* ai);

// Returns the OS error code (or 0 on success).
int SetupSocket();

// Called after Connect() has completed with |net_error|.
void LogConnectCompletion(int net_error);

Expand Down

0 comments on commit 2d6e778

Please sign in to comment.