Clean up comments and code for pin validation.

It should be crystal clear when, and why, pin validation is and is not
performed.

TEST=net_unittests; with an OFFICIAL_BUILD: can still connect to pinned
sites, and https://pinningtest.appspot.com fails with
net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN.


Review URL: https://chromiumcodereview.appspot.com/13466020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192654 0039d316-1c4b-4281-b951-d872f2087c98

net/socket/ssl_client_socket_nss.cc

@@ -3432,15 +3432,26 @@ int SSLClientSocketNSS::DoVerifyCertComplete(int result) {
   // Pinning is only enabled for official builds to make sure that others don't
   // end up with pins that cannot be easily updated.
   //
-  // TODO(agl): we might have an issue here where a request for foo.example.com
+  // TODO(agl): We might have an issue here where a request for foo.example.com
   // merges into a SPDY connection to www.example.com, and gets a different
   // certificate.
 
+  // Perform pin validation if, and only if, all these conditions obtain:
+  //
+  // * a TransportSecurityState object is available;
+  // * the server's certificate chain is valid (or suffers from only a minor
+  //   error);
+  // * the server's certificate chain chains up to a known root (i.e. not a
+  //   user-installed trust anchor); and
+  // * the build is recent (very old builds should fail open so that users
+  //   have some chance to recover).
+  //
   const CertStatus cert_status = server_cert_verify_result_.cert_status;
-  if ((result == OK || (IsCertificateError(result) &&
-                        IsCertStatusMinorError(cert_status))) &&
+  if (transport_security_state_ &&
+      (result == OK ||
+       (IsCertificateError(result) && IsCertStatusMinorError(cert_status))) &&
       server_cert_verify_result_.is_issued_by_known_root &&
-      transport_security_state_) {
+      TransportSecurityState::IsBuildTimely()) {
     bool sni_available =
         ssl_config_.version_max >= SSL_PROTOCOL_VERSION_TLS1 ||
         ssl_config_.version_fallback;
@@ -3451,13 +3462,10 @@ int SSLClientSocketNSS::DoVerifyCertComplete(int result) {
                                                   &domain_state) &&
         domain_state.HasPublicKeyPins()) {
       if (!domain_state.CheckPublicKeyPins(
-               server_cert_verify_result_.public_key_hashes)) {
-        // Pins are not enforced if the build is too old.
-        if (TransportSecurityState::IsBuildTimely()) {
-          result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
-          UMA_HISTOGRAM_BOOLEAN("Net.PublicKeyPinSuccess", false);
-          TransportSecurityState::ReportUMAOnPinFailure(host);
-        }
+              server_cert_verify_result_.public_key_hashes)) {
+        result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
+        UMA_HISTOGRAM_BOOLEAN("Net.PublicKeyPinSuccess", false);
+        TransportSecurityState::ReportUMAOnPinFailure(host);
       } else {
         UMA_HISTOGRAM_BOOLEAN("Net.PublicKeyPinSuccess", true);
       }