Build and send HPKP violation reports

net/spdy/spdy_session.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.
 
 #include "net/spdy/spdy_session.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/basictypes.h"
@@ skipping 580 matching lines @@
       ChannelIDService::GetDomainForHost(new_hostname) !=
       ChannelIDService::GetDomainForHost(old_hostname)) {
     return false;
   }
 
   bool unused = false;
   if (!ssl_info.cert->VerifyNameMatch(new_hostname, &unused))
     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;
   }
 
   return true;
 }
 
 SpdySession::SpdySession(
     const SpdySessionKey& spdy_session_key,
     const base::WeakPtr<HttpServerProperties>& http_server_properties,
@@ skipping 2643 matching lines @@
     if (!queue->empty()) {
       SpdyStreamId stream_id = queue->front();
       queue->pop_front();
       return stream_id;
     }
   }
   return 0;
 }
 
 }  // namespace net