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

DNS bypass #561

Closed
frenchtoastbeer opened this issue Jul 8, 2019 · 15 comments · Fixed by #1277
Closed

DNS bypass #561

frenchtoastbeer opened this issue Jul 8, 2019 · 15 comments · Fixed by #1277
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.

Comments

@frenchtoastbeer
Copy link

frenchtoastbeer commented Jul 8, 2019

It would be nice if there were a way we could bypass DNS resolution and connect directly to an IP address we specify during the request. Something like...

client.skip_dns('127.0.0.1').get("https:/google.com").send()

SNI / cert validation could still work off the domain in the URI.

@seanmonstar
Copy link
Owner

That seems like a fair desire, unsure about the best API to expose.

@seanmonstar seanmonstar added the B-rfc Blocked: Request for comments. More discussion would help move this along. label Jul 8, 2019
@frenchtoastbeer
Copy link
Author

So I toyed with this a little by modifying the async_impl/request struct like:

pub struct Request {
    ip: Option<std::net::Ipv4Addr>,
    method: Method,
    url: Url,
    headers: HeaderMap,
    body: Option<Body>,
}

and then tried compiling / updating over and over again, making the necessary changes at each step to get past whatever errors were introduced. I tried to stay true to how the rest of the library treated an instance of a Request at each step, but I got stuck at...

C:\dns_bypass (master -> origin)λ cargo build
   Compiling hyper v0.12.27 (C:\dns_bypass\.cargo\hyper)
error[E0308]: mismatched types
   --> .cargo\hyper\src\proto\h1\dispatch.rs:464:47
    |
464 |                         Ok(Async::Ready(Some((ip, head, body))))
    |                                               ^^ expected struct proto::MessageHead, found struct http::Ip
    |
    = note: expected type proto::MessageHead<proto::RequestLine>
               found type http::Ip

I didn't think it should be merged into request "headers" as its rather part of the IP header, but taking this approach meant I needed to modify the h2, hyper, and http libraries as well, and possibly more?

@seanmonstar
Copy link
Owner

Before you go to far down that rabbit hole, I think it'd be best to think about the exposed API to the user. The implementation likely doesn't need to bother touching h2/hyper/http, and would probably just adjust reqwest's connect.rs stuff...

@frenchtoastbeer
Copy link
Author

Ahh. Well it's too late for me I already went down the rabbit hole! Anyhow, I think you're right it's best to avoid changes on any dependent libraries. I'll try again sometime this week hopefully and report back.

For the API, I already don't like .skip_dns() because "DNS" is just one of the ways by which an IP could be resolved. I do like that the default behavior is to attempt resolution, since I believe the users of reqwest would prefer not to worry about it.

Perhaps skip_resolution would be better? I still like 'skip', since it implies that the standard behavior is to attempt IP resolution, and shows that by calling that function, no subsequent resolution would occur.

I don't think support for non-IP protocols would be anything in-scope for this library, so accepting an std::net::IpAddr would be best?

Having anything more than a .skip_resolution(std::net::IpAddr) builder method to support this behavior, I think, would be unnecessary.

@seanmonstar
Copy link
Owner

Does curl and other libraries provide an easy option to do this?

@frenchtoastbeer
Copy link
Author

frenchtoastbeer commented Jul 8, 2019

curl has --resolve <host:port:address> Resolve the host+port to this address
wget does similar by over-riding the host header, but I imagine that isn't going to be compatible with SNI?

Now that I'm looking at it, I can see why curl would have a --resolve as opposed to a skip_resolution, as it would matter with redirects. If you skip resolution on the first host, but then get redirected, do you then use DNS for the redirect if it goes to a new host?

In this case, you'd want to simply set up a resolution mask where host A maps to IP A, and let all other names resolve normally. In curls example, they are allowing host A to map to IP A, only on a specific port, otherwise use DNS as normal (I think even for host A on a different port).

That seems a bit more complex than what I was envisioning, where I would simply want to establish a static Host->IP resolution, ignoring the port.

Now that I've seen how curl does it, I think I'd want to leave DNS resolution "on" for any names that weren't already defined, and have a .resolve(&str, std::net::IpAddr) to define name->ip's that would be usable prior to issuing each request. Each .resolve() would append to an internal dns cache that would then be checked prior to doing DNS resolution. I anticipate that there wouldn't be so many DNS over-rides that one would want .resolve() to accept an array of names->IP's.

Since the default resolver is already replaceable with a custom one, maybe all this is really a bogus feature request? All I'd need to do is figure out how to mimic the default resolver and then add my very basic "mask" in front of that resolver for an early return. I'm not sure how to do that, but already it seems that would be a more acceptable direction to take, rather than have all reqwest requests perform a dns cache check for something that probably doesn't exist 99% of the time.

@frenchtoastbeer
Copy link
Author

Maybe I was confused, I'm looking now at the reqwest source and I don't see how to set my own resolver on either the client or the request. Was that part of hyper?

@soyflourbread
Copy link

@seanmonstar Can you reopen this issue? Specifying custom DNS resolution is quite helpful.

@frenchtoastbeer
Copy link
Author

