[syslinux] Domain name search path use during PXE booting

Stephen Bleazard sbleazard at janestreet.com
Fri Jan 8 01:46:57 PST 2016


Domain naming parsing wise, I couldn't find any code or any cases
where the domain search option (119) is processed.

Pointer loop handling: I agree that 32 is pretty arbitrary, the issue
I was trying to prevent is a pointer at the end of a label pointing at
the same label. This would still be backwards but infinite.  However,
this does suggest a solution: The pointer must be before the current
label. I will update the code

Malformed packets are handled by the finite state machine, after each
byte the possible error conditions are checked. Pointing before the
buffer is handled as all dhcp pointers are offsets from the buffer
start and have no sign bit.  When an error is detected the current
entry is aborted and is not included in the final list.

I agree that the local domain could be merged into the DomanSearch
list. To maintain compatibility with the current semantics it would be
at the end of the list.  I searched the code for other usages of
LocalDomain but didn't find any.

Thanks for the feedback

Steve Bleazard


On 7 January 2016 at 18:19, Celelibi <celelibi at gmail.com> wrote:
> 2016-01-07 11:25 UTC+01:00, Stephen Bleazard via Syslinux <syslinux at zytor.com>:
>> Currently it appears that when PXE booting the domain search option is
>> ignored
>> and only the domain name option used for name resolution.
>>
>> The following patch adds support for domain search path usage when PXE
>> booting:
>>   - adds parsing of the DHCP domain search option (119)
>>   - When resolving names via dns_resolv uses the search path if there's no
>>     dot in the name.
>>   - Reverts to the previous behaviour in the absence of a search path, if
>>     the name contains a dot or if no addresses are found when searching the
>>     domain search path.
>>
>> Steve Bleazard
>>
>> --
>> diff -upr syslinux-6.03.orig/core/fs/pxe/dhcp_option.c
>> syslinux-6.03/core/fs/pxe/dhcp_option.c
>> --- syslinux-6.03.orig/core/fs/pxe/dhcp_option.c      2014-10-06
>> 17:27:44.000000000 +0100
>> +++ syslinux-6.03/core/fs/pxe/dhcp_option.c   2016-01-04 07:58:18.158206039
>> +0000
>> @@ -7,6 +7,11 @@
>>  #include "pxe.h"
>>
>>  char LocalDomain[256];
>> +/*
>> + * DomainSearch contains a list of null terminated domain names
>> + * An empty string terminates the list
>> + */
>> +char DomainSearch[257];
>>
>>  int over_load;
>>  uint8_t uuid_type;
>> @@ -133,6 +138,135 @@ static void pxelinux_reboottime(const vo
>>      DHCPMagic |= 8;     /* Got reboot time */
>>  }
>>
>> +/*
>> + * See RFC1035 section 4.1.3 for the format of the domain name as it's a
>> compact format.
>> + */
>> +static void domain_search(const void *data, int opt_len)
>> +{
>> +    const uint8_t *udata = (uint8_t *)data;
>> +    const uint8_t *inp = udata; /* in the buffer, does not follow pointers
>> */
>> +    char *outp = DomainSearch;  /* current location to output chars to */
>> +    char *endp = DomainSearch;  /* end of the last completed domain string
>> */
>> +    const uint8_t *cpp;         /* copy pointer used to follow pointers */
>> +    int pcount;                 /* number of pointers visited in current
>> point copy */
>> +    int len;
>> +    enum { START, LABEL, ADD_LABEL, ADD_DOT, CP_LEN_1, CP_LABEL,
>> CP_ADD_LABEL,
>> +           CP_ADD_DOT, CP_LEN_2, FINISH, END } state, postcpstate;
>> +
>> +    state = START;
>> +    while (state != END) {
>> +        switch (state) {
>> +        case START:
>> +            cpp = udata;
>> +            state = LABEL;
>> +            break;
>> +
>> +        case LABEL:
>> +            len = *inp++;
>> +            if ((len & 0xc0) == 0xc0) {  /* pointer */
>> +                state = CP_LEN_1;
>> +                len = (len & ~0xc0) << 8;
>> +                pcount = 0;
>> +            } else if (len > 0) {       /* label length */
>> +                state = ADD_LABEL;
>> +            } else {                    /* end of string - terminate the
>> string */
>> +                if (outp - endp > 0) {  /* only if not a null string */
>> +                    outp--;             /* backup over the '.' */
>> +                    *outp++ = '\0';
>> +                }
>> +                endp = outp;
>> +            }
>> +            break;
>> +
>> +        case ADD_LABEL:
>> +            *outp++ = (char)*inp++;
>> +            if (--len <= 0)
>> +                state = ADD_DOT;
>> +            break;
>> +
>> +        case ADD_DOT:
>> +            *outp++ = '.';
>> +            state = LABEL;
>> +            break;
>> +
>> +        case CP_LEN_1:
>> +            state = CP_LABEL;
>> +            len += *inp++;
>> +            cpp = udata + len;
>> +            postcpstate = LABEL;
>> +            /*
>> +             * edge condition - the last entry in the buffer is a pointer,
>> +             *   - decrement inp to prevent overflow detection
>> +             *   - set post pointer copying state to FINISH
>> +             */
>> +            if (inp >= udata + opt_len) {
>> +                inp--;
>> +                postcpstate = FINISH;
>> +            }
>> +            break;
>> +
>> +        case CP_LABEL:
>> +            len = *cpp++;
>> +            if ((len & 0xc0) == 0xc0) {
>> +                state = CP_LEN_2;
>> +                len = (len & ~0xc0) << 8;
>> +            } else if (len > 0) {
>> +                state = CP_ADD_LABEL;
>> +            } else {
>> +                /* terminate the string */
>> +                if (outp - endp > 0) {  /* only if not a null string */
>> +                    outp--;             /* backup over the '.' */
>> +                    *outp++ = '\0';
>> +                }
>> +                cpp = udata;            /* prevent cpp going past end of
>> buffer */
>> +                endp = outp;
>> +                state = postcpstate;
>> +            }
>> +            break;
>> +
>> +        case CP_LEN_2:
>> +            state = CP_LABEL;
>> +            len += *cpp++;
>> +            cpp = udata + len;
>> +            /*
>> +             * Detect pointer loops and exit pointer copying
>> +             */
>> +            if (++pcount >= 32) {
>> +                outp = endp;            /* discard looping entry */
>> +                state = postcpstate;
>> +            }
>> +            break;
>> +
>> +        case CP_ADD_LABEL:
>> +            *outp++ = (char)*cpp++;
>> +            if (--len <= 0)
>> +                state = CP_ADD_DOT;
>> +            break;
>> +
>> +        case CP_ADD_DOT:
>> +            *outp++ = '.';
>> +            state = CP_LABEL;
>> +            break;
>> +
>> +        case FINISH:
>> +            state = END;
>> +            *endp = '\0';
>> +            break;
>> +        }
>> +
>> +        if (state != END) {
>> +            if (inp - udata >= opt_len)
>> +                state = FINISH;
>> +
>> +            if (cpp - udata >= opt_len)
>> +                state = FINISH;
>> +
>> +            if (outp - DomainSearch >= sizeof(DomainSearch) - 1)
>> +                state = FINISH;
>> +        }
>> +    }
>> +}
>> +
>
> Isn't there already a function for domain name parsing somewhere that
> you could use instead if reimplementing one? It's a genuine question,
> I don't know this part of the code.
>
> I see you have set a limit of 32 pointers to follow. However, the RFC
> say that pointers can only point backward. I think enforcing the
> restriction would be better than an arbitrary limit.
>
> I haven't read your code carefully yet so don't be offended if I say
> something stupid. :)
> How does your code handle malformed packets? What if we reach the end
> of the data during the reading of the second byte of a pointer? What
> if the pointer points after or before the buffer? What if we reach the
> end of the data at a random point while reading a label?
>
>>
>>  struct dhcp_options {
>>      int opt_num;
>> @@ -150,6 +284,7 @@ static const struct dhcp_options dhcp_op
>>      {61,  client_identifier},
>>      {67,  bootfile_name},
>>      {97,  uuid_client_identifier},
>> +    {119, domain_search},
>>      {209, pxelinux_configfile},
>>      {210, pxelinux_pathprefix},
>>      {211, pxelinux_reboottime}
>> @@ -222,6 +357,7 @@ void parse_dhcp_options(const void *opti
>>   * boot_file - boot file name
>>   * DNSServers        - DNS server IPs
>>   * LocalDomain       - Local domain name
>> + * DomainSearch      - Domain search path
>>   * MAC_len, MAC      - Client identifier, if MAC_len == 0
>>   *
>>   */
>> diff -upr syslinux-6.03.orig/core/fs/pxe/dnsresolv.c
>> syslinux-6.03/core/fs/pxe/dnsresolv.c
>> --- syslinux-6.03.orig/core/fs/pxe/dnsresolv.c        2014-10-06 17:27:44.000000000
>> +0100
>> +++ syslinux-6.03/core/fs/pxe/dnsresolv.c     2015-11-26 16:09:27.904865172
>> +0000
>> @@ -93,6 +93,7 @@ __export uint32_t dns_resolv(const char
>>      err_t err;
>>      struct ip_addr ip;
>>      char fullname[512];
>> +    char *p;
>>
>>      /*
>>       * Return failure on an empty input... this can happen during
>> @@ -110,6 +111,16 @@ __export uint32_t dns_resolv(const char
>>      if (!dns_getserver(0).addr)
>>       return 0;
>>
>> +    /* Do we have a domain search list */
>> +    if (!strchr(name, '.') && DomainSearch[0]) {
>> +        for (p = DomainSearch; *p != '\0'; p += strlen(p) + 1) {
>> +         snprintf(fullname, sizeof fullname, "%s.%s", name, p);
>> +            err = netconn_gethostbyname(fullname, &ip);
>> +            if (!err)
>> +                return ip.addr;
>> +        }
>> +    }
>> +
>>      /* Is it a local (unqualified) domain name? */
>>      if (!strchr(name, '.') && LocalDomain[0]) {
>>       snprintf(fullname, sizeof fullname, "%s.%s", name, LocalDomain);
>
> As I understand the local domain is just a kind of search list with
> one element. Is there something fundamentally different between a
> local domain and the search list?
>
> What's the expected behavior when both a local domain and a search
> list are provided by the DHCP server? Should the search list include
> the local domain? If not, should the local domain be searched before
> or after the search list?
>
> In any case, unless I missed some fundamental difference, I think
> LocalDomain and DomainSearch could be merged together.
>
>> diff -upr syslinux-6.03.orig/core/fs/pxe/pxe.h
>> syslinux-6.03/core/fs/pxe/pxe.h
>> --- syslinux-6.03.orig/core/fs/pxe/pxe.h      2014-10-06 17:27:44.000000000
>> +0100
>> +++ syslinux-6.03/core/fs/pxe/pxe.h   2015-11-26 07:54:01.516841867 +0000
>> @@ -169,6 +169,7 @@ extern uint32_t RebootTime;
>>  extern char boot_file[];
>>  extern char path_prefix[];
>>  extern char LocalDomain[];
>> +extern char DomainSearch[];
>>
>>  extern uint32_t dns_server[];
>>
>
>
>
> Best regards,
> Celelibi


More information about the Syslinux mailing list