[webcrypto] Simplify the AES-KW workaround for NSS and remove valgrind suppression

content/child/webcrypto/platform_crypto_nss.cc

 // 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.
 
 #include "content/child/webcrypto/platform_crypto.h"
 
 #include <cryptohi.h>
 #include <pk11pub.h>
+#include <secerr.h>
 #include <sechash.h>
 
 #include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "content/child/webcrypto/crypto_data.h"
 #include "content/child/webcrypto/status.h"
 #include "content/child/webcrypto/webcrypto_util.h"
 #include "crypto/nss_util.h"
@@ skipping 480 matching lines @@
   crypto::ScopedSECItem param_item(
       PK11_ParamFromIV(CKM_NSS_AES_KEY_WRAP, &iv_item));
   if (!param_item)
     return Status::ErrorUnexpected();
 
   SECItem cipher_text = MakeSECItemForBuffer(wrapped_key_data);
 
   // The plaintext length is always 64 bits less than the data size.
   const unsigned int plaintext_length = wrapped_key_data.byte_length() - 8;
 
+#if defined(USE_NSS)
+  PORT_SetError(0);

[wtc, 2014/03/25 02:02:01]

Nit: add a comment to note this is part of the workaround.

[eroman, 2014/03/25 02:07:05]

Done.

+#endif
+
   crypto::ScopedPK11SymKey new_key(PK11_UnwrapSymKey(wrapping_key->key(),
                                                      CKM_NSS_AES_KEY_WRAP,
                                                      param_item.get(),
                                                      &cipher_text,
                                                      mechanism,
                                                      flags,
                                                      plaintext_length));
+
+#if defined(USE_NSS)
+  // Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=981170
+  // which was fixed in NSS 3.16.0.
+  // If unwrap fails, NSS nevertheless returns a valid-looking PK11SymKey,
+  // with a reasonable length but with key data pointing to uninitialized
+  // memory.
+  // To understand this workaround see the fix for 981170:
+  // https://hg.mozilla.org/projects/nss/rev/753bb69e543c
+  if (!NSS_VersionCheck("3.16") && PORT_GetError() == SEC_ERROR_BAD_DATA)
+    return Status::Error();
+#endif

[wtc, 2014/03/25 02:02:01]

Nit: it may be better to put the workaround after the if (!new_key) check, on
line 539.

[eroman, 2014/03/25 02:07:05]

Done.

+
   // TODO(padolph): Use NSS PORT_GetError() and friends to report a more
   // accurate error, providing if doesn't leak any information to web pages
   // about other web crypto users, key details, etc.
   if (!new_key)
     return Status::Error();
 
-// TODO(padolph): Change to "defined(USE_NSS)" once the NSS fix for
-// https://bugzilla.mozilla.org/show_bug.cgi?id=981170 rolls into chromium.
-#if 1
-  // ------- Start NSS bug workaround
-  // Workaround for https://code.google.com/p/chromium/issues/detail?id=349939
-  // If unwrap fails, NSS nevertheless returns a valid-looking PK11SymKey, with
-  // a reasonable length but with key data pointing to uninitialized memory.
-  // This workaround re-wraps the key and compares the result with the incoming
-  // data, and fails if there is a difference. This prevents returning a bad key
-  // to the caller.
-  const unsigned int output_length = wrapped_key_data.byte_length();
-  std::vector<unsigned char> buffer(output_length, 0);
-  SECItem wrapped_key_item = MakeSECItemForBuffer(CryptoData(buffer));
-  if (SECSuccess != PK11_WrapSymKey(CKM_NSS_AES_KEY_WRAP,
-                                    param_item.get(),
-                                    wrapping_key->key(),
-                                    new_key.get(),
-                                    &wrapped_key_item)) {
-    return Status::Error();
-  }
-  if (wrapped_key_item.len != wrapped_key_data.byte_length() ||
-      memcmp(wrapped_key_item.data,
-             wrapped_key_data.bytes(),
-             wrapped_key_item.len) != 0) {
-    return Status::Error();
-  }
-// ------- End NSS bug workaround
-#endif
 
   *unwrapped_key = new_key.Pass();
   return Status::Success();
 }
 
 }  // namespace
 
 Status ImportKeyRaw(const blink::WebCryptoAlgorithm& algorithm,
                     const CryptoData& key_data,
                     bool extractable,
@@ skipping 763 matching lines @@
                                      key_algorithm,
                                      usage_mask);
   return Status::Success();
 }
 
 }  // namespace platform
 
 }  // namespace webcrypto
 
 }  // namespace content