Update Token Binding code to draft 06

Update Token Binding code to draft 06

BUG=

Committed: https://crrev.com/2840f95e176111e2f468382a51256dc0a34ec547
Cr-Commit-Position: refs/heads/master@{#402364}

[nharper, 2016-06-27 22:51:08]

Patchset #1 (id:1) has been deleted

[nharper, 2016-06-27 22:52:22]

nharper@chromium.org changed reviewers:
+ davidben@chromium.org

[davidben, 2016-06-27 23:39:15]

lgtm

https://codereview.chromium.org/2013303002/diff/20001/net/socket/ssl_client_s...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2013303002/diff/20001/net/socket/ssl_client_s...
net/socket/ssl_client_socket_impl.cc:85: const uint8_t
kTbMinProtocolVersionMinor = 6;
[I am assuming the signature format is the only change.]

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.cc
File net/ssl/token_binding.cc (right):

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:34: bool ECDSA_SIG_to_raw(ECDSA_SIG* ec_sig,
Since these are Chromium functions, probably name them Chromium-style. So
ECDSA_SIGToRaw / RawToECDSA_SIG or something like that. (Although I've always
hated how OPENSSL_UNDERSCORESLookWhenMixedWithGoogleCamelCase.)

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:57: BN_bin2bn(sigp + group_size, group_size,
raw_sig->s) == nullptr) {
Nit: I think Chromium prefers if (!BN_bin2bn(...) || !BN_bin2bn(...)).

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:73: return ECDSA_SIG_to_raw(sig.get(), ec_key, out);
Nit: Check for !sig in case that failed? (I'm like 99% sure it can't, but, I
dunno, maybe someone handed you an ECDSA_METHOD'd ECPrivateKey or something
insane like that.. anyway, it checked before, so *shrug*.)

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:172: if (!sig.get())
Nit: if (!sig)

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:175: ekm.size(), sig.get(), keyp);
Nit: I think I've usually seen !! as the "cast integer to boolean" syntax in
Chromium. I think it's a Windows-ism.

[nharper, 2016-06-27 23:58:30]

https://codereview.chromium.org/2013303002/diff/20001/net/socket/ssl_client_s...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2013303002/diff/20001/net/socket/ssl_client_s...
net/socket/ssl_client_socket_impl.cc:85: const uint8_t
kTbMinProtocolVersionMinor = 6;
On 2016/06/27 23:39:14, davidben wrote:
> [I am assuming the signature format is the only change.]

None of the technical details changed, but the signature format was clarified
and what we had was wrong.

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.cc
File net/ssl/token_binding.cc (right):

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:34: bool ECDSA_SIG_to_raw(ECDSA_SIG* ec_sig,
On 2016/06/27 23:39:14, davidben wrote:
> Since these are Chromium functions, probably name them Chromium-style. So
> ECDSA_SIGToRaw / RawToECDSA_SIG or something like that. (Although I've always
> hated how OPENSSL_UNDERSCORESLookWhenMixedWithGoogleCamelCase.)

Done.

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:57: BN_bin2bn(sigp + group_size, group_size,
raw_sig->s) == nullptr) {
On 2016/06/27 23:39:15, davidben wrote:
> Nit: I think Chromium prefers if (!BN_bin2bn(...) || !BN_bin2bn(...)).

Done.

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:73: return ECDSA_SIG_to_raw(sig.get(), ec_key, out);
On 2016/06/27 23:39:15, davidben wrote:
> Nit: Check for !sig in case that failed? (I'm like 99% sure it can't, but, I
> dunno, maybe someone handed you an ECDSA_METHOD'd ECPrivateKey or something
> insane like that.. anyway, it checked before, so *shrug*.)

Done.

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:172: if (!sig.get())
On 2016/06/27 23:39:15, davidben wrote:
> Nit: if (!sig)

Done.

https://codereview.chromium.org/2013303002/diff/20001/net/ssl/token_binding.c...
net/ssl/token_binding.cc:175: ekm.size(), sig.get(), keyp);
On 2016/06/27 23:39:15, davidben wrote:
> Nit: I think I've usually seen !! as the "cast integer to boolean" syntax in
> Chromium. I think it's a Windows-ism.

Done.

[nharper, 2016-06-27 23:58:45]

The CQ bit was checked by nharper@chromium.org

[nharper, 2016-06-27 23:58:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/2013303002/#ps40001
(title: "fix nits")

[commit-bot: I haz the power, 2016-06-27 23:59:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 01:38:41]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-06-28 01:41:22]

Description was changed from

==========
Update Token Binding code to draft 06

BUG=
==========

to

==========
Update Token Binding code to draft 06

BUG=

Committed: https://crrev.com/2840f95e176111e2f468382a51256dc0a34ec547
Cr-Commit-Position: refs/heads/master@{#402364}
==========

[commit-bot: I haz the power, 2016-06-28 01:41:23]

Patchset 2 (id:??) landed as
https://crrev.com/2840f95e176111e2f468382a51256dc0a34ec547
Cr-Commit-Position: refs/heads/master@{#402364}