Add support for AWS-LC to openssl and openssl-sys crates #1805

skmcgrail · 2023-02-01T23:15:34Z

This pull request adds support for AWS Libcrypto (AWS-LC). This is AWS's BoringSSL fork that has been developed with additional performance optimizations and cryptographic algorithm support. The work to integrate AWS-LC into these crates required minimal changes from the existing BoringSSL support.

There are two feature flags that are added depending on the use-case required by the end-consumer. The first feature is aws-lc which will depend on our standard AWS-LC sys crate bindings which is based off of tagged release's of AWS-LC's main branch. The other feature is aws-lc-fips which will utilize the FIPS version of our sys crate. This currently uses the fips-2022-11-02 branch of AWS-LC which corresponds to our AWS-LC-FIPS 2.0 module with this associated certificate and security policy.

skmcgrail · 2023-02-02T18:55:33Z

Looks like I might have misjudged one of the BoringSSL function signatures, will fix that in a subsequent commit.

skmcgrail · 2023-03-02T17:43:35Z

Hey @sfackler just curious if you've had a chance to consider this pull request? If there are concerns that you may have, or more detail on why we are looking to perform this level of integration please let me know and I would be more then willing to help address those. Thanks!

Skepfyr · 2023-03-12T23:17:45Z

How is AWS-LC versioned? How stable is it and how do the bindings line up? Please correct me if this instance is different but taking on a new flavour of OpenSSL makes every single PR harder to write and review because we have to consider this. BoringSSL is odd because it's entirely unstable, is that also true for AWS-LC?

sfackler · 2023-03-15T00:53:08Z

I had hoped that the OpenSSL-alike situation had stabilized a decade ago with the post-Heartbleed introduction of LibreSSL and BoringSSL. Unfortunately, in the last year or so it appears that both AWS and Alibaba have decided to create their own new forks (or forks of forks) in AWS-LC and BabaSSL and sought support for them in this library.

Frankly, this is a disappointing trend. The open source ecosystem exists in a robust form today because corporations find it more beneficial to contribute improvements back to upstream open source projects than to maintain their own independent forks. I certainly hope that AWS-LC exists purely as a staging ground for the floating set of patches that have not yet been upstreamed!

Whatever the shape of projects like AWS-LC and BabaSSL actually are, I see three general options for them with respect to rust-openssl:

Those projects put in the effort to look enough like their upstream that openssl-sys is happy to link to them as-is.
The teams maintaining the forks of OpenSSL maintain forks of this project.
If the organizations managing the forks are not interested in dealing with this themselves, they can pay us to formally support their forks.

theoparis · 2024-06-13T04:23:27Z

How is AWS-LC versioned? How stable is it and how do the bindings line up? Please correct me if this instance is different but taking on a new flavour of OpenSSL makes every single PR harder to write and review because we have to consider this. BoringSSL is odd because it's entirely unstable, is that also true for AWS-LC?

Is this still being worked on? AWS-LC is supposed to be more stable than boringssl since it has release tags. I also find it helpful that the requirement for Go and Perl is optional in AWS-LC, as well as QUIC being supported.

alex · 2025-03-03T21:09:43Z

I spoke with @skmcgrail the other day, and I think it makes sense for us to add support for aws-lc at this point. It's gained a fair bit of traction in other parts of the rust ecosystem (notably rustls), and so there's value in being able to use the same crypto library consistently.

Separately, I think if we raised our minimum OpenSSL/LibreSSL version, we'd be able to drop a ton of cfg() and reduce our overall maintenance burden.

I'm happy to review this, so let me know when its ready.

alex

A few comments, but mostly this looks ok.

In addition: Let's add this to CI so it doesn't regress

alex · 2025-03-04T01:56:33Z

openssl-sys/Cargo.toml

+aws-lc-sys = { version = "0", features = ["ssl"], optional = true }
+aws-lc-fips-sys = { version = "0", features = ["ssl", "bindgen"], optional = true }


This effectively uses any 0.x version, even if they're not semver compatible?

I guess that's considered ok here because we're just using the bindings which will be stable across them?

