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 based on aws-lc-rs cryptographic library instead of ring #377

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KirilNN
Copy link

@KirilNN KirilNN commented Mar 12, 2024

The goal here is to enable feature based on aws-lc-rs library, so it can be used on demand instead of ring which is not FIPS certified.

Copy link
Owner

@Keats Keats left a comment

Choose a reason for hiding this comment

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

Can you add running with this feature on to the CI?

Cargo.toml Outdated
@@ -50,6 +51,7 @@ criterion = { version = "0.4", default-features = false }
[features]
default = ["use_pem"]
use_pem = ["pem", "simple_asn1"]
aws_lc_rs = ["aws-lc-rs"]
Copy link
Owner

Choose a reason for hiding this comment

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

can we just call it fips? That lib name is not obvious

@KirilNN
Copy link
Author

KirilNN commented Mar 15, 2024

@Keats thanks for taking a look, I changed the feature name and added it to the CI, do you mind taking a look again?

@Keats
Copy link
Owner

Keats commented Mar 15, 2024

Have you seen this PR: #318 ? It's something I was considering

@KirilNN
Copy link
Author

KirilNN commented Mar 18, 2024

Well this is also reasonable imo, for me the question is whether these libs are FIPS compliant, cause this is the biggest issue we are tackling with this PR, as AWS did get it certified. Can we do features as well - like remove ring, get a feature for FIPS using aws lib and all the rest with the lib combo? Does it sound good? I believe removing ring is OK for everyone.

@Keats
Copy link
Owner

Keats commented Mar 18, 2024

It looks like building this lib on windows is problematic? aws/aws-lc#1477

@KirilNN
Copy link
Author

KirilNN commented Mar 21, 2024

@Keats that is very unfortunate. I think that there is a compliant Microsoft library for the same that should be windows compilable. Let me do some research and get back.

@KirilNN
Copy link
Author

KirilNN commented Apr 5, 2024

It looks like the symcrypt from Microsoft does not have wide support of platforms, although has the needed features, so I guess we can either merge the PR you suggest which is not guaranteed we have FIPS compliance, or we wait for the AWS folks to fix the windows build? Or maybe just use ring for windows?

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.

2 participants