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 main into fips-NetOS-2024-06-11 #1673

Closed
wants to merge 61 commits into from
Closed

Conversation

billbo-yang
Copy link
Contributor

Description of changes:

Update fips-NetOS-2024-06-11 to be up to date with main

Call-outs:

N/A

Testing:

N/A

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.

WillChilds-Klein and others added 30 commits June 11, 2024 09:00
@andrewhop noticed that this test didn't cover TLSv1.3 although we'd
expected it to. This is because the previous test case [guards against
TLSv1.3][1] for unrelated reasons. So, we move the test to another case
that does cover TLSv1.3.

[1]: https://github.com/aws/aws-lc/blob/main/ssl/ssl_test.cc#L7786-L7789
### Issues:
Resolves V1401987253

### Description of changes: 
This key buffer is too small for the larger key sizes that the test
uses.

### Call-outs:
It might be worth adding a address sanitizer + ABI test dimension. So
far we haven't hit this before so I didn't add a new docker image with
everything installed for that.

### Testing:
Ran locally and manually reviewed the change for memory safety. 

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.
Take a step towards removing all uses of OPENSSL_armcap_P from the
ARMv8 assembly code.

Change-Id: Ic1a75e107017b33f3e88b8eae503b788e37ca70a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64207
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 5b6a9cf31d13d3b35a59fa674b566cfb553ef22d)
This also removes handling of the empty input, to match what was done
for aarch64. (The C code ensures the function is never called in this
case.)

Bug: 673
Change-Id: I7e868a9eb0b022c22c3f4ba2c8782ae1464c5a52
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64967
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
(cherry picked from commit b3cda5cf4549b131b09e6fc67775717dcfeeae13)
This is unused. Removing it removes a codepath where callers may
inadvertently break internal invariants of the verifier.

It also removes an attractive nuisance: pyca/cryptograpy at one point
intended to use this callback for AIA fetching. They are lucky they
never did because that would have been a security bug. Certificates
returned by this callback are "trusted" which means, if they satisfy the
X509_TRUST criteria (e.g. are self-signed), they would become trust
anchors!

Also remove the getters for the callbacks, as no one is using them. Not
much good can be done by extracting callbacks. Either it is your
X509_STORE, in which case you know your own callbacks, or it is someone
else's, in which case it probably depends on some application-specific
state that you don't know about.

Update-Note: Removed a handful of unused functions.

Change-Id: Ic95db40186a9107e2a3f44028aa28a335653c25a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64987
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit b083a69e5b9a38fcbae710b0c2081de0235d509b)
If we simply set ctx->get_crl to get_crl, as the other callbacks do, we
don't need to branch at the call site. Also get_issuer no longer needs
to be a callback. We can just check if X509_STORE_CTX_set0_trusted_stack
was called.

Change-Id: I42235f141ee9f8463631f56471c12324a8755bba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64988
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit d1831d78c867ba51b3992ccc213fd201d2f4b0f1)
The vast majority of implementations of this callback empirically did
not understand what this API was for, and actually wanted
SSL_CTX_set_cert_verify_callback.

Change-Id: If7961db612932ac9e76ed8482a59442685ece4b5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65007
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
(cherry picked from commit b6e0eba6e62333652290514e51b75b966b27b27c)
Instead of staggering the ranges, we can just generate three variants of
each certificate. Also internal_verify is written in a remarkably goofy
way, so the partial chain case is a little interesting and worth
testing.

Change-Id: I94bfe58f54c5da2b88581af204c4e9d5b919f50b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65047
Reviewed-by: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c0ae579dbbcd47ca60fd9539bf6cfc1bd0b434e9)
Change-Id: I661c1284ba23138074339aec712ee6ba86905518
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65048
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit cc696073cffe7978d489297fbdeac4c0030384aa)
Previously, if we just skipped signature checks, zero tests would fail.
This is perhaps not ideal.

Change-Id: Ife42f32d06c01b48afa9da26a8bd25814f9a909f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65049
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 2e0b9e030e70563afe3ecd44ea9171ea3e6ce51c)
Whenever the object is mutated, we can simply refresh the cached
EVP_PKEY. This aligns with OpenSSL, which computes it eagerly these
days. This removes the need to lock things, and also makes it easy to
implement the get0 versions of the functions from OpenSSL.

