starred/hickory-dns

Fork 0

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

[GH-ISSUE #350] How to set client-subnet for query? #160

New issue

Closed

opened 2026-03-07 22:35:05 +03:00 by kerem · 14 comments

kerem commented

2026-03-07 22:35:05 +03:00

Owner

Originally created by @aggrolite on GitHub (Feb 26, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/350

How can I set client-subnet for a DNS query? Any way to do that with SyncClient.query()?

I'd like to mimic: dig github.com +subnet=1.2.3.4/32

Originally created by @aggrolite on GitHub (Feb 26, 2018). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/350 How can I set client-subnet for a DNS query? Any way to do that with `SyncClient.query()`? I'd like to mimic: `dig github.com +subnet=1.2.3.4/32`

kerem

2026-03-07 22:35:05 +03:00

closed this issue
added the
enhance

crate:client
labels

kerem commented

2026-03-07 22:35:21 +03:00

Author

Owner

@bluejekyll commented on GitHub (Feb 26, 2018):

That’s currently not supported. I haven’t had a need for that myself, but it doesn’t look like it should be easy to add. It looks like that’s an EDNS option. EDNS is supported by TRust-DNS, so it would be a matter of adding the variant the the EDNS options. If this is the Client, then that would be it. For the Resolver there would be a bigger discussion around how to support that in the API.

@bluejekyll commented on GitHub (Feb 26, 2018): That’s currently not supported. I haven’t had a need for that myself, but it doesn’t look like it should be easy to add. It looks like that’s an EDNS option. EDNS is supported by TRust-DNS, so it would be a matter of adding the variant the the EDNS options. If this is the Client, then that would be it. For the Resolver there would be a bigger discussion around how to support that in the API.

kerem commented

2026-03-07 22:35:26 +03:00

Author

Owner

@aggrolite commented on GitHub (Feb 26, 2018):

Okay, thanks. Right now I'd be looking at using client-subnet with Client.

For adding the feature to Client, what approach would make the most sense (I'm new to Rust)?

From my perspective, it makes sense to write the following:

let client = SyncClient::new(conn);

// sets client subnet option and returns error if any
client.set_client_subnet("1.2.3.4/32").unwrap();
// or a generic function for setting any edns option?

let response = client.query(...);

In which case I'm guessing the function would be added to SyncClient, or whatever type it's based on.

@aggrolite commented on GitHub (Feb 26, 2018): Okay, thanks. Right now I'd be looking at using client-subnet with Client. For adding the feature to Client, what approach would make the most sense (I'm new to Rust)? From my perspective, it makes sense to write the following: ```rust let client = SyncClient::new(conn); // sets client subnet option and returns error if any client.set_client_subnet("1.2.3.4/32").unwrap(); // or a generic function for setting any edns option? let response = client.query(...); ``` In which case I'm guessing the function would be added to `SyncClient`, or whatever type it's based on.

kerem commented

2026-03-07 22:35:31 +03:00

Author

Owner

@bluejekyll commented on GitHub (Feb 26, 2018):

That’s one option. There’s another which would be to associate edns which each query or other message, rather than exposing a new api on the client. There are pros and cons to each... not sure which way I’d leaning at the moment.

@bluejekyll commented on GitHub (Feb 26, 2018): That’s one option. There’s another which would be to associate edns which each query or other message, rather than exposing a new api on the client. There are pros and cons to each... not sure which way I’d leaning at the moment.

kerem commented

2026-03-07 22:35:36 +03:00

Author

Owner

@bluejekyll commented on GitHub (Feb 26, 2018):

Let’s see what your method ends up looking like. It might be that we have different options based on different contexts, and there’s no single correct way since EDNS is such a grab bag of options.

@bluejekyll commented on GitHub (Feb 26, 2018): Let’s see what your method ends up looking like. It might be that we have different options based on different contexts, and there’s no single correct way since EDNS is such a grab bag of options.

kerem commented

2026-03-07 22:35:41 +03:00

Author

Owner

@LEXUGE commented on GitHub (Dec 14, 2020):

How is it going here?

@LEXUGE commented on GitHub (Dec 14, 2020): How is it going here?

kerem commented

2026-03-07 22:35:47 +03:00

Author

Owner

@leshow commented on GitHub (Mar 29, 2021):

I stumbled on this issue because I am looking for similar functionality. I'd like to see this taken more generally so we can use it for any opt record. I'm happy to implement if you're open to accepting a PR to trust-dns-client?

As for what to do, right now we have query https://docs.rs/trust-dns-client/0.20.1/trust_dns_client/client/trait.Client.html#method.query

For a dead-simple solution that would provide the functionality, we could add another method that accepted an Edns?

fn query_edns(&self, name: Name, query_class: DNSClass, query_type: RecordType, edns: Edns) -> ClientResult<DnsResponse>

&Edns or Edns?

In any case, that would require making a new Edns per query or at least cloning/moving one. The alternative is set_edns and have it re-use the same edns for every query but I think some kind of query_edns method makes more sense.

Let me know what you think @bluejekyll

edit: another option:

fn send_msg(msg: Message) -> ClientResult<DnsResponse> or something like that?

@leshow commented on GitHub (Mar 29, 2021): I stumbled on this issue because I am looking for similar functionality. I'd like to see this taken more generally so we can use it for any opt record. I'm happy to implement if you're open to accepting a PR to `trust-dns-client`? As for what to do, right now we have `query` https://docs.rs/trust-dns-client/0.20.1/trust_dns_client/client/trait.Client.html#method.query For a dead-simple solution that would provide the functionality, we could add another method that accepted an `Edns`? `fn query_edns(&self, name: Name, query_class: DNSClass, query_type: RecordType, edns: Edns) -> ClientResult<DnsResponse>` `&Edns` or `Edns`? In any case, that would require making a new `Edns` per query or at least cloning/moving one. The alternative is `set_edns` and have it re-use the same edns for every query but I think some kind of `query_edns` method makes more sense. Let me know what you think @bluejekyll edit: another option: `fn send_msg(msg: Message) -> ClientResult<DnsResponse>` or something like that?

kerem commented

2026-03-07 22:35:52 +03:00

Author

Owner

@bluejekyll commented on GitHub (Mar 29, 2021):

@leshow, I wonder if we want to create a new QueryBuilder that would allow for adding all of these options in a more elegant manner? Perhaps rather than Exposing the complexities of Edns directly, the QueryBuilder could have a add_subnet method that would populate the Edns fields for a caller?

Folks can always drop back to the send_msg function for this or more advanced usage.

edit: looking at the interfaces today, maybe a DnsRequestBuilder is the correct thing? And that would just hold a DnsRequest (Message) while it's being built up and then send could be used? https://docs.rs/trust-dns-proto/0.20.1/trust_dns_proto/xfer/dns_handle/trait.DnsHandle.html#tymethod.send

@bluejekyll commented on GitHub (Mar 29, 2021): @leshow, I wonder if we want to create a new `QueryBuilder` that would allow for adding all of these options in a more elegant manner? Perhaps rather than Exposing the complexities of Edns directly, the `QueryBuilder` could have a `add_subnet` method that would populate the Edns fields for a caller? Folks can always drop back to the `send_msg` function for this or more advanced usage. ---- edit: looking at the interfaces today, maybe a `DnsRequestBuilder` is the correct thing? And that would just hold a DnsRequest (Message) while it's being built up and then send could be used? https://docs.rs/trust-dns-proto/0.20.1/trust_dns_proto/xfer/dns_handle/trait.DnsHandle.html#tymethod.send

kerem commented

2026-03-07 22:35:57 +03:00

Author

Owner

@djc commented on GitHub (Mar 29, 2021):

Yeah, builders are the common pattern for stuff like this. I think that could make a lot of sense.

@djc commented on GitHub (Mar 29, 2021): Yeah, builders are the common pattern for stuff like this. I think that could make a lot of sense.

kerem commented

2026-03-07 22:36:02 +03:00

Author

Owner

@leshow commented on GitHub (Mar 29, 2021):

As it happens I do have a half finished branch with *builder pattern types after our last discussion, but I was sort of picturing that to be independent of this. Happy to do the send_msg variant and land builder pattern stuff later though?

I wrote some of this in the PR, but since DnsRequestBuilder has pub fields I thought it'd be a breaking change to replace use_edns with like edns: Option<Edns>

@leshow commented on GitHub (Mar 29, 2021): As it happens I do have a half finished branch with *builder pattern types after our last discussion, but I was sort of picturing that to be independent of this. Happy to do the `send_msg` variant and land builder pattern stuff later though? I wrote some of this in the PR, but since `DnsRequestBuilder` has pub fields I thought it'd be a breaking change to replace `use_edns` with like `edns: Option<Edns>`

kerem commented

2026-03-07 22:36:07 +03:00

Author

Owner

@bluejekyll commented on GitHub (Mar 29, 2021):

DnsRequest is basically just a wrapper on Message... for Client this is probably the correct thing. I noticed you adding a similar method to the Resolver in your PR, using that one, I wonder if there it's more obvious as a LookupBuilder, and probably should be a simpler abstraction then the full power of offering up Message?

@bluejekyll commented on GitHub (Mar 29, 2021): `DnsRequest` is basically just a wrapper on Message... for Client this is probably the correct thing. I noticed you adding a similar method to the Resolver in your PR, using that one, I wonder if there it's more obvious as a `LookupBuilder`, and probably should be a simpler abstraction then the full power of offering up Message?

kerem commented

2026-03-07 22:36:12 +03:00

Author

Owner

@leshow commented on GitHub (Mar 29, 2021):

Woops I meant DnsRequestOptions in that last message. What sort of things would you expose in a hypothetical LookupBuilder that would make it different from a MessageBuilder or something like that?

edit: I see you referred to send, but AFAIK that doesn't exist for SyncClient, so there is no way to send an arbitrary Message with the sync interface

@leshow commented on GitHub (Mar 29, 2021): Woops I meant `DnsRequestOptions` in that last message. What sort of things would you expose in a hypothetical `LookupBuilder` that would make it different from a `MessageBuilder` or something like that? edit: I see you referred to `send`, but AFAIK that doesn't exist for `SyncClient`, so there is no way to send an arbitrary `Message` with the sync interface

kerem commented

2026-03-07 22:36:17 +03:00

Author

Owner

@bluejekyll commented on GitHub (Mar 29, 2021):

One reason I don't want to expose MessageBuilder to users of the Resolver lookup interfaces is that there are a lot of things that are transformed in messages when they are being sent. Simple things like the message_id which changes on each request, and the resolver will make many requests potentially to fulfill a lookup (like CNAME chain resolution). Additionally, there are things like per DNS resolver endpoint EDNS configs, where we may want/need to push EDNS options on initial connection in TCP, but not on every request.

So it's definitely a subset of the Message data. I'm thinking of LookupBuilder as an extension the lookup options today, so it would be Query + some EDNS options data (like subnet), but not the entire EDNS object, because that might have to be generated on a per request basis.

Does that make sense?

Example:

let lookup = LookupBuilder::new()
         .name("www.example.com.")
         .record_type(RecordType::A)
         .edns_subnet([127.0.0])             // maybe a different name
         .validate_dnssec(true)                // and example of something else we might want to add
         .build();

@bluejekyll commented on GitHub (Mar 29, 2021): One reason I don't want to expose `MessageBuilder` to users of the Resolver lookup interfaces is that there are a lot of things that are transformed in messages when they are being sent. Simple things like the `message_id` which changes on each request, and the resolver will make many requests potentially to fulfill a lookup (like `CNAME` chain resolution). Additionally, there are things like per DNS resolver endpoint EDNS configs, where we may want/need to push EDNS options on initial connection in TCP, but not on every request. So it's definitely a subset of the `Message` data. I'm thinking of `LookupBuilder` as an extension the lookup options today, so it would be `Query` + some EDNS options data (like subnet), but not the entire EDNS object, because that might have to be generated on a per request basis. Does that make sense? ---- Example: ```rust let lookup = LookupBuilder::new() .name("www.example.com.") .record_type(RecordType::A) .edns_subnet([127.0.0]) // maybe a different name .validate_dnssec(true) // and example of something else we might want to add .build(); ```

kerem commented

2026-03-07 22:36:22 +03:00

Author

Owner

@leshow commented on GitHub (Mar 29, 2021):

That seems reasonable, I have a few additional questions though 😅

What type does build produce in your above code example? DnsRequest? Message? some new LookupOptions type?

We'll have to also add a method to the Client trait for this to be usable from both the sync/async clients too I think, since there is no send for sync (that I can see)?

@leshow commented on GitHub (Mar 29, 2021): That seems reasonable, I have a few additional questions though :sweat_smile: What type does `build` produce in your above code example? `DnsRequest`? `Message`? some new `LookupOptions` type? We'll have to also add a method to the `Client` trait for this to be usable from both the sync/async clients too I think, since there is no `send` for sync (that I can see)?

kerem commented

2026-03-07 22:36:27 +03:00

Author

Owner

@bluejekyll commented on GitHub (Mar 29, 2021):

I think we should discuss the Resolver and Client changes separately. The Resolver generally hides a lot of details from users of the API, and I can't decide if subnet is a per lookup option, or possibly a general configuration option that should be applied to an entire Resolver instance?

Any chance you could hop on discord to discuss? https://discord.gg/89nxE4n

@bluejekyll commented on GitHub (Mar 29, 2021): I think we should discuss the Resolver and Client changes separately. The Resolver generally hides a lot of details from users of the API, and I can't decide if subnet is a per lookup option, or possibly a general configuration option that should be applied to an entire Resolver instance? Any chance you could hop on discord to discuss? https://discord.gg/89nxE4n

kerem referenced this issue

2026-03-16 01:57:32 +03:00

[PR #160] [MERGED] Example docs #1272