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

Use rusttls for dnssec for peer seeds #2690

Merged
merged 1 commit into from Mar 3, 2021

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Feb 24, 2021

Use dns-over-rustls feature on trust-dns-client crate. This should
remove the openssl-sys dependency from many crates that depend on the
tari-p2p crate.

Note: openssl-sys is still a dependency of hyper and reqwest (merge mining proxy) as well as git2 build dependency
but I expect that removing the openssl dep from tari-p2p will remove it from release builds of the base node and wallet

UPDATE: peer seed resolver works on local base node test

Use `dns-over-rustls` feature on `trust-dns-client` crate. This should
remove the `openssl-sys` dependency from many crates that depend on the
`tari-p2p` crate.
@StriderDM
Copy link
Contributor

Not sure it will be as simple as this, see:
https://crates.io/crates/native-tls
https://crates.io/crates/trust-dns-rustls

In one way or another openssl-sys ends up as a dependency.

The mobile build will need to be updated as well as the mobile apps to remove openssl as a dependency. However it seems that for mobile, openssl will always be used.

@sdbondi
Copy link
Member Author

sdbondi commented Feb 24, 2021

Yeah not 100% sure, difficult to trace all the dependencies.

AFAICS the dns-over-rustls feature does not use native-tls (there is a dns-over-native-tls feature for that)
and trust-dns-rustls only has openssl as a dev dependency. So I expect that we at least won't require openssl for release builds.

Since we are just changing the peer seed resolver (no other component is effected) and that works on my local base node test, I figure it's worth a try.

@StriderDM
Copy link
Contributor

Been running through some of the docs for rustls and it's dependencies.

We will definitely need to check the mobile apps with this, see here:
https://github.com/briansmith/ring#online-automated-testing

For the following platforms:

aarch64-linux-android
armv7-linux-androideabi
aarch64-apple-ios

@hansieodendaal
Copy link
Contributor

Tested this on Windows 10 after removing Open SSL from the path; runtime works perfectly without it.

Note: Tested (1) base node, (2) console wallet, (3) merge mining proxy and (4) mining node.

@stringhandler stringhandler merged commit 565da79 into tari-project:development Mar 3, 2021
@sdbondi sdbondi deleted the p2p-remove-openssl branch March 3, 2021 13:21
@stringhandler stringhandler mentioned this pull request Mar 10, 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.

None yet

4 participants