openssl-sys/build/main.rs

openssl-sys/src/lib.rs

openssl-sys/src/ocsp.rs

openssl/src/ocsp.rs

openssl/src/ssl/test/mod.rs

.github/workflows/ci.yml

openssl-sys/src/lib.rs

…-fips)-sys crate.

skmcgrail · 2025-03-12T18:26:37Z

~~Rebased and appeared to have broken some things, need to get in a bit more. Will ping this when it's ready to look at.~~

Alright looks like we just need to resolve a minor issues when building libssl for x86/i686. I have a PR in AWS-LC to address this: aws/aws-lc#2267

skmcgrail · 2025-03-14T21:09:36Z

All the checks are passing with our latest sys crate release, should be good for eyes again.

alex · 2025-03-14T21:13:04Z

Will add to my TODO list once I'm not sick, thanks.

alex

Very very close, final mile!

Before this is mergable, I want to make sure @sfackler has no objections, but modulo this last comment, this LGTM.

alex · 2025-03-16T21:21:12Z

openssl/src/ocsp.rs

+        const NO_CHECKS = ffi::OCSP_NOCHECKS as c_ulong;
+        const TRUST_OTHER = ffi::OCSP_TRUSTOTHER as c_ulong;
+        const RESPID_KEY = ffi::OCSP_RESPID_KEY as c_ulong;
+        const NO_TIME = ffi::OCSP_NOTIME as c_ulong;


So the implication here is that ffi:: types don't match between aws-lc and other OpenSSL-variants?

Is that an issue for anything else? I guess not?

These aren't types though but preprocessor define values so they are untyped. When generating the Rust bindings with bindgen we have the option to tell it what type should be assigned to these numerical macros: signed or unsigned. With unsigned appearing to be the default that bindgen selects upon reviewing the documentation. AWS-LC and BoringSSL sys crates both default to using the signed types when generating types for numerical macro constants. This is more inline with the behavior defined in the C99 standard for integer constants see 6.4.4.1 where a signed integer would be used first.

As BoringSSL does not support this OCSP functionality this subtlety hadn't been seen yet to date in this project. Which is why it is popping up now. Bindgen supports changing the type for a specific set of macro constant defines, but this is only supported programmatically and not exposed via the bindgen-cli. Thus making that a no-go at the moment.

alex

This LGTM to me. Pinging @sfackler to make sure he's ok too.

alex · 2025-03-18T20:04:28Z

FWIW, at this juncture I think it'd be much easier to merge this PR as-is and expand the exposed functionality in follow-up PRs.

skmcgrail · 2025-03-18T20:05:00Z

Pushed one more commit to enable a few additional capabilities that we can technically support from the AWS-LC side. Added a few tests as well on the cipher side for some of the AES modes.

alex · 2025-03-18T20:06:31Z

Looks like CI is failing.

skmcgrail · 2025-03-18T20:45:26Z

Sad, the remaining failures appear to be around the test cases for AES-128-XTS and AES-256-XTS that I added. I assume there might be some API behavioral differences in the older versions of OpenSSL and for LibreSSL that don't align with how the tests are structured. I can try to investigate why that is happening and what can be changed. I added in the tests since this was a gap in coverage at the moment.

alex · 2025-03-18T20:46:46Z

I would favor reverting the latest set of additions, landing as is, and then you can pursue exposing the additional functionality in follow-up PRs that can all be merged individually.

…

On Tue, Mar 18, 2025 at 4:45 PM Sean McGrail ***@***.***> wrote: Sad, the remaining failures appear to be around the test cases for AES-128-XTS and AES-256-XTS that I added. I assume there might be some API behavioral differences in the older versions of OpenSSL and for LibreSSL that don't align with how the tests are structured. I can try to investigate why that is happening and what can be changed. I added in the tests since this was a gap in coverage at the moment. — Reply to this email directly, view it on GitHub <#1805 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBBDBX3L3G2CZPC7D4D2VCAXZAVCNFSM6AAAAABYH6KFYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZUGY4DIMBTGY> . You are receiving this because you commented.Message ID: ***@***.***> [image: skmcgrail]*skmcgrail* left a comment (sfackler/rust-openssl#1805) <#1805 (comment)> Sad, the remaining failures appear to be around the test cases for AES-128-XTS and AES-256-XTS that I added. I assume there might be some API behavioral differences in the older versions of OpenSSL and for LibreSSL that don't align with how the tests are structured. I can try to investigate why that is happening and what can be changed. I added in the tests since this was a gap in coverage at the moment. — Reply to this email directly, view it on GitHub <#1805 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBBDBX3L3G2CZPC7D4D2VCAXZAVCNFSM6AAAAABYH6KFYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZUGY4DIMBTGY> . You are receiving this because you commented.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

botovq · 2025-03-18T21:34:26Z

Not sure if my comment is visible anywhere, so I repeat it here. That test breakage in CCM mode on old OpenSSL and LibreSSL is an old trap:

        let mut buf = vec![];
        ctx.cipher_update(&aad, None).unwrap();
        ctx.cipher_update_vec(&ct, &mut buf).unwrap();
        ctx.cipher_final_vec(&mut buf).unwrap();

EVP_DecryptFinal_ex() must not be called when decrypting in CCM mode. This is pointed out in
"Authenticated Decryption using CCM mode" in the old OpenSSL wiki. Skipping that ctx_cipher_final_vec() on line 902 for (what will be) LibreSSL 4.1.0 makes tests pass, so that should be easily handled with a cfg_if! skipping this for the affected versions.

skmcgrail · 2025-03-18T23:00:02Z

Not sure if my comment is visible anywhere, so I repeat it here. That test breakage in CCM mode on old OpenSSL and LibreSSL is an old trap:
        let mut buf = vec![];
        ctx.cipher_update(&aad, None).unwrap();
        ctx.cipher_update_vec(&ct, &mut buf).unwrap();
        ctx.cipher_final_vec(&mut buf).unwrap();
EVP_DecryptFinal_ex() must not be called when decrypting in CCM mode. This is pointed out in "Authenticated Decryption using CCM mode" in the old OpenSSL wiki. Skipping that ctx_cipher_final_vec() on line 902 for (what will be) LibreSSL 4.1.0 makes tests pass, so that should be easily handled with a cfg_if! skipping this for the affected versions.

Thanks for tracking this down, very much appreciated. Will have a subsequent PR with that commit here shortly including this suggested fix.

skmcgrail force-pushed the aws-lc-support-final branch 2 times, most recently from 73e0334 to a775d10 Compare February 6, 2023 17:39

skmcgrail force-pushed the aws-lc-support-final branch from a775d10 to 3c18c05 Compare March 3, 2025 21:06

alex reviewed Mar 4, 2025

View reviewed changes

skmcgrail force-pushed the aws-lc-support-final branch 6 times, most recently from 389c9e9 to f6a8368 Compare March 12, 2025 01:30

justsmth reviewed Mar 12, 2025

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

alex reviewed Mar 12, 2025

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

openssl-sys/src/lib.rs Show resolved Hide resolved

skmcgrail force-pushed the aws-lc-support-final branch 3 times, most recently from 94410e5 to 00884aa Compare March 12, 2025 18:19

skmcgrail added 9 commits March 12, 2025 18:23

Add support for aws-lc-sys or aws-lc-fips-sys

c3b714e

Fix clippy and cargo fmt

75e4fd2

Add documentation around the links metadata handling from the aws-lc(…

7d835d0

…-fips)-sys crate.

Add support for OPENSSL_DIR

99bf95f

Resolve OCSP c_ulong flags change

a299e5c

Add support for aws-lc prefixed installs using OPENSSL_DIR

58ba5b2

Handle BIO_get_mem_data macro

5686e16

Pin to specific aws-lc-sys version

eecf75e

cargo fmt

6c1830e

skmcgrail added 2 commits March 12, 2025 18:23

Add AWS-LC to GitHub Actions

64777d9

cleanup comment

fdafb31

skmcgrail force-pushed the aws-lc-support-final branch from 00884aa to fdafb31 Compare March 12, 2025 18:23

