the-tcpdump-group / tcpdump

the TCPdump network dissector
https://www.tcpdump.org/
Other
2.67k stars 838 forks source link

Support for DHCP Option 119 (RFC 3397 - DNS search list)? #400

Open shankarunni opened 10 years ago

shankarunni commented 10 years ago

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..)

infrastation commented 10 years ago

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.

shankarunni commented 10 years ago

Will do shortly, thanks!

On Aug 12, 2014, at 1:30 AM, Denis Ovsienko notifications@github.com wrote:

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.

— Reply to this email directly or view it on GitHub.

shankarunni commented 10 years ago

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").

shankarunni commented 10 years ago

Dang. Can't attach arbitrary binary files to issues in github :-(. Here's a uuencoded dump instead (of the zipped pcap file):

    begin 664 dhcp-option119.pcap.zip
    M4$L#!!0``@`(``M3#$4FP_S3<`$``+@(```3`!P`9&AC<"UO<'1I;VXQ,3DN
    M<&-A<%54"0`#Q4WJ4]I-ZE-U>`L``03U`0``!!0```"[<GC30B8&%@88^/^?
    M@8$12.?[O@J>>(N%(0S(`>'_8."0T^]MS,G!X,K`Z+'&#2@JR!</T074Y\+@
    MS&A2YL+(R,:P:M<F"08L`**?8=B`Y*;@9%-&1G-.1F8V_O+X/SIZEDRL=VS9
    M&2$>-69A*%O.P,-=G)&8EYU8I)N;5/`?17\!,)0S^CGQA;([.)3CT$+9&1[*
    MC*.A3"B4"X&A7-+.PY`%#"L0AFC[T'>;D0$4R@<88T05@:*"?KNY@(D?B+\R
    M.#.X,'K(?V-"2\L@.2Y(!AFFH<QDQ@+R(#!,&?@$K(`D.X<UD.3Q8V0!I3X9
    MD.Q_9K`:-C#)SYR3F%<NREF6FI.?G)-?FL*<EUK"P)R:EWZ`X3\HY-TZ\(:\
    M$CCD=Z&%_%<FM/0]&O*DA7P1,.3S7_#@*UD\P"5++&K)\F$>O&1A'BDE"S.N
    MDL4(%,Y?(?&"M7P!A?+\5WC3MS(X?>]$3=\RGYG00GGXIV]6JJ9O`%!+`0(>
    M`Q0``@`(``M3#$4FP_S3<`$``+@(```3`!@```````````"D@0````!D:&-P
    M+6]P=&EO;C$Q.2YP8V%P550%``/%3>I3=7@+``$$]0$```04````4$L%!@``
    0```!``$`60```+T!````````
    `
    end
infrastation commented 10 years ago

Thank you, I will check and update this issue.

infrastation commented 10 years ago

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.

shankarunni commented 10 years ago

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 notifications@github.com wrote:

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.

— Reply to this email directly or view it on GitHub.

infrastation commented 10 years ago

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.

shankarunni commented 10 years ago

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 notifications@github.com 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.

— Reply to this email directly or view it on GitHub.

fenner commented 5 years ago

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:

    10.0.0.1.67 > 10.0.0.245.68: [udp sum ok] BOOTP/DHCP, Reply, length 320, xid 0xaabab218, secs 1, Flags [none] (0x0000)
          Your-IP 10.0.0.245
          Server-IP 10.0.0.1
          Client-Ethernet-Address 40:6c:8f:4b:33:09
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message Option 53, length 1: Offer
            Server-ID Option 54, length 4: 10.0.0.1
            Lease-Time Option 51, length 4: 3600
            RN Option 58, length 4: 1800
            RB Option 59, length 4: 3150
            Subnet-Mask Option 1, length 4: 255.255.255.0
            BR Option 28, length 4: 10.0.0.255
            Default-Gateway Option 3, length 4: 10.0.0.1
            Domain-Name-Server Option 6, length 4: 10.0.0.1
            Domain-Name Option 15, length 3: "lan"
            Domain-Search Option 119, length 21: velocloud.net., eng.velocloud.net.

Given that option 15 is quoted, what should we do with option 119? Quote each name, like "velocloud.net.", "eng.velocloud.net"?

fenner commented 5 years ago

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.

fxlb commented 5 years ago

remove quotes for option 15 ?

fenner commented 5 years ago

remove quotes for option 15 ?

I'm always a little hesitant to change existing output, for fear of people who write scripts that tcpdump | awk (however wise that is in the first place).

fxlb commented 5 years ago

Ok. Why need for fqdn_print2() and not for DHCP6 ?

fenner commented 5 years ago

Why need for fqdn_print2() and not for DHCP6 ?

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.)

fenner commented 5 years ago

Here's an example of using fqdn_print() with the arguments that dhcp6 passes: I modified the packet dump to swap the Domain-Name and Domain-Search options, and changed the code to not use fqdn_print2(), and as you can see the data of the Domain-Name option ended up in the Domain-Search output:

            Domain-Name-Server Option 6, length 4: 10.0.0.1
            Domain-Search Option 119, length 21: velocloud.net., eng.^ClanM-^?, 
            Domain-Name Option 15, length 3: "lan"
            END Option 255, length 0

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.

fxlb commented 5 years ago

Even with ?

+                                   if((tp = fqdn_print(ndo, tp, bp)) == NULL)
+                                       goto trunc;

(which tool to swap the DHCP options ?)

fenner commented 5 years ago

Even with ?

+                                   if((tp = fqdn_print(ndo, tp, bp)) == NULL)
+                                       goto trunc;

I used fqdn_print(ndo, tp, ep) to mimic how dhcp6 was using it.

(which tool to swap the DHCP options ?)

xxd and vi and parsing the packet by eye.

infrastation commented 5 years ago

Could you confirm fqdn_print() is going to handle compression and splitting correctly in this case?

fenner commented 5 years ago

Could you confirm fqdn_print() is going to handle compression and splitting correctly in this case?

In the example pcap, fqdn_print2() handles

00000880:                                      77  ..N............w
00000890: 1509 7665 6c6f 636c 6f75 6403 6e65 7400  ..velocloud.net.
000008a0: 0365 6e67 c000                           .eng............

as

            Domain-Search Option 119, length 21: velocloud.net. eng.velocloud.net.

i.e., the compression of eng.velocloud.net is clearly handled correctly.

By splitting, do you mean correctly handling \x09velocloud\x03net\x00 as "velocloud.net."?

leslie-qiwa commented 3 years ago

seems like above one is not compression. it is the syntax defined by RFC https://tools.ietf.org/html/rfc3397#section-3

fenner commented 3 years ago

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".

infrastation commented 3 years ago

@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.

fenner commented 3 years ago

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?

infrastation commented 3 years ago

I can spend up to a day on proofreading your implementation if you rebase now.

fenner commented 3 years ago

I can spend up to a day on proofreading your implementation if you rebase now.

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.

infrastation commented 3 years ago

Thank you, I plan to come back with comments soon.

infrastation commented 3 years ago

I went through the encoding sections of RFC 1035, but could not relate that to the changes in the code yet.

fenner commented 3 years ago

There are two main changes:

  1. for DHCP option 119, loop over the option calling fqdn_print2() until we have processed the entire option. This corresponds to this section of RFC3397:
    To enable the searchlist to be encoded compactly, searchstrings in
    the searchlist MUST be concatenated and encoded using the technique
    described in section 4.1.4 of "Domain Names - Implementation And
    Specification" [RFC1035].  In this scheme, an entire domain name or a
    list of labels at the end of a domain name is replaced with a pointer
    to a prior occurrence of the same name.

So each of the searchstrings is encoded as a series of labels (RFC1035:)

Domain names in messages are expressed in terms of a sequence of labels.
Each label is represented as a one octet length field followed by that
number of octets.  Since every domain name ends with the null label of
the root, a domain name is terminated by a length byte of zero.  The
high order two bits of every length octet must be zero, and the
remaining six bits of the length field limit the label to 63 octets or
less.

using compression pointers to eliminate duplication:

In this scheme, an entire domain name or a list of labels at
the end of a domain name is replaced with a pointer to a prior occurance
of the same name.

In the sample pcap, the list is "velocloud.net" and "eng.velocloud.net", and the packet data is:

        0x0130:  0000 ff77 1509 7665 6c6f 636c 6f75 6403  ...w..velocloud.
        0x0140:  6e65 7400 0365 6e67 c000 0304 0a00 0001  net..eng........

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: print a <domain-name> with an explicit end-pointer - <domain-name> refers to RFC1035:

<domain-name> is a domain name represented as a series of labels, and
terminated by a label with zero length.

and

Each label is represented as a one octet length field followed by that
number of octets.  Since every domain name ends with the null label of
the root, a domain name is terminated by a length byte of zero.
  1. A minor change introducing fqdn_print2(), which has the following additional behavior from fqdn_print(): a. It allows explicitly specifying the end of the DNS data. fqdn_print() will decode to the end of the captured packet, but in the DHCP case, the DNS data ends at the end of the DHCP option, not at the end of the packet. b. It allows disabling compression (the pointer that was 0xc000 in the example above). Disabling compression is used in DHCPv6, due to this text from RFC3315:
  2. Representation and Use of Domain Names

    So that domain names may be encoded uniformly, a domain name or a list of domain names is encoded using the technique described in section 3.1 of RFC 1035 [10]. A domain name, or list of domain names, in DHCP MUST NOT be stored in compressed form, as described in section 4.1.4 of RFC 1035.

-- There is a bug in the current code, which I discovered on re-reading RFC3397. It says:

   If multiple Domain Search Options are present, then the data portions
   of all the Domain Search Options are concatenated together as
   specified in "Encoding Long DHCP Options in the Dynamic Host
   Configuration Protocol (DHCPv4)" [RFC3396] and the pointer indicates
   an offset within the complete aggregate block of data.

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)

infrastation commented 3 years ago

Thank you for the explanation, please bear with me.