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

content/child/webcrypto/platform_crypto_nss.cc

Index: content/child/webcrypto/platform_crypto_nss.cc
diff --git a/content/child/webcrypto/platform_crypto_nss.cc b/content/child/webcrypto/platform_crypto_nss.cc
index 478c7aaff73fbb6c873cdcab6465c6de80a146ec..a275c2849a26a49157a2cdc7d4f82c6b6bb478f4 100644
--- a/content/child/webcrypto/platform_crypto_nss.cc
+++ b/content/child/webcrypto/platform_crypto_nss.cc
@@ -519,33 +519,33 @@ Status DoUnwrapSymKeyAesKw(const CryptoData& wrapped_key_data,
   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();
+#if defined(USE_NSS)
+  if (!NSS_VersionCheck("3.16")) {
+    // 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.
+    // This workaround re-wraps the key and compares the result with the

[wtc, 2014/03/25 00:03:25]

This workaround is quite heavyweight.

Since we know NSS calls PORT_SetError(SEC_ERROR_BAD_DATA) before returning
successfully, would it work to call PORT_SetError(0) before calling
PK11_WrapSymKey, and checks PORT_GetError() == SEC_ERROR_BAD_DATA if
PK11_WrapSymKey returns SECSuccess?

[eroman, 2014/03/25 00:18:25]

Excellent idea! Done.

Indeed it looks from https://hg.mozilla.org/projects/nss/rev/753bb69e543c that
should be sufficient.

Using your workaround I can now also remove the valgrind suppression.

+    // 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();