crypto: Implement ECSignatureCreatorImpl for OpenSSL

crypto/ec_signature_creator_openssl.cc

Index: crypto/ec_signature_creator_openssl.cc
diff --git a/crypto/ec_signature_creator_openssl.cc b/crypto/ec_signature_creator_openssl.cc
index 8854f5ed464de9c8fa1f658e1ca31b646cbbdea7..70df13577297d40532d562576bb31bd549baa762 100644
--- a/crypto/ec_signature_creator_openssl.cc
+++ b/crypto/ec_signature_creator_openssl.cc
@@ -4,13 +4,21 @@
 
 #include "crypto/ec_signature_creator_impl.h"
 
+#include <openssl/bn.h>
+#include <openssl/ec.h>
+#include <openssl/ecdsa.h>
+#include <openssl/evp.h>
+#include <openssl/sha.h>
+
 #include "base/logging.h"
+#include "crypto/ec_private_key.h"
+#include "crypto/openssl_util.h"
 
 namespace crypto {
 
 ECSignatureCreatorImpl::ECSignatureCreatorImpl(ECPrivateKey* key)
-    : key_(key) {
-  NOTIMPLEMENTED();
+    : key_(key), signature_len_(0) {

[Ryan Sleevi, 2013/10/15 19:49:01]

STYLE: per the style guide, I believe this should be one-per-line because the
ctor body itself is multi-line.

defer to clang-format here.

[digit1, 2013/10/15 19:54:17]

Thanks, but that's already the output of "git cl format" which invokes
clang-format. Even if I put the second member on a second line, the tool will
put it back there.

+  EnsureOpenSSLInit();
 }
 
 ECSignatureCreatorImpl::~ECSignatureCreatorImpl() {}
@@ -18,14 +26,56 @@ ECSignatureCreatorImpl::~ECSignatureCreatorImpl() {}
 bool ECSignatureCreatorImpl::Sign(const uint8* data,
                                   int data_len,
                                   std::vector<uint8>* signature) {
-  NOTIMPLEMENTED();
-  return false;
+  OpenSSLErrStackTracer err_tracer(FROM_HERE);
+  ScopedOpenSSL<EVP_MD_CTX, EVP_MD_CTX_destroy> ctx(EVP_MD_CTX_create());
+  size_t sig_len = 0;
+  if (!ctx.get() ||
+      !EVP_DigestSignInit(ctx.get(), NULL, EVP_sha256(), NULL, key_->key()) ||
+      !EVP_DigestSignUpdate(ctx.get(), data, data_len) ||
+      !EVP_DigestSignFinal(ctx.get(), NULL, &sig_len))
+    return false;

[wtc, 2013/10/15 23:25:29]

Nit: we usually add curly braces if the conditional expression spans multiple
lines. If this is what "git cl format" outputs, then it's fine.

[digit1, 2013/10/17 14:22:07]

I've added them. Apparently the formatter doesn't seem to care here.

+
+  signature->resize(sig_len);
+  if (!EVP_DigestSignFinal(ctx.get(), &signature->front(), &sig_len))
+    return false;

[wtc, 2013/10/15 23:25:29]

Nit: it may be a good idea to call signature->resize(sig_len) again just in case
the first EVP_DigestSignFinal call returns a generous estimate of the signature
size.

[digit1, 2013/10/17 14:22:07]

I didn't notice this in the function's documentation, this indeed seems
important. Fixed.

+
+  return true;
 }
 
 bool ECSignatureCreatorImpl::DecodeSignature(const std::vector<uint8>& der_sig,
                                              std::vector<uint8>* out_raw_sig) {
-  NOTIMPLEMENTED();
-  return false;
+  OpenSSLErrStackTracer err_tracer(FROM_HERE);
+  // Create ECDSA_SIG object from DER-encoded data.
+  const unsigned char* der_data =
+      reinterpret_cast<const unsigned char*>(der_sig.front());

[wtc, 2013/10/15 23:25:29]

This reinterpret_cast is not necessary because uint8 is the same as unsigned
char.

[digit1, 2013/10/17 14:22:07]

Done.

+  ScopedOpenSSL<ECDSA_SIG, ECDSA_SIG_free> ecdsa_sig(
+      d2i_ECDSA_SIG(NULL, &der_data, static_cast<long>(der_sig.size())));
+  if (!ecdsa_sig.get())
+    return false;
+
+  // The result is made of two 256-bit vectors.
+  const size_t kMaxBitsPerBN = 256;
+  const size_t kMaxBytesPerBN = (kMaxBitsPerBN + 7) / 8;

[wtc, 2013/10/15 23:25:29]

Nit: It is OK to just define kMaxBytesPerBN as 32 and omit kMaxBitsPerBN. 256
bits = 32 bytes is common in crypto code.

If you make this change, also change "two 256-bit vectors" to "two 32-byte
vectors" or "two 32-byte big integers".

[digit1, 2013/10/17 14:22:07]

Done.

+  std::vector<uint8> result;
+  result.resize(2 * kMaxBytesPerBN);
+  memset(&result[0], 0, result.size());
+
+  BIGNUM* r = ecdsa_sig.get()->r;
+  BIGNUM* s = ecdsa_sig.get()->s;
+  int r_bytes = BN_num_bytes(r);
+  int s_bytes = BN_num_bytes(s);
+  // NOTE: Can't really check for equality here since sometimes the value
+  // returned by BN_num_bytes() will be slightly smaller than kMaxBytesPerBN.
+  if (r_bytes > static_cast<int>(kMaxBytesPerBN) ||
+      s_bytes > static_cast<int>(kMaxBytesPerBN)) {
+    DLOG(ERROR) << "Invalid key sizes r(" << r_bytes << ") s(" << s_bytes
+                << ")";
+    return false;
+  }
+  BN_bn2bin(ecdsa_sig.get()->r, &result[kMaxBytesPerBN - r_bytes]);
+  BN_bn2bin(ecdsa_sig.get()->s, &result[2 * kMaxBytesPerBN - s_bytes]);
+  out_raw_sig->swap(result);
+  return true;
 }
 
 }  // namespace crypto