Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge bssl #97

Merged
merged 116 commits into from
Feb 23, 2021
Merged

Merge bssl #97

merged 116 commits into from
Feb 23, 2021

Conversation

bryce-shang
Copy link
Contributor

Issues:

Resolves CryptoAlg-612

Description of changes:

This PR merged latest code of BoringSSL master branch.

Call-outs:

  • NA

Testing:

  • CI

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

davidben and others added 30 commits November 18, 2020 22:56
These APIs were used by Chromium to control the carve-out for the TLS
1.3 downgrade signal. As of
https://chromium-review.googlesource.com/c/chromium/src/+/2324170,
Chromium no longer uses them.

Update-Note: SSL_CTX_set_ignore_tls13_downgrade,
SSL_set_ignore_tls13_downgrade, and SSL_is_tls13_downgrade now do
nothing. Calls sites should be removed. (There are some copies of older
Chromium lying around, so I haven't removed the functions yet.) The
enforcement was already on by default, so this CL does not affect
callers that don't use those functions.

Change-Id: I016af8291cd92051472d239c4650602fe2a68f5b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44124
Reviewed-by: Adam Langley <[email protected]>
It's not even accurate. The term "master key" dates to SSL 2, which we
do not implement. (Starting SSL 3, "key" was replaced with "secret".)
The field stores, at various points, the TLS 1.2 master secret, the TLS
1.3 resumption master secret, and the TLS 1.3 resumption PSK. Simply
rename the field to 'secret', which is as descriptive of a name as we
can get at this point.

I've left SSL_SESSION_get_master_key alone for now, as it's there for
OpenSSL compatibility, as well as references to the various TLS secrets
since those refer to concepts in the spec. (When the dust settles a bit
on rfc8446bis, we can fix those.)

Change-Id: I3c1007eb7982788789cc5db851de8724c7f35baf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44144
Reviewed-by: Adam Langley <[email protected]>
Change-Id: I08cc198f326f02b3f38234b938208ea49a13fab6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44164
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Change-Id: Ib5d69d82c4cfc8cc172bdb5d9a739af53f9d2899
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44165
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Almost everything in <openssl/asn1.h> uses ASN1_STRING, and there are a
lot of unspoken assumptions in the library about the type field, so it
needs quite a bit of text.

Change-Id: Ied56c9428069477da8ecb17a174da4320e573fa1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44184
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
This covers the use of EVP_sha256() added in 8846533.

Change-Id: I8cd4c8e271de6a0b9a926e7186c7b24ffe849d67
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44224
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
…ignal

The docs at os/signal.Notify warn about this signal delivery loss bug at
https://golang.org/pkg/os/signal/#Notify, which says:

    Package signal will not block sending to c: the caller must ensure
    that c has sufficient buffer space to keep up with the expected signal
    rate. For a channel used for notification of just one signal value,
    a buffer of size 1 is sufficient.

Discovered by one of Orijtech, Inc's internal static
analyzers that will eventually be donated to the Go project, and will
then be included when one runs:
    go test

Change-Id: I5713f7087a195ac706240d32b53d2e4855d93a1c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44264
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Clarify that there are no truncation issues on targets where the range
of |unsigned| is smaller than the range of |size_t|.

Ensure that |poly1305_state| is (still) large enough. This is a good
idea independently of this change, but is especially important because
switching the fields to |size_t| might have enlarged the structures.

Change-Id: I16e408229c28fcba6c3592603ddb9431cf1f142d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44244
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
No need to use |sk_new|, which allocates a buffer that will immediately
be realloced.

Change-Id: If0a787beac19933d93c5f9a3a8b560edd027c16c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44205
Reviewed-by: Adam Langley <[email protected]>
ARM Cortex-A57 and Cortex-A72 cores running in 32-bit mode are affected
by silicon errata #1742098 [0] and #1655431 [1], respectively, where the
second instruction of a AES instruction pair may execute twice if an
interrupt is taken right after the first instruction consumes an input
register of which a single 32-bit lane has been updated the last time it
was modified.

Shuffle the counter assignments around a bit so that the most recent
updates when the AES instruction pair executes are 128-bit wide.

[0] ARM-EPM-049219 v23 Cortex-A57 MPCore Software Developers Errata Notice
[1] ARM-EPM-012079 v11.0 Cortex-A72 MPCore Software Developers Errata Notice

(This is imported from upstream's
409c59e8f44ae56f2587cdd8a7ce611d0e3d91d9.)

The change is applied to both 32-bit and 64-bit for simplicity, but there
was no measurable performance difference, so leaving them aligned is
easiest.

Change-Id: Ic8e5f656f59ae8c2ecb2762a066c2c9064bb34c5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44284
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
<openssl/base.h> checks for a supported platform, but we don't check
endianness of ARM and MIPS, which are bi-endian. See
https://crbug.com/1153312#c7.

Switch this around. Documentation on which define is "official" is hard
to come by, so I mostly mimicked Chromium. Chromium detects
little-endian ARM and MIPS with __ARMEL__ and __MIPSEL__ respectively,
without looking at __arm__ or __mips__. It uses __aarch64__
instead of __AARCH64EL__, but I think that's an oversight. I can get
Clang to output for aarch64_be and that defines __aarch64__ with
__AARCH64EB__.

<openssl/arm_arch.h> (which we should simplify and align with base.h
once this CL sticks) also normalizes to __ARMEL__ over __BYTE_ORDER__
and friends. Although, interestingly, arm_arch.h defines its own
__ARMEL__ on GNUC aarch64, even though Clang does *not* define __ARMEL__
on aarch64. (I'm guessing this aligned for the benefit of the "armx"
bi-arch asm files.) This value is based on __BYTE_ORDER__, not
__ARMEL__, but it assumes GNUC arm always defines __ARMEL__, so I think
it's reasonable to assume GNUC aarch64 always defines __AARCH64EL__.

Given all this, probably the simplest thing that's most likely to work
is to use __ARMEL__, __MIPSEL__, and __AARCH64EL__. Note this does not
change the _M_* checks. _M_* are Windows's definitions, which I think we
can reasonably assume come with an endianness opinion. (Windows' ARM and
ARM64 ABIs mandate little-endian.) This aligns with Chromium.

Update-Note: CPU processor defines are a mess. If a little-endian ARM or
MIPS build breaks, some of the assumptions above may be wrong. In that
case, the output $CC -dM -E - < /dev/null on the offending toolchain
will be useful to fix it. If a big-endian ARM or MIPS build breaks, this
is working as intended. Any resulting binaries weren't producing the
right outputs.

Change-Id: I2a9e662d09df119a71226e91716d84e7ac3792aa
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44324
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Change-Id: Iba527924a79733b28b12b65d8e1f613d7819eb34
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44345
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Change-Id: I55ef8c4987c1205de9eb16243ffd4efc6aa1c5bd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44344
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
If I perturb kOrder in the malleability check, our and Wycheproof's
tests don't easily notice. This adds some tests with s above and below
the order. EdDSA hashes the public key with the message, which
frustrates constructing actual boundary cases. Instead, these inputs
were found by generating many signatures.

This isn't ideal, but it is sensitive to the most significant 32 bits.

Change-Id: I7fc03758ab97650d0e94478f355ea7085ae0559a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44346
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
It's insufficient to signal an error when the PWCT fails. We
additionally need to ensure that the invalid key material is not
returned.

Change-Id: Ic5ff719a688985a61c52540ce6d1ed279a493d27
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44306
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
See also CVE-2020-1971, f960d81215ebf3f65e03d4d5d857fb9b666d6920, and
aa0ad2011d3e7ad8a611da274ef7d9c7706e289b from upstream OpenSSL.

Unlike upstream's version, this CL opts for a simpler edipartyname_cmp.
GENERAL_NAME_cmp is already unsuitable for ordering, just equality,
which means there's no need to preserve return values from
ASN1_STRING_cmp. Additionally, the ASN.1 structure implies most fields
cannot be NULL.

(The change from other to x400Address is a no-op. They're the same type.
Just x400Address is a little clearer. Historical quirks of the
GENERAL_NAME structure.)

Change-Id: I4b0ffe8e931c8ef916794a486e6a0d6d684c0cc1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44404
Reviewed-by: Adam Langley <[email protected]>
Also make it a little shorter.

Change-Id: I6ee9d7666e9cf622509c54966a88f899a1974f9f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44405
Reviewed-by: Adam Langley <[email protected]>
Change-Id: Ia3d98b00365ed92cbf7d02cdb55a1a16e431c4f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44384
Reviewed-by: David Benjamin <[email protected]>
This change tweaks our ACVP config to better match what BoringCrypto
has previously tested with CAVP.

Change-Id: I7d7ce5153a3eb7355ae1516f06ff591ee2c9d902
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44385
Reviewed-by: David Benjamin <[email protected]>
Change-Id: I4f4a89f97e2513d8b5b740620989b187a7b44a58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44386
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Bug: 275
Change-Id: I4927c0886e3acf5b39104e3d89ed51d67520a343
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40204
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This imports 1ecc76f6746cefd502c7e9000bdfa4e5d7911386 and
41d62636fd996c031c0c7cef746476278583dc9e from upstream. These would have
rejected the mistake in OpenSSL's EDIPartyName sturcture.

Change-Id: I4eb218f9372bea0f7ff302321b9dc1992ef0c13a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44424
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
This imports d741debb320bf54e8575d35603a44d4eb40fa1f9 from upstream.
We've been managing the shared libraries already because our
arm-xlate.pl automatically adds .hidden to .extern lines, but nice to
reduce the diff. (This does result in some duplicate .hidden lines in
the generated output, but we still want the arm-xlate.pl patch to
automatically hide .globl.)

Removing .comm lines does change the generated output, but having each
asm file define its own copy of OPENSSL_armcap_P as a common symbol
always seemed odd. I recall some weird issue where the armv4.pl files
subtly rely on it for iOS's strange .indirect_symbol machinery. (Not
actually because iOS wants a common symbol but because arm-xlate.pl
repurposes .comm to trigger .indirect_symbol.) Fortunately, aarch64 is
much better about PC-relative addressing, so it should be a no-op.

