Add DER parsing of AlgorithmId for signatures.

net/cert/internal/signature_algorithm.h

+// 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_SIGNATURE_ALGORITHM_H_
+#define NET_CERT_INTERNAL_SIGNATURE_ALGORITHM_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "net/base/net_export.h"
+
+namespace net {
+
+namespace der {
+class Input;
+}

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

nit: }  // namespace der

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

Done.

+
+// Enumeration that describes a (parameter-less) hashing algorithm.

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

nit: This comment doesn't seem to really add much. On a semantic level, it
arguably names rather than describes, but there's arguable nuance in that
terminology.

// The digest algorithm used within a signature.

Might be slightly more direct?

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

Done.

+enum class DigestAlgorithm {
+  Sha1,
+  Sha256,
+  Sha384,
+  Sha512,
+};
+
+// Enumeration that describes a class of algorithm (the parameters to the
+// algorithm are specified separately).

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

// The signature scheme used within a signature. Parameters are specified
separately.

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

Done.

+enum class SignatureAlgorithmId {
+  RsaPkcs1,  // RSA PKCS#1 v1.5
+  RsaPss,    // RSASSA-PSS
+  Ecdsa,     // ECDSA
+};
+
+// Base class for describing algorithm parameters.
+class NET_EXPORT SignatureAlgorithmParameters {
+ public:
+  SignatureAlgorithmParameters() {}
+  virtual ~SignatureAlgorithmParameters(){};
+
+  // Returns true if the two parameters are identical. This must only be called
+  // with instances of the same type (so it is safe for implementations to
+  // cast).
+  virtual bool Equals(const SignatureAlgorithmParameters* other) const = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SignatureAlgorithmParameters);
+};
+
+// Parameters for an RSASSA-PSS signature algorithm.
+//
+// The trailer is assumed to be 1, and the mask gen to be MGF1.

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

Why?

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

Why not?

I don't see any support in BoringSSL for modulating those parameters, nor did I
see where alternate mask gens were enumerated by the specs.

Are you proposing I add a more general structure for mask gen functions, and
have MGF1 be a special case of it?

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

Sorry for being opaque, especially with the round-trip-time. I more meant
"Document why" for these assumptions. My hope is that we document the hell out
of things whenever possible, or even (briefly) summarize some of these CL
comments, because of the subtlety.

For example

// The trailer is assumed to be 1 and the mask generation algorithm to be MGF1,
as that
// is all that is implemented, and any other values while parsing the
AlgorithmIdentifier will thus
// be rejected.

So that if/when there's some other trailer or some other MGF1, it's clear the
reason we didn't add the additional parameters is, like you said, because it's
not implemented and it's all that's specified, so there's no need to explicitly
track/expose via the API.

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

Done.

