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

Add feature flag for enabling FIPS. #268

Merged
merged 7 commits into from
Apr 2, 2024

Conversation

tobz
Copy link
Contributor

@tobz tobz commented Mar 28, 2024

As described.

Adds a new feature flag, fips, to enable FIPS in rustls.

Verified that when enabling the new fips feature flag, that FIPS mode is used in aws-lc-rs (by seeing aws-lc-fips-sys being used.).

Resolves #267

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@djc
Copy link
Member

djc commented Mar 29, 2024

So the fips flag only works on Linux -- this will need some adjustments in the CI workflow to make sure we don't run --all-features on other platforms.

@tobz
Copy link
Contributor Author

tobz commented Mar 29, 2024

I can try and address that here, as well. Probably also worth adding a note to the documentation that it is only supported on Linux, as well.

@tobz
Copy link
Contributor Author

tobz commented Mar 29, 2024

Alright, pushed an update to the README. Let me know what you think about that.

Also pushed an update to the CI to try and automatically collect the set of features to test. I did this for all-features-but-FIPS and for the "defaults + ring" features. It's definitely a little more complex, but it seems, perhaps, somewhat better from an automation standpoint so that updates to the default features don't get stale in the CI definitions.

Totally understand if it feels too complex, though: I'm happy to switch it to just be hard-coded.

If you think it's OK, then I think you just need to approve the workflow to run.

@cpu
Copy link
Member

cpu commented Mar 29, 2024

Totally understand if it feels too complex, though: I'm happy to switch it to just be hard-coded.

I suspect that will be our preference here based on how we've handled this in other Rustls org repos. @djc Do you concur?

@tobz
Copy link
Contributor Author

tobz commented Mar 29, 2024

I'll just approach it that way then.

Also means less faffing about debugging the workflow since it means you having to approve them to run every single time I make a change. 😅

@djc
Copy link
Member

djc commented Mar 29, 2024

Yeah, using yq in the workflow definition is a little too meta for me, and relying on tools that aren't all that familiar to the maintainers (at least to me).

@tobz
Copy link
Contributor Author

tobz commented Mar 31, 2024

Alright, this should be all set now.

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you. The README update is a nice touch 👍

@cpu cpu requested a review from djc April 2, 2024 18:01
@djc djc merged commit 16d7e59 into rustls:main Apr 2, 2024
11 checks passed
@djc
Copy link
Member

djc commented Apr 2, 2024

Thanks! (Bypassed the merge queue in order to squash.)

@tobz tobz deleted the tobz/rustls-aws-lc-fips-feature-flag branch April 25, 2024 13:23
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.

Expose feature flag to enable FIPS compliant build of AWS-LC.
3 participants