Add DER parsing of AlgorithmId for signatures.

net/cert/internal/signature_algorithm.cc

Index: net/cert/internal/signature_algorithm.cc
diff --git a/net/cert/internal/signature_algorithm.cc b/net/cert/internal/signature_algorithm.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4ea2e390d598efd3cceceb18291901fc0b2ed181
--- /dev/null
+++ b/net/cert/internal/signature_algorithm.cc
@@ -0,0 +1,337 @@
+// 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.
+
+#include "net/cert/internal/signature_algorithm.h"
+
+#include <stdint.h>
+
+#include "net/der/input.h"
+#include "net/der/parser.h"
+
+namespace net {
+
+namespace {
+
+// From RFC 3279 section 2.2.1:
+//  sha-1WithRSAEncryption OBJECT IDENTIFIER  ::=  {
+//              iso(1) member-body(2) us(840) rsadsi(113549) pkcs(1)
+//                          pkcs-1(1) 5  }
+// In dotted notation: 1.2.840.113549.1.1.5
+const uint8_t kOidSha1WithRsaEncryption[] =
+    {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x05};
+
+// From RFC 4055 section 6:
+//   pkcs-1  OBJECT IDENTIFIER  ::=  { iso(1) member-body(2)
+//                                 us(840) rsadsi(113549) pkcs(1) 1 }
+
+// From RFC 4055 section 5:
+//   sha256WithRSAEncryption  OBJECT IDENTIFIER  ::=  { pkcs-1 11 }
+// In dotted notation: 1.2.840.113549.1.1.11
+const uint8_t kOidSha256WithRsaEncryption[] =
+    {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b};
+
+// From RFC 4055 section 5:
+//   sha384WithRSAEncryption  OBJECT IDENTIFIER  ::=  { pkcs-1 12 }
+// In dotted notation: 1.2.840.113549.1.1.12
+const uint8_t kOidSha384WithRsaEncryption[] =
+    {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c};
+
+// From RFC 4055 section 5:
+//   sha512WithRSAEncryption  OBJECT IDENTIFIER  ::=  { pkcs-1 13 }
+// In dotted notation: 1.2.840.113549.1.1.13
+const uint8_t kOidSha512WithRsaEncryption[] =
+    {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d};
+
+// From RFC 3279 section 2.2.3:
+//   ansi-X9-62  OBJECT IDENTIFIER ::= {
+//        iso(1) member-body(2) us(840) 10045 }
+//
+//   id-ecSigType OBJECT IDENTIFIER  ::=  {
+//        ansi-X9-62 signatures(4) }
+//
+//   ecdsa-with-SHA1  OBJECT IDENTIFIER ::= {
+//        id-ecSigType 1 }
+// In dotted notation: 1.2.840.10045.4.1
+const uint8_t kOidEcdsaWithSha1[] = {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x01};
+
+// From RFC 5758 section 3.2:
+//   ecdsa-with-SHA256 OBJECT IDENTIFIER ::= { iso(1) member-body(2)
+//        us(840) ansi-X9-62(10045) signatures(4) ecdsa-with-SHA2(3) 2 }
+// In dotted notation: 1.2.840.10045.4.3.2
+const uint8_t kOidEcdsaWithSha256[] =
+    {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02};
+
+// From RFC 5758 section 3.2:
+//   ecdsa-with-SHA384 OBJECT IDENTIFIER ::= { iso(1) member-body(2)
+//        us(840) ansi-X9-62(10045) signatures(4) ecdsa-with-SHA2(3) 3 }
+// In dotted notation: 1.2.840.10045.4.3.3
+const uint8_t kOidEcdsaWithSha384[] =
+    {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x03};
+
+// From RFC 5758 section 3.2:
+//   ecdsa-with-SHA512 OBJECT IDENTIFIER ::= { iso(1) member-body(2)
+//        us(840) ansi-X9-62(10045) signatures(4) ecdsa-with-SHA2(3) 4 }
+// In dotted notation: 1.2.840.10045.4.3.4
+const uint8_t kOidEcdsaWithSha512[] =
+    {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x04};
+
+// From RFC 4055 section 3.1:
+//   id-RSASSA-PSS  OBJECT IDENTIFIER  ::=  { pkcs-1 10 }
+// In dotted notation: 1.2.840.113549.1.1.10
+const uint8_t kOidRsaSsaPss[] =
+    {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0a};
+
+// RFC 5280 section 4.1.1.2 defines signatureAlgorithm as:
+//
+//  AlgorithmIdentifier  ::=  SEQUENCE  {
+//       algorithm               OBJECT IDENTIFIER,
+//       parameters              ANY DEFINED BY algorithm OPTIONAL  }
+WARN_UNUSED_RESULT bool ParseAlgorithmIdentifier(const der::Input& input,
+                                                 der::Input* algorithm,
+                                                 der::Input* parameters) {
+  der::Parser parser(input);
+
+  der::Parser algorithm_identifier_parser;
+  if (!parser.ReadSequence(&algorithm_identifier_parser))
+    return false;
+
+  // There shouldn't be anything after the sequence.
+  if (parser.HasMore())
+    return false;
+
+  if (!algorithm_identifier_parser.ReadTag(der::kOid, algorithm))
+    return false;
+
+  // Read the optional parameters to a der::Input. The parameters can be at
+  // most one TLV (for instance NULL or a sequence).
+  *parameters = der::Input();
+  if (algorithm_identifier_parser.HasMore() &&
+      !algorithm_identifier_parser.ReadRawTLV(parameters)) {
+    return false;
+  }
+  return !algorithm_identifier_parser.HasMore();

[Ryan Sleevi, 2015/07/07 13:55:15]

It's perfectly fine for an algorithm to have additional data.

[eroman, 2015/07/08 01:09:34]

For anyone following, this is the same conversation with more detail on
https://codereview.chromium.org/1217653006/.

Also FTR Mozilla does not allow unrecognized parameters in their signature
AlgorithmIdentifiers:
 
http://lxr.mozilla.org/mozilla-central/source/security/pkix/lib/pkixcheck.cpp#45

[Ryan Sleevi, 2015/07/08 11:44:01]

The result being that as a result if we adopt the 2008 syntax with RFC 5912,
then it's fine to add this check, even though it wasn't strictly required under
the 1988 syntax (as it wasn't expressible positive or negative and the prose
didn't either, although the implication is that because it's an unttaged
optional any, syntax of which was removed in 95, then any extensibility would be
indistinguishable from the ANY and thus ambiguous, thus non-extensible)

[eroman, 2015/07/14 20:31:38]

I have added comments to each place in the code that calls HasMore() referencing
why extra input is disallowed. Thanks for pointing me towards RFC 5912, it
helped clear some stuff up.

+}
+
+// Returns true if the entirety of the input is a NULL value, or is empty.
+WARN_UNUSED_RESULT bool IsNullOrEmpty(const der::Input& input) {
+  if (input.Length() == 0)
+    return true;
+
+  der::Parser parser(input);
+  der::Input null_value;
+  if (!parser.ReadTag(der::kNull, &null_value))
+    return false;
+
+  // NULL values are TLV encoded; the value is expected to be empty.
+  if (null_value.Length() != 0)
+    return false;
+
+  return !parser.HasMore();
+}
+
+// Parses parameters for RSA PKCS#1 v1.5 from |parameters_parser| (ensure there
+// are none), and assigns the resulting algorithm identifier to |*out|.
+//
+// Returns true on success.
+//
+// Note that the parameter parsing used is more permissive than the specs with
+// regards to sha-1WithRSAEncryption - it accepts both NULL and empty
+// parameters.
+//
+// Relevant specs:
+//
+// RFC 4055 section 5 (in reference to sha256WithRSAEncryption et al):
+//   When any of these four object identifiers appears within an
+//   AlgorithmIdentifier, the parameters MUST be NULL.  Implementations
+//   MUST accept the parameters being absent as well as present.
+//
+// RFC 3279 section 2.2.1 (in reference to sha-1WithRSAEncryption):
+//   When any of these three OIDs appears within the ASN.1 type
+//   AlgorithmIdentifier, the parameters component of that type SHALL be
+//   the ASN.1 type NULL.
+WARN_UNUSED_RESULT bool ParseRsaPkcs1(DigestAlgorithm digest,
+                                      const der::Input& params,
+                                      SignatureAlgorithm* out) {
+  if (!IsNullOrEmpty(params))
+    return false;
+
+  out->AssignRsaPkcs1(digest);
+  return true;
+}
+
+// Parses parameters for ECDSA from |parameters_parser| (ensure there
+// are none), and assigns the resulting algorithm identifier to |*out|.
+//
+// Returns true on success.
+//
+// Note that the parameter parsing used is more permissive than the specs with
+// regards - it additionally accepts NULL parameters.

[Ryan Sleevi, 2015/07/07 13:55:15]

Why?

[eroman, 2015/07/08 01:09:34]

Isn't this a moot point based on your earlier comment?

If we allow unrecognized data in AlgorithmIdentifier, wouldn't we also then
accept NULL parameters?
Since it wouldn't be distinguishable from having omitted the optional field, and
then added a new unrecognized field with NULL.

I added the NULL check originally to match Firefox, which allows both empty and
NULL for each flavor of RSA PKCS1 and ECDSA.

[Ryan Sleevi, 2015/07/08 11:44:01]

So if we're adopting the 5912 rules (as discussed elsewhere in this and
related), then rejecting NULL params is in line with the MUST of 5758 (quoted
below) and in line with the relevant sa-ECDSA info classes

   sa-ecdsaWithSHA224 SIGNATURE-ALGORITHM ::= {
    IDENTIFIER ecdsa-with-SHA224
    VALUE ECDSA-Sig-Value
    PARAMS TYPE NULL ARE absent
    HASHES { mda-sha224 }
    PUBLIC-KEYS { pk-ec }
    SMIME-CAPS { IDENTIFIED BY ecdsa-with-SHA224 }
   }

note the reference to, pk-ec

   pk-ec PUBLIC-KEY ::= {
    IDENTIFIER id-ecPublicKey
    KEY ECPoint
    PARAMS TYPE ECParameters ARE required
    -- Private key format not in this module --
    CERT-KEY-USAGE { digitalSignature, nonRepudiation, keyAgreement,
                         keyCertSign, cRLSign }
   }

[eroman, 2015/07/14 20:31:38]

I have re-written most of the RFC references in terms of RFC 5912, as it
provides clearer definitions.

I have changed the parameters parsing for RSA and ECDSA to strictly match RFC
5912, which is to say:
  * RSA must provide a NULL parameters in all cases
  * ECDSA must omit it in all cases

I have updated the tests, and made sure that each of these variations is being
tested.

+//
+// Relevant specs:
+//
+// RFC 5758 section-3.2:
+//   When the ecdsa-with-SHA224, ecdsa-with-SHA256, ecdsa-with-SHA384, or
+//   ecdsa-with-SHA512 algorithm identifier appears in the algorithm field
+//   as an AlgorithmIdentifier, the encoding MUST omit the parameters
+//   field.  That is, the AlgorithmIdentifier SHALL be a SEQUENCE of one
+//   component, the OID ecdsa-with-SHA224, ecdsa-with-SHA256, ecdsa-with-
+//   SHA384, or ecdsa-with-SHA512.
+//
+// RFC 3279 section 2.2.3:
+//   When the ecdsa-with-SHA1 algorithm identifier appears as the
+//   algorithm field in an AlgorithmIdentifier, the encoding MUST omit the
+//   parameters field.  That is, the AlgorithmIdentifier SHALL be a
+//   SEQUENCE of one component: the OBJECT IDENTIFIER ecdsa-with-SHA1.
+WARN_UNUSED_RESULT bool ParseEcdsa(DigestAlgorithm digest,
+                                   const der::Input& params,
+                                   SignatureAlgorithm* out) {
+  if (!IsNullOrEmpty(params))
+    return false;
+
+  out->AssignEcdsa(digest);
+  return true;
+}
+
+// From RFC 4055:
+//    RSASSA-PSS-params  ::=  SEQUENCE  {
+//         hashAlgorithm     [0] HashAlgorithm DEFAULT
+//                                  sha1Identifier,
+//         maskGenAlgorithm  [1] MaskGenAlgorithm DEFAULT
+//                                  mgf1SHA1Identifier,
+//         saltLength        [2] INTEGER DEFAULT 20,
+//         trailerField      [3] INTEGER DEFAULT 1  }
+//
+//      HashAlgorithm  ::=  AlgorithmIdentifier
+//
+//      MaskGenAlgorithm  ::=  AlgorithmIdentifier
+WARN_UNUSED_RESULT bool ParseRsaPss(const der::Input& params,
+                                    SignatureAlgorithm* out) {
+  // TODO(eroman): Implement.
+  return false;
+}
+
+}  // namespace
+
+RsaPssParameters::RsaPssParameters(DigestAlgorithm mgf1_hash,
+                                   uint32_t salt_length)
+    : mgf1_hash_(mgf1_hash), salt_length_(salt_length) {
+}
+
+bool RsaPssParameters::Equals(const SignatureAlgorithmParameters* other) const {
+  const RsaPssParameters* params = static_cast<const RsaPssParameters*>(other);
+  return mgf1_hash_ == params->mgf1_hash_ &&
+         salt_length_ == params->salt_length_;
+}
+
+SignatureAlgorithm::SignatureAlgorithm() {
+  AssignInvalid();
+}
+
+SignatureAlgorithm::~SignatureAlgorithm() {
+}
+
+bool SignatureAlgorithm::IsValid() const {
+  return params_;
+}
+
+SignatureAlgorithmId SignatureAlgorithm::algorithm() const {
+  DCHECK(IsValid());
+  return algorithm_;
+}
+
+DigestAlgorithm SignatureAlgorithm::digest() const {
+  DCHECK(IsValid());
+  return digest_;
+}
+
+bool SignatureAlgorithm::ParseDer(const der::Input& algorithm_identifier) {
+  AssignInvalid();
+
+  der::Input oid;
+  der::Input params;
+  if (!ParseAlgorithmIdentifier(algorithm_identifier, &oid, &params))
+    return false;
+
+  // TODO(eroman): Each OID is tested for equality in order, which is not
+  // particularly efficient.
+
+  if (oid.Equals(der::Input(kOidSha1WithRsaEncryption)))
+    return ParseRsaPkcs1(DigestAlgorithm::Sha1, params, this);
+
+  if (oid.Equals(der::Input(kOidSha256WithRsaEncryption)))
+    return ParseRsaPkcs1(DigestAlgorithm::Sha256, params, this);
+
+  if (oid.Equals(der::Input(kOidSha384WithRsaEncryption)))
+    return ParseRsaPkcs1(DigestAlgorithm::Sha384, params, this);
+
+  if (oid.Equals(der::Input(kOidSha512WithRsaEncryption)))
+    return ParseRsaPkcs1(DigestAlgorithm::Sha512, params, this);
+
+  if (oid.Equals(der::Input(kOidEcdsaWithSha1)))
+    return ParseEcdsa(DigestAlgorithm::Sha1, params, this);
+
+  if (oid.Equals(der::Input(kOidEcdsaWithSha256)))
+    return ParseEcdsa(DigestAlgorithm::Sha256, params, this);
+
+  if (oid.Equals(der::Input(kOidEcdsaWithSha384)))
+    return ParseEcdsa(DigestAlgorithm::Sha384, params, this);
+
+  if (oid.Equals(der::Input(kOidEcdsaWithSha512)))
+    return ParseEcdsa(DigestAlgorithm::Sha512, params, this);
+
+  if (oid.Equals(der::Input(kOidRsaSsaPss)))
+    return ParseRsaPss(params, this);
+
+  return false;  // Unsupported OID.
+}
+
+void SignatureAlgorithm::AssignRsaPkcs1(DigestAlgorithm digest) {
+  algorithm_ = SignatureAlgorithmId::RsaPkcs1;
+  digest_ = digest;
+  params_.reset();
+}
+
+void SignatureAlgorithm::AssignEcdsa(DigestAlgorithm digest) {
+  algorithm_ = SignatureAlgorithmId::Ecdsa;
+  digest_ = digest;
+  params_.reset();
+}
+
+void SignatureAlgorithm::AssignRsaPss(DigestAlgorithm digest,
+                                      DigestAlgorithm mgf1_hash,
+                                      uint32_t salt_length) {
+  algorithm_ = SignatureAlgorithmId::RsaPss;
+  digest_ = digest;
+  params_.reset(new RsaPssParameters(mgf1_hash, salt_length));
+}
+
+bool SignatureAlgorithm::Equals(const SignatureAlgorithm& other) const {
+  if (algorithm_ != other.algorithm_)
+    return false;
+
+  if (digest_ != other.digest_)
+    return false;
+
+  if (!params_ != !other.params_)
+    return false;
+
+  if (params_ && !params_->Equals(other.params_.get()))
+    return false;
+
+  return true;
+}
+
+const RsaPssParameters* SignatureAlgorithm::ParamsForRsaPss() const {
+  if (algorithm_ == SignatureAlgorithmId::RsaPss)
+    return static_cast<RsaPssParameters*>(params_.get());
+  return nullptr;
+}
+
+void SignatureAlgorithm::AssignInvalid() {
+  algorithm_ = static_cast<SignatureAlgorithmId>(-1);
+  digest_ = static_cast<DigestAlgorithm>(-1);
+  params_.reset();
+}
+
+}  // namespace net