The .comm lines have also previously caused weird issues
(https://boringssl-review.googlesource.com/c/boringssl/+/32324), so
it's generally nice to get rid of them.

Update-Note: If aarch64 builds get some weird error about relocations,
it's this CL's fault.

Change-Id: I763ffa6cda750d99694ded8a5b68d7b27b09cfc9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44464
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Change-Id: Ida3ec65e81398881a71828dc1d51cf80be41bdbb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44444
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
8846533 added a “power-on” test for the TLS KDF, but omitted to add
it to the documented list of these tests.

Change-Id: I13dbad4b9359e7dae0938d02ac53e5e011f50824
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44505
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
This should fix the Chromium roll.

Windows shared library builds are fussy about dllexport vs. dllimport in
a way that's incompatible with external uses of the asn1t.h macros. The
issue is the DECLARE_* macros will add dllexport vs. dllimport on the
assumption the symbols are defined in libcrypto, but external
definitions need a different selector.

Rather than add more complex macros for this, just exclude those tests.
Ideally we wouldn't supoport asn1t.h outside the library at all, if we
can manage it, so no sense in trying to make it work.

This excludes both the new and the old tests. Although this has been
working thus far, it only works because we've been setting the
BORINGSSL_IMPLEMENTATION symbol for test targets wrong in Chromium. I'm
confused how that's been working at all (maybe dllexport vs. dllimport
is more lax when it comes to functions rather than variables?), but when
I do it correctly, the ASN1_LINKED_LIST template breaks too.

Change-Id: I391edba1748f66c383ed55a9d23053674bbb876e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44484
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Rather than the FIPS module actively collecting entropy from the CPU or
OS, this change configures Android FIPS to passively receive entropy.

See FIPS IG 7.14 section two.

Change-Id: Ibfc5c5042e560718474b89970199d35b67c21296
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44305
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
As of
https://chromium-review.googlesource.com/c/chromium/tools/build/+/2586225,
we no longer test on Yasm. Yasm hasn't seen a release for over six years
now and is missing support for newer x86 instructions.

This removes the remnants of support for Yasm on the CI. It also removes
the Yasm support we patched into x86nasm.pl, which removes a now
unnecessary divergence from upstream.

Update-Note: If a x86 Windows asm build breaks, switch from Yasm to
NASM. We're also no longer testing NASM on x86_64 Windows, but there
wasn't any patch to revert.

Change-Id: I016bad8757fcc13240db9f56dd622be518e649d7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44564
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Imported from upstream's 617b49db14fa4c1211bfc5d0e88294d0f159c9a9.

Change-Id: I64349b7cbbda8fbacf1e20ca609081ed42f10550
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44565
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This change also drops ex_data from DH objects. The global would need
special handling in the FIPS module, which isn't hard, but just dropping
it saves some of the code-size costs of this change and I cannot find
any signs of use of this functionality.

Change-Id: I984bd70698c2ec329f340d294b3b9ec169cd0c4e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44524
Reviewed-by: David Benjamin <[email protected]>
davidben and others added 22 commits February 2, 2021 21:34
This fixes the build for folks not using bcm.c.

Change-Id: I47935d8af7cb5a12ff2918ee2a8774182681d930
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45384
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
test_fips probably needs to exercise everything that we have self-tests
for.

(The following change will eliminate the duplication of the code to
create the FFDH group. For reasons, that can't be done in this change.)

Change-Id: Ia72064db77381e7cf396a34b4723b2607f26f00b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45404
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Change-Id: Ica2248a0c7f90b3cc13dfea79c95277313c4eb58
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45424
Reviewed-by: Adam Langley <[email protected]>
This is only used for testing acvptool but, yea, |memcmp| doesn't return
a bool 😳

This wasn't noticed because "ver" mode was missing from the registration
and thus from the test vectors.

Change-Id: I181c9b66aea4032543d39ebcc8728a01e0f34f55
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45464
Commit-Queue: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
CMAC-AES isn't inside our FIPS module, it's only included in
modulewrapper in order to test acvptool. Mark it with a special tag to
avoid it appearing when dumping regcap JSON because NIST paperwork is
such that it's better not to ACVP test such code.

Change-Id: I0c6d3a38bce9bf5766b889677eb3f7de94262c24
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45465
Reviewed-by: David Benjamin <[email protected]>
When a feature is enabled statically in the build config, the compiler
defines __ARM_NEON and also considers itself free to emit NEON code.
In this case, there is no need to check for NEON support at runtime
since the binary will not work without NEON anyway. Moving the check to
compile time lets us drop unused code.

Chrome has required NEON on Android for nearly five years now. However,
historically there was a bad CPU which broke on some NEON code, but not
others. See https://crbug.com/341598 and https://crbug.com/606629. We
worked around that CPU by parsing /proc/cpuinfo and intentionally
dropping the optimization. This is not a stable situation, however, as
we're hoping the compiler is not good enough at emitting NEON to trigger
this bug.

Since then, the number of affected devices has dropped, and Chrome has
raised the minimum Android requirement to L. The Net.HasBrokenNEON
metric from Chrome is now well in the noise.

This CL stops short of removing the workaround altogether because some
consumers of Cronet are unsure whether they needed this workaround.
Those consumers also build without __ARM_NEON, so gating on that works
out. We'll decide what to do with it pending metrics from them.

Update-Note: Builds with __ARM_NEON (-mfpu=neon) will now drop about
30KiB of dead code, but no longer work (if they even did before) on a
particular buggy CPU. Builds without __ARM_NEON are not affected.

Change-Id: Id8f7bccfb75afe0a1594572ea20c51d275b0a256
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45484
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This is largely some cleanup so the three features follow the same
patterns and is hopefully cleaner (no more separate static and
non-static paths). The practical impact is probably nil. (Linux-based
ARM builds with crypto extensions as a baseline, if any exist, save
binary size.)

Change-Id: I2214b1a54e2074024b8eeb51799a08b94646cbf3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45485
Reviewed-by: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Sometimes JSON vector files contain a header element that must be
duplicated into the output and sometimes they don't. Auto-detect this by
looking for a “url” field in the first element.

Change-Id: I76046adb8ea64fe5ac9bae9d6583546504723918
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45524
Reviewed-by: David Benjamin <[email protected]>
This aligns with OpenSSL's behavior. RFC7301 says servers should return
no_application_protocol if the client supported ALPN but no common
protocol was found. We currently interpret all values as
SSL_TLSEXT_ERR_NOACK. Instead, implement both modes and give guidance on
whne to use each. (NOACK is still useful because the callback may be
shared across multiple configurations, some of which don't support ALPN
at all. Those would want to return NOACK to ignore the list.)

To match upstream, I've also switched SSL_R_MISSING_ALPN, added for
QUIC, to SSL_R_NO_APPLICATION_PROTOCOL.

Update-Note: Callers that return SSL_TLSEXT_ERR_ALERT_FATAL from the
ALPN callback will change behavior. The old behavior may be restored by
returning SSL_TLSEXT_ERR_NOACK, though see the documentation for new
recommendations on return values.

Change-Id: Ib7917b5f8a098571bed764c79aa7a4ce0f728297
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45504
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Newer versions of Bazel use a different setting for the crosstool_top
flag, depending on the NDK toolchain in use. This change detects these
crosstools and builds them using Android flags.

Fixes: 180083900
Change-Id: I937d18e53d72b2911e1c472adbce65282d31885d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45564
Commit-Queue: Justin Paupore <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
This lets the builders pass it in via gclient_vars. Once this lands,
I'll make the builders fill it in, at which point we can remove the
magic 'env' value and the logic in the recipe.

Change-Id: Idfc4db3e4cdecf62eacbb2925fd545e1a76b2c79
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45624
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
This fixes an issue in a99308f that caused Bazel builds to break during
the analysis phase.

Change-Id: Ib26e70a52730f04905c2b2f137674f297488ec4f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45665
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Update-Note: No one uses this function. It had a NULL dereference in
some error cases. See CVE-2021-23841.

Change-Id: Ie1cc97615ac8b674147715d7d62e62faf218ae65
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45684
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
CVE-2021-23840

(Imported from upstream's 6a51b9e1d0cf0bf8515f7201b68fb0a3482b3dc1.)

This differs slightly from upstream's version:

- EVP_R_OUTPUT_WOULD_OVERFLOW didn't seem necessary when ERR_R_OVERFLOW
  already exists. (Also since we use CIPHER_R_*, it wouldn't have helped
  with compatibility anyway. Though there's probably something to be
  said for us folding CIPHER_R_* back into EVP_R_*.)

- For simplicity, just check in_len + bl at the top, rather than trying
  to predict the exact number of bytes written.

Update-Note: Passing extremely large input lengths into EVP_CipherUpdate
will now fail. Use EVP_AEAD instead, which is size_t-based and has more
explicit output bounds.

Change-Id: I31835c89dcdecb6b112828f57deb798dc7187db5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45685
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
While I'm here, rewrite it a bit to align more with our preferred style.
(No assignments in conditions, no need for NULL checks on free
functions.)

See also 64a1b940d2b640e5edf0feae90e81bbb6b4941e7 from upstream.

Change-Id: I99a122343541e89d5950888de2c708cfa3ec45e2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45686
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
I've left libc++ and Android tools for now. libc++ is running into
https://crbug.com/1166707. I'm not sure what's wrong with the Android
tools. (CMAKE_LINKER isn't defined for some reason, but it's defined on
my machine.)

We'll also want to update the builders before the NDK anyway. The new
NDK now defaults to ANDROID_ARM_NEON=TRUE.

Change-Id: I1c0fbc3e26368c04d31464477a51e04209aec7ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45544
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
@bryce-shang bryce-shang requested a review from nebeid February 23, 2021 15:55
@bryce-shang bryce-shang merged commit e50a2f3 into aws:main Feb 23, 2021
@bryce-shang bryce-shang deleted the merge-bssl branch April 28, 2021 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.