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

Consider reexporting rustls #300

Closed
Ralith opened this issue Apr 7, 2019 · 11 comments
Closed

Consider reexporting rustls #300

Ralith opened this issue Apr 7, 2019 · 11 comments

Comments

@Ralith
Copy link
Collaborator

Ralith commented Apr 7, 2019

rustls ClientConfig and ServerConfig structs are consumed by quinn-proto's public API, but cannot be constructed by a caller without additionally depending upon rustls. This is an ergonomic wart, and could become particularly annoying if a caller needs a different major version of rustls in their crate than ours. Reexporting the crate, and referencing its types directly rather than through the internal crypto facade when they appear in public locations, provides convenient access to exactly the version our interfaces need.

This is contrary to the spirit of our internal efforts to avoid being tightly coupled with rustls, but those efforts aren't currently exposed regardless.

@Ralith
Copy link
Collaborator Author

Ralith commented Apr 9, 2019

We also have a ring SigningKey in the EndpointConfig, though that's easier to paper over with a newtype.

@djc
Copy link
Member

djc commented Apr 9, 2019

I'd prefer to just push through to start exposing the traits, I just need to explore the best way to go about it. My main open question is if it's possible to close a type over an internal type parameter.

@Ralith
Copy link
Collaborator Author

Ralith commented Apr 9, 2019

I'd prefer to just push through to start exposing the traits

I think we'll still want rustls to Just Work by default at least at the high level API, since TLS is standard QUIC and rustls is by far the most convenient impl. This seems difficult to do without publicly referencing its types, even if only by implementing public traits for them and placing them in default type parameters.

close a type over an internal type parameter

I'm not sure what this is describing; could you provide an example?

@djc
Copy link
Member

djc commented Apr 9, 2019

I think we'll still want rustls to Just Work by default at least at the high level API, since TLS is standard QUIC and rustls is by far the most convenient impl. This seems difficult to do without publicly referencing its types, even if only by implementing public traits for them and placing them in default type parameters.

I wanted to hang this off ClientConfig and ServerConfig types that contain dyn trait impls to kick off CryptoSessions somehow. Seems to me like we can make this ergonomic enough while still allowing users to parametrize with different TLS/crypto implementations.

I'm not sure what this is describing; could you provide an example?

Basically, can we have a ConnectionRef type without type parameters that contains a parametrized Connection type (potentially if we Box the Connection, which might make sense anyway).

@Ralith
Copy link
Collaborator Author

Ralith commented Apr 9, 2019

Seems to me like we can make this ergonomic enough while still allowing users to parametrize with different TLS/crypto implementations.

I think this is achievable, I just want to make sure that users will get rustls out of the box if they don't go out of their way for something else.

Basically, can we have a ConnectionRef type without type parameters that contains a parametrized Connection type (potentially if we Box the Connection, which might make sense anyway).

This is generally feasible through trait objects, though it can be a little boilerplatey. I expect the public interface of Connection/Endpoint won't actually expose any genericness aside from the config structs, so if we write a trait that forwards all the method calls and box that it should just work.

@djc
Copy link
Member

djc commented Mar 12, 2020

I now think it makes sense to reexport rustls conditional on the relevant features. I feel like the crypto session provider dependency is fairly fundamental and, although we can try to make the API opaque for simple cases, there's enough moving bits and pieces here that wrapping everything is practically quite hard.

@jonatanzeidler
Copy link
Contributor

Please consider reexporting ClientConfig and ServerConfig from rustls, too. Otherwise one can run into incompatible versions with his own rustls dependency, which happened to me in a project. See this minimal example.

@djc
Copy link
Member

djc commented Jul 13, 2023

@jonatanzeidler want to submit a PR to implement this?

@rageshkrishna
Copy link

Running into this with a brand new project that picks uses rustls 0.22. Is there any plan to create a release that includes this fix any time soon?

@djc
Copy link
Member

djc commented Jan 12, 2024

Yes: #1715.

@djc
Copy link
Member

djc commented Jan 12, 2024

See also #1737.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants