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

Decide what to do about rustls defaulting to aws-lc-rs instead of ring #751

Closed
Diggsey opened this issue Apr 22, 2024 · 13 comments
Closed

Comments

@Diggsey
Copy link

Diggsey commented Apr 22, 2024

rustls now requires aws-lc as a default dependency, which doesn't build without a load of extra dependencies. This crate should specify default-features = false so that end users can continue to use the ring backend of rustls

@algesten algesten changed the title Do not enable default features for rustls Decide what to do about rustls defaulting to aws-lc-rs instead of ring Apr 23, 2024
@algesten
Copy link
Owner

algesten commented Apr 23, 2024

Hi @Diggsey, welcome to ureq!

I haven't made up my mind on how to deal with this situation. ureq was built on the idea of having very few dependencies. When rustls changed to aws-lc-rs, I think we end up with some tough decisions. There might be some way forward I can't see, so I'm happy for input. Observations in no particular order:

  • Rustls wants the application to set their process default provider, libraries like ureq should not touch this
    * Rustls defaults to aws-lc-rs and no ring, while rustls-webpki defaults to ring and no aws-lc-rs (as of 23 Apr 2024).
    * Ureq depends on both rustls and rustls-webpki, which means if we do nothing, we get both aws-lc-rs and ring (which goes against the idea of having minimal amount of deps)
  • aws-lc-rs claims to not require cmake for non-fips builds as of 1.7.0
  • I still see cmake as a build dep despite aws-lc-rs being 1.7.0, pulled in via aws-lc-sys. Unclear what they mean by (aws-lc-rs not requiring cmake)[https://github.com/aws/aws-lc-rs/blob/4216305d6fdff26d95713fad32d45d3f75b44ef5/aws-lc-sys/Cargo.toml#L54] (as of 23 Apr 2024)
  • Additional problems around nasm being required on windows
$ cargo tree -i cmake
cmake v0.1.50
[build-dependencies]
└── aws-lc-sys v0.15.0
    └── aws-lc-rs v1.7.0

I've had no ureq user asking for us to upgrade to rustls 0.23 – obviously just a matter of time though. I hope we can stall and have a clearer way forward once the dust settles.

@ctz
Copy link

ctz commented Apr 23, 2024

  • Ureq depends on both rustls and rustls-webpki, which means if we do nothing, we get both aws-lc-rs and ring (which goes against the idea of having minimal amount of deps)

Just on this point, I don't see any direct use of rustls-webpki in ureq. Maybe I missed it. Could that dependency could be dropped?

@algesten
Copy link
Owner

@ctz Yes I believe it can! Thank you for pointing it out!

Just on this point, I don't see any direct use of rustls-webpki in ureq. Maybe I missed it. Could that dependency could be dropped?

#752

@algesten
Copy link
Owner

Updated my above comment to reflect this.

@Diggsey
Copy link
Author

Diggsey commented Apr 23, 2024

Rustls wants the application to set their process default provider, libraries like ureq should not touch this

Right, all ureq needs to do is specify default-features = false when depending on rustls. No further changes are necessary.

If you don't want users of ureq to need to explicitly enable the feature on rustls then you could also choose to pass-through the features to rustls.

@Diggsey
Copy link
Author

Diggsey commented Apr 23, 2024

Libraries in general should always be passing default-features = false as to do otherwise prevents downstream crates from having the choice.

@algesten
Copy link
Owner

Libraries in general should always be passing default-features = false as to do otherwise prevents downstream crates from having the choice.

Yeah. I know.

Right, all ureq needs to do is specify default-features = false

That doesn't work because it make ureq unusable "out of the box". If you depend on ureq, you'd additional have to depend on rustls with a selection of backend, or ureq won't compile. I think we'll go with the advice here:

Dirkjan Ochtman:
IMO for your minimal-dependencies use case it is fine to stick with ring for the time being -- and I don't think you're the only library making that choice

@Diggsey
Copy link
Author

Diggsey commented Apr 23, 2024

That doesn't work because it make ureq unusable "out of the box". If you depend on ureq, you'd additional have to depend on rustls with a selection of backend, or ureq won't compile. I think we'll go with the rustls/rustls#1913 (comment):

You can re-expose the features from ureq - for example, ureq could have a default-feature which enables ring, and a separate feature which enables the aws-lc feature of rustls.

@algesten
Copy link
Owner

You can re-expose the features from ureq - for example, ureq could have a default-feature which enables ring, and a separate feature which enables the aws-lc feature of rustls.

True, I'm just not convinced I want to introduce more feature flags unless I absolutely have to. ureq is > 1.x.x, which means there's a kind of "contract" to be non-breaking. It's also rustls intention that the user picks backend using CryptoProvider and I'd like to avoid confusion how ureq would need to be configured in relation to this.

@algesten
Copy link
Owner

Having read up some more on aws-lc (I never encountered it before), I find the situation perplexing. aws-lc implements OpenSSL API, which means it already got a TLS frontend. Rustls is using the same crypto as aws-lc, but provides its own TLS API. So we got two TLS frontends for the same crypto backend.

I always thought Rustls ambition was to provide a "pure Rust" TLS implementation. The original README said "Rustls is a modern TLS library written in Rust". However, I clearly read things into that which was never stated.

With ring, it was of course never pure (it's a mix of asm/C/Rust) and from my limited understanding of crypto, many of the optimizations required for modern crypto primitives are hand written assembly, which will take a long time, if ever, to replace with any Rust equivalent (or at least the asm! macro). However I used to believe Rustls ambition was to aim towards a pure Rust TLS library.

From my (extremely Rust centric) view, aws-lc seems like "Yet Another OpenSSL API" derivative – why I would pick that one over another (BoringSSL, LibreSSL, OpenSSL, etc) with a safe Rust wrapper? Maybe they are at the forefront of quantum resistance, FIPS or something – but clearly there are reasons people much closer to the problem see that I don't.

For me, cmake, nasm and golang (uhm, lol?) as build requirement (yes I know golang is only for FIPS) moves rustls much further away from any "pure Rust" ideals – but then again, that was probably never the goal.

@Diggsey
Copy link
Author

Diggsey commented Apr 23, 2024

Did you mean to post this on the other other issue? (I agree completely btw)

@algesten
Copy link
Owner

algesten commented Apr 23, 2024

Did you mean to post this on the other other issue? (I agree completely btw)

No, I don't have the emotional bandwidth to engage and argue a case. In the other thread someone already said that it's about different prioritizations – which is of course true. I also know that everyone involved in rustls are deeply invested in Rust in many different projects. They are closer to this problem and likely have different prioritizations than me.

My comment is to document this for ureq's side, where I do get to pick the priorities. I'm perplexed (and sad), that a project I rate very highly took this turn, but I also don't have any choice that is more closely aligned with what I want. It is what it is.

@algesten
Copy link
Owner

Closing since we're moving to ureq 3.x.

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

No branches or pull requests

3 participants