Build and send HPKP violation reports

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 1eaa715b28a09b671cfcac2668e14cfe4c96ee90..2ea761bd899b78018e5143f1467054698bc6db21 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -598,10 +598,13 @@ bool SpdySession::CanPool(TransportSecurityState* transport_security_state,
     return false;
 
   std::string pinning_failure_log;
+  // TODO(estark): replace 0 below with the port of the connection
+  // (though it won't actually be used since reports aren't getting
+  // sent).
   if (!transport_security_state->CheckPublicKeyPins(
-          new_hostname,
-          ssl_info.is_issued_by_known_root,
-          ssl_info.public_key_hashes,
+          HostPortPair(new_hostname, 0), ssl_info.is_issued_by_known_root,
+          ssl_info.public_key_hashes, ssl_info.unverified_cert.get(),
+          ssl_info.cert.get(), TransportSecurityState::DISABLE_PIN_REPORTS,

[davidben, 2015/07/24 20:42:55]

Should this send reports? I believe if we get this far, this is a certificate
that's otherwise valid for the name, which suggests yes.

I guess one problem is that CanPool will get queried every single time we
locally compare the session, so long as the other valid session is alive, and so
we might go crazy?

Anyway, if we don't want to send reports, worth a brief comment as to why not.

[estark, 2015/07/25 00:10:31]

Yeah... just because the certificate is valid in every other way, I don't think
it means we should treat it as a violation. Ryan had an example (which I might
mangle in the retelling) about inbox.google.com having a cert that's valid for
inbox.g.com and mail.g.com, but mail.g.com having its own cert and pins. It
seems weird to send a report for mail.g.com just because the browser happened to
have an inbox.google.com connection lying around (especially because there's
nothing a site operator can or should do with such a report -- it's not a
misconfiguration, not an attacker, nothing to fix... it's just something that
can happen in completely normal operation).
Done.

[davidben, 2015/07/25 01:48:58]

Hrm. Okay, so it's specifically that the two certificates are owned by the same
entity but that one entity decided to pin on something else. That... seems sort
of a contrived situation, but I suppose it's possible. I could imagine it being
more likely if you have wildcards. It also seems a slightly weird deployment
strategy. There's no point in inbox.google.com and mail.google.com having
different sets of keys in this case; if inbox.google.com's key is compromised,
mail.google.com is still bust, pinning aside.

If the two certificates weren't owned by the same entity though, it's definitely
still a pinning violation. If the certificate is valid for mail.google.com and
evil.com, it can be used to speak for mail.google.com.

Actually, doesn't this mean that, if I have a bad certificate for example.com,
which sets a pin, I just need to ensure that bad certificate is also good for a
site I own and then, whenever I mount an attack, I make sure I have a SPDY
session to my domain open first? In fact, that's really easy. I make my server
shut off the connection if SNI = mail.google.com and only send the cert on SNI =
evil.com.

Then, though I'm still beaten by pinning (for users who have already observe the
pin), I at least won't get reported.

           &pinning_failure_log)) {
     return false;
   }