starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 19:25:56 +03:00

[GH-ISSUE #2125] Low level send method in resolver #893

New issue

Open

opened 2026-03-16 00:46:30 +03:00 by kerem · 10 comments

kerem commented

2026-03-16 00:46:30 +03:00

Owner

Originally created by @andrewbaxter on GitHub (Jan 6, 2024).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2125

Is your feature request related to a problem? Please describe.
I'm implementing a dns server that handles some requests and forwards the rest on to another server. I was using the lower level client APIs to do this since I could forward the request parameters verbatim, as well as the response details.

I was looking into adding DoT today but it seems much more complex - specifically IIUC the hickory-client interface doesn't do any connection pooling.

As an alternative I started looking into using resolver directly, but it doesn't accept any advanced request parameters and the Lookup response similarly doesn't have any response details.

Describe the solution you'd like

I'm not sure if there's complex handling in Resolver that prevents this, but to expose a send method that takes a Message or something as a parameter and returns the full response (what lookup is doing under the hood, at some level).

Describe alternatives you've considered

Implement my own, somehow using NameServerPool but I wasn't sure how many other internals would have to come with that - its use in Resolver is fairly indirect and it seemed like it might not be appropriate for UDP upstream servers, so I'd be splitting more code paths, etc.
Create a new connection for each request, but I think this could have dire performance and fd usage consequences

Additional context
Similar: https://github.com/hickory-dns/hickory-dns/issues/1613

Originally created by @andrewbaxter on GitHub (Jan 6, 2024). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2125 **Is your feature request related to a problem? Please describe.** I'm implementing a dns server that handles some requests and forwards the rest on to another server. I was using the lower level client APIs to do this since I could forward the request parameters verbatim, as well as the response details. I was looking into adding DoT today but it seems much more complex - specifically IIUC the `hickory-client` interface doesn't do any connection pooling. As an alternative I started looking into using resolver directly, but it doesn't accept any advanced request parameters and the Lookup response similarly doesn't have any response details. **Describe the solution you'd like** I'm not sure if there's complex handling in Resolver that prevents this, but to expose a `send` method that takes a `Message` or something as a parameter and returns the full response (what `lookup` is doing under the hood, at some level). **Describe alternatives you've considered** - Implement my own, somehow using `NameServerPool` but I wasn't sure how many other internals would have to come with that - its use in `Resolver` is fairly indirect and it seemed like it might not be appropriate for UDP upstream servers, so I'd be splitting more code paths, etc. - Create a new connection for each request, but I think this could have dire performance and fd usage consequences **Additional context** Similar: https://github.com/hickory-dns/hickory-dns/issues/1613

kerem commented

2026-03-16 00:46:36 +03:00

Author

Owner

@djc commented on GitHub (Jan 8, 2024):

Just exposing a send() method that can send any message on a Resolver or AsyncResolver feels like a bit of a layering violation? I feel like the resolver abstraction kind of requires that the message is actually a query, for example.

@djc commented on GitHub (Jan 8, 2024): Just exposing a `send()` method that can send any message on a `Resolver` or `AsyncResolver` feels like a bit of a layering violation? I feel like the resolver abstraction kind of requires that the message is actually a query, for example.

kerem commented

2026-03-16 00:46:41 +03:00

Author

Owner

@bluejekyll commented on GitHub (Jan 8, 2024):

I have started some work in this area specifically for recursive resolution. I'd be curious what other changes you think would be necessary, here's an example of how that's being used, here: https://github.com/hickory-dns/hickory-dns/blob/main/crates/recursor/src/recursor_pool.rs#L98C27-L98C27

That's based on the GenericNameServerPool, which is really just a NameServerPool with a predefined GenericNameServer: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L51

The NameServerPool itself does implement DnsHandle, which has a send method for a Request: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L232

That will return a single DnsResponse... In terms of advanced parameters, we have this type that can be used to pass information to the underlying protocol, though I've never been very happy with it, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L17

DnsRequest is really just a Message with the DnsRequestOptions, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L50

So I think this already exists?

@bluejekyll commented on GitHub (Jan 8, 2024): I have started some work in this area specifically for recursive resolution. I'd be curious what other changes you think would be necessary, here's an example of how that's being used, here: https://github.com/hickory-dns/hickory-dns/blob/main/crates/recursor/src/recursor_pool.rs#L98C27-L98C27 That's based on the GenericNameServerPool, which is really just a NameServerPool with a predefined GenericNameServer: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L51 The NameServerPool itself does implement DnsHandle, which has a send method for a Request: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L232 That will return a single DnsResponse... In terms of advanced parameters, we have this type that can be used to pass information to the underlying protocol, though I've never been very happy with it, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L17 DnsRequest is really just a Message with the DnsRequestOptions, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L50 So I think this already exists?

kerem commented

2026-03-16 00:46:46 +03:00

Author

Owner

@andrewbaxter commented on GitHub (Jan 9, 2024):

So I think this already exists?

Oh I didn't realize NameServerPool implemented DnsHandle, and yeah that looks like maybe it's exactly what I want? So is that basically Resolver but without the high level interface?

I'd be interested in trying it out in any case. It looks like it's pub(crate) though ATM and I'm not sure how many other internals would need to be opened up to use it.

@andrewbaxter commented on GitHub (Jan 9, 2024): > So I think this already exists? Oh I didn't realize NameServerPool implemented DnsHandle, and yeah that looks like maybe it's exactly what I want? So is that basically Resolver but without the high level interface? I'd be interested in trying it out in any case. It looks like it's `pub(crate)` though ATM and I'm not sure how many other internals would need to be opened up to use it.

kerem commented

2026-03-16 00:46:51 +03:00

Author

Owner

@andrewbaxter commented on GitHub (Jan 9, 2024):

And yeah, didn't mean to imply that I specifically wanted this in Resolver.

@andrewbaxter commented on GitHub (Jan 9, 2024): And yeah, didn't mean to imply that I specifically wanted this in Resolver.

kerem commented

2026-03-16 00:46:57 +03:00

Author

Owner

@bluejekyll commented on GitHub (Jan 9, 2024):

I’d have to review all the details here, but I think it primarily will not do secondary lookups, like CNAME chain resolution (if it’s not in the original response packet). So I wouldn’t call it the resolver, it’s really just a connection pool (with some extra logic to deal with UDP upgrades to TCP in certain cases).

Since I’m using it across crates, the recursor pool is marked crate only, but the NameServerPool which I think is what you’re interested in should be available publicly. NameServerPool could theoretically be moved into the proto crate I think, and then be used with the Client.

@bluejekyll commented on GitHub (Jan 9, 2024): I’d have to review all the details here, but I think it primarily will not do secondary lookups, like CNAME chain resolution (if it’s not in the original response packet). So I wouldn’t call it the resolver, it’s really just a connection pool (with some extra logic to deal with UDP upgrades to TCP in certain cases). Since I’m using it across crates, the recursor pool is marked crate only, but the NameServerPool which I think is what you’re interested in should be available publicly. NameServerPool could theoretically be moved into the proto crate I think, and then be used with the Client.

kerem commented

2026-03-16 00:47:02 +03:00

Author

Owner

@djc commented on GitHub (Jan 9, 2024):

Moving NameServerPool down into the proto crate seems like an interesting idea. 👍

@djc commented on GitHub (Jan 9, 2024): Moving `NameServerPool` down into the proto crate seems like an interesting idea. 👍

kerem commented

2026-03-16 00:47:07 +03:00

Author

Owner

@andrewbaxter commented on GitHub (Jan 9, 2024):

Oh awesome, I'll try that out! And thanks for the detailed replies!

So it looks like idiomatic usage (forwarding a request) is basically

let mut name_servers = NameServerConfigGroup::new();
name_servers.push(NameServerConfig::new(SocketAddr::new(...), Protocol::Udp));
let pool = NameServerPool::from_config(
            name_servers,
            ResolverOpts::default(),
            GenericConnector::new(TokioRuntimeProvider::new()),
);
...
let resp = self1.upstream.send(DnsRequest::new({
    let mut m = Message::new();
    m.add_query({
        let mut q =
            Query::query(
                Name::from(request.query().name()),
                request.query().query_type(),
            );
        q.set_query_class(request.query().query_class());
        q
    });
    m
}, DnsRequestOptions::default())).next().await;
match resp {
    Some(Ok(r)) => ...,
    Some(Err(e)) => ...,
    None => ...
}

@andrewbaxter commented on GitHub (Jan 9, 2024): Oh awesome, I'll try that out! And thanks for the detailed replies! So it looks like idiomatic usage (forwarding a request) is basically ```rust let mut name_servers = NameServerConfigGroup::new(); name_servers.push(NameServerConfig::new(SocketAddr::new(...), Protocol::Udp)); let pool = NameServerPool::from_config( name_servers, ResolverOpts::default(), GenericConnector::new(TokioRuntimeProvider::new()), ); ... let resp = self1.upstream.send(DnsRequest::new({ let mut m = Message::new(); m.add_query({ let mut q = Query::query( Name::from(request.query().name()), request.query().query_type(), ); q.set_query_class(request.query().query_class()); q }); m }, DnsRequestOptions::default())).next().await; match resp { Some(Ok(r)) => ..., Some(Err(e)) => ..., None => ... } ```

kerem commented

2026-03-16 00:47:12 +03:00

Author

Owner

@andrewbaxter commented on GitHub (Jan 18, 2024):

This is working for me! The only trick was empty responses get turned into errors, but I think transforming them back is pretty safe and it had all the info I needed.

Should I leave this open/rename it about moving the type?

Also if I may, I think the name NameServerPool is confusing led to me skipping over it. I assumed it was a pool that returned configured name server information or something. At the moment the docs just say "Abstract interface for mocking purpose".

I think ideally there'd be something saying "If you need a lower level interface for name resolution, you can use implementations of DnsHandle" and allow users to look for implementers, but unfortunately docs.rs doesn't have the ability to search for implementations (and it's harder cross-crate).

DnsHandle docs say "A trait for implementing high level functions of DNS" which suggests it's intended for internal use only by the wording. I think it's a very usable interface and promoting it would make me less nervous.

I'd be happy to make doc changes if you can suggest the best places to put the info.

Edit: A name like NameServerPoolHandler or NameServerPoolResolver (I know it's not a full resolver, but as an example) something to indicate it's not just an auxiliary data structure would help.

@andrewbaxter commented on GitHub (Jan 18, 2024): This is working for me! The only trick was empty responses get turned into errors, but I think transforming them back is pretty safe and it had all the info I needed. Should I leave this open/rename it about moving the type? Also if I may, I think the name `NameServerPool` is confusing led to me skipping over it. I assumed it was a pool that returned configured name server information or something. At the moment the docs just say "Abstract interface for mocking purpose". I think ideally there'd be something saying "If you need a lower level interface for name resolution, you can use implementations of `DnsHandle`" and allow users to look for implementers, but unfortunately docs.rs doesn't have the ability to search for implementations (and it's harder cross-crate). `DnsHandle` docs say "A trait for implementing high level functions of DNS" which suggests it's intended for internal use only by the wording. I think it's a very usable interface and promoting it would make me less nervous. I'd be happy to make doc changes if you can suggest the best places to put the info. Edit: A name like `NameServerPoolHandler` or `NameServerPoolResolver` (I know it's not a full resolver, but as an example) something to indicate it's not just an auxiliary data structure would help.

kerem commented

2026-03-16 00:47:17 +03:00

Author

Owner

@andrewbaxter commented on GitHub (Sep 1, 2024):

I've been working with DNS64 with the google and cloudflare DNS64 servers. The google servers have been somewhat unreliable and aren't returning records, like I get a SOA response for github.com and was getting no records (A or AAAA) for the local amazon domains (cloudflare is fine).

I feel like, being a lower level structure, NameServerPool might have a more defined scope of features. I'm not sure if Hickory has the ability to query multiple servers and corroborate the results right now, but I feel like it may be the sort of feature that could be added to Resolver but out of scope of NameServerPool?

@andrewbaxter commented on GitHub (Sep 1, 2024): I've been working with DNS64 with the google and cloudflare DNS64 servers. The google servers have been somewhat unreliable and aren't returning records, like I get a SOA response for `github.com` and was getting no records (A or AAAA) for the local amazon domains (cloudflare is fine). I feel like, being a lower level structure, `NameServerPool` might have a more defined scope of features. I'm not sure if Hickory has the ability to query multiple servers and corroborate the results right now, but I feel like it may be the sort of feature that could be added to `Resolver` but out of scope of `NameServerPool`?

kerem commented

2026-03-16 00:47:22 +03:00

Author

Owner

@djc commented on GitHub (Sep 2, 2024):

I think this would involve splitting up ResolverOpts in (at least two) parts to split things that are about pooling from things that conceptually live at a higher level (like LookupIpStrategy). So yes, pushing down NameServerPool would at least require some rethinking what conceptually makes sense.

@djc commented on GitHub (Sep 2, 2024): I think this would involve splitting up `ResolverOpts` in (at least two) parts to split things that are about pooling from things that conceptually live at a higher level (like `LookupIpStrategy`). So yes, pushing down `NameServerPool` would at least require some rethinking what conceptually makes sense.