Add a histogram to measure the frequency of OCSP stapling support by servers

net/socket/ssl_client_socket_nss.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file includes code SSLClientSocketNSS::DoVerifyCertComplete() derived
 // from AuthCertificateCallback() in
 // mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp.
 
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ skipping 2377 matching lines @@
     return;
 
   nss_handshake_state_.sct_list_from_tls_extension = std::string(
       reinterpret_cast<char*>(signed_cert_timestamps->data),
       signed_cert_timestamps->len);
 }
 
 void SSLClientSocketNSS::Core::UpdateStapledOCSPResponse() {
   const SECItemArray* ocsp_responses =
       SSL_PeerStapledOCSPResponses(nss_fd_);
-  if (!ocsp_responses || !ocsp_responses->len)
+  bool ocsp_requested =
+      IsOCSPStaplingSupported() || ssl_config_.signed_cert_timestamps_enabled;
+  bool ocsp_responses_present = ocsp_responses && ocsp_responses->len;
+  UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled",

[jar (doing other things), 2014/02/19 22:29:16]

This looks fine... but considering how little space you're using for a
boolean... you *could* break out the 4 cases into an UMA_HISTOGRAM_ENUMERATION,
rather than aggregating them via the && on the next line.  YMMV... your choice.

+      ocsp_requested && ocsp_responses_present);

[wtc, 2014/02/19 21:09:19]

It should be unnecessary to test ocsp_requested. NSS doesn't allow a server to
send us an unsolicited extension, so if OCSP responses are present, we must have
requested OCSP stapling.

[wtc, 2014/02/19 22:00:30]

1. I guess what you have in mind is this:

  if (ocsp_requested)
    UMA_HISTOGRAM_BOOLEAN("Net.OCSPResponseStapled", ocsp_responses_present);

That is, the histogram is only updated when we requested OCSP stapling.

2. We can call SSL_OptionGet(nss_fd_, SSL_ENABLE_OCSP_STAPLING) to get
the value of ocsp_requested. We will need to declare ocsp_requested as
a PRBool.

[Ryan Sleevi, 2014/02/19 23:12:18]

Thanks for pointing this out. I've instead chosen to only record this histogram
when we've actually *requested* OCSP stapling. With CT, this is effectively
becoming "always", but at least it keeps the histogram clean.

+  if (!ocsp_responses_present)
     return;
 
   nss_handshake_state_.stapled_ocsp_response = std::string(
       reinterpret_cast<char*>(ocsp_responses->items[0].data),
       ocsp_responses->items[0].len);
 
   // TODO(agl): figure out how to plumb an OCSP response into the Mac
   // system library and update IsOCSPStaplingSupported for Mac.
   if (IsOCSPStaplingSupported()) {
   #if defined(OS_WIN)
@@ skipping 781 matching lines @@
   rv = SSL_OptionSet(nss_fd_, SSL_CBC_RANDOM_IV, PR_TRUE);
   if (rv != SECSuccess)
     LogFailedNSSFunction(net_log_, "SSL_OptionSet", "SSL_CBC_RANDOM_IV");
 
 // Added in NSS 3.15
 #ifdef SSL_ENABLE_OCSP_STAPLING
   // Request OCSP stapling even on platforms that don't support it, in
   // order to extract Certificate Transparency information.
   rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_OCSP_STAPLING,
                      (IsOCSPStaplingSupported() ||
                       ssl_config_.signed_cert_timestamps_enabled));

[wtc, 2014/02/19 21:09:19]

Perhaps we should save this boolean in a data member, for later use in
Core::UpdateStapledOCSPResponse. Unfortunately there are two classes involved,
so it may not be that straightforward.

[wtc, 2014/02/19 22:00:30]

Please ignore this comment. I found a better solution.

   if (rv != SECSuccess) {
     LogFailedNSSFunction(net_log_, "SSL_OptionSet",
                          "SSL_ENABLE_OCSP_STAPLING");
   }
 #endif
 
   rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_SIGNED_CERT_TIMESTAMPS,
                      ssl_config_.signed_cert_timestamps_enabled);
   if (rv != SECSuccess) {
     LogFailedNSSFunction(net_log_, "SSL_OptionSet",
@@ skipping 371 matching lines @@
         SignedCertificateTimestampAndStatus(*iter,
                                             ct::SCT_STATUS_LOG_UNKNOWN));
   }
 }
 
 ServerBoundCertService* SSLClientSocketNSS::GetServerBoundCertService() const {
   return server_bound_cert_service_;
 }
 
 }  // namespace net