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

Allow different tls implementations with cargo features #74

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

simao
Copy link
Contributor

@simao simao commented Jul 8, 2021

This is a first try to fix #53 and #21

It uses different features to allow the user to choose a tls implementation. For example, all of the following work:

cargo build # Will use with_tls+with_https using nativetls

cargo build --all-features # Will compile with with_tls+with_https+with_nativetls+withrustls but it will use nativetls

cargo build --no-default-features --features with_tls,with_https,with_rustls # will use rustls

cargo build --no-default-features --features with_tls,with_https,with_nativetls_vendored # will use nativetls_vendored

When using rustls or nativetls_vendored, we can compile dog using musl:

cargo build --target x86_64-unknown-linux-musl --no-default-features --features with_tls,with_https,with_nativetls_vendored

--features with_tls,with_rustls is supported currently, but maybe we should not support this combination at the moment, because of rustls/rustls#281 briansmith/webpki#54 rustls/rustls#184 This means for this to work you'd have to use a valid dns name as a dns server:

$ cargo run --no-default-features --features with_tls,with_rustls -- github.com -S @one.one.one.one
A github.com. 47s   140.82.121.4

With an ip address for dns server:

$ cargo run --no-default-features --features with_tls,with_rustls -- github.com -S @1.1.1.1
Error [tls]: InvalidDNSNameError

But it works with nativetls:

$ cargo run --no-default-features --features with_tls,with_nativetls_vendored -- github.com -S @1.1.1.1
A github.com. 30s   140.82.121.3

DNS over HTTPS works with rustls though:

$ cargo run --no-default-features --features with_https,with_rustls -- github.com -H @https://cloudflare-dns.com/dns-query
A github.com. 50s   140.82.121.3

--all-features is supported as per https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features To not complicate builds etc.

@CosmicToast
Copy link

It would be nice if --features with_tls,with_rustls was to work (potentially later).
I distribute statically linked binaries (they're regularly rebuilt), with a goal to have them work across multiple distributions.
The issue with openssl is that the certificate locations are hard-coded build-time, and there's no fallback mechanism (whether it be alternative directories or bundled certs), which means that it doesn't find certs in other distributions.

Go's native tls solves this by looking at all of the common directories, and webpki-roots obviously solves this as well (without any real downsides, since my binaries are rebuilt regularly).
Still, with_tls isn't as important as with_https in this case, I think, and this is a clear improvement over the current state of things, in my opinion (I currently build dog with idna only).

@ogham
Copy link
Owner

ogham commented Oct 7, 2021

Thanks for this, this is great! And sorry it took so long.

I heed the warning about briansmith/webpki#54. I'm going to leave it as a viable combination, but document the discrepancy in the compilation instructions.

@ogham ogham merged commit 1698ea6 into ogham:master Oct 7, 2021
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.

Introduce openssl-vendored or Consider using rustls (#21)
3 participants