Update Token Binding code to the latest drafts

net/ssl/token_binding.cc

Index: net/ssl/token_binding.cc
diff --git a/net/ssl/token_binding.cc b/net/ssl/token_binding.cc
index 63fc9dbaf5cf1b1fb3bf761fff02eb6a684ad5b9..91fe62913468bfc2bb6085593b0ec5c528d4fa85 100644
--- a/net/ssl/token_binding.cc
+++ b/net/ssl/token_binding.cc
@@ -32,9 +32,10 @@ bool BuildTokenBindingID(crypto::ECPrivateKey* key, CBB* out) {
           NULL) != kUncompressedPointLen) {
     return false;
   }
-  CBB ec_point;
+  CBB public_key, ec_point;
   return CBB_add_u8(out, TB_PARAM_ECDSAP256) &&
-         CBB_add_u8_length_prefixed(out, &ec_point) &&
+         CBB_add_u16_length_prefixed(out, &public_key) &&
+         CBB_add_u8_length_prefixed(&public_key, &ec_point) &&

[davidben, 2016/09/19 21:24:14]

I... okay. They could also have said:

  struct {
      TokenBindingKeyParameters key_parameters;
      opaque key<1..2^16-1>;
  } TokenBindingID;

Then elsewhere you say:

This specification defines the following key types and their serializations.
rsa2048_pkcs1.5 and rsa2048_pss use an RSAPublicKey. ecdsap256 uses such and
such encoding.

That's how we avoid the extra length prefix in TLS 1.3.

PS: If we're stuck with TB thinking it's the center of the universe and defining
custom serializations, they should at least rename RSAPublicKey since
RSAPublicKey is the name of an ASN.1 type. But I also think we should very
strongly prefer that TB not get in the key serialization game. ECPoint should
also be renamed because ECPoint is the name of a TLS structure that's different
(it includes the leading byte).

[nharper, 2016/09/19 22:32:23]

Maybe that can be changed in a future draft.
You can open an issue at https://github.com/TokenBinding/Internet-Drafts/issues
about the naming of RSAPublicKey if you want.

          CBB_add_bytes(&ec_point, point_buf + 1, kUncompressedPointLen - 1) &&
          CBB_flush(out);
 }
@@ -68,14 +69,26 @@ ECDSA_SIG* RawToECDSA_SIG(EC_KEY* ec, base::StringPiece sig) {
 
 }  // namespace
 
-bool SignTokenBindingEkm(base::StringPiece ekm,
-                         crypto::ECPrivateKey* key,
-                         std::vector<uint8_t>* out) {
-  const uint8_t* ekm_data = reinterpret_cast<const uint8_t*>(ekm.data());
+bool CreateTokenBindingSignature(base::StringPiece ekm,
+                                 TokenBindingType type,
+                                 crypto::ECPrivateKey* key,
+                                 std::vector<uint8_t>* out) {
+  crypto::ScopedEVP_MD_CTX digest_ctx(EVP_MD_CTX_create());
+  uint8_t tb_type = static_cast<uint8_t>(type);
+  uint8_t key_type = static_cast<uint8_t>(TB_PARAM_ECDSAP256);
+  uint8_t digest[EVP_MAX_MD_SIZE];
+  unsigned int digest_len;
+  if (!EVP_DigestInit(digest_ctx.get(), EVP_sha256()) ||
+      !EVP_DigestUpdate(digest_ctx.get(), &tb_type, 1) ||
+      !EVP_DigestUpdate(digest_ctx.get(), &key_type, 1) ||
+      !EVP_DigestUpdate(digest_ctx.get(), ekm.data(), ekm.size()) ||
+      !EVP_DigestFinal_ex(digest_ctx.get(), digest, &digest_len)) {
+    return false;
+  }
   EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(key->key());
   if (!ec_key)
     return false;
-  crypto::ScopedECDSA_SIG sig(ECDSA_do_sign(ekm_data, ekm.size(), ec_key));
+  crypto::ScopedECDSA_SIG sig(ECDSA_do_sign(digest, digest_len, ec_key));
   if (!sig)
     return false;
   return ECDSA_SIGToRaw(sig.get(), ec_key, out);
@@ -137,7 +150,7 @@ TokenBinding::TokenBinding() {}
 
 bool ParseTokenBindingMessage(base::StringPiece token_binding_message,
                               std::vector<TokenBinding>* token_bindings) {
-  CBS tb_message, tb, ec_point, signature, extensions;
+  CBS tb_message, tb, public_key, ec_point, signature, extensions;
   uint8_t tb_type, tb_param;
   CBS_init(&tb_message,
            reinterpret_cast<const uint8_t*>(token_binding_message.data()),
@@ -146,7 +159,8 @@ bool ParseTokenBindingMessage(base::StringPiece token_binding_message,
     return false;
   while (CBS_len(&tb)) {
     if (!CBS_get_u8(&tb, &tb_type) || !CBS_get_u8(&tb, &tb_param) ||
-        !CBS_get_u8_length_prefixed(&tb, &ec_point) ||
+        !CBS_get_u16_length_prefixed(&tb, &public_key) ||
+        !CBS_get_u8_length_prefixed(&public_key, &ec_point) ||

[davidben, 2016/09/19 21:24:14]

CBS_len(&public_key) != 0 ||

[nharper, 2016/09/19 22:32:23]

CBS_get_u8_length_prefixed will return 0 (false) if public_key is too short, and
the whole thing will short circuit if the line above fails (which would result
in an uninitialized public_key, and anyway calling CBS_len() on a null cbs will
still de-ref it), so I don't see any use for this check.

[davidben, 2016/09/19 22:35:40]

It's to ensure there isn't garbage trailing the ECPoint in public_key.

[nharper, 2016/09/19 22:45:06]

Oh! that makes sense. Done.

         !CBS_get_u16_length_prefixed(&tb, &signature) ||
         !CBS_get_u16_length_prefixed(&tb, &extensions) ||
         tb_param != TB_PARAM_ECDSAP256 ||
@@ -167,9 +181,10 @@ bool ParseTokenBindingMessage(base::StringPiece token_binding_message,
   return true;
 }
 
-bool VerifyEKMSignature(base::StringPiece ec_point,
-                        base::StringPiece signature,
-                        base::StringPiece ekm) {
+bool VerifyTokenBindingSignature(base::StringPiece ec_point,
+                                 base::StringPiece signature,
+                                 TokenBindingType type,
+                                 base::StringPiece ekm) {
   if (ec_point.size() != kUncompressedPointLen - 1)
     return false;
   uint8_t x9_62_ec_point[kUncompressedPointLen];
@@ -184,11 +199,23 @@ bool VerifyEKMSignature(base::StringPiece ec_point,
     return false;
   }
 
+  crypto::ScopedEVP_MD_CTX digest_ctx(EVP_MD_CTX_create());
+  uint8_t tb_type = static_cast<uint8_t>(type);
+  uint8_t key_type = static_cast<uint8_t>(TB_PARAM_ECDSAP256);
+  uint8_t digest[EVP_MAX_MD_SIZE];
+  unsigned int digest_len;
+  if (!EVP_DigestInit(digest_ctx.get(), EVP_sha256()) ||
+      !EVP_DigestUpdate(digest_ctx.get(), &tb_type, 1) ||
+      !EVP_DigestUpdate(digest_ctx.get(), &key_type, 1) ||
+      !EVP_DigestUpdate(digest_ctx.get(), ekm.data(), ekm.size()) ||
+      !EVP_DigestFinal_ex(digest_ctx.get(), digest, &digest_len)) {
+    return false;
+  }
+
   crypto::ScopedECDSA_SIG sig(RawToECDSA_SIG(keyp, signature));
   if (!sig)
     return false;
-  return !!ECDSA_do_verify(reinterpret_cast<const uint8_t*>(ekm.data()),
-                           ekm.size(), sig.get(), keyp);
+  return !!ECDSA_do_verify(digest, digest_len, sig.get(), keyp);
 }
 
 }  // namespace net