Allow depending on aws-lc-rs fips without pulling in non-fips#432
Allow depending on aws-lc-rs fips without pulling in non-fips#432j-baker wants to merge 2 commits into
Conversation
At present, this crate nicely supports the FIPS mode of aws-lc-rs.
However, one is presented with a catch-22; if one depends on _only_
the fips feature of this crate, the crate will fail to compile as
critical codepaths have been commented out (the aws-lc-rs feature flag
not being on).
so
```
rcgen = { version = "0.whatever", features = ["fips"] # fails to compile
rcgen = { version = "0.whatever", features = ["aws-lc-rs", "fips"] # links against non-FIPS crypto.
```
Meanwhile, if one depends on the aws-lc-rs feature, it also enables the
non-fips backend of aws-lc-rs, meaning that one can't use cargo deny as
easily to block non-FIPS crypto backends.
I wasn't sure how to avoid this. One approach would simply be to depend
on aws-lc-rs with default features on. Another would be to have the
feature gates dotted around the codebase be on any(aws_lc_rs, fips)
rather than just aws_lc_rs.
I figured a nice approach might be to have either feature trigger an
'aws-lc-like' feature which is what can be used through the codebase, as
this minimises the likelihood that future dev work will re-introduce the
dependency.
However, this does introduce a public-facing internal feature flag.
Looking forward to a review; happy to make changes.
|
It's not entirely clear to me what problem you are trying to solve here. |
|
Apologies, let me be more clear. Right now, I have a use case that uses rcgen. I would like to use aws-lc-rs in FIPS mode. I would additionally like to use linting to enforce that I don't accidentally bring back a non-FIPS usage of aws-lc-rs. By far the easiest way to do this is to use cargo deny to block the usage of the aws-lc-sys crate through my workspace. When I depend on rcgen, I add the 'fips' feature. My code then fails to compile with errors like: This is happening because with the 'fips' feature only, the dependencies are correct, but certain cfg blocks conditionally compile based on the aws-lc-rs feature flag, and not the fips one. One can get the system to compile by additionally adding the "aws-lc-rs" feature. However, this will then also compile in "aws-lc-sys", even though it will then be unused. This is unhelpful for purposes of linting, and a bit redundant. This PR is an attempt to resolve this issue by instead having a feature flag that both modes enable, and switching aws-lc-rs behaviour on this alone. The issue could also be resolved by having every Does that make sense? |
| [features] | ||
| default = ["ring"] | ||
| aws_lc_rs = ["dep:aws-lc-rs", "rcgen/aws_lc_rs", "aws-lc-rs/aws-lc-sys"] | ||
| aws_lc_rs = ["_aws_lc_like", "rcgen/aws_lc_rs", "aws-lc-rs/aws-lc-sys"] |
There was a problem hiding this comment.
It's not obvious to me that we need to propagate this setup to the rustls-cert-gen crate, since that is not intended to be used as a library anyway.
| x509-parser = ["dep:x509-parser", "rcgen/x509-parser"] | ||
| # Internal feature: enabled automatically whenever `aws_lc_rs` or `fips` is on. | ||
| # Do not enable directly; use `aws_lc_rs` or `fips` instead. | ||
| _aws_lc_like = [] |
There was a problem hiding this comment.
Also not sure it's necessary/useful to extend this to our local tests.
|
Is this an issue you run into with other rustls ecosystem crates that support a FIPS feature, or is it unique to rcgen? |
At present, this crate nicely supports the FIPS mode of aws-lc-rs. However, one is presented with a catch-22; if one depends on only the fips feature of this crate, the crate will fail to compile as critical codepaths have been commented out (the aws-lc-rs feature flag not being on).
so
Meanwhile, if one depends on the aws-lc-rs feature, it also enables the non-fips backend of aws-lc-rs, meaning that one can't use cargo deny as easily to block non-FIPS crypto backends.
I wasn't sure how to avoid this. One approach would simply be to depend on aws-lc-rs with default features on. Another would be to have the feature gates dotted around the codebase be on any(aws_lc_rs, fips) rather than just aws_lc_rs.
I figured a nice approach might be to have either feature trigger an 'aws-lc-like' feature which is what can be used through the codebase, as this minimises the likelihood that future dev work will re-introduce the dependency.
However, this does introduce a public-facing internal feature flag.
Looking forward to a review; happy to make changes as necessary! Just trying to be an OSS good citizen!