WebCrypto: Implement crypto.subtle.deriveKey (chromium-side).

content/child/webcrypto/openssl/ecdh_openssl.cc

Index: content/child/webcrypto/openssl/ecdh_openssl.cc
diff --git a/content/child/webcrypto/openssl/ecdh_openssl.cc b/content/child/webcrypto/openssl/ecdh_openssl.cc
index d6228cba802ea41a5d113439806043f354d2a333..342064aa67d2c42f7d619e8d060797194f95e12b 100644
--- a/content/child/webcrypto/openssl/ecdh_openssl.cc
+++ b/content/child/webcrypto/openssl/ecdh_openssl.cc
@@ -49,7 +49,8 @@ class EcdhImplementation : public EcAlgorithm {
 
   Status DeriveBits(const blink::WebCryptoAlgorithm& algorithm,
                     const blink::WebCryptoKey& base_key,
-                    unsigned int length_bits,
+                    bool has_optional_length_bits,
+                    unsigned int optional_length_bits,
                     std::vector<uint8_t>* derived_bytes) const override {
     if (base_key.type() != blink::WebCryptoKeyTypePrivate)
       return Status::ErrorUnexpectedKeyType();
@@ -78,13 +79,6 @@ class EcdhImplementation : public EcAlgorithm {
       return Status::ErrorEcdhCurveMismatch();
     }
 
-    // Handle the empty length case now to avoid calling an undefined
-    // |&derived_bytes->front()| later.
-    if (length_bits == 0) {
-      derived_bytes->clear();
-      return Status::Success();
-    }
-
     crypto::ScopedEC_KEY public_key_ec(
         EVP_PKEY_get1_EC_KEY(AsymKeyOpenSsl::Cast(public_key)->key()));
 
@@ -100,6 +94,18 @@ class EcdhImplementation : public EcAlgorithm {
     int field_size_bytes = NumBitsToBytes(
         EC_GROUP_get_degree(EC_KEY_get0_group(private_key_ec.get())));
 
+    // If a desired key length was not specified, default to the field size
+    // (rounded up to nearest byte).
+    unsigned int length_bits =
+        has_optional_length_bits ? optional_length_bits : field_size_bytes * 8;
+
+    // Handle the empty length case now to avoid calling an undefined
+    // |&derived_bytes->front()| later.

[Ryan Sleevi, 2014/12/10 03:21:26]

Is this really still applicable, given line 100? This only handles if the caller
sets Length Bits == 0, which is... not a key?

[eroman, 2014/12/10 17:58:55]

Calling deriveBits() with a length of zero is perfectly valid according to the
spec. The spec says to generate the full output of ECDH, and then "Return the
first length bits of secret."

So in the case of specifying "length: 0" the result should be an empty buffer.

This comment is simply saying that it handles the zero-length case before
calling ECDH_compute_key() below, to avoid worrying about that line calling
&derived_bytes->front() on an empty buffer (which would be incorrect)

Note that "has_optional_length_bits" does not imply that that length was
non-zero. Merely that it was provided

[Ryan Sleevi, 2014/12/11 23:23:48]

Ah, right, for deriveBits, zero bits are allowed.

Although I wonder if perhaps it should be prohibited? Makes it easier to specify
short-circuiting here.

My main goal in spec'ing it would be making sure that short circuits are
permitted, and that you don't have to compute-and-discard but can instead
short-out. Would you agree?

[eroman, 2014/12/12 01:21:17]

I am fine with prohibiting.
I agree that not having to compute ECDH output in this case is the right
implementation strategy (and what this code currently does).

I am not convinced the spec needs more words to allow such a short-circuit. The
only observable difference I can see is that by _not_ short-circuiting you have
the possibility of hitting an operation error if ECDH_compute_key() fails.
Computing ECDH output itself is unlikely to have observable side-effects.

If short-circuiting is explicitly baked into the spec, I think all the preceding
error tests (do the curves match, are they supported curves, etc.) should still
run. Just not the actual ECDH algorithm.

+    if (length_bits == 0) {
+      derived_bytes->clear();
+      return Status::Success();
+    }
+
     if (length_bits > static_cast<unsigned int>(field_size_bytes * 8))
       return Status::ErrorEcdhLengthTooBig(field_size_bytes * 8);