Skip to content

Commit

Permalink
Metric & meta-metric for CECPQ1 handshake latency.
Browse files Browse the repository at this point in the history
To fairly measure the latency impact of CECPQ1, we need to measure only
hosts that will negotiate it.  Define a small group of Google hosts that
we believe ought to be negotiating CECPQ1, and a new histogram that
measures latency when connecting to these hosts.

In addition, add a meta-metric that (in the experiment group only)
allows us to verify that CECPQ1 is being negotiated in cases where we
expect it to be.  This will serve as a check on the quality of the first
metric.

BUG=626363

Review-Url: https://codereview.chromium.org/2192053002
Cr-Commit-Position: refs/heads/master@{#411518}
  • Loading branch information
mab authored and Commit bot committed Aug 12, 2016
1 parent 5f12b67 commit e0d8c58
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 10 deletions.
18 changes: 18 additions & 0 deletions net/socket/ssl_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

#include "net/socket/ssl_client_socket.h"

#include "base/feature_list.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/sparse_histogram.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "crypto/ec_private_key.h"
#include "net/base/net_errors.h"
#include "net/socket/ssl_client_socket_impl.h"
Expand All @@ -15,6 +17,13 @@

namespace net {

namespace {
#if !defined(OS_NACL)
const base::Feature kPostQuantumExperiment{"SSLPostQuantumExperiment",
base::FEATURE_DISABLED_BY_DEFAULT};
#endif
} // namespace

SSLClientSocket::SSLClientSocket()
: signed_cert_timestamps_received_(false),
stapled_ocsp_response_received_(false) {}
Expand Down Expand Up @@ -79,6 +88,15 @@ bool SSLClientSocket::IgnoreCertError(int error, int load_flags) {
IsCertificateError(error);
}

// static
bool SSLClientSocket::IsPostQuantumExperimentEnabled() {
#if !defined(OS_NACL)
return base::FeatureList::IsEnabled(kPostQuantumExperiment);
#else
return false;
#endif
}

// static
std::vector<uint8_t> SSLClientSocket::SerializeNextProtos(
const NextProtoVector& next_protos) {
Expand Down
5 changes: 5 additions & 0 deletions net/socket/ssl_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
// establishing the connection (or NULL if no channel ID was used).
virtual crypto::ECPrivateKey* GetChannelIDKey() const = 0;

// Returns true if the CECPQ1 (experimental post-quantum) experiment is
// enabled. This should be removed after the experiment is ended, around
// 2017-18.
static bool IsPostQuantumExperimentEnabled();

protected:
void set_signed_cert_timestamps_received(
bool signed_cert_timestamps_received) {
Expand Down
10 changes: 1 addition & 9 deletions net/socket/ssl_client_socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/feature_list.h"
#include "base/lazy_instance.h"
#include "base/macros.h"
#include "base/memory/singleton.h"
Expand Down Expand Up @@ -86,11 +85,6 @@ const uint8_t kTbProtocolVersionMinor = 8;
const uint8_t kTbMinProtocolVersionMajor = 0;
const uint8_t kTbMinProtocolVersionMinor = 6;

#if !defined(OS_NACL)
const base::Feature kPostQuantumExperiment{"SSLPostQuantumExperiment",
base::FEATURE_DISABLED_BY_DEFAULT};
#endif

bool EVP_MDToPrivateKeyHash(const EVP_MD* md, SSLPrivateKey::Hash* hash) {
switch (EVP_MD_type(md)) {
case NID_md5_sha1:
Expand Down Expand Up @@ -1008,8 +1002,7 @@ int SSLClientSocketImpl::Init() {
// supported. As DHE is being deprecated, don't add a cipher only to remove it
// immediately.
std::string command;
#if !defined(OS_NACL)
if (base::FeatureList::IsEnabled(kPostQuantumExperiment)) {
if (SSLClientSocket::IsPostQuantumExperimentEnabled()) {
// These are experimental, non-standard ciphersuites. They are part of an
// experiment in post-quantum cryptography. They're not intended to
// represent a de-facto standard, and will be removed from BoringSSL in
Expand All @@ -1028,7 +1021,6 @@ int SSLClientSocketImpl::Init() {
"CECPQ1-ECDSA-AES256-GCM-SHA384:");
}
}
#endif
command.append("ALL:!SHA256:!SHA384:!DHE-RSA-AES256-GCM-SHA384:!aPSK:!RC4");

if (ssl_config_.require_ecdhe)
Expand Down
23 changes: 22 additions & 1 deletion net/socket/ssl_client_socket_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {

const char *str, *cipher_str, *mac_str;
bool is_aead;
bool is_cecpq1 = false;
SSLCipherSuiteToStrings(&str, &cipher_str, &mac_str, &is_aead,
cipher_suite);
// UMA_HISTOGRAM_... macros cache the Histogram instance and thus only work
Expand All @@ -384,7 +385,7 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_KeyExchange.ECDHE",
ssl_info.key_exchange_info);
} else if (strncmp(str, "CECPQ1_", 7) == 0) {
// Nothing.
is_cecpq1 = true;
} else {
DCHECK_EQ(0, strcmp(str, "RSA"));
}
Expand Down Expand Up @@ -427,6 +428,26 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
base::TimeDelta::FromMilliseconds(1),
base::TimeDelta::FromMinutes(1),
100);

// These are hosts that we expect to always offer CECPQ1. Connections
// to them, whether or not this browser is in the experiment group, form
// the basis of our comparisons.
bool cecpq1_supported =
(host == "play.google.com" || host == "checkout.google.com" ||
host == "wallet.google.com");
if (cecpq1_supported) {
UMA_HISTOGRAM_CUSTOM_TIMES(
"Net.SSL_Connection_Latency_PostQuantumSupported_Full_Handshake",
connect_duration, base::TimeDelta::FromMilliseconds(1),
base::TimeDelta::FromMinutes(1), 100);
if (SSLClientSocket::IsPostQuantumExperimentEnabled()) {
// But don't trust that these hosts offer CECPQ1: make sure. If
// we're doing everything right on the server side, |is_cecpq1|
// should always be true if we get here, modulo MITM.
UMA_HISTOGRAM_BOOLEAN("Net.SSL_Connection_PostQuantum_Negotiated",
is_cecpq1);
}
}
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31738,6 +31738,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram
name="Net.SSL_Connection_Latency_PostQuantumSupported_Full_Handshake"
units="ms">
<owner>mab@chromium.org</owner>
<summary>
Time from when the Connect() starts until it completes (full handshakes
only), for a set of domains that we expect to always offer the experimental
post-quantum (CECPQ1) ciphersuites.
</summary>
</histogram>

<histogram name="Net.SSL_Connection_Latency_Resume_Handshake" units="ms">
<owner>agl@chromium.org</owner>
<summary>
Expand All @@ -31746,6 +31757,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="Net.SSL_Connection_PostQuantum_Negotiated"
enum="BooleanSupported">
<owner>mab@chromium.org</owner>
<summary>
For only browsers in the post-quantum (CECPQ1) ciphersuite experiment,
counts the full TLS handshakes where CECPQ1 was, or was not, negotiated on
hosts where we expect it to be negotiated.
</summary>
</histogram>

<histogram name="Net.SSL_EVCertificateCTCompliance"
enum="CTRequirementCompliance">
<obsolete>
Expand Down

0 comments on commit e0d8c58

Please sign in to comment.