Change-Id: Ib17b654af694817edc43e4742d9baf9ed05c676e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65050
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 5b3dc49c1271554f73b976c2c625600d6bd912b0)
Also X509_REQ_check_private_key didn't handle unknown key type case.
Clean those up and align with X509_check_private_key.

Change-Id: I7b16f85662943e4b226221a01e1092cf62afc643
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65051
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c1c7f0929f18fabc74004242d3e4ce03f54511d0)
No change to tests, but makes it easier to test X509_STORE_CTX-level
features.

Change-Id: I26a02ce51b4970aedce06f64d4ebef0dba154597
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65052
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit e40882e848fdc03e31f974bbad6825f6886a5a38)
This seems to only be reachable via the verify callback, but it is
possible for internal_verify to see a single-element certificate that
isn't self-signed. There's a special error code for this. We probably
can safely change it, but cover the codepath in the meantime.

Change-Id: Id4c81e1826f0b43b369e8f00de36313e5fa4360d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65053
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 35f5a321296977809cd89a49ec400310c2bba78b)
### Description of changes: 
Merging from Upstream considering commits
- then commits between
google/boringssl@5b6a9cf
(Dec 19, 2023) and
google/boringssl@1749dc9
(Jan 4 2023)

### Call-outs:
See internal document as well as "AWS-LC" notes inserted in some of the
commit messages for additions/deviations from the upstream commit.

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.
Implement and use a single version of point addition
for implementations of NIST curves P-384, P-521, and
Fiat-crypto based implementation of P-256. The change
does not affect performance.
This allows operating systems to insist on IBT
enforcement as an exploit mitigation mechanism without
needing to make an exception for anything using a
bundled boringssl, such as chrome, mono, and qtwebengine.

Change-Id: Iac28dd3d2af177b89ffde10ae97bce23739feb94
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60625
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

(cherry picked from commit 9fc1c33e9c21439ce5f87855a6591a9324e569fd)
Change-Id: Ie64ed383a1f6b63a0624c8a6f64d92a33dabf56e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61485
Auto-Submit: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>

(cherry picked from commit 51ed32f1971956a904ce7b3a7ff10716e76eecd4)
Curl's tip of main is failing against a compiler warning specific to
gcc-12 at the moment with `-Werror` enabled. This is causing errors in
our integration CI. We enable the flag since Curl's CI enables it, but
they run against gcc-11 for their
CI at the moment.

I've submitted an issue to curl to fix it
(curl/curl#13932), but we should use gcc-11
for our Curl CI to avoid with curl's main branch that are out of our
control.

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.
Ruby consumes NETSCAPE_SPKI_print for debugging purposes.
This adds support for the symbol for easier integration.

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.
This implements two minor symbols and a few no-op flags for Ruby
support.

New symbols:
* PKCS12_new
* CONF_get1_default_config_file

No-op flags:
* SSL_OP_CRYPTOPRO_TLSEXT_BUG
* SSL_OP_SAFARI_ECDHE_ECDSA_BUG
* SSL_OP_TLSEXT_PADDING

### Call-outs:
All of these no-ops are following a precedent in AWS-LC. We may have to
have a discussion on whether to support CONF modules further down the
line, but exposing this as a no-op for now.

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.
All this flag does is cause verify_cb to be called with ok=2 after
policy validation happens, breaking the otherwise strict 0/1 behavior of
the callback.

We can't quite remove the symbol because a lot of bindings libraries
wrap it without realizing what it does. But no one actually uses it,
because it's pretty useless. Since we now always (other than the
bad_chain thing) check policies and that happens last, this flag really
means "please call the verify callback an extra time at the end with
ok=2".

Update-Note: X509_V_FLAG_NOTIFY_POLICY is now a no-op. This is not
expected to impact anyone.

Change-Id: I892a872181d1c1836ef2533ac616edfb6b3b5836
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65087
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit b2e57a1c132a34938ee3051d57c5dfa2ef64ff42)
If a verify_cb consistently returns -1, it would broadly get treated as
success, except the final call would leak into ok and come out of
X509_verify_cert and read as failure. Prevent this ambiguity by
requiring the return value be 0 or 1, and aborting otherwise.

Update-Note: If the verify callback returns anything other than 0 or 1,
X509_verify_cert will now crash in BSSL_CHECK. If this happens, fix the
callback to use the correct return value.

Change-Id: I0394e68febe9f22a42bcd5de8ea4f3a82b07c862
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65107
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 7a813621dac6878ab53b6ed7392939a8982226e8)
All callers I found seem to be compatible with this. Using the non-const
pointer isn't very useful because you cannot resize the value. Let's try
const-correcting it and we'll revert if it's too annoying to fix.

Update-Note: The above functions are now const-correct. Store the result
in a const pointer to avoid compatibility issues.

Change-Id: Id4a1c7223fbb333716906e20844bf8795118a8ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65128
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
(cherry picked from commit 3ef8cbc419c3143fb3218eac1b1162515573ecb0)
In particular, deprecate get_crl and check_crl. Only gRPC uses them, and
does so incorrectly. It is impossible to implement those callbacks
correctly.

In AWS-LC we didn't take this part of the commit:
  Also remove X509_STORE_CTX_set_cert. No one uses it, and it's redundant
  with X509_STORE_CTX_init. We should remove X509_STORE_CTX_set_chain too,
  but there are some callers to cleanup.

Bug: 426
Change-Id: I846f8292a5f5d6cc3b5d2a576bfaf86e9ea84bdc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65147
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit e9539957ce42b07dc6f8b9bd23c28c7d2ef2bd3b)
Bug: 426
Change-Id: Ie1ba74a940db1525926da1856bb98d350d977674
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65149
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 2a88b4b65da2794378044be9d125ce987ffd39f3)
WillChilds-Klein and others added 13 commits June 19, 2024 16:56
This commit adds two functions used by OpenLDAP 2.5.17+. Both functions
are very straightforward.

From OpenSSL's [docs][1] for `EVP_md_null`:

>A "null" message digest that does nothing: i.e. the hash it returns is
of zero length.

From OpenSSL's [docs][2] for `SSL_set_ciphersuites`:

> SSL_set_ciphersuites() is the same as SSL_CTX_set_ciphersuites()
except it configures the ciphersuites for ssl.

[1]: https://www.openssl.org/docs/man1.1.1/man3/EVP_md_null.html
[2]:
https://www.openssl.org/docs/man1.1.1/man3/SSL_set_ciphersuites.html
### Description of changes: 
Prepare for v1.30.0 release

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.
### Description of changes: 
* Close file-descriptor if `HAZMAT_init_sysgenid_file` fails to sync
sysgenid test file.

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.
### Description of changes: 
Revert: 
* #1641
* #1628

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.
### Description of changes: 
Prepare for release v1.30.1

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.
### Issues:
N/A

### Description of changes: 
Set an appropriate RSA flag when stripped private keys are loaded.

### Call-outs:
Point out areas that need special attention or support during the review
process. Discuss architecture or design changes.

### Testing:
How is this change tested (unit tests, fuzz tests, etc.)? Are there any
testing steps to be verified by the reviewer?

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.

Co-authored-by: dkostic <[email protected]>
### Description of changes: 
Propogates script parameters to cmake command

### Testing:
```
❯ ./util/build_compilation_database.sh -DFIPS=1 -DBUILD_SHARED_LIBS=1
...
+ mkdir -p /var/folders/bd/5rrlftjx2098f44h7jbj3n400000gr/T/tmp.69gTlsxzTf/AWS-LC-BUILD
+ cmake /Users/justsmth/repos/aws-lc -B /var/folders/bd/5rrlftjx2098f44h7jbj3n400000gr/T/tmp.69gTlsxzTf/AWS-LC-BUILD -GNinja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DFIPS=1 -DBUILD_SHARED_LIBS=1
...
...


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.
PR #1526 introduced the `OPENSSL_NO_TLS_PHA` directive mostly for the
purposes of AWS-LC's compatibility with CPython, but in [cpython PR
#117785](python/cpython#117785) @encukou points
out that detecting the absence of OpenSSL's own
`SSL_VERIFY_POST_HANDSHAKE` directive is sufficient. This change removes
AWS-LC's `OPENSSL_NO_TLS_PHA` directive in favor of detecting absence of
`SSL_VERIFY_POST_HANDSHAKE`.
Previously, `DH_check` allowed only primes with certain
properties when the generator was equal to 2 or 5. We remove
this requirement to:
  - be more consistent with OpenSSL that doesn't have these
    checks. As a consequence, it was possible to generate DH
    parameters with OpenSSL that wouldn't pass AWS-LC `DH_check`.
  - not break customers who use one of the standard DH groups
    from RFC's 3526 and 7919 and call `DH_check` on it (like s2n-tls).

(Addresses CryptoAlg-2490)
* Update ACVP tool to properly interact with SHAKE vectors
* Update modulewrapper to run SHAKE tests correctly
* Update modulewrapper to have updated registration json for SHAKE
* Update ACVP tool tests
@billbo-yang billbo-yang requested a review from a team as a code owner June 28, 2024 20:00
davidben and others added 6 commits June 29, 2024 00:36
SSL_select_next_proto has some fairly complex preconditions:

- The peer and supported list must be valid protocol lists
- The supported list must not be empty. The peer list may be empty due
  to one of NPN's edge cases.

In the context of how this function is meant to be used, these are
reasonable preconditions. The caller should not serialize its own list
wrong, and it makes no sense to try to negotiate a protocol when you
don't support any protocols. In particular, it complicates NPN's weird
"opportunistic" protocol.

However, the preconditions are unchecked and a bit subtle. Violating
them will result in memory errors. Bad syntax on the protocol lists is
mostly not a concern (you should encode your own list correctly and the
library checks the peer's list), but the second rule is somewhat of a
mess in practice:

Despite having the same precondition in reality, OpenSSL did not
document this. Their documentation implies things which are impossible
without this precondition, but they forgot to actually write down the
precondition. There's an added complexity that OpenSSL never updated the
parameter names to match the role reversal between ALPN and NPN.

There are thus a few cases where a buggy caller may pass an empty
"supported" list.

- An NPN client called SSL_select_next_proto despite not actually
  supporting any NPN protocols.

- An NPN client called SSL_select_next_proto, flipped the parameters,
  and the server advertised no protocols.

- An ALPN server called SSL_select_next_proto, passed its own list in as
  the second parameter, despite not actually supporting any ALPN
  protocols.

In these scenarios, the "opportunistic" protocol returned in the
OPENSSL_NPN_NO_OVERLAP case will be out of bounds. If the caller
discards it, this does not matter. If the caller returns it through the
NPN or ALPN selection callback, they have a problem. ALPN servers are
expected to discard it, though some may be buggy. NPN clients may
implement either behavior.

Older versions of some callers have exhibited variations on the above
mistakes, so empirically folks don't always get it right. OpenSSL's
wrong documentation also does not help matters. Instead, have
SSL_select_next_proto just check these preconditions. That is not a
performance-sensitive function and these preconditions are easy to
check. While I'm here, rewrite it with CBS so it is much more
straightforwardly correct.

What to return when the preconditions fail is tricky, but we need to
output *some* protocol, so we output the empty protocol. This, per the
previous test and doc fixes, is actually fine in NPN, so one of the
above buggy callers is not retroactively made OK. But it is not fine in
ALPN, so we still need to document that callers need to avoid this
state. To that end, revamp the documentation a bit.

Thanks to Joe Birr-Pixton for reporting this!

Fixed: 735
Change-Id: I4378a082385e7334e6abaa6705e6b15d6843f6c5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69028
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c1d9ac02514a138129872a036e3f8a1074dcb8bd)
SSL_select_next_proto has some fairly complex preconditions:

- The peer and supported list must be valid protocol lists
- The supported list must not be empty. The peer list may be empty due
to one of NPN's edge cases.

In the context of how this function is meant to be used, these are
reasonable preconditions. The caller should not serialize its own list
wrong, and it makes no sense to try to negotiate a protocol when you
don't support any protocols. In particular, it complicates NPN's weird
"opportunistic" protocol.

However, the preconditions are unchecked and a bit subtle. Violating
them will result in memory errors. Bad syntax on the protocol lists is
mostly not a concern (you should encode your own list correctly and the
library checks the peer's list), but the second rule is somewhat of a
mess in practice:

Despite having the same precondition in reality, OpenSSL did not
document this. Their documentation implies things which are impossible
without this precondition, but they forgot to actually write down the
precondition. There's an added complexity that OpenSSL never updated the
parameter names to match the role reversal between ALPN and NPN.

There are thus a few cases where a buggy caller may pass an empty
"supported" list.

- An NPN client called SSL_select_next_proto despite not actually
supporting any NPN protocols.

- An NPN client called SSL_select_next_proto, flipped the parameters,
and the server advertised no protocols.

- An ALPN server called SSL_select_next_proto, passed its own list in as
the second parameter, despite not actually supporting any ALPN
protocols.

In these scenarios, the "opportunistic" protocol returned in the
OPENSSL_NPN_NO_OVERLAP case will be out of bounds. If the caller
discards it, this does not matter. If the caller returns it through the
NPN or ALPN selection callback, they have a problem. ALPN servers are
expected to discard it, though some may be buggy. NPN clients may
implement either behavior.

Older versions of some callers have exhibited variations on the above
mistakes, so empirically folks don't always get it right. OpenSSL's
wrong documentation also does not help matters. Instead, have
SSL_select_next_proto just check these preconditions. That is not a
performance-sensitive function and these preconditions are easy to
check. While I'm here, rewrite it with CBS so it is much more
straightforwardly correct.

What to return when the preconditions fail is tricky, but we need to
output *some* protocol, so we output the empty protocol. This, per the
previous test and doc fixes, is actually fine in NPN, so one of the
above buggy callers is not retroactively made OK. But it is not fine in
ALPN, so we still need to document that callers need to avoid this
state. To that end, revamp the documentation a bit.

Thanks to Joe Birr-Pixton for reporting this!

Fixed: 735
Change-Id: I4378a082385e7334e6abaa6705e6b15d6843f6c5 Reviewed-on:
https://boringssl-review.googlesource.com/c/boringssl/+/69028
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit c1d9ac02514a138129872a036e3f8a1074dcb8bd)

### Issues:
Resolves #ISSUE-NUMBER1
Addresses #ISSUE-NUMBER2

### Description of changes: 
Describe AWS-LC’s current behavior and how your code changes that
behavior. If there are no issues this pr is resolving, explain why this
change is necessary.

### Call-outs:
Point out areas that need special attention or support during the review
process. Discuss architecture or design changes.

### Testing:
How is this change tested (unit tests, fuzz tests, etc.)? Are there any
testing steps to be verified by the reviewer?

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.
### Issues:
Addresses CryptoAlg-2421

### Description of changes: 
No one should start using these DES functions, or continue using DES in
general. However, for legacy customers that can't change this PR adds a
few small utility functions and aligns AWS-LC with the behavior they
expect from OpenSSL.

### Call-outs:
I updated DES_set_key to perform the same checks as OpenSSL and updated
internal usages to use DES_set_key_unchecked.

### Testing:
Added new tests and ensured existing tests still pass. 

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.
### Issues:
Addresses: aws/aws-lc-rs#427

### Description of changes: 
* Allow fallback when `AT_HWCAP2` is not defined (for older glibc).
> 
>       AT_HWCAP2 (since glibc 2.18)
>              Further machine-dependent hints about processor
>              capabilities.
>

### Testing:
* I've not yet been able to setup an environment that replicates [the
failure seen
downstream](https://github.com/rust-lang/rustup/actions/runs/9720503984/job/26831972336?pr=3898)
for `arm-unknown-linux-gnueabihf`:
```
  In file included from /cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.18.0/aws-lc/crypto/fipsmodule/bcm.c:81:
  /cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.18.0/aws-lc/crypto/fipsmodule/cpucap/cpu_arm_linux.c: In function 'aws_lc_0_18_0_OPENSSL_cpuid_setup':
  /cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.18.0/aws-lc/crypto/fipsmodule/cpucap/cpu_arm_linux.c:121:38: error: 'AT_HWCAP2' undeclared (first use in this function); did you mean 'AT_HWCAP'?
       unsigned long hwcap2 = getauxval(AT_HWCAP2);
                                        ^~~~~~~~~
                                        AT_HWCAP
  /cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.18.0/aws-lc/crypto/fipsmodule/cpucap/cpu_arm_linux.c:121:38: note: each undeclared identifier is reported only once for each function it appears in
  /cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.18.0/aws-lc/crypto/fipsmodule/bcm.c: At top level:
  cc1: error: unrecognized command line option '-Wno-c11-extensions' [-Werror]
  cc1: all warnings being treated as errors
  ninja: build stopped: subcommand failed.
```

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.
Scalar encoding for scalar multiplication for curves P-384 and P-521
was implemented for each curve separately and with hard-coded
parameters. This commit refactors the encoding function to be
generic and uses removes the hard-coded ones.
### Description of changes: 
[Draft
release](https://github.com/aws/aws-lc/releases/edit/untagged-11ac3412a1f5ec7633c3)

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.
@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 86.05442% with 164 lines in your changes missing coverage. Please review.

Project coverage is 78.21%. Comparing base (4e54dd8) to head (3da2c4a).

Files Patch % Lines
crypto/fipsmodule/rand/snapsafe_detect.c 52.83% 25 Missing ⚠️
crypto/fipsmodule/kdf/sskdf.c 86.36% 21 Missing ⚠️
tool-openssl/x509.cc 48.78% 21 Missing ⚠️
crypto/x509/x509_vfy.c 69.84% 19 Missing ⚠️
crypto/evp_extra/p_kem.c 85.91% 10 Missing ⚠️
crypto/fipsmodule/evp/evp_ctx.c 66.66% 9 Missing ⚠️
tool-openssl/x509_test.cc 72.72% 5 Missing and 4 partials ⚠️
crypto/x509/x509_test.cc 94.36% 8 Missing ⚠️
crypto/x509/x509_req.c 63.15% 7 Missing ⚠️
ssl/ssl_lib.cc 85.71% 7 Missing ⚠️
... and 12 more
Additional details and impacted files
@@                    Coverage Diff                    @@
##           fips-NetOS-2024-06-11    #1673      +/-   ##
=========================================================
+ Coverage                  78.13%   78.21%   +0.08%     
=========================================================
  Files                        562      571       +9     
  Lines                      94667    95405     +738     
  Branches                   13576    13705     +129     
=========================================================
+ Hits                       73964    74621     +657     
- Misses                     20111    20173      +62     
- Partials                     592      611      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kexgaber and others added 3 commits July 1, 2024 14:19
### Issues:
CryptoAlg-2522

### Description of changes: 

Currently, `NULL` passed as the value of `out` can result in a
segmentation fault. This change adds checks to the HMAC one-shot API and
HMAC_final function to handle scenarios where `NULL` is passed as a
value to the `out` parameter to return from the functions and prevent
further computation.

### Testing:

Added additional test case in `crypto/hmac_extra/hmac_test.cc` to verify
the behavior when `NULL` is passed as a value to `out` in both
functions.

### Call-outs:

- OpenSSL supports this differently by allowing the computation to occur
but allocating a throw-away array [OpenSSL
implementation](https://github.com/openssl/openssl/blob/1977c00f00ad0546421a5ec0b40c1326aee4cddb/crypto/hmac/hmac.c#L233)

- We should also evaluate if this change should be back-ported to the
FIPS branches

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.

Co-authored-by: Sean McGrail <[email protected]>
Ruby exposes the `EC_GROUP` seed functions and allows you to set the
seed with `EC_GROUP_set_seed`. It's one thing that are `EC_GROUP`s are
static and immutable, but setting the seed is more prevalent to custom
curves. We don't encourage using custom curves, so I've chose to mark it
as a no-op and deprecated.
We could arguably support `EC_GROUP_get0_seed` and
`EC_GROUP_get_seed_len` for our named curves, but the seed value is only
used during the initial curve parameter generation. We've chosen to mark
this as a no-op for now, implementing them creates additional complexity
that doesn't really provide additional value to the consumer.

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.
 [CPython PR #117785][1], CPython can now build against AWS-LC
without any source code modifications. The only patches we still require
are to configure the build and work around ([expected][2]) test
failures.

[1]: python/cpython#117785
[2]:
https://discuss.python.org/t/support-building-ssl-and-hashlib-modules-against-aws-lc/44505/4
andrewhop
andrewhop previously approved these changes Jul 2, 2024
ec2-test-framework is having failures due to the instance timeout we
originally set. The ultimate goal is to parallelize this, but extending
the timeout for now.

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.
Description: 
- Developed structure for new OpenSSL tools 
- Contains initial implementation for OpenSSL x509 tool, options -in and
-out (x509.cc), and unit test (x509_test.cc)
- x509_test.cc contains test portions ultimately to be used for future
options but unnecessary for -in/-out unit test

_Files expected to change_


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.
@billbo-yang
Copy link
Contributor Author

Closing since we're locking down the fips branch

@billbo-yang billbo-yang closed this Jul 3, 2024
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.