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 support for hyper-rustls #43

Closed
wants to merge 1 commit into from
Closed

Conversation

wezm
Copy link

@wezm wezm commented Aug 18, 2019

Description

Allow the hyper client to be configured to use hyper-rustls for TLS abilities, eliminating the dependency on the OS TLS library/OpenSSL.

Motivation and Context

Fixes #40

This is a quick pass at using hyper-rustls instead of native-tls. rustls makes cross-compiling easier and eliminates a dependency on a C library (OpenSSL) on open source OSes.

How Has This Been Tested?

These changes build with cargo build --no-default-features --features hyper-client,hyper-rustls,rustls. However, cargo test --no-default-features --features hyper-client,hyper-rustls,rustls fails. It appears that running the tests is broken on master with native-tls too, as cargo test --no-default-features --features hyper-client,hyper-tls,native-tls also fails.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@yoshuawuyts
Copy link
Member

@wezm I'm kind of wondering if we shouldn't straight up replace hyper's tls impl with rustls. There's quite a few options possible for hyper and if possible I'd like to prevent a combinatorial explosion in our configs for hyper features. How do you feel about only exposing rustls as Hyper's only TLS impl?

@wezm
Copy link
Author

wezm commented Aug 24, 2019

How do you feel about only exposing rustls as Hyper's only TLS impl?

That would be fine with me.

@hirrolot
Copy link

@wezm I'm kind of wondering if we shouldn't straight up replace hyper's tls impl with rustls. There's quite a few options possible for hyper and if possible I'd like to prevent a combinatorial explosion in our configs for hyper features. How do you feel about only exposing rustls as Hyper's only TLS impl?

I think that not all users of Surf may want to use Rustls and forget about a native TLS implementation (see the discussion in #40).

@yoshuawuyts
Copy link
Member

@Gymmasssorla We should probably make http-client public so any kind of configuration can be created.

@yoshuawuyts
Copy link
Member

http-client is public now, and lives as part of a separate crate. We should pick the work in this patch back up there. It's currently being integrated back into Surf; so fingers crossed we can add support for this.

Going to go ahead and close this patch for now, as we can't merge it into this repo anymore. Thanks heaps @wezm for your work!

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.

Rustls optional dependency
3 participants