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

Update pq branch 01 17 23 #757

Merged
merged 69 commits into from
Jan 17, 2023

Conversation

jakemas
Copy link
Contributor

@jakemas jakemas commented Jan 17, 2023

Merge main

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

davidben and others added 30 commits December 13, 2022 13:49
We have two places where the current cap on BIGNUM sizes (64 MiB) is too
large, both involving Montgomery reduction: bn_mul_mont allocates a
spare value on the stack, and BN_mod_exp_mont_constime needs to allocate
a buffer of up to 64 contiguous values, which may overflow an int.

Make BN_MONT_CTX reject any BIGNUM larger than 8 KiB. This is 65,536
bits which is well above our maximum RSA key size, 16,384 bits. Ideally
we'd just apply this in bn_wexpand, to all BIGNUMs across the board, but
we found one caller that depends on creating an 8 MiB BIGNUM.

Update-Note: This will not affect any cryptography implemented by
BoringSSL, such as RSA, but other callers may run into this limit. If
necessary, we can raise this a bit, but the stack allocation means we
don't want to go *significantly* beyond what's in this CL.

Fixed: 541
Change-Id: Ia00f3ea6714a5042434f446943db55a533752dc5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55266
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit db10ae50361a05508683282fcd8d2344a6a1517c)
It's weird to have both "in" and "input" in the same function. Also the
vector const refs can be spans. This is a bit of preparatory cleanup
along the way to a larger refactor so we can do negative tests and test
the weird EVP_Cipher API more coherently.

Bug: 494
Change-Id: I5cddf1b3ab88b3419bd88ce15bee56a2016bcd57
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55385
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit e0d601a57fde7d67a1c771e7d87468faf1f8fe55)
EVP_CIPH_NO_PADDING is a no-op when block_size is one, yet we sized the
output expecting it to always add a byte of padding. (I don't think this
makes a difference because most call sites of DoCipher set
EVP_CIPH_NO_PADDING.)

Bug: 494
Change-Id: Ic75e48a60e669270a093416b862ec03706e1d6ef
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55386
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 7f7599a7264ddde9c720db49135ec919593789e2)
The main FileTest callback didn't support invalid ciphertexts, while the
Wycheproof one did but reimplemented a lot of logic to discover
variations.

Rework this a bit so we have a single TestCipher function that's
agnostic to the FileTest setup. The Operation enum gains a
kInvalidDecrypt mode to specify that the ciphertext is invalid. As part
of this, we tighten up the test

This is to make the later changes easier:

1. We don't have any negative tests for EVP_CIPHER's tag check at all!
   We only test it for EVP_AEAD right now. Now TestCipher can express
   this.

2. The weird EVP_Cipher API has no tests. It has a weird calling
   convention that was easier to test if calling code knew whether to
   expect success or failure. Previously, kInvalidDecrypt was
   implemented by checking if DoCipher failed, which makes it harder to
   embed success/failure-specific assertions.

Along the way, I've made each function responsible for running through
the input once, with separate TestCipherAPI and TestLowLevelAPI helpers.
Otherwise we have to reset the spans and keep track of the state of
intermediate values.

Bug: 494
Change-Id: Ieab87c0c76586aefeb596cc90edd4910ff1b962f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55387
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit 1ce2ec71c3e5563fcebc08bbe67ad5f11add02d8)
The most likely use of EVP_CIPHER_CTX copy is to work around
EVP_CIPHER_CTX's lack of separation between "key" and "pending
operation". That is, you'd probably first configure the key, save that
as the precomputed AES key schedule, and then mint copies for each
operation to avoid mutating your key-only EVP_CIPHER_CTX. Rearrange
things to test that point.

But, while I'm here, we can just sprinkle copies throughout the whole
operation and ensure it still works.

Bug: 494
Change-Id: I464aa265cee317020d6b3ae3fd2ebfa92d7e12ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55388
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
(cherry picked from commit b5b1c61f604f0568e333a4f54eae279e0d867c08)
See upstream's 70d8b304d01b9e0c4ec182db20c33aa0698cda51. Also import
upstream's AES-192-GCM tests, now that we support that in EVP_CIPHER
too.

Bug: 494
Change-Id: I157ebe23f32147214641aeb664ce7db2e21bf86a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55389
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 9992ad269d120e512c1b5831bfb649c76a253eec)
This API supports that too, so we should test it.

Bug: 494
Change-Id: If978705b3120697cc18fc91c06ca950e0e93bcf3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55390
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 8417bacf195cc710650e2f74f3c0040697af3b5d)
It would be nice to have a single-shot EVP_CIPHER_CTX API. This function
is not it.

EVP_Cipher is absurd. It's actually just exposing the internal
EVP_CIPHER 'cipher' callback, whose calling convention is extremely
complex. We've currently documented it as a "single-shot" API, but it's
not single-shot either, as it does update cipher state. It just can't
update across block boundaries.

It is particularly bizarre for "custom ciphers", which include AEADs,
which completely changes the return value convention from
bytes_written/-1 to 1/0, but also adds a bunch of magic NULL behaviors:

- out == NULL, in != NULL: supply AAD
- out != NULL, in != NULL: bulk encrypt/decrypt
- out == NULL, in == NULL: compute/check the tag

Moreover, existing code, like OpenSSH, relies on this behavior. To
ensure we don't break it when refactoring EVP_CIPHER internals, capture
the current behavior in tests. But also, no one should be using this in
new code, so deprecate it.

Upstream hasn't quite deprecated it, they now say "Due to the
constraints of the API contract of this function it shouldn't be used in
applications, please consider using EVP_CipherUpdate() and
EVP_CipherFinal_ex() instead."

Bug: 494
Change-Id: Icfe39a8fbbc860b03c9861f4164b7ee8da340216
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55391
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit e40d0f8ee1361fbff2927a6806c755acea79a521)
Used on the Android platform.

Change-Id: I99f1f56c6a09852ec918816591371426390f1873
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55405
Commit-Queue: Pete Bentley <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit afbd7cf2ae197bfe83163ba47d1be7960a6cb4ba)
Interestingly, the Chromium version is a bit behind the default. I've
set it to match the Chromium version.

Bug: chromium:1340825
Change-Id: Ia956e42897add5849dfe09ec8e3a19f704ed9641
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55425
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 7ad733c81abbf1d6bb7df67b886b7e4a49e08a6d)
SSL_SIGN_RSA_PKCS1_MD5_SHA1 does not really exist, but is a private use
value we allocated to internally represent the TLS 1.0/1.1 RSA signature
algorithm. (Unlike the TLS 1.0/1.1 ECDSA signature algorithm, which is
the same as SSL_SIGN_ECDSA_SHA1, the RSA one is a bespoke MD5+SHA1
concatenation which never appears in TLS 1.2 and up.)

Although documented that you're not to use it with
SSL_CTX_set_verify_algorithm_prefs and
SSL_CTX_set_signing_algorithm_prefs (it only exists for
SSL_PRIVATE_KEY_METHOD), there's nothing stopping a caller from passing
it in.

Were you to do so anyway, we'd get confused and sign or verify it at TLS
1.2. This CL is the first half of a fix: since we already have
pkey_supports_algorithm that checks a (version, sigalg, key) tuple, that
function should just know this is not a 1.2-compatible algorithm.

A subsequent CL will also fix those APIs to not accept invalid values
from the caller, since these invalid calls will still, e.g., dump
garbage values on the wire.

Change-Id: I119503f9742a17952ed08e5815fb3d1419fd4a12
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55445
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit e8f57ca134ffd297e5c46505c86ff7001ef32f7b)
It should not be possible to make BoringSSL request unknown signature
algorithms, or the special SSL_SIGN_RSA_PKCS1_MD5_SHA1 value, in the
ClientHello or CertificateRequest.

Update-Note: This CL makes unknown values fail
SSL_set_verify_algorithm_prefs, etc. SSL_SIGN_RSA_PKCS1_MD5_SHA1 is
silently dropped from the list, rather than an error because, although
documented as incorrect, this hole in the abstraction seems to be
confusing. I think there's some code in Chromium which accidentally puts
it in the signing prefs (wrong but harmless) and I often need to explain
to folks that it doesn't belowing in verify prefs (puts it in the
ClientHello). This makes us tolerate the value by ignoring it.

This makes the previous pkey_supports_algorithm change moot because we'd
never get that far with SSL_SIGN_RSA_PKCS1_MD5_SHA1, but I think the
check, but I think the check belongs in that function too.

The test also reveals that some of our tests have been accidentally
passing zero into the preference list all this time.

Change-Id: I76d4eb98682515c3b819e0ed8d44f2d708a98975
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55446
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 3a1b7306ac18110024f1b716c2aa61f17732e02a)
We currently shift between unsigned long and int.

Bug: 516
Change-Id: I9e3fcc9393e24a352a2c08b9df0650a508d7a60b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55448
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit 55c32b64297ff725978112bc2d7c20dca5e495dc)
tag and utype are always accessed as int, so make the structs match.
Boolean ASN1_ITEMs put an ASN1_BOOLEAN in it->size, so add a cast. Also
fix the time set_string functions to call the underlying CBS parser
directly, so they don't need to put a strlen into an int.

Bug: 516
Change-Id: Ie10e7eaf58ec0b0dec59813a0ddcb0197fce1fd1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55449
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 9211b80f1e4d66982b17134b61f11db1b8a8a192)
Some of them were flagging -Wshorten-64-to-32 warnings. None of these
values are long, so just remove them. (I suspect this assumes unsigned
int is at least 32-bit, but we already assume this rather than wrap all
32-bit constants in UINT32_C(x).)

Ideally the c2l and l2c macros could be replaced with the load/store
functions but, like with the ciphers in decrepit, this is probably not
worth the effort for DES.

Bug: 516
Change-Id: I19e8cd4a321c20b9a22e4c007d943185c10755bb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55450
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit e30a652a36fd65a7cf85c49cdd38aab25ea5bd72)
Along the way, this fixes some size_t truncations.

Bug: 516
Change-Id: Iff0cf6ced0b7deb4c48c268e051a4da433088056
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55453
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit c477d1e33ed9fca7c75e375dcd51f1d644e1aded)
We were mixing uint64_t and unsigned, which flagged -Wshorten-64-to-32.
While I'm here, switch the iteration count to uint64_t to cut down on
uses of 'unsigned'. While we have no real risk of overflow a u32 here,
counting the number of times we perform some operation in a loop would
probably best be u64.

(I'm guessing we originally used unsigned mostly so that %u worked. But
PRIu64 exists, though it's wordy.)

Bug: 516
Change-Id: I6abc24ecb029c2c223bb940c903d497868bab9fc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55455
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 1a46f8fb6fb55247d43a0406edb9eca62e4c12c1)
On Apple platforms, Jitter RNG implementation uses `mach_absolute_time`
function to get the current value of the clock. However, this function
deals in "tick" units, not in time units. On Intel based Apple
platforms 1 tick = 1 ns, which is good enough timer resolution for
Jitter. On M1 macbooks 1 tick ~ 41.67 ns. This seems to be a problem
for Jitter since it causes frequent failures to produce enough entropy
on M1 macbooks. Another potential cause for the issue might be that
`mach_absolute_time` clock doesn't increment while they system is
asleep. I am not sure what is the definition of "asleep" on M1 macs,
how it correlates to performance and efficiency cores and different
power saving states, etc. The upstream Jitter RNG repository has
recently added support for using the system counter on `aarch64`
which seems like way to go for all 64-bit Arm platforms.

So this change:
  - Adds the system counter calls on `aarch64` platforms,
  - Removes the `__MACH__` special case.
* Print the full stack trace if the UndefinedBehaviorSanitizer detects an issue. Update the benchmark readme to clarify usecases and build options
BIO_ctrl is one of OpenSSL's ioctl-style patterns, where they jam many
different function signatures into one type. BIO_ctrl returns long for
the sake of other operations, but many of them are only allowed to
return int.

Bug: 516
Change-Id: Ieffad1da89c60a538f142b12bdebdb950efd5c6a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55454
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 02f7705dff4c75439fe75e7e34086ebb3a82c2db)
- Adds the full set of architectures for Linux for which there are
  assembly sources listed.
- Adds Android, mostly parallel to Linux.
- Adds the other Apple OSs, parallel to macOS.

Bug: 531

Change-Id: I8bb609d3563b2d151a404f8468b4c6b22c2692f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55485
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 99eac424d84d34395c7eea7a90d419e23c00dfc2)
Less code, and internally handles overflows. (Although this one cannot
overflow.)

Bug: 516
Change-Id: I3c2718075689d2815a43534a578a323c52787223
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55452
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 29723828eca43b2dfa5fe476edc4e01ca8072d2a)
That test cert expires in 2099, which is a ways off but if this code is
somehow still around by then, let's save the future some pain. With this
fixed, our test all pass at least through the year 3000, so we're
hopefully clear of timebombs.

Change-Id: Ie9dcbc4f4db70c6bcc1ae9717c6e1ee89eb4195c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55625
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit c7b255e5bb14e6a44058c4fe16e88237d07f8378)
Bug: 516
Change-Id: Ifd381d1a2ed30aed6ffe84eb83d8fb4d93ec02ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55451
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit 7ff871acbe1c58d142f089e228626e6a63ad5fe0)
Change-Id: I42d210ae4be106fb2898b986bb17dfe293454828
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55665
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit ba509dca55bbf1fc116d58035359e12ec2e33f22)
This function reports when security-critical checks on the X.509 key
usage extension would have failed, but were skipped due to the temporary
exception in SSL_set_enforce_rsa_key_usage. This function is meant to
aid deployments as they work through enabling this.

Change-Id: Ice0359879c0a6cbe55bf0cb81a63685506883123
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55465
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit a614d46d40509ea2f0c10d005972a08909c32b8c)
davidben and others added 27 commits January 4, 2023 15:03
The buffer length is int, so the output also fits in int.

Bug: 516
Change-Id: I8e59a2109f38c81ac58f1a8f1e7d739c8b0d1c7c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55707
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 23b1ec016404607bd5e867cf5d6fc199294d09d6)
First, rename to x509v3_conf_name_matches and flip the result value. We
don't need to preserve the positive vs negative return of strncmp here.
The rename is because "name" can mean so many things in the context of
X.509. Here, it's specifically the name of a CONF_VALUE.

Finally, fix it to be size_t-clean.

Bug: 516
Change-Id: I1c3039d9c6ce70cde669e07f943ad1e25fb49dc1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55705
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit de5bb39125a7758a71274e81bfb1ce68f0bd2148)
Scrutinize found that the client-side's session reuse counter
was unreachable. This moves the counter to a more suitable
place and adds tests to verify that.
* OpenSSL Crate Compatibility Support

* Enable SSL Feature Flag

* Remove ssl.h from rust_wrapper.h

* use std::os::raw::{c_long, c_char, c_void}

* Again: use std::os::raw::{c_long, c_char, c_void}

* No need for "vendored" openssl

* Bump crate version to 0.3

Co-authored-by: Justin W Smith <[email protected]>
Compiler flags are not yet populated during compiler feature probing for gcc/clang. Better model compiler flags during compiler feature probing by moving the probing to succeed it.
…AL2022 (aws#745)

Run posix_tests and fips_tests on AL2022 on X86.
The public API already expects them to be uint32_t. Fix the internals to
match.

Bug: 516
Change-Id: Ia683cc2fac559ebe0b3c7045e4db551224677c28
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55706
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 35a31162ffb3a9f6bf7c543e2d516d5cdb339334)
Change-Id: Ifd5aceed9556324cd2f3ed4440664f7e0535e80f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55606
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit 02397c7c973e9dbdda0f4d610e91395b7a72d3c6)
Change-Id: Iae74a5d82715e16eacfc73e02a1b8190d06cdbc8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55607
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit d77fdbff010ee70776036c41155d1b3711ede548)
We already tell people not to use these APIs, but some do anyway. Those
that do should be warned about the streaming implications.

Change-Id: I67a9e1bb94aec2217b7c53849ec676b1c3dddb3c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55392
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit e2e613c269a6bb3d7c0271150fff48d11fdbbace)
X509 verification currently lazily fills in some fields on a lock. Test
that it works correctly. Confirmed that TSan can catch data races
intentionally patched into v3_purp.c.

Change-Id: Ia0e8d81bb6ba4b9ade1a47edcb48404902f4ae8c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55745
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit 030693dc23c91894432aa1ae28b43d2c00d4f421)
That loop is just sk_ASN1_OBJECT_deep_copy.

Change-Id: Idc9db7f8e0ac28c853415813f49b1441b646c246
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55746
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit 42ba95f5c6afcf9105266225341dbe86741b1b9c)
Change-Id: I880dab43471b12c564909fbc00800b8f7a46575f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55747
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit dc139e2e6585fdd3ed8d77d41aab85c5c6c98463)
I inadvertently removed it, but set_string(NULL, str) would validate str
without writing an object. OpenSSL's habit of dual-use functions isn't
great (this behavior masked a bug in another project), but I apparently
even documented it in the header, so restore the behavior.

Change-Id: I8b4dbe5a2b21eb59cb20e4c845b17761329b34a1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55785
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
(cherry picked from commit 97dd962a20b1d19c9e569cf8756fbcfc48ff7c73)
X509_policy_check returns -1 if some certificate had an unparseable
extension, in which case it sets EXFLAG_INVALID_POLICY on it. The
calling code then iterates over the certificates to find the offending
one, so the callback has a chance to undo it. But it skips i = 0, the
leaf, and instead just silentely returns success.

We really should cut down on the callback's ability to mess things up
here but, in the meantime, fix this. Also add a test covering this case.

While I'm here, I've updated make_invalid_extensions.go, which I pulled
some code from, to rename fooOrPanic to mustFoo. That seems to be the
convention in the Go standard library. (regexp.MustCompile, etc.)

Change-Id: Ib07c9f4175e66483bd7c0f7d49aea931bf36e53f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55748
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit d1b20a9580aebb6fbb0b1b2408cf1221d83afb71)
X509 objects do some deferred parsing. Make sure we cover that code with
fuzzers.

Change-Id: I618e90aaf4d8decbc3af59f36910feb9949a8cd2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55751
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 715a0a1da0050a49bad2047051628fdaf7421353)
Change-Id: I738490d70208885481f638273d663140444c3a76
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55825
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 837ade76fd2a0ce8a310b0d7c8d7b523fcb23047)
The clang script needed to be tweaked slightly because they've since
changed the URL. Also libc++ now needs to be built as C++20. (The
bundled libc++ is only built in some of our test configs, so this
doesn't imply a C++20 dependency across the board.)

Change-Id: I0a9e3aed71268bcd37059af8549a23cfc0270b05
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55272
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit aa72a6c3fadff91b815b5bd62b8ab1bb4afe2b91)

* Add AL2022 sanitizers for x86/arm with Clang 14, remove sanitizer config with Ubuntu and Clang 9, and fix minor issues flagged by the newer sanitizers

Co-authored-by: David Benjamin <[email protected]>
This fixes the gcc release build for static FIPS on ARM. It
had been working on other compiler combinations for ARM, like
gcc debug or clang release/debug.

The following steps were done to fix this:
1. Fix three `delocate.peg` parsing issues.
2. An offset exists at the end of `:lo12:.LC9+8`, which
`delocate.go` was choking on. Removing the check gets around
this.
3. The gcc release build does not use `.comm` to define common
symbols. This creates a gap in the bss accessor functions, so we
use `bl` to catch these accessor function symbols instead.
4. Older versions of the gcc assembler maps variables of the
same value to each other, and skips label creation. 
`delocate.go` relies on these labels to determine if a symbol is
"known"; if a label doesn't exist, it's addressed at the end of
`bcm-delocated.S`. Rearranging the variable definition forces a
label creation and gets around the unwanted assembler
optimization.

Co-authored-by: Ubuntu <[email protected]>
@andrewhop andrewhop merged commit a425db0 into aws:integrate-pq Jan 17, 2023
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.