Refactor WebCrypto code

content/child/webcrypto/nss/aes_key_nss.h

Index: content/child/webcrypto/nss/aes_key_nss.h
diff --git a/content/child/webcrypto/nss/aes_key_nss.h b/content/child/webcrypto/nss/aes_key_nss.h
new file mode 100644
index 0000000000000000000000000000000000000000..f9821264ab1b407904db6fd75c8f8ec5188b37f5
--- /dev/null
+++ b/content/child/webcrypto/nss/aes_key_nss.h
@@ -0,0 +1,70 @@
+// 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_NSS_AES_NSS_H_
+#define CONTENT_CHILD_WEBCRYPTO_NSS_AES_NSS_H_
+
+#include <pkcs11t.h>
+
+#include "content/child/webcrypto/algorithm.h"
+
+namespace content {
+
+namespace webcrypto {
+
+// Base class for AES algorithms that provides the implementation for key
+// creation and export.
+class AesAlgorithm : public AlgorithmImplementation {
+ public:
+  AesAlgorithm(CK_MECHANISM_TYPE import_mechanism,
+               CK_FLAGS import_flags,
+               blink::WebCryptoKeyUsageMask all_key_usages,
+               const char* jwk_suffix);
+
+  // Shortcut for creating an encryption algorithm, with support for encrypt,
+  // decrypt, wrap, unwrap.
+  AesAlgorithm(CK_MECHANISM_TYPE import_mechanism, const char* jwk_suffix);

[Ryan Sleevi, 2014/07/12 00:55:27]

Is this really a suffix or a prefix? And what's the lifetime here? I fear you
potentially allow stack-allocated variables in, which then cause lifetime issues
on line 63. Why not make it explicit with std::string(), and let the small
string optimization amortize any perceived performance overheads?

[eroman, 2014/07/12 01:59:30]

A128CBC --> "CBC" is the jwk_suffix expected here.
Similarly A192KW --> "KW" is the jwk_suffix

And what's the lifetime here? I fear you
Done

+
+  virtual Status GenerateSecretKey(const blink::WebCryptoAlgorithm& algorithm,
+                                   bool extractable,
+                                   blink::WebCryptoKeyUsageMask usage_mask,
+                                   blink::WebCryptoKey* key) const OVERRIDE;
+
+  virtual Status VerifyKeyUsagesBeforeImportKey(
+      blink::WebCryptoKeyFormat format,
+      blink::WebCryptoKeyUsageMask usage_mask) const OVERRIDE;

[Ryan Sleevi, 2014/07/12 00:55:27]

Please keep the ordering the same on all derived methods. This is out of order
with the main Algorithm.

[eroman, 2014/07/12 01:59:30]

Acknowledged.

+
+  virtual Status VerifyKeyUsagesBeforeGenerateKey(
+      blink::WebCryptoKeyUsageMask usage_mask) const OVERRIDE;
+
+  virtual Status ImportKeyRaw(const CryptoData& key_data,
+                              const blink::WebCryptoAlgorithm& algorithm,
+                              bool extractable,
+                              blink::WebCryptoKeyUsageMask usage_mask,
+                              blink::WebCryptoKey* key) const OVERRIDE;
+
+  virtual Status ImportKeyJwk(const CryptoData& key_data,
+                              const blink::WebCryptoAlgorithm& algorithm,
+                              bool extractable,
+                              blink::WebCryptoKeyUsageMask usage_mask,
+                              blink::WebCryptoKey* key) const OVERRIDE;
+
+  virtual Status ExportKeyRaw(const blink::WebCryptoKey& key,
+                              std::vector<uint8>* buffer) const OVERRIDE;
+
+  virtual Status ExportKeyJwk(const blink::WebCryptoKey& key,
+                              std::vector<uint8>* buffer) const OVERRIDE;
+
+ private:
+  const CK_MECHANISM_TYPE import_mechanism_;
+  const CK_FLAGS import_flags_;
+  const blink::WebCryptoKeyUsageMask all_key_usages_;
+  const char* const jwk_suffix_;
+};
+
+}  // namespace webcrypto
+
+}  // namespace content
+
+#endif  // CONTENT_CHILD_WEBCRYPTO_NSS_AES_NSS_H_