[webcrypto] Add error messages for failed operations.

content/renderer/webcrypto/webcrypto_util.h

 // Copyright (c) 2013 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_RENDERER_WEBCRYPTO_WEBCRYPTO_UTIL_H_
 #define CONTENT_RENDERER_WEBCRYPTO_WEBCRYPTO_UTIL_H_
 
 #include <map>
 #include <string>
 #include <vector>
 #include "base/basictypes.h"
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/WebArrayBuffer.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
 
 namespace content {
 
 namespace webcrypto {
 
+// Status indicates whether an operation completed successfully, or with an
+// error. The error is used for verification in unit-tests, as well as for
+// display to the user.
+//
+// As such, it is important that errors DO NOT reveal any sensitive material
+// (like key bytes).
+//
+// Care must be taken with what errors are reported back to blink when doing
+// compound operations like unwrapping a JWK key. In this case, errors
+// generated by the JWK import are not appropriate to report since the wrapped
+// JWK is not visible to the caller.
+class CONTENT_EXPORT Status {
+ public:
+  // Returns true if the Status represents an error (any one of them).
+  bool IsError() const;
+
+  // Returns true if the Status represent success.
+  bool IsSuccess() const;
+
+  // Returns a UTF-8 error message (non-localized) describing the error. This
+  // message is intended to be displayed in the dev tools console.
+  std::string ToString() const;
+
+  // Constructs a status representing success.
+  static Status Success();
+
+  // Constructs a status representing a generic error. It contains no extra
+  // details.
+  static Status Error();
+
+  // ------------------------------------
+  // Errors when importing a JWK formatted key
+  // ------------------------------------
+
+  // The key bytes could not parsed as JSON dictionary. This either
+  // means there was a parsing error, or the JSON was not of a dictionary.

[Ryan Sleevi, 2014/01/28 21:11:58]

nit: "JSON was not of a dictionary" - "JSON object was not convertable to a
dictionary" ?

[eroman, 2014/01/28 22:59:08]

Done.

+  static Status ErrorJwkNotDictionary();
+
+  // The required "kty" parameter was missing, or is not a string.
+  static Status ErrorJwkMissingKty();

[Ryan Sleevi, 2014/01/28 21:11:58]

This seems to be an overloaded error, especially given how much JSON can co-erce
things to strings when treating in JS.

Is the plan to treat all non-coercable data types (eg: a list, a dict) as
missing?

[eroman, 2014/01/28 22:59:08]

I am unaware of any type coercion mandated by JWK parsing, or generic coercion
implied by JSON.

The current implementation requires strong matching of types, and does not try
to stringify types (for instance if you pass an integer where a string was
expected, that is a failure, not a stringification to decimal form).

There are some aspects to the particular implementation which concern me, and I
will follow up on (have added a TODO). For instance, optional fields are
considered as "existing" only if the propery is present AND the correct type. So
if "extractable: 0" was set, that would be disregarded, which seems dangerous.
Instead I believe that should error, since the optional field was present but of
the wrong type.

I will follow up on this issue separately, and perhaps introduce some new errors
to capture these nuances.

In the current CL I would prefer to match the current errors.

+
+  // The "extractable" parameter was set, however it contradicted the one
+  // specified by the Web Crypto call.
+  static Status ErrorJwkExtractableInconsistent();

[Ryan Sleevi, 2014/01/28 21:11:58]

nit:
The JWK "extractable" attribute was present, but was incompatible with the value
requested by the Web Crypto caller.

Since it's not about "set", it's about the particular value (false), and being
incompatible with the API.

[eroman, 2014/01/28 22:59:08]

Done.

+
+  // The "alg" parameter could not be converted to an equivalent
+  // WebCryptoAlgorithm. Either it was malformed, or unrecognized.

[Ryan Sleevi, 2014/01/28 21:11:58]

unnecessary ,

When in a list of two, it's not necessary to include the comma.

[eroman, 2014/01/28 22:59:08]

Done.

+  static Status ErrorJwkUnrecognizedAlgorithm();
+
+  // The "alg" parameter contradicts the (optional) Algorithm which was
+  // specified by the Web Crypto import operation.

[Ryan Sleevi, 2014/01/28 21:11:58]

I'd prefer "inconsistent" or "incompatible" over "contradict", if only that
"contradict" suggests asserting the opposite or that one value is more true than
the other, when really, we don't know which is "right"

[eroman, 2014/01/28 22:59:08]

Done.

+  static Status ErrorJwkAlgorithmInconsistent();
+
+  // The "alg" parameter was not provided, however neither was an algorithm
+  // provided by the Web Crypto import operation.
+  static Status ErrorJwkAlgorithmMissing();
+
+  // The "use" parameter was specified, however it couldn't be converted to an
+  // equivalent Web Crypto usage.
+  static Status ErrorJwkUnrecognizedUsage();
+
+  // The "use" parameter was specified, however it contradicts the one specifed
+  // by the Web Crypto import operation.
+  static Status ErrorJwkUsageInconsistent();
+
+  // The "k" parameter was either missing, or could not be parsed as a base-64
+  // encoded string, or the decoded bytes were empty.
+  static Status ErrorJwkDecodeK();
+
+  // The "n" parameter was either missing, or could not be parsed as a base-64

[Ryan Sleevi, 2014/01/28 21:11:58]

drop the first "or" (missing, could not be parsed ..., or the decoed)

Same with others.

[eroman, 2014/01/28 22:59:08]

Done.

+  // encoded string, or the decoded bytes were empty.
+  static Status ErrorJwkDecodeN();
+
+  // The "e" parameter was either missing, or could not be parsed as a base-64
+  // encoded string, or the decoded bytes were empty.
+  static Status ErrorJwkDecodeE();
+
+  // TODO(eroman): Private key import through JWK is not yet supported.
+  static Status ErrorJwkRsaPrivateKeyUnsupported();
+
+  // The "kty" parameter was given and was a string, however it was
+  // unrecognized.
+  static Status ErrorJwkUnrecognizedKty();
+
+  // ------------------------------------
+  // Other errors
+  // ------------------------------------
+
+  // No key data was provided when importing an spki, pkcs8, or jwk formatted
+  // key. This does not apply to raw format, since it is possible to have empty
+  // key data there.
+  static Status ErrorImportEmptyKeyData();
+
+  // The wrong key was used for the operation. For instance, a public key was
+  // used to verify a RsaSsaPkcs1v1_5 signature, or tried exporting a private
+  // key using spki format.
+  static Status ErrorUnexpectedKeyType();
+
+  // When doing an AES-CBC encryption/decryption, the "iv" parameter was not 16
+  // bytes.
+  static Status ErrorIncorrectSizedAesCbcIv();
+
+  // The data provided to an encrypt/decrypt/sign/verify operation was too
+  // large. This can either represent an internal limitation (for instance
+  // representing buffer lengths as uints), or an algorithm restriction (for
+  // instance RSAES can operation on messages relative to the length of the
+  // key's modulus).
+  static Status ErrorDataTooBig();

[Ryan Sleevi, 2014/01/28 21:11:58]

s/Big/Large, to match both your description and the error message.

[eroman, 2014/01/28 22:59:08]

Done.

+
+  // Something was unsupported or unimplemented. This can mean the algorithm in
+  // question was unsupported, some parameter combination was unsupported, or
+  // something has not yet been implemented.
+  static Status ErrorUnsupported();
+
+  // Something unexpected happened in the code, which implies there is a
+  // source-level bug. These should not happen, but safer to fail than simply
+  // DCHECK.
+  static Status ErrorUnexpected();
+
+  // The authentication tag length specified for AES-GCM encrypt/decrypt was
+  // either greater than 128 bits, or it was not a multiple of 8 bits.
+  // (zero length is allowed).
+  static Status ErrorInvalidAesGcmTagLength();
+
+  // The "publicExponent" used to generate a key was invalid: either no bytes
+  // were specified, or the number was too large to fit into an "unsigned long"
+  // (implemention limitation), or the exponent was zero.
+  static Status ErrorGenerateKeyPublicExponent();
+
+  // The algorithm was null when importing a raw-formatted key. In this case it
+  // is required.
+  static Status ErrorMissingAlgorithmImportRawKey();
+
+  // The modulus bytes were empty when importing an RSA public key.
+  static Status ErrorImportRsaEmptyModulus();
+
+  // The the modulus length was zero bits when generating an RSA public key.
+  static Status ErrorGenerateRsaZeroModulus();
+
+  // The exponent bytes were empty when importing an RSA public key.
+  static Status ErrorImportRsaEmptyExponent();
+
+  // An unextractable key was used by an operation which exports the key data.
+  static Status ErrorKeyNotExtractable();
+
+  // The key length specified when generating a key was invalid. Either it was
+  // zero, or it was not a multiple of 8 bits.
+  static Status ErrorGenerateKeyLength();
+
+ private:
+  // |error_details_utf8| can be NULL to indicate there was no error.
+  // Otherwise it is a UTF-8 string literal (the pointer must remain valid).
+  explicit Status(const char* error_details_utf8);
+
+  const char* error_details_;
+};
+
 // Returns a pointer to the start of |data|, or NULL if it is empty. This is a
 // convenience function for getting the pointer, and should not be used beyond
 // the expected lifetime of |data|.
 CONTENT_EXPORT const uint8* Uint8VectorStart(const std::vector<uint8>& data);
 
 // Shrinks a WebArrayBuffer to a new size.
 // TODO(eroman): This works by re-allocating a new buffer. It would be better if
 //               the WebArrayBuffer could just be truncated instead.
 void ShrinkBuffer(blink::WebArrayBuffer* buffer, unsigned new_size);
 
@@ skipping 57 matching lines @@
     uint8 tag_length_bytes);
 
 // Returns the internal block size for SHA-*
 unsigned int ShaBlockSizeBytes(blink::WebCryptoAlgorithmId hash_id);
 
 }  // namespace webcrypto
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_WEBCRYPTO_WEBCRYPTO_UTIL_H_