net: allow CRLSets to block specific SPKIs.

net/base/crl_set.h

Index: net/base/crl_set.h
diff --git a/net/base/crl_set.h b/net/base/crl_set.h
index a41cf0c392a0cabb6e0018c1a59fbc5cab9885dc..624e6711228aa114ff33fdeebb26d2df5cc1b931 100644
--- a/net/base/crl_set.h
+++ b/net/base/crl_set.h
@@ -17,6 +17,10 @@
 #include "base/time.h"
 #include "net/base/net_export.h"
 
+namespace base {
+class DictionaryValue;
+}
+
 namespace net {
 
 // A CRLSet is a structure that lists the serial numbers of revoked
@@ -39,10 +43,12 @@ class NET_EXPORT CRLSet : public base::RefCountedThreadSafe<CRLSet> {
 
   // CheckCertificate returns the information contained in the set for a given
   // certificate:
+  //   spki_hash: the SHA256 of the SubjectPublicKeyInfo

[wtc, 2012/01/10 00:50:59]

Nit: add "of the certificate".

[agl, 2012/01/10 16:15:29]

Done.

   //   serial_number: the serial number of the certificate
   //   issuer_spki_hash: the SHA256 of the SubjectPublicKeyInfo of the CRL

[wtc, 2012/01/10 00:50:59]

You should describe what happens if issuer_spki_hash is empty
(the function returns GOOD),  because you call
CheckCertificate() with an empty issuer_spki_hash in
x509_certificate_nss.cc (the first iteration of the for
loop).

[agl, 2012/01/10 16:15:29]

(Have split the function to hopefully make it clearer.)

   //       signer
   Result CheckCertificate(
+      const base::StringPiece& spki_hash,
       const base::StringPiece& serial_number,
       const base::StringPiece& issuer_spki_hash) const;
 
@@ -78,7 +84,7 @@ class NET_EXPORT CRLSet : public base::RefCountedThreadSafe<CRLSet> {
  private:
   CRLSet();
 
-  static CRLSet* CRLSetFromHeader(base::StringPiece header);
+  bool CopyBlockedSPKIsFromHeader(base::DictionaryValue* header_dict);

[wtc, 2012/01/10 00:50:59]

Nit: perhaps call these "revoked SPKIs" rather than "blocked
SPKIs"?

Unlike the original function CRLSetFromHeader, I cannot
guess how CopyBlockedSPKIsFromHeader works by looking at
the function prototype alone.  So a brief description would
be helpful.

(What wasn't obvious to me was the output of this function.
I just looked at its implementation and saw that the output
is the blocked_spkis_ member.)

[agl, 2012/01/10 16:15:29]

(See other comment for why I want to stick with "blocked".)
Done.

 
   uint32 sequence_;
   CRLList crls_;
@@ -87,6 +93,9 @@ class NET_EXPORT CRLSet : public base::RefCountedThreadSafe<CRLSet> {
   // and |crls_index_by_issuer_| because, when applying a delta update, we need
   // to identify a CRL by index.
   std::map<std::string, size_t> crls_index_by_issuer_;
+  // blocked_spkis_ contains the SHA256 hash of SPKIs which are to be blocked no

[wtc, 2012/01/10 00:50:59]

Nit: hash => hashes ?

+  // matter where in a certificate chain they might appear.
+  std::vector<std::string> blocked_spkis_;
 };
 
 }  // namespace net