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

feat: make scram an optional feature #180

Merged
merged 6 commits into from
Apr 28, 2024
Merged

Conversation

sunng87
Copy link
Owner

@sunng87 sunng87 commented Apr 26, 2024

This patch makes scram an optional feature completely as it introduces a few cypto libraries you might not want if you do not use scram in your application.

cc @serprex , this changes some behaviour introduced in #179

Cargo.toml Outdated
default = ["server-api-ring"]
server-api-ring = ["server-api", "ring"]
server-api-aws-lc-rs = ["server-api", "aws-lc-rs"]
default = ["server-api"]
Copy link
Contributor

@serprex serprex Apr 26, 2024

Choose a reason for hiding this comment

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

scram should probably be enabled by default, tokio-rustls needs to bring in either ring or aws-lc-rs anyways, so even if someone disables scram they'll end up needing to specify rustls dependency themselves to enable a crypto provider, so even if someone disables scram you'll want separate ring/aws-lc-rs features

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for the notes. I will be thinking about that.

@sunng87
Copy link
Owner Author

sunng87 commented Apr 27, 2024

The updated feature sets:

  • server-api-aws-lc-rs as default, to align with upstream's defaults. It includes tokio-rustls with aws-lc-rs turned on
  • server-api-ring as alternative to use ring for rustls
  • scram as optional feature to enable scram authentication, it has to be used together with either server-api-aws-lc-rs or server-api-ring

@serprex
Copy link
Contributor

serprex commented Apr 27, 2024

x509-certificate brings in ring still so scram will always bring ring & aws-lc-rs unless using server-api-ring. But that's implementation detail, maybe in future x509-certificate can add aws-lc-rs feature. Otherwise LGTM

@sunng87 sunng87 merged commit ae78d0b into master Apr 28, 2024
6 checks passed
@sunng87 sunng87 deleted the chore/more-feature-gates branch April 28, 2024 03:31
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.

None yet

2 participants