Refactor WebCrypto code

content/child/webcrypto/algorithm.h

+// Copyright 2014 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 CONTENT_CHILD_WEBCRYPTO_CRYPTO_ALGORITHM_H_
+#define CONTENT_CHILD_WEBCRYPTO_CRYPTO_ALGORITHM_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "third_party/WebKit/public/platform/WebCrypto.h"
+
+namespace content {
+
+namespace webcrypto {
+
+class Status;
+class CryptoData;
+
+// AlgorithmImplementation is a base class for *executing* the operations of an
+// algorithm (generating keys, encrypting, signing, etc.).
+//
+// This is in contrast to blink::WebCryptoAlgorithm which instead *describes*
+// the operation and its parameters.
+//
+// AlgorithmImplementation has reasonable default implementations for all
+// methods which behave as if the operation is it is unsupported, so
+// implementations need only override the applicable methods.
+//
+// Unless stated otherwise methods of AlgorithmImplementation are responsible
+// for sanitizing their inputs. The following can be assumed:
+//
+//   * |algorithm.id()| and |key.algorithm.id()| matches the algorithm under
+//     which the implementation was registerd.
+//   * |algorithm| has the correct parameters type for the operation.
+//   * The key usages have already been verified. In fact in the case of calls
+//     to Encrypt()/Decrypt() the corresponding key usages may not be present
+//     (when wrapping/unwrapping).
+class AlgorithmImplementation {

[Ryan Sleevi, 2014/07/10 23:20:54]

We generally encourage pure virtual interfaces when describing this.

Here, you've provided default values for everything, which is a bit odd, though
understandable.

+ public:
+  virtual ~AlgorithmImplementation();
+
+  virtual Status Encrypt(const blink::WebCryptoAlgorithm& algorithm,
+                         const blink::WebCryptoKey& key,
+                         const CryptoData& data,
+                         std::vector<uint8>* buffer) const;
+
+  virtual Status Decrypt(const blink::WebCryptoAlgorithm& algorithm,
+                         const blink::WebCryptoKey& key,
+                         const CryptoData& data,
+                         std::vector<uint8>* buffer) const;
+
+  virtual Status Sign(const blink::WebCryptoAlgorithm& algorithm,
+                      const blink::WebCryptoKey& key,
+                      const CryptoData& data,
+                      std::vector<uint8>* buffer) const;
+
+  virtual Status VerifySignature(const blink::WebCryptoAlgorithm& algorithm,

[Ryan Sleevi, 2014/07/10 23:20:54]

Why is this VerifySignature, when the corresponding WebCrypto method is "Verify"
(a name chosen because it also verifies MACs, which are not signatures)

[eroman, 2014/07/11 00:27:36]

History: the Blink name for the method is verifySignature() because verify()
collided with a macro on MAC OS X.

I should be able to call this Verify() without such a collision, will try the
change.

+                                 const blink::WebCryptoKey& key,
+                                 const CryptoData& signature,
+                                 const CryptoData& data,
+                                 bool* signature_match) const;
+
+  virtual Status Digest(const blink::WebCryptoAlgorithm& algorithm,
+                        const CryptoData& data,
+                        std::vector<uint8>* buffer) const;
+
+  virtual scoped_ptr<blink::WebCryptoDigestor> CreateDigestor(
+      blink::WebCryptoAlgorithmId algorithm) const;

[Ryan Sleevi, 2014/07/10 23:20:54]

This feels like it's at the wrong layer.

[eroman, 2014/07/11 00:27:36]

OK I'll try to move this out. It was convenient having it here since it avoided
needing to define extra headers and openssl vs blink dispatches.

+
+  // -----------------------------------------------
+  // Key generation
+  // -----------------------------------------------
+  // When generating a key, VerifyKeyUsagesVeforeGenerateKey() will always be
+  // called before GenerateSecretKey(). Similarly when generating a keypair
+  // VerifyKeyUsagesBeforeGenerateKey() will always be called before
+  // GenerateKeyPair().
+
+  virtual Status VerifyKeyUsagesBeforeGenerateKey(
+      blink::WebCryptoKeyUsageMask usage_mask) const;
+
+  virtual Status GenerateSecretKey(const blink::WebCryptoAlgorithm& algorithm,
+                                   bool extractable,
+                                   blink::WebCryptoKeyUsageMask usage_mask,
+                                   blink::WebCryptoKey* key) const;
+
+  virtual Status VerifyKeyUsagesBeforeGenerateKeyPair(
+      blink::WebCryptoKeyUsageMask combined_usage_mask,
+      blink::WebCryptoKeyUsageMask* public_usage_mask,
+      blink::WebCryptoKeyUsageMask* private_usage_mask) const;
+
+  virtual Status GenerateKeyPair(
+      const blink::WebCryptoAlgorithm& algorithm,
+      bool extractable,
+      blink::WebCryptoKeyUsageMask public_usage_mask,
+      blink::WebCryptoKeyUsageMask private_usage_mask,
+      blink::WebCryptoKey* public_key,
+      blink::WebCryptoKey* private_key) const;
+
+  // -----------------------------------------------
+  // Key import
+  // -----------------------------------------------
+  // VerifyKeyUsagesBeforeImportKey() will always be called before either
+  // importing a key, or unwrapping a key.
+  //
+  // Note that when the format is JWK it may be unknown what the valid key
+  // usages are (since the key type will not be known yet).
+
+  virtual Status VerifyKeyUsagesBeforeImportKey(
+      blink::WebCryptoKeyFormat format,
+      blink::WebCryptoKeyUsageMask usage_mask) const;
+
+  virtual Status ImportKeyRaw(const CryptoData& key_data,
+                              const blink::WebCryptoAlgorithm& algorithm,
+                              bool extractable,
+                              blink::WebCryptoKeyUsageMask usage_mask,
+                              blink::WebCryptoKey* key) const;
+
+  virtual Status ImportKeyPkcs8(const CryptoData& key_data,
+                                const blink::WebCryptoAlgorithm& algorithm,
+                                bool extractable,
+                                blink::WebCryptoKeyUsageMask usage_mask,
+                                blink::WebCryptoKey* key) const;
+
+  virtual Status ImportKeySpki(const CryptoData& key_data,
+                               const blink::WebCryptoAlgorithm& algorithm,
+                               bool extractable,
+                               blink::WebCryptoKeyUsageMask usage_mask,
+                               blink::WebCryptoKey* key) const;
+
+  virtual Status ImportKeyJwk(const CryptoData& key_data,
+                              const blink::WebCryptoAlgorithm& algorithm,
+                              bool extractable,
+                              blink::WebCryptoKeyUsageMask usage_mask,
+                              blink::WebCryptoKey* key) const;

[Ryan Sleevi, 2014/07/10 23:20:54]

Not sure why this multitude of methods, rather than having an enum for format,
and letting the implementation handle format dispatch internally.

Ditto export key.

[eroman, 2014/07/11 00:27:36]

This is about reducing code duplication.

Without a central dispatch, every implementation of AlgorithmImplementation ends
up needing to do something like:

switch (format) {
  case Spki:
    return ImportSpki(...);
  case Pkcs8:
    return ImportPkcs8(...);
  case Jwk:
    return ImportJwk(...);
  default:
    return Status::ErrorUnsupportedImportKeyFormat();
}

Not the worst thing in the world, but something we can avoid.

Do you want me to change it?

+
+  // -----------------------------------------------
+  // Key export
+  // -----------------------------------------------
+
+  virtual Status ExportKeyRaw(const blink::WebCryptoKey& key,
+                              std::vector<uint8>* buffer) const;
+
+  virtual Status ExportKeyPkcs8(const blink::WebCryptoKey& key,
+                                std::vector<uint8>* buffer) const;
+
+  virtual Status ExportKeySpki(const blink::WebCryptoKey& key,
+                               std::vector<uint8>* buffer) const;
+
+  virtual Status ExportKeyJwk(const blink::WebCryptoKey& key,
+                              std::vector<uint8>* buffer) const;
+};
+
+}  // namespace webcrypto
+
+}  // namespace content
+
+#endif  // CONTENT_CHILD_WEBCRYPTO_CRYPTO_ALGORITHM_H_