-
Notifications
You must be signed in to change notification settings - Fork 861
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
Support for DHCP Option 119 (RFC 3397 - DNS search list)? #400
Comments
IPv4 DHCP option 119 looks similar to the IPv6 ND RA DNSSL option, which is supported. I could check if it is possible to model one after the other. Could you provide a small .pcap file with a example of the DHCP option? Thank you. |
Will do shortly, thanks! On Aug 12, 2014, at 1:30 AM, Denis Ovsienko [email protected] wrote:
|
I'm attaching a small zipped pcap file with a few DHCP packets. Packet 5 has the DHCPREQUEST, with the option 55 (parameter request list) containing "119", and packet 6 has the reply, with the option 119 included. Wireshark is able to show this information correctly, btw - they identify the option name as "Domain Search", and show the two domains in this example ("velocloud.net" and "eng.velocloud.net"). |
Dang. Can't attach arbitrary binary files to issues in github :-(. Here's a uuencoded dump instead (of the zipped pcap file):
|
Thank you, I will check and update this issue. |
The Domain Search (119) option is a bit more complex because it encourages RFC1035 compression (DNSSL encoding forbids compression) and RFC3396 splitting (multiple DNSSL occurrences are to be interpreted independently from each other). I will look more. |
Thank you, Denis. I, too, got a little lost when I got to the splitting long options. Though if we can just get in some basic support for the decompression of single options (within the usual limits), that itself would be a big win. On Aug 13, 2014, at 2:15 PM, Denis Ovsienko [email protected] wrote:
|
I still don't have a clear idea for the solution, let me deassign this issue for the time being. Please post here any useful feedback you have. |
That's all right, Denis. Thanks for taking a look it at - I was just hoping that someone might have something up their sleeve. Let's just leave this as an open, un-assigned request. On Aug 29, 2014, at 12:45 PM, Denis Ovsienko [email protected] wrote:
|
This is actually surprisingly easy. (I guess I'm the one with something up my sleeve). Here's my current output from the supplied pcap:
Given that option 15 is quoted, what should we do with option 119? Quote each name, like |
I see that the DHCP6 option is space separated with no quotes: case DH6OPT_DOMAIN_LIST:
tp = (const u_char *)(dh6o + 1);
while (tp < cp + sizeof(*dh6o) + optlen) {
ND_PRINT(" ");
if ((tp = fqdn_print(ndo, tp, cp + sizeof(*dh6o) + optlen)) == NULL)
goto trunc;
} so perhaps the dhcp printer should mimic that. |
remove quotes for option 15 ? |
I'm always a little hesitant to change existing output, for fear of people who write scripts that |
Ok. |
DHCP6 should be converted to fqdn_print2(). With fqdn_print() a badly encoded DNS name in the middle of an option can overrun into other DHCP data, as long as it hasn't overrun the packet data. It will print some nice garbage and maybe not print the other DHCP6 fields, but won't overrun the packet data so isn't dangerous. DHCP6 is actually passing what should be the 4th arg to fqdn_print2() to the 3rd arg of fqdn_print(). Luckily, this is only used for compression, and compression is prohibited in DHCP6 (see RFC3315 section 8). But a DHCP6 packet using compression will behave as though the end of the option was the beginning of the DNS, so will have similar results to what I describe above. (Perhaps fqdn_print2() should take another argument: is compression allowed, so that the DHCP6 callers can ask fqdn_print2() to point out that compression is used.) |
Here's an example of using
Again, you can see that it stopped printing when it got to the end of the packet (the END option is 0xff, which is M-^?), so there's no overflow concern, but it is not very pretty. I think a NULL 3rd arg to fqdn_print2() can imply that compression is not supported, I'll try that out. |
Even with ? + if((tp = fqdn_print(ndo, tp, bp)) == NULL)
+ goto trunc; (which tool to swap the DHCP options ?) |
I used
|
Could you confirm |
In the example pcap,
as
i.e., the compression of eng.velocloud.net is clearly handled correctly. By splitting, do you mean correctly handling |
Also fix some minor bugs in dhcp6 and hncp handling of domain names Closes the-tcpdump-group#400
seems like above one is not compression. it is the syntax defined by RFC https://tools.ietf.org/html/rfc3397#section-3 |
That RFC refers to RFC 1035 section 4.1.4, "Message compression". |
@fenner, do you feel like respinning this change now or after the more important work is done? It would be good to have this eventually reviewed and merged. |
I've had other changes where I rebased regularly, which turned out to be a wasted effort. Are you saying that if I rebase this now, someone will look at it? |
I can spend up to a day on proofreading your implementation if you rebase now. |
Also fix some minor bugs in dhcp6 and hncp handling of domain names Closes the-tcpdump-group#400
Thanks. I've rebased, tidied up some minor details, and made sure the tests pass. Luckily there was no EXTRACT_ stuff to have to change. |
Thank you, I plan to come back with comments soon. |
Also fix some minor bugs in dhcp6 and hncp handling of domain names Closes the-tcpdump-group#400
I went through the encoding sections of RFC 1035, but could not relate that to the changes in the code yet. |
There are two main changes:
So each of the searchstrings is encoded as a series of labels (RFC1035:)
using compression pointers to eliminate duplication:
In the sample pcap, the list is "velocloud.net" and "eng.velocloud.net", and the packet data is:
At 0x135, there's a length 9 and "velocloud", then a length 3 and "net", then a 0 (the null label that terminates the string). Next, there's a length 3 and "eng", and 0xc000, which is a pointer to offset 000, meaning, start at the beginning and add the .velocloud.net. This is what the comment above fqdn_print2() is referring to:
and
--
Doing this right probably means implementing the RFC3396 algorithm for all options, looping over the whole packet to create a full option store in memory and then printing them all. (This of course creates the dilemma that the GET macros assume you've been printing all along, not creating a buffer, and also don't support retrieving data from a buffer that is not the packet, so there's a couple of levels of "I don't really have a great way to do that" to implement RFC3396) |
Thank you for the explanation, please bear with me. |
Also fix some minor bugs in dhcp6 and hncp handling of domain names Closes the-tcpdump-group#400
I see that support for this commonly-used option is still missing. Basically, a payload has to be decoded as described in http://tools.ietf.org/search/rfc3397 (specifically, the compression algorithm in http://tools.ietf.org/search/rfc1035#section-4.1.4).
Has this topic come up before? Is there a reason why this is still excluded? (I realize that the RFC is still in "draft" status, but so are so many others..)
The text was updated successfully, but these errors were encountered: