Return enum from TransportSecurityState::CheckPublicKeyPins

net/quic/crypto/proof_verifier_chromium.cc

Index: net/quic/crypto/proof_verifier_chromium.cc
diff --git a/net/quic/crypto/proof_verifier_chromium.cc b/net/quic/crypto/proof_verifier_chromium.cc
index 48b408be1578dff615de38c2693a92ced16be4dd..49e341c96ee7932647d8072cfb143c58df28fcf3 100644
--- a/net/quic/crypto/proof_verifier_chromium.cc
+++ b/net/quic/crypto/proof_verifier_chromium.cc
@@ -336,18 +336,26 @@ int ProofVerifierChromium::Job::DoVerifyCertComplete(int result) {
 
   if (transport_security_state_ &&
       (result == OK ||
-       (IsCertificateError(result) && IsCertStatusMinorError(cert_status))) &&
-      !transport_security_state_->CheckPublicKeyPins(
-          HostPortPair(hostname_, port_),
-          cert_verify_result.is_issued_by_known_root,
-          cert_verify_result.public_key_hashes, cert_.get(),
-          cert_verify_result.verified_cert.get(),
-          TransportSecurityState::ENABLE_PIN_REPORTS,
-          &verify_details_->pinning_failure_log)) {
-    if (cert_verify_result.is_issued_by_known_root)
-      result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
-    else
-      verify_details_->pkp_bypassed = true;
+       (IsCertificateError(result) && IsCertStatusMinorError(cert_status)))) {
+    TransportSecurityState::PKPStatus pin_validity =
+        transport_security_state_->CheckPublicKeyPins(
+            HostPortPair(hostname_, port_),
+            cert_verify_result.is_issued_by_known_root,
+            cert_verify_result.public_key_hashes, cert_.get(),
+            cert_verify_result.verified_cert.get(),
+            TransportSecurityState::ENABLE_PIN_REPORTS,
+            &verify_details_->pinning_failure_log);
+    switch (pin_validity) {
+      case TransportSecurityState::PKPStatus::VIOLATED:
+        result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
+        break;
+      case TransportSecurityState::PKPStatus::BYPASSED:
+        verify_details_->pkp_bypassed = true;
+        break;

[Ryan Sleevi, 2016/06/15 02:04:03]

Should this be an explicit/intentional

// Fall through

rather than break;

So that it's clear we're treating bypassed as equal to OK

[dadrian, 2016/06/15 02:47:55]

I'm wary of using any non-empty fall through, just in a case a future enum value
is added. Instead, we could add a comment indicating we're treating bypass as
equal to OK.

+      case TransportSecurityState::PKPStatus::OK:
+        // Do nothing.
+        break;
+    }
   }
 
   if (result != OK) {