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

fix(bindings): Increase aws-lc-rs minimum version #5042

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

goatgoose
Copy link
Contributor

@goatgoose goatgoose commented Jan 16, 2025

Description of changes:

#5028 updated the minimum supported version of aws-lc-rs to 1.6.4, because aws/aws-lc-rs#335 is required in order to build s2n-tls-sys. However, aws/aws-lc-rs#335 modifies three crates: aws-lc-rs, aws-lc-sys, and aws-lc-fips-sys. In order to build s2n-tls, the changes are required in aws-lc-rs and aws-lc-sys for non-FIPS builds, and aws-lc-rs and aws-lc-fips-sys for FIPS builds.

aws-lc-rs 1.6.4 is the first aws-lc-rs version to enforce that aws/aws-lc-rs#335 is included in aws-lc-sys, since it depends on aws-lc-sys 0.14.0.

However, aws-lc-rs 1.12.0 (released ~1 month ago) is the first aws-lc-rs version to enforce that aws/aws-lc-rs#335 is included in aws-lc-fips-sys, since it started depending on aws-lc-fips-sys 0.13.0. All versions before that only depended on 0.12.0

It's therefore currently possible for a dependency closure to include s2n-tls, aws-lc-rs 1.6.4, and aws-lc-fips-sys 1.12.0, which doesn't include aws/aws-lc-rs#335. This causes the s2n-tls build to fail. To ensure that aws/aws-lc-rs#335 is included in the build, we need aws-lc-fips-sys to be at least 0.12.2, which is only enforced in aws-lc-rs 1.12.0, so this PR updates the aws-lc-rs minimum version to 1.12.0.

Call-outs:

Alternative solutions:

  • We could decide that specifying proper minimum versions is not worth requiring such a strict aws-lc-rs version.
  • We could maybe specify the minimum aws-lc-fips-sys version as an optional dependency in s2n-tls-sys, which gets enabled when the fips feature is enabled. This would prevent us from requiring such a strict aws-lc-rs minimum version for non-FIPS builds. However, aws-lc-fips-sys can be included in a dependency closure as a result of another crate enabling fips in aws-lc-rs, so this would not resolve the minimum version issue in this case.

Note that this was not caught by the new minimum versions test added in #5028 because it currently only tests minimum versions of direct dependencies (aws-lc-rs). For indirect dependencies (aws-lc-fips-sys), the highest supported versions are tested.

Testing:

I ran the minimal-versions test on my smithy-rs PR locally:

❯ cargo minimal-versions check --all-features
...
info: running `cargo hack check --all-features`
info: running `cargo check --all-features` on aws-smithy-http-client (1/1)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.68s

I didn't update the minimum test to include transitive dependencies, as this is not recommended in the Rust documentation.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant