net: allow CRLSets to block specific SPKIs.

net/base/crl_set.h

 // 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.
 
 #ifndef NET_BASE_CRL_SET_H_
 #define NET_BASE_CRL_SET_H_
 #pragma once
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/string_piece.h"
 #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
 // certificates from a number of issuers where issuers are identified by the
 // SHA256 of their SubjectPublicKeyInfo.
 class NET_EXPORT CRLSet : public base::RefCountedThreadSafe<CRLSet> {
  public:
   enum Result {
     REVOKED,  // the certificate should be rejected.
     UNKNOWN,  // the CRL for the certificate is not included in the set.
     GOOD,  // the certificate is not listed.
   };
 
   ~CRLSet();
 
   // Parse parses the bytes in |data| and, on success, puts a new CRLSet in
   // |out_crl_set| and returns true.
   static bool Parse(base::StringPiece data,
                     scoped_refptr<CRLSet>* out_crl_set);
 
   // 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;
 
   // ApplyDelta returns a new CRLSet in |out_crl_set| that is the result of
   // updating the current CRL set with the delta information in |delta_bytes|.
   bool ApplyDelta(const base::StringPiece& delta_bytes,
                   scoped_refptr<CRLSet>* out_crl_set);
 
   // GetIsDeltaUpdate extracts the header from |bytes|, sets *is_delta to
   // whether |bytes| is a delta CRL set or not and returns true. In the event
@@ skipping 15 matching lines @@
   typedef std::vector< std::pair<std::string, std::vector<std::string> > >
       CRLList;
 
   // crls returns the internal state of this CRLSet. It should only be used in
   // testing.
   const CRLList& crls() const;
 
  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_;
   // crls_index_by_issuer_ maps from issuer SPKI hashes to the index in |crls_|
   // where the information for that issuer can be found. We have both |crls_|
   // 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
 
 #endif  // NET_BASE_CRL_SET_H_