Not sure how that happened.....

71a9ac9

skmcgrail force-pushed the aws-lc-support-final branch from 6e14170 to 71a9ac9 Compare March 14, 2025 20:48

alex reviewed Mar 16, 2025

View reviewed changes

alex approved these changes Mar 17, 2025

View reviewed changes

skmcgrail force-pushed the aws-lc-support-final branch from cb2a15c to f40a401 Compare March 18, 2025 20:04

skmcgrail requested a review from alex March 18, 2025 20:05

skmcgrail force-pushed the aws-lc-support-final branch 2 times, most recently from 8c80c70 to 759b9f8 Compare March 18, 2025 20:18

skmcgrail force-pushed the aws-lc-support-final branch from 759b9f8 to 71a9ac9 Compare March 18, 2025 21:04

alex merged commit dde9ffb into sfackler:master Mar 18, 2025
158 checks passed

skmcgrail mentioned this pull request Mar 18, 2025

Enable additional capabilities for AWS-LC #2386

Merged

cedricschwyter mentioned this pull request Apr 5, 2025

chore(deps): bump openssl from 0.10.70 to 0.10.72 in /src-tauri woollygoods/huehuehue#399

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for AWS-LC to openssl and openssl-sys crates #1805

Add support for AWS-LC to openssl and openssl-sys crates #1805

skmcgrail commented Feb 1, 2023 •

edited

Loading

skmcgrail commented Feb 2, 2023

skmcgrail commented Mar 2, 2023

Skepfyr commented Mar 12, 2023

sfackler commented Mar 15, 2023

theoparis commented Jun 13, 2024 •

edited

Loading

alex commented Mar 3, 2025

alex left a comment

alex Mar 4, 2025

skmcgrail commented Mar 12, 2025 •

edited

Loading

skmcgrail commented Mar 14, 2025

alex commented Mar 14, 2025

alex left a comment

alex Mar 16, 2025

skmcgrail Mar 17, 2025

alex Mar 17, 2025

alex left a comment

alex commented Mar 18, 2025

skmcgrail commented Mar 18, 2025

alex commented Mar 18, 2025

skmcgrail commented Mar 18, 2025

alex commented Mar 18, 2025 via email

botovq commented Mar 18, 2025 •

edited

Loading

skmcgrail commented Mar 18, 2025

		aws-lc-sys = { version = "0", features = ["ssl"], optional = true }
		aws-lc-fips-sys = { version = "0", features = ["ssl", "bindgen"], optional = true }

Add support for AWS-LC to openssl and openssl-sys crates #1805

Add support for AWS-LC to openssl and openssl-sys crates #1805

Conversation

skmcgrail commented Feb 1, 2023 • edited Loading

skmcgrail commented Feb 2, 2023

skmcgrail commented Mar 2, 2023

Skepfyr commented Mar 12, 2023

sfackler commented Mar 15, 2023

theoparis commented Jun 13, 2024 • edited Loading

alex commented Mar 3, 2025

alex left a comment

Choose a reason for hiding this comment

alex Mar 4, 2025

Choose a reason for hiding this comment

skmcgrail commented Mar 12, 2025 • edited Loading

skmcgrail commented Mar 14, 2025

alex commented Mar 14, 2025

alex left a comment

Choose a reason for hiding this comment

alex Mar 16, 2025

Choose a reason for hiding this comment

skmcgrail Mar 17, 2025

Choose a reason for hiding this comment

alex Mar 17, 2025

Choose a reason for hiding this comment

alex left a comment

Choose a reason for hiding this comment

alex commented Mar 18, 2025

skmcgrail commented Mar 18, 2025

alex commented Mar 18, 2025

skmcgrail commented Mar 18, 2025

alex commented Mar 18, 2025 via email

botovq commented Mar 18, 2025 • edited Loading

skmcgrail commented Mar 18, 2025

skmcgrail commented Feb 1, 2023 •

edited

Loading

theoparis commented Jun 13, 2024 •

edited

Loading

skmcgrail commented Mar 12, 2025 •

edited

Loading

botovq commented Mar 18, 2025 •

edited

Loading