Roll src/third_party/boringssl/src 0d81373f9..1991af690

Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227be924c08bf1ec5..1991af69006a8fe4faf9b80b2340688846e62514

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45edf51d9d7f
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
Committed: https://crrev.com/0f3127d8ff177305fe9992e87b4f096598fb787a
Cr-Commit-Position: refs/heads/master@{#425147}

[davidben, 2016-10-06 22:55:11]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 22:55:49]

Dry run: 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-10-06 23:22:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-06 23:22:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[davidben, 2016-10-07 19:40:57]

Description was changed from

==========
Roll src/third_party/boringssl/src 0d81373f9..93d242bde

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-15#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

to

==========
Roll src/third_party/boringssl/src 0d81373f9..49cdd4699

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-15#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

[davidben, 2016-10-07 19:58:47]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-07 19:59:09]

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

[davidben, 2016-10-07 21:15:48]

davidben@chromium.org changed reviewers:
+ eroman@chromium.org, lgarron@chromium.org, mpearson@chromium.org,
svaldez@chromium.org

[davidben, 2016-10-07 21:15:49]

svaldez for overall review and net in particular

eroman for components/webcrypto
mpearson for histograms.xml
lgarron for chrome/, content/, and third_party/WebKit (this is all DevTools
security panel stuff)

[Mark P, 2016-10-07 21:31:47]

histograms.xml lgtm

Note that I didn't check your math.  (I know sometimes I check your math but
today I'm pressed for time.)

--mark

[commit-bot: I haz the power, 2016-10-07 23:59:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-07 23:59:54]

Dry run: This issue passed the CQ dry run.

[svaldez, 2016-10-10 14:17:21]

Any idea about why the asm files gained newlines?

https://codereview.chromium.org/2395323002/diff/20001/net/ssl/ssl_cipher_suit...
File net/ssl/ssl_cipher_suite_names.h (right):

https://codereview.chromium.org/2395323002/diff/20001/net/ssl/ssl_cipher_suit...
net/ssl/ssl_cipher_suite_names.h:24: // In the case of a TLS 1.3 AEAD-only
cipher suite, *key_exchange_str is nullptr
Wrap?

[davidben, 2016-10-10 15:39:38]

Whitespace and junk comes from this:
https://boringssl.googlesource.com/boringssl/+/1f53747baa8052e12709244a3567e6...

I'll see about sending upstream a change to purge those...

https://codereview.chromium.org/2395323002/diff/20001/net/ssl/ssl_cipher_suit...
File net/ssl/ssl_cipher_suite_names.h (right):

https://codereview.chromium.org/2395323002/diff/20001/net/ssl/ssl_cipher_suit...
net/ssl/ssl_cipher_suite_names.h:24: // In the case of a TLS 1.3 AEAD-only
cipher suite, *key_exchange_str is nullptr
These seem to be breaking at sentences, so I just left them alone.

[svaldez, 2016-10-10 15:40:57]

lgtm

[lgarron, 2016-10-10 19:58:49]

chrome/, content/, and WebKit LGTM assuming you're okay with the higher layers
knowing the semantics of the is_tls13 boolean.

[davidben, 2016-10-10 19:59:59]

On 2016/10/10 19:58:49, lgarron wrote:
> chrome/, content/, and WebKit LGTM assuming you're okay with the higher layers
> knowing the semantics of the is_tls13 boolean.

Yeah, I don't think we can sanely hide that. :-( Maybe if we take away
decompositions, I dunno.

[davidben, 2016-10-10 20:08:36]

davidben@chromium.org changed reviewers:
+ felt@chromium.org, pfeldman@chromium.org

[davidben, 2016-10-10 20:08:37]

+pfeldman for third_party/WebKit and content/child/web_url_loader_impl.cc
+felt for chrome/browser/ssl

https://codereview.chromium.org/2395323002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2395323002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/browser_protocol.json:1030: { "name":
"keyExchange", "type": "string", "description": "Key Exchange used by the
connection, or the empty string if not applicable." },
Making a formerly required field optional doesn't seem to work. Unfortunately,
this decomposition was a bad way to build a semi-stable interface for reporting
TLS info. I'll muse on a better one later (fundamentally the only thing that's
okay to send across is protocol-level cipher suite) but, in the meantime, this
is the sanest option I see.

[eroman, 2016-10-10 20:27:59]

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
File components/webcrypto/algorithms/aes_kw.cc (right):

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:33: // These length checks are done in
order to give a more specific
crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:41: if (raw_key.size() != 16 &&
raw_key.size() != 32)
Can you remove this check? (key import enforces this invariant, I don't think
there is any need to test it here).

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:51: base::CheckedNumeric<size_t>
length(data.byte_length());
Should this be an |int| instead ? Although the API for AES_key_wrap() takes
sizes in size_t, looks like it is internally constrained by an |int|. Doesn't
matter for correctness here, but would provide for a more accurate error
message.

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:69: // These length checks are done in
order to give a more specific
crypto::OpenSSLErrStackTracer ?

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:77: if (raw_key.size() != 16 &&
raw_key.size() != 32)
Same comment as above -- we can forego this check

[felt, 2016-10-10 21:31:28]

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
File chrome/browser/ssl/chrome_security_state_model_client.cc (right):

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:132: } else {
Why do we need this else catch-all now?

[davidben, 2016-10-10 21:38:19]

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
File chrome/browser/ssl/chrome_security_state_model_client.cc (right):

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:132: } else {
On 2016/10/10 21:31:28, felt wrote:
> Why do we need this else catch-all now?

It's because line 115 (on LHS) is gone. base::ASCIIToUTF16 seems to be fine with
NULLs (treats as empty string), but it seemed poor to pass it in nonetheless.

[davidben, 2016-10-10 21:39:12]

(Oops, missed that I still have eroman's comments to go through. Will resolve
those later today.)

[felt, 2016-10-10 22:04:07]

c/b/ssl lgtm

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
File chrome/browser/ssl/chrome_security_state_model_client.cc (right):

https://codereview.chromium.org/2395323002/diff/20001/chrome/browser/ssl/chro...
chrome/browser/ssl/chrome_security_state_model_client.cc:132: } else {
On 2016/10/10 21:38:19, davidben wrote:
> On 2016/10/10 21:31:28, felt wrote:
> > Why do we need this else catch-all now?
> 
> It's because line 115 (on LHS) is gone. base::ASCIIToUTF16 seems to be fine
with
> NULLs (treats as empty string), but it seemed poor to pass it in nonetheless.

Oh, right.

[davidben, 2016-10-10 23:49:55]

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
File components/webcrypto/algorithms/aes_kw.cc (right):

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:33: // These length checks are done in
order to give a more specific
On 2016/10/10 20:27:59, eroman wrote:
> crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);

Done. aes.h actually does not touch the error queue, but yeah this is probably
prudent.

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:41: if (raw_key.size() != 16 &&
raw_key.size() != 32)
On 2016/10/10 20:27:59, eroman wrote:
> Can you remove this check? (key import enforces this invariant, I don't think
> there is any need to test it here).

Done.

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:51: base::CheckedNumeric<size_t>
length(data.byte_length());
On 2016/10/10 20:27:59, eroman wrote:
> Should this be an |int| instead ? Although the API for AES_key_wrap() takes
> sizes in size_t, looks like it is internally constrained by an |int|. Doesn't
> matter for correctness here, but would provide for a more accurate error
> message.

Well, it's also more fundamentally constrained by uint32_t because it needs to
express a length in four bytes internally. I considered that, but then we need
to write even more casts and whatnot. Kind of a nuisance. The original code also
didn't do this but was subject to a similar constraint.

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:69: // These length checks are done in
order to give a more specific
On 2016/10/10 20:27:59, eroman wrote:
> crypto::OpenSSLErrStackTracer ?

Done.

https://codereview.chromium.org/2395323002/diff/20001/components/webcrypto/al...
components/webcrypto/algorithms/aes_kw.cc:77: if (raw_key.size() != 16 &&
raw_key.size() != 32)
On 2016/10/10 20:27:59, eroman wrote:
> Same comment as above -- we can forego this check

Done.

[davidben, 2016-10-10 23:50:02]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 23:50:19]

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

[eroman, 2016-10-10 23:52:53]

lgtm

[commit-bot: I haz the power, 2016-10-11 01:17:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 01:17:16]

Dry run: This issue passed the CQ dry run.

[davidben, 2016-10-12 06:40:00]

pfeldman: friendly ping

[davidben, 2016-10-13 17:14:43]

On 2016/10/12 06:40:00, davidben wrote:
> pfeldman: friendly ping

pfeldman: Ping. Could you take a look at third_party/WebKit and
content/child/web_url_loader_impl.cc? This is all DevTools stuff. It's just
making the "key exchange" optional (or rather documenting that the empty string
is funny). The decomposition shouldn't have been put in browser_protocol.json to
begin with, but that's done, so the field needs to be made optional now.

[pfeldman, 2016-10-13 17:29:48]

lgtm

[davidben, 2016-10-13 17:41:19]

The CQ bit was checked by davidben@chromium.org

[davidben, 2016-10-13 17:41:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
lgarron@chromium.org, svaldez@chromium.org, felt@chromium.org,
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/2395323002/#ps40001
(title: "eroman comments")

[commit-bot: I haz the power, 2016-10-13 17:41:58]

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-10-13 17:44:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 17:44:37]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[davidben, 2016-10-13 19:26:34]

Description was changed from

==========
Roll src/third_party/boringssl/src 0d81373f9..49cdd4699

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-15#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

to

==========
Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

[davidben, 2016-10-13 19:28:52]

The CQ bit was checked by davidben@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 19:29:12]

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

[davidben, 2016-10-13 19:40:59]

The CQ bit was unchecked by davidben@chromium.org

[davidben, 2016-10-13 19:41:06]

The CQ bit was checked by davidben@chromium.org

[davidben, 2016-10-13 19:41:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
lgarron@chromium.org, svaldez@chromium.org, felt@chromium.org,
mpearson@chromium.org, eroman@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2395323002/#ps60001
(title: "rebase, roll further")

[commit-bot: I haz the power, 2016-10-13 19:41:28]

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-10-13 20:56:36]

Description was changed from

==========
Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

to

==========
Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

[commit-bot: I haz the power, 2016-10-13 20:56:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-13 20:58:25]

Description was changed from

==========
Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495
==========

to

==========
Roll src/third_party/boringssl/src 0d81373f9..1991af690

https://boringssl.googlesource.com/boringssl/+log/0d81373f9169637ff935449227b...

This requires a change in WebCrypto. BoringSSL's
https://boringssl.googlesource.com/boringssl/+/97227dc52d9e6620c8b83eb1dd4d45...
switches to aligning with upstream's APIs for AES key wrap.

This also teaches DevTools how to handle the new draft TLS 1.3 ciphers
and adds them to histograms.xml. See
https://tools.ietf.org/html/draft-ietf-tls-tls13-16#appendix-A.4

Finally, it adds the CECPQ1 experimental ciphers which we forgot to add
there earlier.

BUG=639495

Committed: https://crrev.com/0f3127d8ff177305fe9992e87b4f096598fb787a
Cr-Commit-Position: refs/heads/master@{#425147}
==========

[commit-bot: I haz the power, 2016-10-13 20:58:27]

Patchset 4 (id:??) landed as
https://crrev.com/0f3127d8ff177305fe9992e87b4f096598fb787a
Cr-Commit-Position: refs/heads/master@{#425147}