Skip to content

Feat: DNS lookup via the proxy (DoH) #507

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2025
Merged

Feat: DNS lookup via the proxy (DoH) #507

merged 1 commit into from
Jun 6, 2025

Conversation

@JoseSK999
Copy link
Contributor

@JoseSK999 JoseSK999 commented Jun 3, 2025 •
edited
Loading

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other:

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other:

Description

This implements a simple DNS lookup routed through the SOCKS5 proxy, such that we don't leak that we use Bitcoin and utreexo to the system-configured DNS server (usually from our ISP), or any eavesdropper.

We use DNS-over-HTTPS (DoH) which means the DNS query is encrypted. The proxy won't be able to read the contents of the query nor the response. If we use tor as proxy, then the exit node won't see the contents either.

This is also an advantage over the system DNS lookup, as most OS don't use DoT/DoH by default, they use plaintext UDP/TCP on port 53.

This simple implementation uses the ureq crate (similar to reqwest but with compatible deps MSRV) and calls the dns.google JSON API. Google will only learn the proxy IP (or tor exit node IP) but not ours.

Notes to the reviewers

ureq vs other crates (click to expand)

I had first implemented this with reqwest but many of its dependencies require a higher rust version (1.82 or more). Then I saw the isahc crate but it doesn't seem maintained anymore. Also the minreq crate that we already have in our cargo.lock doesn't support SOCKS proxies. So ureq seems the best replacement for reqwest.

Unfortunately ureq doesn't support socks5h, which means the dns.google hostname is resolved locally, but we don't leak that we use Bitcoin anymore.

Cargo issues

The cargo.lock file diff is generated by cargo +1.74.1 generate-lockfile, but that bumps the version of a dependency of criterion called half to one with higher rust requirements:

error: package `half v2.6.0` cannot be built because it requires rustc 1.81 or newer, while the currently active rustc version is 1.74.1
Either upgrade to rustc 1.81 or newer, or use
cargo update [email protected] --precise ver

So I just called cargo +1.74.1 update [email protected] --precise 2.4.1 which is the same version we have been using, so no diff.

Another important thing is that we need the ureq/rustls-no-provider feature which avoids the rustls/ring feature, or we get some runtime panics in rustls:

  thread 'main' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.27/src/crypto/mod.rs:249:14:
  no process-level CryptoProvider available -- call CryptoProvider::install_default() before this point

EDIT: The reason for this error is that rustls allows different crypto backends, and by default it uses the aws-lc-rs one, as explained in their docs. So when we use tokio-rustls in florestad and floresta-electrum, we are using this default backend. But then the ureq crate (with default features) tries to use rustls with the alternative ring backend, creating a backend conflict that errors at runtime.

In this PR we simply keep using the aws-lc-rs backend already imported by tokio-rustls. Another option would be to use the ring one (we already have this crate in our cargo.lock anyway).

@Davidson-Souza Davidson-Souza added the enhancement New feature or request label Jun 3, 2025
@Davidson-Souza
Copy link
Member

Nice! DoH is so underrated IMO. I even have a proxy that does DoH-over-SOCKS to achieve something similar, but system-wise. I'll review the code soon.

I had first implemented this with reqwest but many of its dependencies require a higher rust version (1.82 or more).

Also, reqwest brings quite a lot of indirect dependencies, I prefer ureq.

Also the minreq crate that we already have in our cargo.lock doesn't support SOCKS proxies. So ureq seems the best replacement for reqwest.

minreq is pulled from jsonrpc. If having two http clients is something we would want to avoid, it seems pretty straightforward to use either use ureq as backend for the json-rpc client, we only need to implement one trait. Or we can use the simple-http implementation, that has no external dependencies, and can do SOCKS (so floresta-cli will work over tor)

Davidson-Souza
Davidson-Souza reviewed Jun 5, 2025
View reviewed changes
Copy link
Member

@Davidson-Souza Davidson-Souza left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still lgetting the panic you've mentioned on 0824ab9

thread 'florestad' panicked at /registry/src/index.crates.io-6f17d22bba15001f/ureq-3.0.11/src/tls/rustls.rs:143:9:
No CryptoProvider for Rustls. Either enable feature `rustls`, or set process
            default using CryptoProvider::set_default(), or configure
            TlsConfig::rustls_crypto_provider()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@JoseSK999
Copy link
Contributor Author

JoseSK999 commented Jun 5, 2025 •
edited
Loading

I'm still lgetting the panic you've mentioned on 0824ab9

thread 'florestad' panicked at /registry/src/index.crates.io-6f17d22bba15001f/ureq-3.0.11/src/tls/rustls.rs:143:9:
No CryptoProvider for Rustls. Either enable feature `rustls`, or set process
            default using CryptoProvider::set_default(), or configure
            TlsConfig::rustls_crypto_provider()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Wow, I thought it was working on my side, but you are right it doesn't. The error I mentioned was that both rustls backends were set at the same time, here we get the opposite thing. We need to enable the aws_lc_rs backend for the rustls that ureq uses as we have disabled all backends for it.

@JoseSK999
Feat: DNS lookup via the proxy (DoH)
4a0a43c 
This implements a simple DNS lookup over the SOCKS5 proxy, such that we don't leak that we use Bitcoin **and** utreexo to the system-configured DNS server (usually from our ISP), or any eavesdropper.

We use DNS-over-HTTPS (DoH) which means the DNS query is encrypted. The proxy won't be able to read the contents of the query nor the response. If we use tor as proxy, then the exit node won't see the contents either.

This is also an advantage over the system DNS lookup, as most OS don't use DoT/DoH by default, they use plaintext UDP/TCP on port 53.

This simple implementation uses the `ureq` crate (similar to `reqwest` but with compatible deps MSRV) and calls the dns.google JSON API. Google will only learn the proxy IP (or tor exit node IP) but not ours.

In `florestad` I had to change the proxy address parsing code a bit, such that we directly parse a IP:port.
@JoseSK999
Copy link
Contributor Author

Now it works! I had to configure the ureq Agent. This was explained here in their docs

Davidson-Souza
Davidson-Souza approved these changes Jun 6, 2025
View reviewed changes
Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4a0a43c

@Davidson-Souza Davidson-Souza merged commit 614f138 into vinteumorg:master Jun 6, 2025
10 checks passed
@JoseSK999 JoseSK999 deleted the proxy-dns branch June 6, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Davidson-Souza Davidson-Souza Davidson-Souza approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JoseSK999 @Davidson-Souza