Implement VerifySignedData() for ECDSA, RSA PKCS#1 and RSA PSS.

net/cert/internal/verify_signed_data.h

Index: net/cert/internal/verify_signed_data.h
diff --git a/net/cert/internal/verify_signed_data.h b/net/cert/internal/verify_signed_data.h
new file mode 100644
index 0000000000000000000000000000000000000000..fc622e8f563ab2edb037393e5eb5d669a78de78b
--- /dev/null
+++ b/net/cert/internal/verify_signed_data.h
@@ -0,0 +1,38 @@
+// 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_VERIFY_SIGNED_DATA_H_
+#define NET_CERT_INTERNAL_VERIFY_SIGNED_DATA_H_
+
+#include "base/compiler_specific.h"
+#include "net/base/net_export.h"
+
+namespace net {
+
+namespace der {
+class Input;
+}  // namespace der
+
+class SignatureAlgorithm;
+
+// Verifies that |signature_value| is a valid signature of |signed_data| using
+// the algorithm |signature_algorithm| and the public key |public_key|.
+//
+//   |signature_algorithm| - The parsed AlgorithmIdentifier
+//   |signed_data| - The blob of data to verify
+//   |signature_value_bit_string| - The DER-encoded BIT STRING representing the

[davidben, 2015/07/22 17:20:47]

Maybe s/BIT STRING/BIT STRING (contents|element)/ just so it's clear which one
you intended.

I don't see the ReadBitStringNoUnusedBits definition, but I'm guessing you made
it the element? Contents might be more consistent with the other
parser.h/parse_values.h splits. Otherwise the caller needs to Parser.ReadRawTLV
and we parse the length twice.

(You don't need to re-regenerate all your tests. Just parse out the element
wrapper in the test harness.)

[eroman, 2015/07/22 17:46:10]

Regenerating the test data for signature is not a problem, as I already have an
automated way of doing this. So if we decide on that approach I can strip the
bitstring prefix.

That said, I think it is advantageous to have the signature_value_bit_string be
the full TLV (i.e. start with 03 ) rather than its contents, and think it is the
most consistent approach.

Note that other parameters are also taking the outtermost type rather than the
contents. For instance the public_key is expected to be a sequence, and not the
contents of the sequence. Similarly the signature algorithm parsing functions
take a sequence and not the contents of that sequence.

If we wanted to take the contents of bit string I would argue we should do
similarly for these other parameters, since otherwise signature value is a bit
unique. But I would also argue that taking the sequence contents rather than the
sequence encoding for things like parsing of spki and algorithm identifiers is
worse.

Reading the full TLV is already required for the signed data (say
tbsCertificate) so I don't think it makes things any less understandable to be
doing this for signatureValue/algorithm/publicKey

[davidben, 2015/07/22 17:49:40]

Oh, good point about all the other inputs. Full element it is then. I guess
there's not much need to clarify the comment either since DER-encoded BIT STRING
and DER-encoded SubjectPublicKeyInfo presumably refer to the same thing.

+//       signature's value (to be interpreted according to the signature
+//       algorithm).
+//   |public_key| - A DER-encoded SubjectPublicKeyInfo.
+//
+// Returns true if verification was successful.
+NET_EXPORT bool VerifySignedData(const SignatureAlgorithm& signature_algorithm,
+                                 const der::Input& signed_data,
+                                 const der::Input& signature_value_bit_string,
+                                 const der::Input& public_key)
+    WARN_UNUSED_RESULT;
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_VERIFY_SIGNED_DATA_H_