Commit 09c3d073 authored by davidben's avatar davidben Committed by Commit bot

Align SSLClientSocketOpenSSL and SSLClientSocketNSS histograms.

This makes a handful of small changes to align the two implementations:

- Remove Net.SSLv3FallbackToRenegoPatchedServer. It's no longer necessary. In
  fact, the majority of SSLv3 fallbacks are to servers that /are/ renego patched.

- Gather Net.SSLCertVerificationTime* on OpenSSL.

- Move histogram calls on OpenSSL out of GetSSLInfo as that is not called once
  per handshake. Net.RenegotiationExtensionSupported is moved to common code and
  RecordChannelIDSupport to the handshake success codepath.

Net.OCSPResponseStapled is left ungathered as BoringSSL does not yet support
OCSP stapling.

BUG=405631

Review URL: https://codereview.chromium.org/493793003

Cr-Commit-Position: refs/heads/master@{#291752}
parent 21c97c1f
...@@ -2456,26 +2456,6 @@ void SSLClientSocketNSS::Core::UpdateConnectionStatus() { ...@@ -2456,26 +2456,6 @@ void SSLClientSocketNSS::Core::UpdateConnectionStatus() {
VLOG(1) << "The server " << host_and_port_.ToString() VLOG(1) << "The server " << host_and_port_.ToString()
<< " does not support the TLS renegotiation_info extension."; << " does not support the TLS renegotiation_info extension.";
} }
UMA_HISTOGRAM_ENUMERATION("Net.RenegotiationExtensionSupported",
peer_supports_renego_ext, 2);
// We would like to eliminate fallback to SSLv3 for non-buggy servers
// because of security concerns. For example, Google offers forward
// secrecy with ECDHE but that requires TLS 1.0. An attacker can block
// TLSv1 connections and force us to downgrade to SSLv3 and remove forward
// secrecy.
//
// Yngve from Opera has suggested using the renegotiation extension as an
// indicator that SSLv3 fallback was mistaken:
// tools.ietf.org/html/draft-pettersen-tls-version-rollback-removal-00 .
//
// As a first step, measure how often clients perform version fallback
// while the server advertises support secure renegotiation.
if (ssl_config_.version_fallback &&
channel_info.protocolVersion == SSL_LIBRARY_VERSION_3_0) {
UMA_HISTOGRAM_BOOLEAN("Net.SSLv3FallbackToRenegoPatchedServer",
peer_supports_renego_ext == PR_TRUE);
}
} }
if (ssl_config_.version_fallback) { if (ssl_config_.version_fallback) {
......
...@@ -507,6 +507,8 @@ void SSLClientSocketOpenSSL::Disconnect() { ...@@ -507,6 +507,8 @@ void SSLClientSocketOpenSSL::Disconnect() {
cert_key_types_.clear(); cert_key_types_.clear();
client_auth_cert_needed_ = false; client_auth_cert_needed_ = false;
start_cert_verification_time_ = base::TimeTicks();
npn_status_ = kNextProtoUnsupported; npn_status_ = kNextProtoUnsupported;
npn_proto_.clear(); npn_proto_.clear();
...@@ -598,11 +600,6 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) { ...@@ -598,11 +600,6 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
ssl_info->channel_id_sent = WasChannelIDSent(); ssl_info->channel_id_sent = WasChannelIDSent();
ssl_info->pinning_failure_log = pinning_failure_log_; ssl_info->pinning_failure_log = pinning_failure_log_;
RecordChannelIDSupport(channel_id_service_,
channel_id_xtn_negotiated_,
ssl_config_.channel_id_enabled,
crypto::ECPrivateKey::IsSupported());
const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_); const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl_);
CHECK(cipher); CHECK(cipher);
ssl_info->security_bits = SSL_CIPHER_get_bits(cipher, NULL); ssl_info->security_bits = SSL_CIPHER_get_bits(cipher, NULL);
...@@ -611,11 +608,8 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) { ...@@ -611,11 +608,8 @@ bool SSLClientSocketOpenSSL::GetSSLInfo(SSLInfo* ssl_info) {
SSL_CIPHER_get_id(cipher), 0 /* no compression */, SSL_CIPHER_get_id(cipher), 0 /* no compression */,
GetNetSSLVersion(ssl_)); GetNetSSLVersion(ssl_));
bool peer_supports_renego_ext = !!SSL_get_secure_renegotiation_support(ssl_); if (!SSL_get_secure_renegotiation_support(ssl_))
if (!peer_supports_renego_ext)
ssl_info->connection_status |= SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION; ssl_info->connection_status |= SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION;
UMA_HISTOGRAM_ENUMERATION("Net.RenegotiationExtensionSupported",
implicit_cast<int>(peer_supports_renego_ext), 2);
if (ssl_config_.version_fallback) if (ssl_config_.version_fallback)
ssl_info->connection_status |= SSL_CONNECTION_VERSION_FALLBACK; ssl_info->connection_status |= SSL_CONNECTION_VERSION_FALLBACK;
...@@ -904,6 +898,11 @@ int SSLClientSocketOpenSSL::DoHandshake() { ...@@ -904,6 +898,11 @@ int SSLClientSocketOpenSSL::DoHandshake() {
} }
} }
RecordChannelIDSupport(channel_id_service_,
channel_id_xtn_negotiated_,
ssl_config_.channel_id_enabled,
crypto::ECPrivateKey::IsSupported());
// Verify the certificate. // Verify the certificate.
const bool got_cert = !!UpdateServerCert(); const bool got_cert = !!UpdateServerCert();
DCHECK(got_cert); DCHECK(got_cert);
...@@ -993,6 +992,7 @@ int SSLClientSocketOpenSSL::DoChannelIDLookupComplete(int result) { ...@@ -993,6 +992,7 @@ int SSLClientSocketOpenSSL::DoChannelIDLookupComplete(int result) {
int SSLClientSocketOpenSSL::DoVerifyCert(int result) { int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
DCHECK(server_cert_.get()); DCHECK(server_cert_.get());
DCHECK(start_cert_verification_time_.is_null());
GotoState(STATE_VERIFY_CERT_COMPLETE); GotoState(STATE_VERIFY_CERT_COMPLETE);
CertStatus cert_status; CertStatus cert_status;
...@@ -1004,6 +1004,8 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) { ...@@ -1004,6 +1004,8 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
return OK; return OK;
} }
start_cert_verification_time_ = base::TimeTicks::Now();
int flags = 0; int flags = 0;
if (ssl_config_.rev_checking_enabled) if (ssl_config_.rev_checking_enabled)
flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED; flags |= CertVerifier::VERIFY_REV_CHECKING_ENABLED;
...@@ -1030,6 +1032,16 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) { ...@@ -1030,6 +1032,16 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) {
int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) { int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) {
verifier_.reset(); verifier_.reset();
if (!start_cert_verification_time_.is_null()) {
base::TimeDelta verify_time =
base::TimeTicks::Now() - start_cert_verification_time_;
if (result == OK) {
UMA_HISTOGRAM_TIMES("Net.SSLCertVerificationTime", verify_time);
} else {
UMA_HISTOGRAM_TIMES("Net.SSLCertVerificationTimeError", verify_time);
}
}
bool sni_available = ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1 || bool sni_available = ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1 ||
ssl_config_.version_fallback; ssl_config_.version_fallback;
const CertStatus cert_status = server_cert_verify_result_.cert_status; const CertStatus cert_status = server_cert_verify_result_.cert_status;
......
...@@ -232,6 +232,7 @@ class SSLClientSocketOpenSSL : public SSLClientSocket { ...@@ -232,6 +232,7 @@ class SSLClientSocketOpenSSL : public SSLClientSocket {
CertVerifier* const cert_verifier_; CertVerifier* const cert_verifier_;
scoped_ptr<SingleRequestCertVerifier> verifier_; scoped_ptr<SingleRequestCertVerifier> verifier_;
base::TimeTicks start_cert_verification_time_;
// The service for retrieving Channel ID keys. May be NULL. // The service for retrieving Channel ID keys. May be NULL.
ChannelIDService* channel_id_service_; ChannelIDService* channel_id_service_;
......
...@@ -514,6 +514,11 @@ int SSLConnectJob::DoSSLConnectComplete(int result) { ...@@ -514,6 +514,11 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
SSLConnectionStatusToCipherSuite( SSLConnectionStatusToCipherSuite(
ssl_info.connection_status)); ssl_info.connection_status));
UMA_HISTOGRAM_BOOLEAN(
"Net.RenegotiationExtensionSupported",
(ssl_info.connection_status &
SSL_CONNECTION_NO_RENEGOTIATION_EXTENSION) == 0);
if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_RESUME) { if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_RESUME) {
UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Resume_Handshake", UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Resume_Handshake",
connect_duration, connect_duration,
......
...@@ -17064,6 +17064,9 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -17064,6 +17064,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
<histogram name="Net.SSLv3FallbackToRenegoPatchedServer" <histogram name="Net.SSLv3FallbackToRenegoPatchedServer"
enum="TLSRenegotiationPatched"> enum="TLSRenegotiationPatched">
<obsolete>
Removed on 2014-08-20.
</obsolete>
<owner>Please list the metric's owners. Add more owner tags as needed.</owner> <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<summary> <summary>
The number of times that we have performed SSLv3 fallback and found a TLS The number of times that we have performed SSLv3 fallback and found a TLS
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment