Replace the hardcoded ECH config, which wasn't updated for draft-13,
with a call to SSL_marshal_ech_config.
Bug: 275, oss-fuzz:38054
Change-Id: I10c12b22015c9c0cb90dd6185eb375153a2531f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49445
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Having APIs named "session" and "ID" appears to be far too tempting for
developers, mistaking it as some application-level notion of session.
Update the documentation, in hopes of discouraging this mistake.
Change-Id: Ifd9516287092371d4701114771eff6640df1bcb0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49405
Reviewed-by: Adam Langley <agl@google.com>
This doesn't affect RSA key generation, which uses
BN_prime_checks_for_generation.
Change-Id: Ibf32c0c4bc9fed369e8f8a1efea72c5bd39185a9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49426
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
These are a little odd with the ASN1_ENCODING paths. And there were some
bugs previously around CHOICE types. Nothing defines them, inside or
outside BoringSSL, so remove them.
Change-Id: Id2954fef8ee9637f36f7511b51dc0adc2557e3ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49352
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
it->funcs is only an ASN1_AUX for ASN1_ITYPE_SEQUENCE and
ASN1_ITYPE_CHOICE. Fortunately, the other possible types for it->funcs
are larger than ASN1_AUX and we don't touch the result when we
shouldn't, so this is merely a strict aliasing violation.
Change-Id: I29e94249e0b137fe8df0b16254366ae6705c8784
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49351
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See also 006906cddda37e24a66443199444ef4476697477 from OpenSSL, though
this CL uses a different strategy from upstream. Upstream makes
ASN1_item_ex_i2d continue to allow optionals and checks afterwards at
every non-optional call site. This CL pushes down an optional parameter
and says functions cannot omit items unless explicitly allowed.
I think this is a better default, though it is a larger change. Fields
are only optional when they come from an ASN1_TEMPLATE with the OPTIONAL
flag. Upstream's strategy misses top-level calls.
This CL additionally adds checks for optional ASN1_TEMPLATEs in contexts
where it doesn't make sense. Only fields of SEQUENCEs and SETs may be
OPTIONAL, but the ASN1_ITEM/ASN1_TEMPLATE split doesn't quite match
ASN.1 itself. ASN1_TEMPLATE is additionally responsible for
explicit/implicit tagging, and SEQUENCE/SET OF. That means CHOICE arms
and the occasional top-level type (ASN1_ITEM_TEMPLATE) use ASN1_TEMPLATE
but will get confused if marked optional.
As part of this, i2d_FOO(NULL) now returns -1 rather than "successfully"
writing 0 bytes. If we want to allow NULL at the top-level, that's not
too hard to arrange, but our CBB-based i2d functions do not.
Update-Note: Structures with missing mandatory fields can no longer be
encoded. Note that, apart from the cases already handled by preceding
CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main
downstream impact I've seen of this particular change is in combination
with other bugs. Consider a caller that does:
GENERAL_NAME *name = GENERAL_NAME_new();
name->type = GEN_DNS;
name->d.dNSName = DoSomethingComplicated(...);
Suppose DoSomethingComplicated() was actually fallible and returned
NULL, but the caller forgot to check. They'd now construct a
GENERAL_NAME with a missing field. Previously, this would silently
serialize some garbage (omitted field) or empty string. Now we fail to
encode, but the true error was the uncaught DoSomethingComplicated()
failure. (Which likely was itself a bug.)
Bug: 429
Change-Id: I37fe618761be64a619be9fdc8d416f24ecbb8c46
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49350
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See https://github.com/openssl/openssl/issues/16538
Update-Note: A default-constructed object with a required ANY or
string-like CHOICE field cannot be encoded until the field is specified.
Note this affects i2d_X509: notBefore and notAfter are string-like
CHOICEs in OpenSSL.
Bug: 429
Change-Id: I97d971fa588ab72be25a4c1eb7310ed330f16c4f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49349
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
asn1_ex_i2c actually does have an error condition, it just wasn't being
handled.
628b3c7f2f, imported from upstream's
f3f8e72f494b36d05e0d04fe418f92b692fbb261, tried to check for OID-less
ASN1_OBJECTs and return an error. But it and the upstream change didn't
actually work. -1 in this function means to omit the object, so OpenSSL
was silently misinterpreting the input structure.
This changes the calling convention for asn1_ex_i2c to support this. It
is, unfortunately, a little messy because:
1. One cannot check for object presense without walking the
ASN1_ITEM/ASN1_TEMPLATE structures. You can *almost* check if *pval
is NULL, but ASN1_BOOLEAN is an int with -1 to indicate an omitted
optional. There are also FBOOLEAN/TBOOLEAN types that omit FALSE/TRUE
for DEFAULT. Thus, without more invasive changes, asn1_ex_i2c must be
able to report an omitted element.
2. While the i2d functions report an omitted element by successfully
writing zero bytes, i2c only writes the contents. It thus must
distinguish between an omitted element and an element with
zero-length contents.
3. i2c_ASN1_INTEGER and i2c_ASN1_BIT_STRING return zero on error rather
than -1. Those error paths are not actually reachable because they
only check for NULL. In fact, OpenSSL has even unexported them. But I
found a few callers. Rather than unwind all this and change the
calling convention, I've just made it handle 0 and map to -1 for now.
It's all a no-op anyway, and hopefully we can redo all this with CBB
later.
I've just added an output parameter for now.
In writing tests, I also noticed that the hand-written i2d_ASN1_OBJECT
and i2d_ASN1_BOOLEAN return the wrong value for errors, so I've fixed
that.
Update-Note: A default-constructed object with a required ASN1_OBJECT
field can no longer be encoded without initializing the ASN1_OBJECT.
Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests
that try to serialize an X509_new() must fill in all required fields.
(Production code is unlikely to be affected because the output was
unparsable anyway, while tests sometimes wouldn't notice.)
Bug: 429
Change-Id: I04417f5ad6b994cc5ccca540c8a7714b9b3af33d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49348
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This handles normal CHOICE types. A follow-up CL will handle MSTRING and
ANY types.
Update-Note: An invalid CHOICE object (e.g. GENERAL_NAME) will now fail
when encoded, rather than be silently omitted. In particular, CHOICE
objects are default-initialized by tasn_new.c in an empty -1 state.
Structures containing a required CHOICE field can no longer be encoded
without filling in the CHOICE.
Bug: 429
Change-Id: I7011deadf518ddc344a56b07a0e268ceaae17fe0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49347
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This function forgot to handle errors in ASN1_item_ex_i2d. It also
checked x509_name_canon for ret < 0, when x509_name_canon returns a
boolean. For consistency, I've switched to x509_name_encode to return a
boolean as well. It doesn't actually need to return a length because
it's responsible for filling in a->bytes.
(This is also far from thread-safe, but I'll figure out what to do there
separately.)
Bug: 429
Change-Id: I1dddeab320018be4b837f95001cbeeba4e25f0a1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49346
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
tasn_enc.c was missing lots of error checks and mixed up 0 and -1
returns. Document all the internal calling conventions, as best as I can
tell, and fix things up.
There are also error cases it forgets to check (it generally does not
notice missing non-OPTIONAL fields). This CL only addresses errors it
already tries to report. Subsequent CLs will add in the missing error
cases. And then if it all sticks, I'm hoping we can rewrite this with
CBB. Rewriting tsan_dec.c to CBS would also be good, but that will be
more difficult as we need to clear out BER first.
Update-Note: Some error cases which were silently misinterpreted as
missing OPTIONAL elements will now cause encoding to fail.
Bug: 429
Change-Id: Ibbb3eba08eb8f8f878930c9456edc8c74479aade
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49345
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
GCC 11.2.1 reportedly warns that CTR_DRBG_init may be passed an
uninitialized personalization buffer. This appears to be a false
positive, because personalization_len will be zero. But it's easy enough
to zero-initialize it, so silence the warning.
Bug: 432
Change-Id: I20f6b74e09f19962e8cae37d45090ff3d1c0215d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49245
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Later CLs will clean up the ClientHello construction a bit (draft-12
avoids computing ClientHelloOuter twice). I suspect the transcript
handling on the client can also be simpler, but I'll see what's
convenient after I've changed how ClientHelloOuter is constructed.
Changes of note between draft-10 and draft-13:
- There is now an ECH confirmation signal in both HRR and SH. We don't
actually make much use of this in our client right now, but it
resolves a bunch of weird issues around HRR, including edge cases if
HRR applies to one ClientHello but not the other.
- The confirmation signal no longer depends on key_share and PSK, so we
don't have to work around a weird ordering issue.
- ech_is_inner is now folded into the main encrypted_client_hello code
point. This works better with some stuff around HRR.
- Padding is moved from the padding extension, computed with
ClientHelloInner, to something we fill in afterwards. This makes it
easier to pad up the whole thing to a multiple of 32. I've accordingly
updated to the latest recommended padding construction, and updated
the GREASE logic to match.
- ech_outer_extensions is much easier to process because the order is
required to be consistent. We were doing that anyway, and now a simple
linear scan works.
- ClientHelloOuterAAD now uses an all zero placeholder payload of the
same length. This lets us simplify the server code, but, for now, I've
kept the client code the same. I'll follow this up with a CL to avoid
computing ClientHelloOuter twice.
- ClientHelloOuterAAD is allowed to contain a placeholder PSK. I haven't
filled that in and will do it in a follow-up CL.
Bug: 275
Change-Id: I7464345125c53968b2fe692f9268e392120fc2eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48912
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Hopefully it's a little clearer that this may be called whether or not
ECH is offered. (And whether or not it's a server.)
Bug: 275
Change-Id: I39c8ce5758543a0cfda84652b3fc0a5b9669fd0a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49165
Reviewed-by: Matt Mueller <mattm@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This unexports X509, X509_CINF, X509_NAME_ENTRY, X509_NAME, X509_OBJECT,
X509_LOOKUP_METHOD, X509_STORE, X509_LOOKUP, and X509_STORE_CTX.
Note this means X509_STORE_CTX can no longer be stack-allocated.
Update-Note: Patch cl/390055173 into the roll that includes this. This
unexports most of the X.509 structs, aligning with OpenSSL. Use the
accessor APIs instead.
Bug: 425
Change-Id: I53e915bfae3b8dc4b67642279d0e54dc606f2297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48985
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
V_ASN1_APP_CHOOSE has been discouraged by OpenSSL since 2000:
https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=CHANGES;h=824f421b8d331ba2a2009dbda333a57493bedb1e;hb=fb047ebc87b18bdc4cf9ddee9ee1f5ed93e56aff#l10848
Instead, upstream recommends an MBSTRING_* constant.
https://www.openssl.org/docs/man1.1.1/man3/X509_NAME_add_entry_by_NID.html
This function is a bit overloaded:
MBSTRING_* means "Decode my input from this format and then re-encode it
using whatever string type best suits the NID (usually UTF8String, but
some NIDs require PrintableString)".
V_ASN1_APP_CHOOSE means "This is a Latin-1 string. Without looking at
the NID, pick one of PrintableString, IA5String, or T61String".
The latter is almost certainly not what callers want. If they want a
particular type, they can always force it by passing a particular
V_ASN1_* constant. This removes the only use of ASN1_PRINTABLE_type
within the library, though there is one external use still.
Update-Note: V_ASN1_APP_CHOOSE is removed. I only found one use, which
has been fixed.
Change-Id: Id36376dd0ec68559bbbb366e2305d42be5ddac67
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49067
Reviewed-by: Adam Langley <agl@google.com>
The old loop read one byte past the length. It also stopped the loop
too early on interior NUL. See also upstream's
https://github.com/openssl/openssl/pull/16433, though I've opted to
rewrite the function entirely rather than use their fix.
Also deduplicate the PrintableString check.
Change-Id: Ia8bd282047c2a2ed1d5e71a68a3947c7c108df95
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49066
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
strchr is interprets the trailing NUL as part of the string, so
is_printable thought NUL was allowed. Just write the code in the obvious
way and let the compiler figure it out. (It seems to make a clever
bitmask or something.)
Update-Note: ASN1_mbstring_ncopy will no longer allow PrintableString
for strings containing NUL.
Change-Id: I3675191ceb44c06f0ac7b430f88272cabf392d35
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49065
Reviewed-by: Adam Langley <agl@google.com>
The bulk of RSA_check_key is spent in bn_div_consttime, which is a naive
but constant-time long-division algorithm for the few places that divide
by a secret even divisor: RSA keygen and RSA import. RSA import is
somewhat performance-sensitive, so pick some low-hanging fruit:
The main observation is that, in all but one call site, the bit width of
the divisor is public. That means, for an N-bit divisor, we can skip the
first N-1 iterations of long division because an N-1-bit remainder
cannot exceed the N-bit divisor.
One minor nuisance is bn_lcm_consttime, used in RSA keygen has a case
that does *not* have a public bit width. Apply the optimization there
would leak information. I've implemented this as an optional public
lower bound on num_bits(divisor), which all but that call fills in.
Before:
Did 5060 RSA 2048 private key parse operations in 1058526us (4780.2 ops/sec)
Did 1551 RSA 4096 private key parse operations in 1082343us (1433.0 ops/sec)
After:
Did 11532 RSA 2048 private key parse operations in 1084145us (10637.0 ops/sec) [+122.5%]
Did 3542 RSA 4096 private key parse operations in 1036374us (3417.7 ops/sec) [+138.5%]
Bug: b/192484677
Change-Id: I893ebb8886aeb8200a1a365673b56c49774221a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49106
Reviewed-by: Adam Langley <agl@google.com>
We do non-trivial work when parsing RSA private keys (RSA_check_key)
and, in some consumers, this is performance-sensitive.
Bug: b/192484677
Change-Id: Ic27f5f11d8bd030de77dd500a826fb2dd7c5b75d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49105
Reviewed-by: Adam Langley <agl@google.com>
Although we defined a CBS -> Span<const uint8_t> conversion, MSVC 2015
keeps trying to call the Span(const Container&) constructor. It seems to
not correctly SFINAE the existence of data() and size() members unless
the expression is inlined into the default template argument.
Change-Id: I4e88f820b78ce72ad1b014b5bae0830bc7d099d4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48945
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
tls13_init_key_schedule calls InitHash internally, but we also call
InitHash earlier at various times. On the client, we do it early to
handle HelloRetryRequest and 0-RTT. ECH draft-12 will also need to do it
early. Apparently we do it early on the server too.
Probably tls13_init_key_schedule doesn't need to call InitHash, but for
now, it is an easy check in SSLTranscript.
Change-Id: I5473047c1f29bdeb60901e4e6e80979e592bd6e9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48911
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
std::initializer_list appears to work by instantiating a T[N] at the
call site (which is what we were doing anyway), so I don't believe there
is a runtime dependency.
This also adds a way for individual entries to turn themselves off,
which means we don't need to manually check for some unsolicited
extensions.
Change-Id: I40f79b6a0e9c005fc621f4a798fe201bfbf08411
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48910
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We do this enough that it's worth extracting a common parser. And this
gives a struct we can pass around. Note this moves the server extensions
block parsing out of ssl_scan_serverhello_tlsext.
I've also consolidated a few error conditions to tighten the code up a
bit: the TLS 1.2 code distinguishes unknown from unadvertised cipher,
while the TLS 1.3 code didn't. And seeing the wrong legacy version
number in TLS 1.3 is really just a syntax error since it's not the
version field anymore. (RFC8446 specifies the value.)
Change-Id: Ia2f44ff9a3899b5a594569f1b258f2b487930496
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48908
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Ran the following command at OpenSSL commit
18622c7625436d7f99c0f51895c4d3cea233c62e:
./build-fuzz/fuzz/cert -merge=1 -max_len=10000 fuzz/cert_corpus/ ~/openssl/fuzz/corpora/x509
Change-Id: I22c4051351138736a0fa0202c0977ca9afc6924c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49047
Reviewed-by: Adam Langley <agl@google.com>
Given the error handling issues in the previous CL, we'll probably be
chasing down bugs in there for a while.
Change-Id: I7a219e0fe2496f602d38b4bd0fcd5585ebd72cb7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49046
Reviewed-by: Adam Langley <agl@google.com>
See upstream commits:
32f3b98d1302d4c0950dc1bf94b50269b6edbd95
432f8688bb72e21939845ac7a69359ca718c6676
7bb50cbc4af78a0c8d36fdf2c141ad1330125e2f
8c74c9d1ade0fbdab5b815ddb747351b8b839641
Change-Id: Iff614260c1b1582856edb4ae7a226f2e07537698
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49045
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Subsequent CLs will add some fuzzers, etc., that'll help with catching
this.
Change-Id: I10a8e4b2f23ffd07b124e725c1f7454e7ea6f2dd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49025
Reviewed-by: Adam Langley <agl@google.com>
See also 8393de42498f8be75cf0353f5c9f906a43a748d2 from upstream and
CBS-2021-3712. But rather than do that, I've rewritten it with CBS, so
it's a bit clearer. The previous commit added tests.
Change-Id: Ie52e28f07b9bf805c8730eab7be5d40cb5d558b6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49008
Reviewed-by: Adam Langley <agl@google.com>
See also 174ba8048a7f2f5e1fca31cfb93b1730d9db8300 from upstream. This
differs from the upstream CL in that:
- We don't silently drop trailing NULs.
- As a NUL-terminated C string, the empty string is a non-NULL pointer
to an array containing a zero byte. Use the latter consistently.
Change-Id: I99c6c4c26be5a1771c56c6ab356425f1b85be41d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49006
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This imports part of the fix for CVE-2021-3712, commits
d9d838ddc0ed083fb4c26dd067e71aad7c65ad16,
5f54e57406ca17731b9ade3afd561d3c652e07f2,
23446958685a593d4d9434475734b99138902ed2,
and bb4d2ed4091408404e18b3326e3df67848ef63d0 from upstream. The
others will be imported in follow-up CLs.
Change-Id: Ic35aeb3895935ee94b82a295efade32782e8d1bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49005
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We fill in placeholder values of all zeros fairly often in TLS now,
as workarounds for messages being constructed in the wrong order.
draft-12 of ECH adds even more of these. Add a helper so we don't need
to interrupt an || chain with a memset.
Change-Id: Id4f9d988ee67598645a01637cc9515b475c1aec2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48909
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The session ID field cannot exceed 32 bytes, and we size various buffers
based on this. Test that our parsers correctly handle this.
Also fix the -wait-for-debugger flag. I broke it recently by removing
the statusShimStarted message.
Change-Id: I29bb177f29a79bb4904fb5ba3cedfb0b6b856061
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48907
Reviewed-by: Adam Langley <agl@google.com>
The cipher suite, like the version, is determined by the first server
message, independent of whether it's ServerHello or HelloRetryRequest.
We can simplify this by just processing it before we branch on which it
was.
Change-Id: I747f515e9e5b05a42cbed6e7844808d0fc79a30b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48906
Reviewed-by: Adam Langley <agl@google.com>
GCC 6.1 was released more than five years ago, April 27, 2016. We can
thus drop some bits in the CMake files.
https://gcc.gnu.org/releases.htmlhttps://gcc.gnu.org/develop.html#num_scheme
Also note in BUILDING.md that VS2015 will no longer be supported next
year. Then we can cycle our CQ to testing VS2017 + VS2019. (We're
currently not testing VS2019 at all, though so far it hasn't been an
issue.) I've been running into some VS2015-only C++ issues around
conversions, so once we stop testing it, I expect it'll break.
Change-Id: I7a3020df2acd61d57409108aa4d99c840b5ca994
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48925
Reviewed-by: Adam Langley <agl@google.com>