+class NET_EXPORT RsaPssParameters : public SignatureAlgorithmParameters {
+ public:
+  RsaPssParameters(DigestAlgorithm mgf1_hash, uint32_t salt_length);
+
+  bool Equals(const SignatureAlgorithmParameters* other) const override;
+
+  DigestAlgorithm mgf1_hash() const { return mgf1_hash_; }
+  uint32_t salt_length() const { return salt_length_; }
+
+ private:
+  const DigestAlgorithm mgf1_hash_;
+  const uint32_t salt_length_;
+};
+
+// SignatureAlgorithm describes a signature algorithm and its parameters. This
+// corresponds to "AlgorithmIdentifier" from RFC 5280.
+class NET_EXPORT SignatureAlgorithm {
+ public:
+  // Creates an empty (invalid) signature algorithm. Callers should use one of
+  // the Assign* methods to initialize it.
+  SignatureAlgorithm();

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

Why init+Assign vs a factory create? Is the assumption to avoid the heap?

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

Using factory methods creates the need to support assignment or copy or move.

Currently the parameters are held by a scoped_ptr<> so assignment or copy are
not trivial.

Maybe move support would be sufficient, but it seemed like more effort than it
was worth, so I just stuck with assignment to keep it simple.

[Ryan Sleevi, 2015/07/08 11:49:05]

Sorry, by factory method I meant something like

scoped_ptr<SignatureAlgorithm> CreateRsaPkcs(...);
scoped_ptr<SignatureAlgorithm> CreateEcdsa(...);
scoped_ptr<SignatureAlgorithm> CreateRsaPss(...);
scoped_ptr<SignatureAlgorithm> ParseDer(...);

with nullptr == unable to create. This makes it more explicit than 

SignatureAlgorithm foo;
// foo.ParseDer(...) || foo.AssignRsaPkcs1(...)
if (!foo.IsValid())

The WUR for ParseDer is understandably trying to make sure an uninitialized Foo
isn't used, but it does seem more overhead w/ the need to check IsValid(). It
also makes it weird for situations like ParamsForRsaPss() returning a structure
that could be invalidated by a subsequent Assign* call.

[The assumption here is that SignatureAlgorithm is moved to a protected/private
ctor as appropriate, and only the dtor is exposed]

By the factory scoped_ptr<> method, you can make the SignatureAlgorithm wholly
immutable and singly-owned, but at the cost of going to the heap. I had assumed
you were making a performance tradeoff there in avoiding the heap, but it sounds
like you weren't?

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

Done.
(Except renamed ParseDer --> CreateFromDer() for consistency)

+  ~SignatureAlgorithm();
+
+  // Returns true if the SignatureAlgorithm has been initialized. Accessors
+  // should not be called on invalid SignatureAlgorithms.
+  //
+  // Note that IsValid() does not imply that the algorithm and its parameters
+  // are valid in a cryptographic sense, just that the object itself has been
+  // initialized.
+  bool IsValid() const WARN_UNUSED_RESULT;
+
+  SignatureAlgorithmId algorithm() const;
+  DigestAlgorithm digest() const;
+
+  // Assigns the SignatureAlgorithm by parsing a DER-encoded
+  // "AlgorithmIdentifier" (RFC 5280). Returns true on success.
+  bool ParseDer(const der::Input& algorithm_identifier) WARN_UNUSED_RESULT;
+
+  // Sets the SignatureAlgorithm to the given type and parameters.
+  void AssignRsaPkcs1(DigestAlgorithm digest);
+  void AssignEcdsa(DigestAlgorithm digest);
+  void AssignRsaPss(DigestAlgorithm digest,
+                    DigestAlgorithm mgf1_hash,
+                    uint32_t salt_length);
+
+  // Returns true if |*this| is equivalent to |other|. This compares both the
+  // algorithm ID and each parameter for equality.
+  bool Equals(const SignatureAlgorithm& other) const WARN_UNUSED_RESULT;
+
+  // The following methods retrieve the parameters for the signature algorithm.
+  //
+  // The correct parameters should be chosen based on the algorithm ID. For
+  // instance a SignatureAlgorithm with |algorithm() == RsaPss| should retrieve
+  // parameters via ParametersForRsaPss().
+  //
+  // The returned pointer is non-owned, and will be valid only as long as the
+  // parameters remain alive. This correspond with the lifetime of the
+  // SignatureAlgorithm, provided there are no intervening calls to Assign*() or
+  // Parse*().
+  const RsaPssParameters* ParamsForRsaPss() const;
+
+ private:
+  // Clears the SignatureAlgorithm, rendering it in an invalid state as if it
+  // had just been default-constructed.
+  void AssignInvalid();
+
+  SignatureAlgorithmId algorithm_;
+  DigestAlgorithm digest_;
+  scoped_ptr<SignatureAlgorithmParameters> params_;
+
+  DISALLOW_COPY_AND_ASSIGN(SignatureAlgorithm);
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_INTERNAL_SIGNATURE_ALGORITHM_H_