I was the one that closed it, re-opening now.

@x1957
Copy link
Contributor

x1957 commented Feb 11, 2020

A quite useful feature.

@CliffHan
Copy link

CliffHan commented Mar 2, 2020

That's really useful, especially for us to bypass DNS spoofing.

@soyflourbread
Copy link

It seems that it involves modifying hyper’s source code for per-request DNS bypassing to work.

@quasicomputational
Copy link

I think that the API presented to users for this could also be useful for #39.

@campbellC
Copy link
Contributor

campbellC commented Mar 21, 2021

I've played around with an implementation of this and it's definitely bubbling up to cause some ugliness so I think there are some choices to make.

Currently the HttpConnector is an enum with either the GaiResolver or the TrustDNS resolver. Here's my attempt to make a generic wrapper resolver that would allow a user to add specific overrides:

    async fn new_wrapping_resolver<R, U>(
        mut wrapped_resolver: R,
        overrides: std::sync::Arc<HashMap<String, SocketAddr>>,
    ) -> impl Service<Name, Response = impl Iterator<Item = SocketAddr>>
    where
        R: Service<Name, Response = U> + 'static,
        U: Iterator<Item = SocketAddr>,
    {
        type EitherOverrideOrWrapped<U> = itertools::Either<std::iter::Once<SocketAddr>, U>;
        tower::service_fn(move |name: Name| {
            if let Some(overridden) = overrides.get(name.as_str()) {
                return futures_util::future::ok::<EitherOverrideOrWrapped<U>, R::Error>(
                    EitherOverrideOrWrapped::Left(std::iter::once(*overridden)),
                )
                .left_future();
            } else {
                return wrapped_resolver
                    .call(name)
                    .map(|result| result.map(|iter| EitherOverrideOrWrapped::Right(iter)))
                    .right_future();
            }
        })
    }

This is the first time I've written this kind of futures + iterators code so this may be unidiomatic (happy to rework if there are better options). NB: this adds tower and itertools as new dependencies of reqwest.

The implication of this though is that the resolver becomes a new unnamed type, and there is an overhead for clients that are not using overrides at all.

Other options for a design here that I can see:

  • A new WrappedResolver Struct generic over the resolver. Then two new constructors for HttpConnector with names new_gai_with_overrides() and new_trust_dns_with_overrides(). Then a third variant of HttpConnector with a generic inner type. I don't know enough about how the different resolvers are used to know whether this has more implications and this means two new functions per resolver being added.
  • Make HttpConnector a generic new_type over hyper::client::HttpConnector rather than an enum. This would simplify connect.rs (I think) but would presumably impact the rest of the crate.

If anyone has any input on how this could be cleanly implemented I'm happy to take a swing and get a PR up.

@campbellC
Copy link
Contributor

campbellC commented Mar 21, 2021

As for user API my suggestion would be:

    reqwest::Client::builder()
        .resolve("example.com", "127.0.0.1")
        .resolve("example2.com", "127.0.0.1")
        .build()?

This would be in keeping with curls API.

There is a question as to whether we want to allow custom resolves to map the same domain to multiple IP addresses. The use case I'm motivated by here is for testing against a local running TLS server so this isn't an issue but there may be other use cases that would prefer multiple potential IPs.

campbellC added a commit to campbellC/reqwest that referenced this issue Jun 1, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
campbellC added a commit to campbellC/reqwest that referenced this issue Jun 1, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
campbellC added a commit to campbellC/reqwest that referenced this issue Jun 3, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
campbellC added a commit to campbellC/reqwest that referenced this issue Jun 5, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
campbellC added a commit to campbellC/reqwest that referenced this issue Jun 16, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
campbellC added a commit to campbellC/reqwest that referenced this issue Jun 16, 2021
…tar#561)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
seanmonstar pushed a commit that referenced this issue Jun 16, 2021
…1277)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes #561
pfernie added a commit to pfernie/reqwest that referenced this issue Jun 25, 2021
* master:
  add option to disable http2 upgrade (seanmonstar#1292)
  Fix documentation of http1_title_case_headers() (seanmonstar#1294)
  CI: make a single final job that depends on all others (seanmonstar#1291)
  Fix bare url warnings in `multipart` docs (seanmonstar#1289)
  v0.11.4
  Allow overriding of DNS resolution to specified IP addresses(seanmonstar#561) (seanmonstar#1277)
  WASM: Add `try_clone` implementations to `Request` and `RequestBuilder` (seanmonstar#1286)
  Add native-tls-alpn feature (seanmonstar#1283)
Nutomic pushed a commit to Nutomic/reqwest that referenced this issue Nov 7, 2024
…tar#561) (seanmonstar#1277)

This change allows users to bypass the selected DNS resolver for
specific domains. The allows, for example, to make calls to a local TLS
server by rerouting a given domain to 127.0.0.1.

The approach I've taken for the design is to wrap the resolver in an
outer service. This leads to a fair amount of boilerplate code mainly to
be able to explain the typing to the compiler. The actual business logic
is very simple for the number of lines involved.

Closes seanmonstar#561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants