Introduce CertVerifier::RequestParams

net/cert/cert_verifier.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_CERT_CERT_VERIFIER_H_
 #define NET_CERT_CERT_VERIFIER_H_
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/macros.h"
 #include "net/base/completion_callback.h"
+#include "net/base/hash_value.h"
 #include "net/base/net_export.h"
+#include "net/cert/x509_certificate.h"
 
 namespace net {
 
 class BoundNetLog;
 class CertVerifyResult;
 class CRLSet;
-class X509Certificate;
 
 // CertVerifier represents a service for verifying certificates.
 //
 // CertVerifiers can handle multiple requests at a time.
 class NET_EXPORT CertVerifier {
  public:
   class Request {
    public:
     Request() {}
 
@@ skipping 36 matching lines @@
     // for certificates issued by non-public trust anchors. Failure to check
     // revocation is treated as a hard failure.
     // Note: If VERIFY_CERT_IO_ENABLE is not also supplied, certificates
     // that chain to local trust anchors will likely fail - for example, due to
     // lacking fresh cached revocation issue (Windows) or because OCSP stapling
     // can only provide information for the leaf, and not for any
     // intermediates.
     VERIFY_REV_CHECKING_REQUIRED_LOCAL_ANCHORS = 1 << 4,
   };
 
+  // The parameters for doing a Verify(). |certificate|, |hostname|, and
+  // |flags| are required. The rest are optional.

[eroman, 2016/05/18 01:43:18]

Are you going to move documentation for the various inputs when updating
Verify() to use this?

[Ryan Sleevi, 2016/05/18 02:02:02]

Yes, when updating Verify() to use this.

+  class RequestParams {
+   public:
+    RequestParams(X509Certificate* certificate,
+                  const std::string& hostname,
+                  int flags,
+                  const std::string& ocsp_response,
+                  const CertificateList& additional_trust_anchors);
+    RequestParams(const RequestParams& other);
+    ~RequestParams();
+
+    const std::string& hostname() const { return hostname_; }
+    int flags() const { return flags_; }
+    const std::vector<SHA1HashValue> request_data() const {
+      return request_data_;
+    }
+
+    bool operator<(const RequestParams& other) const;
+
+   private:
+    std::string hostname_;
+    int flags_;
+    std::vector<SHA1HashValue> request_data_;

[eroman, 2016/05/18 01:43:18]

Can you speak to the design choice of using a vector of hashes rather than a
single hash? Was this to provide a particular sorting stability? Reduce the time
spent hashing? Other?
Using a single cryptographically secure hash would use less memory.

Also why are we OK with using SHA1 here? If we believe SHA1 to be susceptible to
preimage attacks (now or in the near future), using it to key the certificate
cache doesn't sound like a good idea.

[Ryan Sleevi, 2016/05/18 02:02:02]

While I appreciate all of this feedback, I don't think it's good to do on this
CL. I'm trying to minimize changes to ease the implementation transition, and am
not comfortable with scope creep.

(In my earlier version, I actually did move to switch to SHA-256, but felt that
was too significant a change for this CL; similar, using a single hash -
although that has issues with separating out distinct fields)

[eroman, 2016/05/18 02:06:43]

OK, we can discuss that separately.

+  };
+
   // When the verifier is destroyed, all certificate verification requests are
   // canceled, and their completion callbacks will not be called.
   virtual ~CertVerifier() {}
 
   // Verifies the given certificate against the given hostname as an SSL server.
   // Returns OK if successful or an error code upon failure.
   //
   // The |*verify_result| structure, including the |verify_result->cert_status|
   // bitmask, is always filled out regardless of the return value.  If the
   // certificate has multiple errors, the corresponding status flags are set in
@@ skipping 19 matching lines @@
   // be passed to the callback when available.
   //
   // On asynchronous completion (when Verify returns ERR_IO_PENDING) |out_req|
   // will be reset with a pointer to the request. Freeing this pointer before
   // the request has completed will cancel it.
   //
   // If Verify() completes synchronously then |out_req| *may* be reset to
   // nullptr. However it is not guaranteed that all implementations will reset
   // it in this case.
   //
-  // TODO(rsleevi): Move CRLSet* out of the CertVerifier signature.
+  // TODO(rsleevi): Update this to use RequestParams as part of the signature.
   virtual int Verify(X509Certificate* cert,
                      const std::string& hostname,
                      const std::string& ocsp_response,
                      int flags,
                      CRLSet* crl_set,
                      CertVerifyResult* verify_result,
                      const CompletionCallback& callback,
                      std::unique_ptr<Request>* out_req,
                      const BoundNetLog& net_log) = 0;
 
   // Returns true if this CertVerifier supports stapled OCSP responses.
   virtual bool SupportsOCSPStapling();
 
   // Creates a CertVerifier implementation that verifies certificates using
   // the preferred underlying cryptographic libraries.
   static std::unique_ptr<CertVerifier> CreateDefault();
 };
 
 }  // namespace net
 
 #endif  // NET_CERT_CERT_VERIFIER_H_