webcrypto: Add ECDSA algorithm (chromium-side)

content/child/webcrypto/openssl/ecdsa_openssl.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 <openssl/ec.h>
+#include <openssl/ec_key.h>
+#include <openssl/evp.h>
+
+#include "base/logging.h"
+#include "content/child/webcrypto/algorithm_implementation.h"
+#include "content/child/webcrypto/crypto_data.h"
+#include "content/child/webcrypto/generate_key_result.h"
+#include "content/child/webcrypto/openssl/ec_key_openssl.h"
+#include "content/child/webcrypto/openssl/key_openssl.h"
+#include "content/child/webcrypto/openssl/util_openssl.h"
+#include "content/child/webcrypto/status.h"
+#include "content/child/webcrypto/webcrypto_util.h"
+#include "crypto/openssl_util.h"
+#include "crypto/scoped_openssl_types.h"
+#include "crypto/secure_util.h"
+#include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
+#include "third_party/WebKit/public/platform/WebCryptoKey.h"
+#include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
+
+namespace content {
+
+namespace webcrypto {
+
+namespace {
+
+// Extracts the OpenSSL key and digest from a WebCrypto key + algorithm. The
+// returned pkey pointer will remain valid as long as |key| is alive.
+Status GetPKeyAndDigest(const blink::WebCryptoAlgorithm& algorithm,
+                        const blink::WebCryptoKey& key,
+                        EVP_PKEY** pkey,
+                        const EVP_MD** digest) {
+  *pkey = AsymKeyOpenSsl::Cast(key)->key();
+  *digest = GetDigest(algorithm.ecdsaParams()->hash().id());
+  if (!*digest)
+    return Status::ErrorUnsupported();
+  return Status::Success();
+}
+
+// Gets the EC key's order size in bytes.
+//
+// TODO(eroman): Is there a more direct way of doing this? In BoringSSL's
+// ECDSA_size() implementation it calls ec->ecdsa_meth->group_order_size(ec).

[davidben, 2014/11/07 00:17:21]

We could probably expose something of the sort. (And swap out that default
implementation's call to BN_num_bits with BN_num_bytes...)

Is WebCrypto intended to be used with keys with custom *_METHODs?

[eroman, 2014/11/08 01:52:57]

I have replaced this with a call to EC_GROUP_get_degree(), which I think
accomplishes what I want.

+Status GetEcGroupOrderSize(EVP_PKEY* pkey, size_t* order_size_bytes) {
+  crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+
+  crypto::ScopedEC_KEY ec(EVP_PKEY_get1_EC_KEY(pkey));
+  if (!ec.get())
+    return Status::ErrorUnexpected();
+
+  const EC_GROUP* group = EC_KEY_get0_group(ec.get());
+
+  crypto::ScopedBIGNUM order(BN_new());
+  if (!EC_GROUP_get_order(group, order.get(), NULL))
+    return Status::OperationError();
+
+  *order_size_bytes = BN_num_bytes(order.get());
+  return Status::Success();
+}
+
+// Formats a DER-encoded signature (ECDSA-Sig-Value as specified in RFC 3279) to
+// the signature format expected by WebCrypto (raw concatenated "r" and "s").
+//
+// TODO(eroman): Where is the specification for WebCrypto's signature format?
+Status ConvertDerSignatureToWebCryptoSignature(
+    EVP_PKEY* key,
+    std::vector<uint8_t>* signature) {
+  crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+
+  const unsigned char* der_data = &signature->front();
+  crypto::ScopedECDSA_SIG ecdsa_sig(
+      d2i_ECDSA_SIG(NULL, &der_data, static_cast<long>(signature->size())));
+  if (!ecdsa_sig.get())

[davidben, 2014/11/07 00:17:21]

  || der_data != &signature->front() + signature->size()

Not that BoringSSL should ever give back an invalid ECDSA_SIG, but we should
make sure there's no trailing input.

[eroman, 2014/11/08 01:52:57]

Actually, see the comment below.

I have been finding EVP_DigestSignFinal() does not return the correct size to be
able to do this test (It appears to always just be ECDSA_size() and zero-added
when the DER is shorter).

Maybe there is a bug in my code? I will investigate further and open a bug if in
fact this is the case.

[davidben, 2014/11/10 20:23:59]

BoringSSL really shouldn't be padding that. At a glance the code seems fine
though. The length comes eventually from i2d_ECDSA_SIG's return value which
wouldn't know anything about ECDSA_size.

The NULL better case of EVP_DigestSignFinal only returns the maximum size of the
signature, but you seem to be accounting for that. Is it possible that you
weren't in a previous version and that's where you saw it? (I just threw
together a test program and EVP_DigestSignFinal seems to behave correctly.)

[eroman, 2014/11/10 20:55:52]

I will debug this further and get back to you.
Note that it didn't happen every time, but every so often there would be 1 or 2
zeros at the end of the signature.

[eroman, 2014/11/10 22:02:59]

Added the length check.

(I can no longer reproduce the issue, must have been a bug in my test setup).

+    return Status::OperationError();
+
+  // Note that |der_data| is updated to point to past the end of the DER
+  // structure. It is possible there are unconsumed bytes at the end, and
+  // this is OK. (Since the ECDSA signature appears to be padded up to
+  // ECDSA_size()).
+
+  // Determine the maximum length of r and s.
+  size_t order_size_bytes;
+  Status status = GetEcGroupOrderSize(key, &order_size_bytes);
+  if (status.IsError())
+    return status;
+

[davidben, 2014/11/07 00:17:21]

This dance can just be two calls to BN_bn2bin_padded.

[eroman, 2014/11/08 01:52:57]

Done.

+  const BIGNUM* r = ecdsa_sig.get()->r;
+  const BIGNUM* s = ecdsa_sig.get()->s;
+  unsigned int r_size_bytes = BN_num_bytes(r);
+  unsigned int s_size_bytes = BN_num_bytes(s);
+
+  // This shouldn't happen since the values of "r" and "s" were computed by
+  // BoringSSL.
+  if (r_size_bytes > order_size_bytes || s_size_bytes > order_size_bytes)
+    return Status::ErrorUnexpected();
+
+  // Concatenate "r" and "s", zero-padding each to length |order_size_bytes|.
+  signature->clear();
+  signature->resize(order_size_bytes * 2, 0);
+  BN_bn2bin(ecdsa_sig.get()->r,
+            &((*signature)[order_size_bytes - r_size_bytes]));
+  BN_bn2bin(ecdsa_sig.get()->s,
+            &((*signature)[signature->size() - s_size_bytes]));
+  return Status::Success();
+}
+
+// Formats a WebCrypto ECDSA signature to a DER-encoded signature
+// (ECDSA-Sig-Value as specified in RFC 3279).
+//
+// TODO(eroman): What is the specification for WebCrypto's signature format?
+//
+// If the signature length is incorrect (not 2 * order_size), then
+// Status::Success() is returned and |*incorrect_length| is set to true;
+//
+// Otherwise on success, der_signature is filled with a ASN.1 encoded
+// ECDSA-Sig-Value.
+Status ConvertWebCryptoSignatureToDerSignature(
+    EVP_PKEY* key,
+    const CryptoData& signature,
+    std::vector<uint8_t>* der_signature,
+    bool* incorrect_length) {
+  crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+
+  // Determine the length of r and s.
+  size_t order_size_bytes;
+  Status status = GetEcGroupOrderSize(key, &order_size_bytes);
+  if (status.IsError())
+    return status;
+
+  // If the size of the signature is incorrect, verification must fail. Success
+  // is returned here rather than an error, so that the caller can fail
+  // verification with a boolean, rather than reject the promise with an
+  // exception.
+  if (signature.byte_length() != 2 * order_size_bytes) {
+    *incorrect_length = true;
+    return Status::Success();
+  }
+
+  *incorrect_length = false;
+
+  // Construct an ECDSA_SIG from |signature|.
+  crypto::ScopedECDSA_SIG ecdsa_sig(ECDSA_SIG_new());
+  if (!ecdsa_sig)
+    return Status::OperationError();
+
+  ecdsa_sig->r = BN_bin2bn(signature.bytes(), order_size_bytes, NULL);
+  ecdsa_sig->s =
+      BN_bin2bn(signature.bytes() + order_size_bytes, order_size_bytes, NULL);

[davidben, 2014/11/07 00:17:21]

BN_bin2bn can return NULL. Dunno if you want to check for that... it's only on
allocation failure, but then again, this should be true[*] of ECDSA_SIG_new too.

[*] Probably? It's still using the macro-ridden old ASN.1 code, so who knows?

[eroman, 2014/11/08 01:52:57]

Done.

+
+  // Determine the size of the DER-encoded signature.
+  int der_encoding_size = i2d_ECDSA_SIG(ecdsa_sig.get(), NULL);
+  if (der_encoding_size < 0)
+    return Status::OperationError();
+
+  // DER-encode the signature.
+  der_signature->resize(der_encoding_size);
+  uint8_t* result = &der_signature->front();
+  if (0 > i2d_ECDSA_SIG(ecdsa_sig.get(), &result))
+    return Status::OperationError();
+
+  return Status::Success();
+}
+
+class EcdsaImplementation : public EcAlgorithm {
+ public:
+  EcdsaImplementation()
+      : EcAlgorithm(blink::WebCryptoKeyUsageVerify,
+                    blink::WebCryptoKeyUsageSign) {}
+
+  const char* GetJwkAlgorithm(
+      const blink::WebCryptoNamedCurve curve) const override {
+    switch (curve) {
+      case blink::WebCryptoNamedCurveP256:
+        return "ES256";
+      case blink::WebCryptoNamedCurveP384:
+        return "ES384";
+      case blink::WebCryptoNamedCurveP521:
+        // This is not a typo! ES512 means P-521 with SHA-512.
+        return "ES512";
+      default:
+        return NULL;
+    }
+  }
+
+  Status Sign(const blink::WebCryptoAlgorithm& algorithm,
+              const blink::WebCryptoKey& key,
+              const CryptoData& data,
+              std::vector<uint8_t>* buffer) const override {
+    if (key.type() != blink::WebCryptoKeyTypePrivate)
+      return Status::ErrorUnexpectedKeyType();
+
+    crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+    crypto::ScopedEVP_MD_CTX ctx(EVP_MD_CTX_create());
+
+    EVP_PKEY* private_key = NULL;
+    const EVP_MD* digest = NULL;
+    Status status = GetPKeyAndDigest(algorithm, key, &private_key, &digest);
+    if (status.IsError())
+      return status;
+
+    // NOTE: A call to EVP_DigestSignFinal() with a NULL second parameter
+    // returns a maximum allocation size, while the call without a NULL returns
+    // the real one, which may be smaller.
+    size_t sig_len = 0;
+    if (!ctx.get() ||
+        !EVP_DigestSignInit(ctx.get(), NULL, digest, NULL, private_key) ||
+        !EVP_DigestSignUpdate(ctx.get(), data.bytes(), data.byte_length()) ||
+        !EVP_DigestSignFinal(ctx.get(), NULL, &sig_len)) {
+      return Status::OperationError();
+    }
+
+    buffer->resize(sig_len);
+    if (!EVP_DigestSignFinal(ctx.get(), &buffer->front(), &sig_len))
+      return Status::OperationError();
+    buffer->resize(sig_len);
+
+    // ECDSA signing in BoringSSL outputs a DER-encoded (r,s). WebCrypto however
+    // expects a padded bitstring that is r concatenated to s. Convert to the
+    // expected format.
+    return ConvertDerSignatureToWebCryptoSignature(private_key, buffer);
+  }
+
+  Status Verify(const blink::WebCryptoAlgorithm& algorithm,
+                const blink::WebCryptoKey& key,
+                const CryptoData& signature,
+                const CryptoData& data,
+                bool* signature_match) const override {
+    if (key.type() != blink::WebCryptoKeyTypePublic)
+      return Status::ErrorUnexpectedKeyType();
+
+    crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
+    crypto::ScopedEVP_MD_CTX ctx(EVP_MD_CTX_create());
+
+    EVP_PKEY* public_key = NULL;
+    const EVP_MD* digest = NULL;
+    Status status = GetPKeyAndDigest(algorithm, key, &public_key, &digest);
+    if (status.IsError())
+      return status;
+
+    std::vector<uint8_t> der_signature;
+    bool incorrect_length_signature;
+    status = ConvertWebCryptoSignatureToDerSignature(
+        public_key, signature, &der_signature, &incorrect_length_signature);
+    if (status.IsError())
+      return status;
+
+    if (incorrect_length_signature) {
+      *signature_match = false;
+      return Status::Success();
+    }
+
+    if (!EVP_DigestVerifyInit(ctx.get(), NULL, digest, NULL, public_key) ||
+        !EVP_DigestVerifyUpdate(ctx.get(), data.bytes(), data.byte_length())) {
+      return Status::OperationError();
+    }
+
+    // Note that the return value can be:
+    //   1 --> Success
+    //   0 --> Verification failed
+    //  <0 --> Operation error (including if DER-encoded ECDSA signature cannot
+    //         be decoded). However this error should not be possible since the
+    //         DER was created by BoringSSL.
+    int rv = EVP_DigestVerifyFinal(ctx.get(), &der_signature.front(),
+                                   der_signature.size());
+    *signature_match = rv == 1;
+    return rv >= 0 ? Status::Success() : Status::OperationError();

[davidben, 2014/11/07 00:17:21]

Actually this just returns 0 or 1 now, as of
https://boringssl-review.googlesource.com/2064
https://boringssl-review.googlesource.com/1333

It does mean you can no longer distinguish the two failure cases, though it
doesn't look like this code cares.

[eroman, 2014/11/08 01:52:57]

Done.

+  }
+};
+
+}  // namespace
+
+AlgorithmImplementation* CreatePlatformEcdsaImplementation() {
+  return new EcdsaImplementation;
+}
+
+}  // namespace webcrypto
+
+}  // namespace content