Add some policy controls for VerifySignedData().

net/cert/internal/verification_policy.h

Index: net/cert/internal/verification_policy.h
diff --git a/net/cert/internal/verification_policy.h b/net/cert/internal/verification_policy.h
new file mode 100644
index 0000000000000000000000000000000000000000..c255b38bfcf50957862acb24e2caf3acf5e4510d
--- /dev/null
+++ b/net/cert/internal/verification_policy.h
@@ -0,0 +1,62 @@
+// Copyright 2015 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_INTERNAL_VERIFICATION_POLICY_H_
+#define NET_CERT_INTERNAL_VERIFICATION_POLICY_H_
+
+#include "base/compiler_specific.h"
+#include "net/base/net_export.h"
+#include "net/cert/internal/signature_algorithm.h"
+
+namespace net {
+
+class SignatureAlgorithm;
+
+// This interface specifies policies that apply when verifying signed data.
+class NET_EXPORT VerificationPolicy {

[Ryan Sleevi, 2015/07/28 21:14:53]

Naming wise, this seems like it should be SignaturePolicy? It's what sort of
signatures are accepted?

[eroman, 2015/07/29 02:44:43]

Done.

+ public:
+  virtual ~VerificationPolicy() {}
+
+  // Implementations should return true if |algorithm| is an allowed digest
+  // algorithm. This will be called *anywhere* a hash algorithm is used, which
+  // includes the main digest algorithm for RSA PKCS#1 v1.5, ECDSA, and
+  // RSASSA-PSS, and also includes the hash function used by RSASSA-PSS mask
+  // gen function.
+  virtual bool IsAcceptableDigestAlgorithm(DigestAlgorithm algorithm) const = 0;

[Ryan Sleevi, 2015/07/28 21:14:53]

I'm a little torn on this interface, whether it should take the signature as a
whole. The concerns for MGF-1 are different from that of RSA-SSA, for example.

[eroman, 2015/07/29 02:44:43]

Done (Changed to take a signature algorithm instead).

Taking the signature algorithm is more flexible, and probably the right API.
(My original concern was that it would be easy to forget about RSASSA-PSS
parameters)

+
+  // Implementations should return true if |curve_nid| is an allowed
+  // elliptical curve. |curve_nid| is an object ID from BoringSSL (for example
+  // NID_secp384r1).
+  virtual bool IsAcceptableCurveForEcdsa(int curve_nid) const = 0;
+
+  // Implementations should return true if |modulus_length_bits| is an allowed
+  // RSA key size in bits.
+  virtual bool IsAcceptableModulusLengthForRsa(
+      size_t modulus_length_bits) const = 0;

[Ryan Sleevi, 2015/07/28 21:14:53]

We arguably similarly have an EC policy (nothing less than 256 is acceptable).
I'm wondering if that should generalize into a minimum size policy for an
algorithm.

Similarly, a maximum size may also be desirable to express, since the
implementation may support greater than the CPU allows. Although I'm not sure
how necessary that is.

[eroman, 2015/07/29 02:44:43]

This should already be covered by the IsAcceptableCurveForEcdsa() which lets you
accept/reject based on the named curve.

The key size in this case is implied from the named curve itself, and the
default implementation will enforce a minimum key size of 256 bits by virtue of
accepting curves secp256r1, secp384r1, secp521r1.

I could modify the API to explicitly take the group degree size in bits (i.e.
256, 384, 521 for the currently supported curves) to make this more apparent,
but I suspect it is easy enough to do this given the named curve (just a few
function calls in OpenSSL) that this isn't worthwhile.

+};
+
+// BaseVerificationPolicy is a concrete implementation of VerificationPolicy
+// that applies some default behaviors, and allows configuring the minimum RSA
+// key length.
+//
+//   * Accepts all digest algorithms: SHA-1, SHA-256, SHA-384, SHA-512
+//   * Accepts the following curves: secp256r1, secp384r1, secp521r1
+class NET_EXPORT BaseVerificationPolicy : public VerificationPolicy {

[Ryan Sleevi, 2015/07/28 21:14:53]

Curious why this split?

That is, why a pure virtual plus a concrete? Why not an overridable concrete
implementation? It seems that most implementations are likely to derive from
this policy, rather than the pure virtual, and then... what, delegate down to
the base class? Or do you see this being something compositional?

[eroman, 2015/07/29 02:44:43]

My reason for the split mainly came because of not wanting the
"min_rsa_modulus_length_bits" member to be in the base class (since if you
implemented a modulus size policy that didn't use a minimum, this is a pointless
member).

I have changed this around a bit so that:
  * The base class includes the default policy, and enforces RSA key sizes >=
2048
  * A helper class provides a canned implemenation for applying a variable
minimum RSA key size

+ public:
+  explicit BaseVerificationPolicy(size_t min_rsa_modulus_length_bits);
+
+  bool IsAcceptableDigestAlgorithm(DigestAlgorithm algorithm) const override;
+
+  bool IsAcceptableCurveForEcdsa(int curve_nid) const override;
+
+  bool IsAcceptableModulusLengthForRsa(
+      size_t modulus_length_bits) const override;
+
+ private:
+  const size_t min_rsa_modulus_length_bits_;
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_VERIFICATION_POLICY_H_