[webcrypto] Add error messages for failed operations.

content/renderer/webcrypto/webcrypto_impl_nss.cc

 // Copyright 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.
 
 #include "content/renderer/webcrypto/webcrypto_impl.h"
 
 #include <cryptohi.h>
 #include <pk11pub.h>
 #include <sechash.h>
 
@@ skipping 94 matching lines @@
 
   PK11_EncryptDecryptFunction pk11_encrypt_func_;
   PK11_EncryptDecryptFunction pk11_decrypt_func_;
 };
 
 base::LazyInstance<AesGcmSupport>::Leaky g_aes_gcm_support =
     LAZY_INSTANCE_INITIALIZER;
 
 namespace content {
 
+using webcrypto::Status;

[Ryan Sleevi, 2014/01/24 01:21:50]

ditto re: using

+
 namespace {
 
 class SymKeyHandle : public blink::WebCryptoKeyHandle {
  public:
   explicit SymKeyHandle(crypto::ScopedPK11SymKey key) : key_(key.Pass()) {}
 
   PK11SymKey* key() { return key_.get(); }
 
  private:
   crypto::ScopedPK11SymKey key_;
@@ skipping 58 matching lines @@
     case blink::WebCryptoAlgorithmIdSha384:
       return CKM_SHA384_HMAC;
     case blink::WebCryptoAlgorithmIdSha512:
       return CKM_SHA512_HMAC;
     default:
       // Not a supported algorithm.
       return CKM_INVALID_MECHANISM;
   }
 }
 
-bool AesCbcEncryptDecrypt(
+Status AesCbcEncryptDecrypt(
     CK_ATTRIBUTE_TYPE operation,
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
   DCHECK_EQ(blink::WebCryptoAlgorithmIdAesCbc, algorithm.id());
   DCHECK_EQ(algorithm.id(), key.algorithm().id());
   DCHECK_EQ(blink::WebCryptoKeyTypeSecret, key.type());
   DCHECK(operation == CKA_ENCRYPT || operation == CKA_DECRYPT);
 
   SymKeyHandle* sym_key = reinterpret_cast<SymKeyHandle*>(key.handle());
 
   const blink::WebCryptoAesCbcParams* params = algorithm.aesCbcParams();
   if (params->iv().size() != AES_BLOCK_SIZE)
-    return false;
+    return Status::ErrorIncorrectSizedIv();
 
   SECItem iv_item;
   iv_item.type = siBuffer;
   iv_item.data = const_cast<unsigned char*>(params->iv().data());
   iv_item.len = params->iv().size();
 
   crypto::ScopedSECItem param(PK11_ParamFromIV(CKM_AES_CBC_PAD, &iv_item));
   if (!param)
-    return false;
+    return Status::Error();
 
   crypto::ScopedPK11Context context(PK11_CreateContextBySymKey(
       CKM_AES_CBC_PAD, operation, sym_key->key(), param.get()));
 
   if (!context.get())
-    return false;
+    return Status::Error();
 
   // Oddly PK11_CipherOp takes input and output lengths as "int" rather than
   // "unsigned". Do some checks now to avoid integer overflowing.
   if (data_size >= INT_MAX - AES_BLOCK_SIZE) {
     // TODO(eroman): Handle this by chunking the input fed into NSS. Right now
     // it doesn't make much difference since the one-shot API would end up
     // blowing out the memory and crashing anyway. However a newer version of
     // the spec allows for a sequence<CryptoData> so this will be relevant.
-    return false;
+    return Status::ErrorDataTooBig();
   }
 
   // PK11_CipherOp does an invalid memory access when given empty decryption
   // input, or input which is not a multiple of the block size. See also
   // https://bugzilla.mozilla.com/show_bug.cgi?id=921687.
   if (operation == CKA_DECRYPT &&
       (data_size == 0 || (data_size % AES_BLOCK_SIZE != 0))) {
-    return false;
+    return Status::Error();
   }
 
   // TODO(eroman): Refine the output buffer size. It can be computed exactly for
   //               encryption, and can be smaller for decryption.
   unsigned output_max_len = data_size + AES_BLOCK_SIZE;
   CHECK_GT(output_max_len, data_size);
 
   *buffer = blink::WebArrayBuffer::create(output_max_len, 1);
 
   unsigned char* buffer_data = reinterpret_cast<unsigned char*>(buffer->data());
 
   int output_len;
   if (SECSuccess != PK11_CipherOp(context.get(),
                                   buffer_data,
                                   &output_len,
                                   buffer->byteLength(),
                                   data,
                                   data_size)) {
-    return false;
+    return Status::Error();
   }
 
   unsigned int final_output_chunk_len;
   if (SECSuccess != PK11_DigestFinal(context.get(),
                                      buffer_data + output_len,
                                      &final_output_chunk_len,
                                      output_max_len - output_len)) {
-    return false;
+    return Status::Error();
   }
 
   webcrypto::ShrinkBuffer(buffer, final_output_chunk_len + output_len);
-  return true;
+  return Status::Success();
 }
 
 // Helper to either encrypt or decrypt for AES-GCM. The result of encryption is
 // the concatenation of the ciphertext and the authentication tag. Similarly,
 // this is the expectation for the input to decryption.
-bool AesGcmEncryptDecrypt(
+Status AesGcmEncryptDecrypt(
     bool encrypt,
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
   DCHECK_EQ(blink::WebCryptoAlgorithmIdAesGcm, algorithm.id());
   DCHECK_EQ(algorithm.id(), key.algorithm().id());
   DCHECK_EQ(blink::WebCryptoKeyTypeSecret, key.type());
 
   if (!g_aes_gcm_support.Get().IsSupported())
-    return false;
+    return Status::ErrorUnsupported();
 
   SymKeyHandle* sym_key = reinterpret_cast<SymKeyHandle*>(key.handle());
 
   const blink::WebCryptoAesGcmParams* params = algorithm.aesGcmParams();
-  if (!params)
-    return false;
+  if (!params) {
+    return Status::ErrorUnexpected();
+  }

[Ryan Sleevi, 2014/01/24 01:21:50]

I'm not sure why you added braces here, especially considering lines like
291/292 or 335/336.

I agree there's inconsistency, but if the goal is to resolve that, this addition
doesn't.

[eroman, 2014/01/24 03:19:21]

Removed the braces; modifying existing style was not my intention. I verified
that the other lines being edited don't change the brace style.

I can do a style sweep and switch everything to one style or another as a
seprate CL if you want, but both styles are allowed by the styleguide.

[Ryan Sleevi, 2014/01/24 20:27:01]

I'm aware both styles are permitted, and don't really care other than that it's
consistent within it's area (for example, all of net/ discourages it, with the
exception of the shared QUIC bits -_-)

Per http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml 's
background, all I care about is consistency - within a file and within a
'module'.

[eroman, 2014/01/24 21:33:10]

Sure, I'll do a separate pass to resolve this inconsistency.

 
   // TODO(eroman): The spec doesn't define the default value. Assume 128 for now
   // since that is the maximum tag length:
   // http://www.w3.org/2012/webcrypto/track/issues/46
   unsigned tag_length_bits = 128;
   if (params->hasTagLengthBits()) {
     tag_length_bits = params->optionalTagLengthBits();
   }
 
-  if (tag_length_bits > 128) {
-    return false;
+  if (tag_length_bits > 128 || tag_length_bits % 8 != 0) {

[Ryan Sleevi, 2014/01/24 01:21:50]

nit: add parens for clarity (tag_length_bits % 8) != 0

[eroman, 2014/01/24 03:19:21]

Done.

+    return Status::ErrorInvalidAesGcmTagLength();
   }
-
-  if (tag_length_bits % 8 != 0)
-    return false;
   unsigned tag_length_bytes = tag_length_bits / 8;
 
   CK_GCM_PARAMS gcm_params = {0};
   gcm_params.pIv =
       const_cast<unsigned char*>(algorithm.aesGcmParams()->iv().data());
   gcm_params.ulIvLen = algorithm.aesGcmParams()->iv().size();
 
   gcm_params.pAAD =
       const_cast<unsigned char*>(params->optionalAdditionalData().data());
   gcm_params.ulAADLen = params->optionalAdditionalData().size();
 
   gcm_params.ulTagBits = tag_length_bits;
 
   SECItem param;
   param.type = siBuffer;
   param.data = reinterpret_cast<unsigned char*>(&gcm_params);
   param.len = sizeof(gcm_params);
 
   unsigned buffer_size = 0;
 
   // Calculate the output buffer size.
   if (encrypt) {
     // TODO(eroman): This is ugly, abstract away the safe integer arithmetic.
     if (data_size > (UINT_MAX - tag_length_bytes))
-      return false;
+      return Status::ErrorDataTooBig();
     buffer_size = data_size + tag_length_bytes;
   } else {
     // TODO(eroman): In theory the buffer allocated for the plain text should be
     // sized as |data_size - tag_length_bytes|.
     //
     // However NSS has a bug whereby it will fail if the output buffer size is
     // not at least as large as the ciphertext:
     //
     // https://bugzilla.mozilla.org/show_bug.cgi?id=%20853674
     //
     // From the analysis of that bug it looks like it might be safe to pass a
     // correctly sized buffer but lie about its size. Since resizing the
     // WebCryptoArrayBuffer is expensive that hack may be worth looking into.
     buffer_size = data_size;
   }
 
   *buffer = blink::WebArrayBuffer::create(buffer_size, 1);
   unsigned char* buffer_data = reinterpret_cast<unsigned char*>(buffer->data());
 
   PK11_EncryptDecryptFunction func =
     encrypt ? g_aes_gcm_support.Get().pk11_encrypt_func() :
               g_aes_gcm_support.Get().pk11_decrypt_func();
 
   unsigned int output_len = 0;
   SECStatus result = func(sym_key->key(), CKM_AES_GCM, &param,
                           buffer_data, &output_len, buffer->byteLength(),
                           data, data_size);
 
   if (result != SECSuccess)
-    return false;
+    return Status::Error();
 
   // Unfortunately the buffer needs to be shrunk for decryption (see the NSS bug
   // above).
   webcrypto::ShrinkBuffer(buffer, output_len);
 
-  return true;
+  return Status::Success();
 }
 
 CK_MECHANISM_TYPE WebCryptoAlgorithmToGenMechanism(
     const blink::WebCryptoAlgorithm& algorithm) {
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdAesCbc:
     case blink::WebCryptoAlgorithmIdAesGcm:
     case blink::WebCryptoAlgorithmIdAesKw:
       return CKM_AES_KEY_GEN;
     case blink::WebCryptoAlgorithmIdHmac:
@@ skipping 24 matching lines @@
   }
   return true;
 }
 
 bool IsAlgorithmRsa(const blink::WebCryptoAlgorithm& algorithm) {
   return algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5 ||
          algorithm.id() == blink::WebCryptoAlgorithmIdRsaOaep ||
          algorithm.id() == blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5;
 }
 
-bool ImportKeyInternalRaw(
+Status ImportKeyInternalRaw(
     const unsigned char* key_data,
     unsigned key_data_size,
     const blink::WebCryptoAlgorithm& algorithm,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
   DCHECK(!algorithm.isNull());
 
   blink::WebCryptoKeyType type;
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdHmac:
     case blink::WebCryptoAlgorithmIdAesCbc:
     case blink::WebCryptoAlgorithmIdAesKw:
     case blink::WebCryptoAlgorithmIdAesGcm:
       type = blink::WebCryptoKeyTypeSecret;
       break;
     // TODO(bryaneyler): Support more key types.
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 
   // TODO(bryaneyler): Need to split handling for symmetric and asymmetric keys.
   // Currently only supporting symmetric.
   CK_MECHANISM_TYPE mechanism = CKM_INVALID_MECHANISM;
   // Flags are verified at the Blink layer; here the flags are set to all
   // possible operations for this key type.
   CK_FLAGS flags = 0;
 
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdHmac: {
       const blink::WebCryptoHmacParams* params = algorithm.hmacParams();
       if (!params) {
-        return false;
+        return Status::ErrorUnexpected();
       }
 
       mechanism = WebCryptoHashToHMACMechanism(params->hash());
       if (mechanism == CKM_INVALID_MECHANISM) {
-        return false;
+        return Status::ErrorUnsupported();
       }
 
       flags |= CKF_SIGN | CKF_VERIFY;
 
       break;
     }
     case blink::WebCryptoAlgorithmIdAesCbc: {
       mechanism = CKM_AES_CBC;
       flags |= CKF_ENCRYPT | CKF_DECRYPT;
       break;
     }
     case blink::WebCryptoAlgorithmIdAesKw: {
       mechanism = CKM_NSS_AES_KEY_WRAP;
       flags |= CKF_WRAP | CKF_WRAP;
       break;
     }
     case blink::WebCryptoAlgorithmIdAesGcm: {
       if (!g_aes_gcm_support.Get().IsSupported())
-        return false;
+        return Status::ErrorUnsupported();
       mechanism = CKM_AES_GCM;
       flags |= CKF_ENCRYPT | CKF_DECRYPT;
       break;
     }
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 
   DCHECK_NE(CKM_INVALID_MECHANISM, mechanism);
   DCHECK_NE(0ul, flags);
 
   SECItem key_item = {
       siBuffer,
       const_cast<unsigned char*>(key_data),
       key_data_size
   };
 
   crypto::ScopedPK11Slot slot(PK11_GetInternalSlot());
   crypto::ScopedPK11SymKey pk11_sym_key(
       PK11_ImportSymKeyWithFlags(slot.get(),
                                  mechanism,
                                  PK11_OriginUnwrap,
                                  CKA_FLAGS_ONLY,
                                  &key_item,
                                  flags,
                                  false,
                                  NULL));
   if (!pk11_sym_key.get()) {
-    return false;
+    return Status::Error();
   }
 
   *key = blink::WebCryptoKey::create(new SymKeyHandle(pk11_sym_key.Pass()),
                                       type, extractable, algorithm, usage_mask);
-  return true;
+  return Status::Success();
 }
 
-bool ExportKeyInternalRaw(
+Status ExportKeyInternalRaw(
     const blink::WebCryptoKey& key,
     blink::WebArrayBuffer* buffer) {
 
   DCHECK(key.handle());
   DCHECK(buffer);
 
-  if (key.type() != blink::WebCryptoKeyTypeSecret || !key.extractable())
-    return false;
+  if (!key.extractable())
+    return Status::ErrorKeyNotExtractable();
+  if (key.type() != blink::WebCryptoKeyTypeSecret)
+    return Status::ErrorUnexpectedKeyType();
 
   SymKeyHandle* sym_key = reinterpret_cast<SymKeyHandle*>(key.handle());
 
   if (PK11_ExtractKeyValue(sym_key->key()) != SECSuccess)
-    return false;
+    return Status::Error();
 
   const SECItem* key_data = PK11_GetKeyData(sym_key->key());
   if (!key_data)
-    return false;
+    return Status::Error();
 
   *buffer = webcrypto::CreateArrayBuffer(key_data->data, key_data->len);
 
-  return true;
+  return Status::Success();
 }
 
 typedef scoped_ptr<CERTSubjectPublicKeyInfo,
                    crypto::NSSDestroyer<CERTSubjectPublicKeyInfo,
                                         SECKEY_DestroySubjectPublicKeyInfo> >
     ScopedCERTSubjectPublicKeyInfo;
 
 // Validates an NSS KeyType against a WebCrypto algorithm. Some NSS KeyTypes
 // contain enough information to fabricate a Web Crypto algorithm, which is
 // returned if the input algorithm isNull(). This function indicates failure by
@@ skipping 17 matching lines @@
     case rsaPssKey:
     case rsaOaepKey:
       // TODO(padolph): Handle other key types.
       break;
     default:
       break;
   }
   return blink::WebCryptoAlgorithm::createNull();
 }
 
-bool ImportKeyInternalSpki(
+Status ImportKeyInternalSpki(
     const unsigned char* key_data,
     unsigned key_data_size,
     const blink::WebCryptoAlgorithm& algorithm_or_null,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
   DCHECK(key);
 
   if (!key_data_size)
-    return false;
+    return Status::ErrorEmptyKeyData();
   DCHECK(key_data);
 
   // The binary blob 'key_data' is expected to be a DER-encoded ASN.1 Subject
   // Public Key Info. Decode this to a CERTSubjectPublicKeyInfo.
   SECItem spki_item = {siBuffer, const_cast<uint8*>(key_data), key_data_size};
   const ScopedCERTSubjectPublicKeyInfo spki(
       SECKEY_DecodeDERSubjectPublicKeyInfo(&spki_item));
   if (!spki)
-    return false;
+    return Status::Error();
 
   crypto::ScopedSECKEYPublicKey sec_public_key(
       SECKEY_ExtractPublicKey(spki.get()));
   if (!sec_public_key)
-    return false;
+    return Status::Error();
 
   const KeyType sec_key_type = SECKEY_GetPublicKeyType(sec_public_key.get());
   blink::WebCryptoAlgorithm algorithm =
       ResolveNssKeyTypeWithInputAlgorithm(sec_key_type, algorithm_or_null);
   if (algorithm.isNull())
-    return false;
+    return Status::Error();
 
   *key = blink::WebCryptoKey::create(
       new PublicKeyHandle(sec_public_key.Pass()),
       blink::WebCryptoKeyTypePublic,
       extractable,
       algorithm,
       usage_mask);
 
-  return true;
+  return Status::Success();
 }
 
-bool ExportKeyInternalSpki(
+Status ExportKeyInternalSpki(
     const blink::WebCryptoKey& key,
     blink::WebArrayBuffer* buffer) {
 
   DCHECK(key.handle());
   DCHECK(buffer);
 
-  if (key.type() != blink::WebCryptoKeyTypePublic || !key.extractable())
-    return false;
+  if (!key.extractable())
+    return Status::ErrorKeyNotExtractable();
+  if (key.type() != blink::WebCryptoKeyTypePublic)
+    return Status::ErrorUnexpectedKeyType();
 
   PublicKeyHandle* const pub_key =
       reinterpret_cast<PublicKeyHandle*>(key.handle());
 
   const crypto::ScopedSECItem spki_der(
       SECKEY_EncodeDERSubjectPublicKeyInfo(pub_key->key()));
   if (!spki_der)
-    return false;
+    return Status::Error();
 
   DCHECK(spki_der->data);
   DCHECK(spki_der->len);
 
   *buffer = webcrypto::CreateArrayBuffer(spki_der->data, spki_der->len);
 
-  return true;
+  return Status::Success();
 }
 
-bool ImportKeyInternalPkcs8(
+Status ImportKeyInternalPkcs8(
     const unsigned char* key_data,
     unsigned key_data_size,
     const blink::WebCryptoAlgorithm& algorithm_or_null,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
   DCHECK(key);
 
   if (!key_data_size)
-    return false;
+    return Status::ErrorEmptyKeyData();
   DCHECK(key_data);
 
   // The binary blob 'key_data' is expected to be a DER-encoded ASN.1 PKCS#8
   // private key info object.
   SECItem pki_der = {siBuffer, const_cast<uint8*>(key_data), key_data_size};
 
   SECKEYPrivateKey* seckey_private_key = NULL;
   crypto::ScopedPK11Slot slot(PK11_GetInternalSlot());
   if (PK11_ImportDERPrivateKeyInfoAndReturnKey(
           slot.get(),
           &pki_der,
           NULL,  // nickname
           NULL,  // publicValue
           false,  // isPerm
           false,  // isPrivate
           KU_ALL,  // usage
           &seckey_private_key,
           NULL) != SECSuccess) {
-    return false;
+    return Status::Error();
   }
   DCHECK(seckey_private_key);
   crypto::ScopedSECKEYPrivateKey private_key(seckey_private_key);
 
   const KeyType sec_key_type = SECKEY_GetPrivateKeyType(private_key.get());
   blink::WebCryptoAlgorithm algorithm =
       ResolveNssKeyTypeWithInputAlgorithm(sec_key_type, algorithm_or_null);
   if (algorithm.isNull())
-    return false;
+    return Status::Error();
 
   *key = blink::WebCryptoKey::create(
       new PrivateKeyHandle(private_key.Pass()),
       blink::WebCryptoKeyTypePrivate,
       extractable,
       algorithm,
       usage_mask);
 
-  return true;
+  return Status::Success();
 }
 
 }  // namespace
 
 void WebCryptoImpl::Init() {
   crypto::EnsureNSSInit();
 }
 
-bool WebCryptoImpl::EncryptInternal(
+Status WebCryptoImpl::EncryptInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
 
   DCHECK_EQ(algorithm.id(), key.algorithm().id());
   DCHECK(key.handle());
   DCHECK(buffer);
 
   // TODO(eroman): Use a switch() statement.
   if (algorithm.id() == blink::WebCryptoAlgorithmIdAesCbc) {
     return AesCbcEncryptDecrypt(
         CKA_ENCRYPT, algorithm, key, data, data_size, buffer);
   } else if (algorithm.id() == blink::WebCryptoAlgorithmIdAesGcm) {
     return AesGcmEncryptDecrypt(
         true, algorithm, key, data, data_size, buffer);
   } else if (algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5) {
 
     // RSAES encryption does not support empty input
     if (!data_size)
-      return false;
+      return Status::ErrorEmptyData();
     DCHECK(data);
 
     if (key.type() != blink::WebCryptoKeyTypePublic)
-      return false;
+      return Status::ErrorUnexpectedKeyType();
 
     PublicKeyHandle* const public_key =
         reinterpret_cast<PublicKeyHandle*>(key.handle());
 
     const unsigned encrypted_length_bytes =
         SECKEY_PublicKeyStrength(public_key->key());
 
     // RSAES can operate on messages up to a length of k - 11, where k is the
     // octet length of the RSA modulus.
     if (encrypted_length_bytes < 11 || encrypted_length_bytes - 11 < data_size)
-      return false;
+      return Status::ErrorDataTooBig();
 
     *buffer = blink::WebArrayBuffer::create(encrypted_length_bytes, 1);
     unsigned char* const buffer_data =
         reinterpret_cast<unsigned char*>(buffer->data());
 
     if (PK11_PubEncryptPKCS1(public_key->key(),
                              buffer_data,
                              const_cast<unsigned char*>(data),
                              data_size,
                              NULL) != SECSuccess) {
-      return false;
+      return Status::Error();
     }
-    return true;
+    return Status::Success();
   }
 
-  return false;
+  return Status::ErrorUnsupported();
 }
 
-bool WebCryptoImpl::DecryptInternal(
+Status WebCryptoImpl::DecryptInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
 
   DCHECK_EQ(algorithm.id(), key.algorithm().id());
   DCHECK(key.handle());
   DCHECK(buffer);
 
   // TODO(eroman): Use a switch() statement.
   if (algorithm.id() == blink::WebCryptoAlgorithmIdAesCbc) {
     return AesCbcEncryptDecrypt(
         CKA_DECRYPT, algorithm, key, data, data_size, buffer);
   } else if (algorithm.id() == blink::WebCryptoAlgorithmIdAesGcm) {
     return AesGcmEncryptDecrypt(
         false, algorithm, key, data, data_size, buffer);
   } else if (algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5) {
 
     // RSAES decryption does not support empty input
     if (!data_size)
-      return false;
+      return Status::ErrorEmptyData();
     DCHECK(data);
 
     if (key.type() != blink::WebCryptoKeyTypePrivate)
-      return false;
+      return Status::ErrorUnexpectedKeyType();
 
     PrivateKeyHandle* const private_key =
         reinterpret_cast<PrivateKeyHandle*>(key.handle());
 
     const int modulus_length_bytes =
         PK11_GetPrivateModulusLen(private_key->key());
     if (modulus_length_bytes <= 0)
-      return false;
+      return Status::ErrorEmptyModulus();
     const unsigned max_output_length_bytes = modulus_length_bytes;
 
     *buffer = blink::WebArrayBuffer::create(max_output_length_bytes, 1);
     unsigned char* const buffer_data =
         reinterpret_cast<unsigned char*>(buffer->data());
 
     unsigned output_length_bytes = 0;
     if (PK11_PrivDecryptPKCS1(private_key->key(),
                               buffer_data,
                               &output_length_bytes,
                               max_output_length_bytes,
                               const_cast<unsigned char*>(data),
                               data_size) != SECSuccess) {
-      return false;
+      return Status::Error();
     }
     DCHECK_LE(output_length_bytes, max_output_length_bytes);
     webcrypto::ShrinkBuffer(buffer, output_length_bytes);
-    return true;
+    return Status::Success();
   }
 
-  return false;
+  return Status::ErrorUnsupported();
 }
 
-bool WebCryptoImpl::DigestInternal(
+Status WebCryptoImpl::DigestInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
   HASH_HashType hash_type = WebCryptoAlgorithmToNSSHashType(algorithm);
   if (hash_type == HASH_AlgNULL) {
-    return false;
+    return Status::ErrorUnsupported();
   }
 
   HASHContext* context = HASH_Create(hash_type);
   if (!context) {
-    return false;
+    return Status::Error();
   }
 
   HASH_Begin(context);
 
   HASH_Update(context, data, data_size);
 
   unsigned hash_result_length = HASH_ResultLenContext(context);
   DCHECK_LE(hash_result_length, static_cast<size_t>(HASH_LENGTH_MAX));
 
   *buffer = blink::WebArrayBuffer::create(hash_result_length, 1);
 
   unsigned char* digest = reinterpret_cast<unsigned char*>(buffer->data());
 
   unsigned result_length = 0;
   HASH_End(context, digest, &result_length, hash_result_length);
 
   HASH_Destroy(context);
 
-  return result_length == hash_result_length;
+  if (result_length != hash_result_length)
+    return Status::ErrorUnexpected();

[Ryan Sleevi, 2014/01/24 01:21:50]

Note: This strikes me as a dangerous pattern (even if pre-existing), because it
leaves me unclear what the status of |*buffer| is (in this case, it refers to a
buffer I'm supposed to free, even though things failed). Is it actually freed?

[eroman, 2014/01/24 03:19:21]

WebArrayBuffer manages the underlying blink buffers, so there are no leaks here.
Once the caller's WebArrayBuffer goes out of scope, it will release the final
refcount on the WTF::ArrayBuffer.

As far as the bigger question on contract, the current contract is that the
caller provides a WebArrayBuffer whose contents are replaced with the result of
the operation. In the case of failures, the contents may still have been
replaced, however an error status is returned indicating that it is not
relevant. We could guarantee that the provided WebArrayBuffer is only mutated on
success, but that seems like more work for no gain.

[Ryan Sleevi, 2014/01/24 20:27:01]

Reasonable.

My concern here with this pattern is trying to find a good API contract that can
ensure that even if an operation fails, the buffer contents won't leak back into
the Blink side, since that would be Real Bad (mmkay).

From my quick can, it looked like you were always stack-allocating a
blink::WebArrayBuffer in the !Internal calls (eg: WebCrypto::Digest), so this
should be fine, I just want to request you double check that's the case.

[eroman, 2014/01/24 21:33:10]

Correct. It is only possible to stack allocate blink::WebArrayBuffer so I am not
concerned about leaks. I have reviewed the code and I don't see any issues.

+
+  return Status::Success();
 }
 
-bool WebCryptoImpl::GenerateKeyInternal(
+Status WebCryptoImpl::GenerateKeyInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
   CK_MECHANISM_TYPE mech = WebCryptoAlgorithmToGenMechanism(algorithm);
   unsigned int keylen_bytes = 0;
   blink::WebCryptoKeyType key_type = blink::WebCryptoKeyTypeSecret;
 
   if (mech == CKM_INVALID_MECHANISM) {
-    return false;
+    return Status::ErrorUnsupported();
   }
 
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdAesCbc:
     case blink::WebCryptoAlgorithmIdAesGcm:
     case blink::WebCryptoAlgorithmIdAesKw: {
       const blink::WebCryptoAesKeyGenParams* params =
           algorithm.aesKeyGenParams();
       DCHECK(params);
       // Ensure the key length is a multiple of 8 bits. Let NSS verify further
       // algorithm-specific length restrictions.
       if (params->lengthBits() % 8)
-        return false;
+        return Status::ErrorKeyLength();
       keylen_bytes = params->lengthBits() / 8;
       key_type = blink::WebCryptoKeyTypeSecret;
       break;
     }
     case blink::WebCryptoAlgorithmIdHmac: {
       const blink::WebCryptoHmacKeyParams* params = algorithm.hmacKeyParams();
       DCHECK(params);
       if (params->hasLengthBytes()) {
         keylen_bytes = params->optionalLengthBytes();
       } else {
         keylen_bytes = webcrypto::ShaBlockSizeBytes(params->hash().id());
       }
 
       key_type = blink::WebCryptoKeyTypeSecret;
       break;
     }
 
     default: {
-      return false;
+      return Status::ErrorUnsupported();
     }
   }
 
   if (keylen_bytes == 0) {
-    return false;
+    return Status::ErrorKeyLength();
   }
 
   crypto::ScopedPK11Slot slot(PK11_GetInternalKeySlot());
   if (!slot) {
-    return false;
+    return Status::Error();
   }
 
   crypto::ScopedPK11SymKey pk11_key(
       PK11_KeyGen(slot.get(), mech, NULL, keylen_bytes, NULL));
 
   if (!pk11_key) {
-    return false;
+    return Status::Error();
   }
 
   *key = blink::WebCryptoKey::create(
       new SymKeyHandle(pk11_key.Pass()),
       key_type, extractable, algorithm, usage_mask);
-  return true;
+  return Status::Success();
 }
 
-bool WebCryptoImpl::GenerateKeyPairInternal(
-    const blink::WebCryptoAlgorithm& algorithm,
+Status WebCryptoImpl::GenerateKeyPairInternal(
+   const blink::WebCryptoAlgorithm& algorithm,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* public_key,
     blink::WebCryptoKey* private_key) {
 
   // TODO(padolph): Handle other asymmetric algorithm key generation.
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5:
     case blink::WebCryptoAlgorithmIdRsaOaep:
     case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5: {
       const blink::WebCryptoRsaKeyGenParams* const params =
           algorithm.rsaKeyGenParams();
       DCHECK(params);
 
       crypto::ScopedPK11Slot slot(PK11_GetInternalKeySlot());
+      if (!slot)
+        return Status::Error();
+
       unsigned long public_exponent;
-      if (!slot || !params->modulusLengthBits() ||
-          !BigIntegerToLong(params->publicExponent().data(),
+      if (!params->modulusLengthBits())
+        return Status::ErrorEmptyModulus();
+
+      if (!BigIntegerToLong(params->publicExponent().data(),
                             params->publicExponent().size(),
-                            &public_exponent) ||
-          !public_exponent) {
-        return false;
+                            &public_exponent) || !public_exponent) {
+        return Status::ErrorPublicExponent();
       }
 
       PK11RSAGenParams rsa_gen_params;
       rsa_gen_params.keySizeInBits = params->modulusLengthBits();
       rsa_gen_params.pe = public_exponent;
 
       // Flags are verified at the Blink layer; here the flags are set to all
       // possible operations for the given key type.
       CK_FLAGS operation_flags;
       switch (algorithm.id()) {
         case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5:
         case blink::WebCryptoAlgorithmIdRsaOaep:
           operation_flags = CKF_ENCRYPT | CKF_DECRYPT | CKF_WRAP | CKF_UNWRAP;
           break;
         case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5:
           operation_flags = CKF_SIGN | CKF_VERIFY;
           break;
         default:
           NOTREACHED();
-          return false;
+          return Status::ErrorUnexpected();
       }
       const CK_FLAGS operation_flags_mask = CKF_ENCRYPT | CKF_DECRYPT |
                                             CKF_SIGN | CKF_VERIFY | CKF_WRAP |
                                             CKF_UNWRAP;
       const PK11AttrFlags attribute_flags = 0;  // Default all PK11_ATTR_ flags.
 
       // Note: NSS does not generate an sec_public_key if the call below fails,
       // so there is no danger of a leaked sec_public_key.
       SECKEYPublicKey* sec_public_key;
       crypto::ScopedSECKEYPrivateKey scoped_sec_private_key(
           PK11_GenerateKeyPairWithOpFlags(slot.get(),
                                           CKM_RSA_PKCS_KEY_PAIR_GEN,
                                           &rsa_gen_params,
                                           &sec_public_key,
                                           attribute_flags,
                                           operation_flags,
                                           operation_flags_mask,
                                           NULL));
       if (!private_key) {
-        return false;
+        return Status::Error();
       }
 
       *public_key = blink::WebCryptoKey::create(
           new PublicKeyHandle(crypto::ScopedSECKEYPublicKey(sec_public_key)),
           blink::WebCryptoKeyTypePublic,
           true,
           algorithm,
           usage_mask);
       *private_key = blink::WebCryptoKey::create(
           new PrivateKeyHandle(scoped_sec_private_key.Pass()),
           blink::WebCryptoKeyTypePrivate,
           extractable,
           algorithm,
           usage_mask);
 
-      return true;
+      return Status::Success();
     }
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 }
 
-bool WebCryptoImpl::ImportKeyInternal(
+Status WebCryptoImpl::ImportKeyInternal(
     blink::WebCryptoKeyFormat format,
     const unsigned char* key_data,
     unsigned key_data_size,
     const blink::WebCryptoAlgorithm& algorithm_or_null,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
   switch (format) {
     case blink::WebCryptoKeyFormatRaw:
       // A 'raw'-formatted key import requires an input algorithm.
       if (algorithm_or_null.isNull())
-        return false;
+        return Status::ErrorMissingAlgorithmRawKey();
       return ImportKeyInternalRaw(key_data,
                                   key_data_size,
                                   algorithm_or_null,
                                   extractable,
                                   usage_mask,
                                   key);
     case blink::WebCryptoKeyFormatSpki:
       return ImportKeyInternalSpki(key_data,
                                    key_data_size,
                                    algorithm_or_null,
                                    extractable,
                                    usage_mask,
                                    key);
     case blink::WebCryptoKeyFormatPkcs8:
       return ImportKeyInternalPkcs8(key_data,
                                     key_data_size,
                                     algorithm_or_null,
                                     extractable,
                                     usage_mask,
                                     key);
     default:
       // NOTE: blink::WebCryptoKeyFormatJwk is handled one level above.
-      return false;
+      return Status::ErrorUnsupported();
   }
 }
 
-bool WebCryptoImpl::ExportKeyInternal(
+Status WebCryptoImpl::ExportKeyInternal(
     blink::WebCryptoKeyFormat format,
     const blink::WebCryptoKey& key,
     blink::WebArrayBuffer* buffer) {
   switch (format) {
     case blink::WebCryptoKeyFormatRaw:
       return ExportKeyInternalRaw(key, buffer);
     case blink::WebCryptoKeyFormatSpki:
       return ExportKeyInternalSpki(key, buffer);
     case blink::WebCryptoKeyFormatPkcs8:
       // TODO(padolph): Implement pkcs8 export
-      return false;
+      return Status::ErrorUnsupported();
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 }
 
-bool WebCryptoImpl::SignInternal(
+Status WebCryptoImpl::SignInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* data,
     unsigned data_size,
     blink::WebArrayBuffer* buffer) {
 
   // Note: It is not an error to sign empty data.
 
   DCHECK(buffer);
   DCHECK_NE(0, key.usages() & blink::WebCryptoKeyUsageSign);
 
   blink::WebArrayBuffer result;
 
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdHmac: {
       const blink::WebCryptoHmacParams* params = algorithm.hmacParams();
       if (!params) {
-        return false;
+        return Status::ErrorUnexpected();
       }
 
       SymKeyHandle* sym_key = reinterpret_cast<SymKeyHandle*>(key.handle());
 
       DCHECK_EQ(PK11_GetMechanism(sym_key->key()),
                 WebCryptoHashToHMACMechanism(params->hash()));
 
       SECItem param_item = { siBuffer, NULL, 0 };
       SECItem data_item = {
         siBuffer,
         const_cast<unsigned char*>(data),
         data_size
       };
       // First call is to figure out the length.
       SECItem signature_item = { siBuffer, NULL, 0 };
 
       if (PK11_SignWithSymKey(sym_key->key(),
                               PK11_GetMechanism(sym_key->key()),
                               &param_item,
                               &signature_item,
                               &data_item) != SECSuccess) {
-        NOTREACHED();
-        return false;
+        return Status::Error();
       }
 
       DCHECK_NE(0u, signature_item.len);
 
       result = blink::WebArrayBuffer::create(signature_item.len, 1);
       signature_item.data = reinterpret_cast<unsigned char*>(result.data());
 
       if (PK11_SignWithSymKey(sym_key->key(),
                               PK11_GetMechanism(sym_key->key()),
                               &param_item,
                               &signature_item,
                               &data_item) != SECSuccess) {
-        NOTREACHED();
-        return false;
+        return Status::Error();
       }
 
       DCHECK_EQ(result.byteLength(), signature_item.len);
 
       break;
     }
     case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5: {
-      if (key.type() != blink::WebCryptoKeyTypePrivate ||
-          webcrypto::GetInnerHashAlgorithm(algorithm).isNull())
-        return false;
+      if (key.type() != blink::WebCryptoKeyTypePrivate)
+        return Status::ErrorUnexpectedKeyType();
+
+      if (webcrypto::GetInnerHashAlgorithm(algorithm).isNull())
+        return Status::ErrorUnexpected();
 
       PrivateKeyHandle* const private_key =
           reinterpret_cast<PrivateKeyHandle*>(key.handle());
       DCHECK(private_key);
       DCHECK(private_key->key());
 
       // Pick the NSS signing algorithm by combining RSA-SSA (RSA PKCS1) and the
       // inner hash of the input Web Crypto algorithm.
       SECOidTag sign_alg_tag;
       switch (webcrypto::GetInnerHashAlgorithm(algorithm).id()) {
         case blink::WebCryptoAlgorithmIdSha1:
           sign_alg_tag = SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION;
           break;
         case blink::WebCryptoAlgorithmIdSha224:
           sign_alg_tag = SEC_OID_PKCS1_SHA224_WITH_RSA_ENCRYPTION;
           break;
         case blink::WebCryptoAlgorithmIdSha256:
           sign_alg_tag = SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION;
           break;
         case blink::WebCryptoAlgorithmIdSha384:
           sign_alg_tag = SEC_OID_PKCS1_SHA384_WITH_RSA_ENCRYPTION;
           break;
         case blink::WebCryptoAlgorithmIdSha512:
           sign_alg_tag = SEC_OID_PKCS1_SHA512_WITH_RSA_ENCRYPTION;
           break;
         default:
-          return false;
+          return Status::ErrorUnsupported();
       }
 
       crypto::ScopedSECItem signature_item(SECITEM_AllocItem(NULL, NULL, 0));
       if (SEC_SignData(signature_item.get(),
                        data,
                        data_size,
                        private_key->key(),
                        sign_alg_tag) != SECSuccess) {
-        return false;
+        return Status::Error();
       }
 
       result = webcrypto::CreateArrayBuffer(signature_item->data,
                                             signature_item->len);
 
       break;
     }
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 
   *buffer = result;
-  return true;
+  return Status::Success();
 }
 
-bool WebCryptoImpl::VerifySignatureInternal(
+Status WebCryptoImpl::VerifySignatureInternal(
     const blink::WebCryptoAlgorithm& algorithm,
     const blink::WebCryptoKey& key,
     const unsigned char* signature,
     unsigned signature_size,
     const unsigned char* data,
     unsigned data_size,
     bool* signature_match) {
 
   if (!signature_size)
-    return false;
+    return Status::ErrorEmptySignatureData();
   DCHECK(signature);
 
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdHmac: {
       blink::WebArrayBuffer result;
-      if (!SignInternal(algorithm, key, data, data_size, &result)) {
-        return false;
+      Status status = SignInternal(algorithm, key, data, data_size, &result);
+      if (status.IsError()) {
+        return status;
       }
 
       // Handling of truncated signatures is underspecified in the WebCrypto
       // spec, so here we fail verification if a truncated signature is being
       // verified.
       // See https://www.w3.org/Bugs/Public/show_bug.cgi?id=23097
       *signature_match =
           result.byteLength() == signature_size &&
           crypto::SecureMemEqual(result.data(), signature, signature_size);
 
       break;
     }
     case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5: {
       if (key.type() != blink::WebCryptoKeyTypePublic)
-        return false;
+        return Status::ErrorUnexpectedKeyType();
 
       PublicKeyHandle* const public_key =
           reinterpret_cast<PublicKeyHandle*>(key.handle());
       DCHECK(public_key);
       DCHECK(public_key->key());
 
       const SECItem signature_item = {
           siBuffer,
           const_cast<unsigned char*>(signature),
           signature_size
@@ skipping 10 matching lines @@
         case blink::WebCryptoAlgorithmIdSha256:
           hash_alg_tag = SEC_OID_SHA256;
           break;
         case blink::WebCryptoAlgorithmIdSha384:
           hash_alg_tag = SEC_OID_SHA384;
           break;
         case blink::WebCryptoAlgorithmIdSha512:
           hash_alg_tag = SEC_OID_SHA512;
           break;
         default:
-          return false;
+          return Status::ErrorUnsupported();
       }
 
       *signature_match =
           SECSuccess == VFY_VerifyDataDirect(data,
                                              data_size,
                                              public_key->key(),
                                              &signature_item,
                                              SEC_OID_PKCS1_RSA_ENCRYPTION,
                                              hash_alg_tag,
                                              NULL,
                                              NULL);
 
       break;
     }
     default:
-      return false;
+      return Status::ErrorUnsupported();
   }
 
-  return true;
+  return Status::Success();
 }
 
-bool WebCryptoImpl::ImportRsaPublicKeyInternal(
+Status WebCryptoImpl::ImportRsaPublicKeyInternal(
     const unsigned char* modulus_data,
     unsigned modulus_size,
     const unsigned char* exponent_data,
     unsigned exponent_size,
     const blink::WebCryptoAlgorithm& algorithm,
     bool extractable,
     blink::WebCryptoKeyUsageMask usage_mask,
     blink::WebCryptoKey* key) {
 
-  if (!modulus_size || !exponent_size)
-    return false;
+  if (!modulus_size)
+    return Status::ErrorEmptyModulus();
+
+  if (!exponent_size)
+    return Status::ErrorEmptyExponent();
+
   DCHECK(modulus_data);
   DCHECK(exponent_data);
 
   // NSS does not provide a way to create an RSA public key directly from the
   // modulus and exponent values, but it can import an DER-encoded ASN.1 blob
   // with these values and create the public key from that. The code below
   // follows the recommendation described in
   // https://developer.mozilla.org/en-US/docs/NSS/NSS_Tech_Notes/nss_tech_note7
 
   // Pack the input values into a struct compatible with NSS ASN.1 encoding, and
@@ skipping 10 matching lines @@
   const SEC_ASN1Template rsa_public_key_template[] = {
       {SEC_ASN1_SEQUENCE, 0, NULL, sizeof(RsaPublicKeyData)},
       {SEC_ASN1_INTEGER, offsetof(RsaPublicKeyData, modulus), },
       {SEC_ASN1_INTEGER, offsetof(RsaPublicKeyData, exponent), },
       {0, }};
 
   // DER-encode the public key.
   crypto::ScopedSECItem pubkey_der(SEC_ASN1EncodeItem(
       NULL, NULL, &pubkey_in, rsa_public_key_template));
   if (!pubkey_der)
-    return false;
+    return Status::Error();
 
   // Import the DER-encoded public key to create an RSA SECKEYPublicKey.
   crypto::ScopedSECKEYPublicKey pubkey(
       SECKEY_ImportDERPublicKey(pubkey_der.get(), CKK_RSA));
   if (!pubkey)
-    return false;
+    return Status::Error();
 
   *key = blink::WebCryptoKey::create(new PublicKeyHandle(pubkey.Pass()),
                                      blink::WebCryptoKeyTypePublic,
                                      extractable,
                                      algorithm,
                                      usage_mask);
-  return true;
+  return Status::Success();
 }
 
 }  // namespace content