Syslinux - Sep 2015 - pxelinux tries to load ldlinux.c32 from DHCP server, instead of next-server

On Sat, Sep 12, 2015 at 08:42:15AM -0400, Gene Cumm via Syslinux
wrote:
> On Sat, Sep 12, 2015 at 7:37 AM, Gene Cumm <gene.cumm at gmail.com>
wrote:
> > On Sat, Sep 12, 2015 at 7:08 AM, Gene Cumm <gene.cumm at
gmail.com> wrote:
> >> On Sat, Sep 12, 2015 at 5:54 AM, Teun Docter
> >> <teun.docter at brightcomputing.com> wrote:
> >>> On 2015-09-12 04:58, Gene Cumm wrote:
> >>>>>
> >>>>> I've captured the following DHCP ACK:
> >>>>>
> >>>>> 10.141.20.1.bootps > 10.141.20.2.bootpc: [udp sum
ok] BOOTP/DHCP, Reply,
> >>>>> length 399, xid 0xf0eb955a, secs 14, Flags [none]
(0x0000)
> >>>>>            Your-IP 10.141.20.2
> >>>>>            Server-IP 10.141.255.254
> >>>>>            Client-Ethernet-Address fa:16:3e:08:31:b9
> >>>>>            Vendor-rfc1048 Extensions
> >>>>>              Magic Cookie 0x63825363
> >>>>>              DHCP-Message Option 53, length 1: ACK
> >>>>>              Server-ID Option 54, length 4:
10.141.20.1
> >>>>>              Lease-Time Option 51, length 4: 86400
> >>>>>              RN Option 58, length 4: 43200
> >>>>>              RB Option 59, length 4: 75600
> >>>>>              TFTP Option 66, length 7:
"master^@"
> >>>>
> >>>>
> >>>> What does the name "master.openstacklocal" point
to?  I typically set
> >>>> dhcp-66 to the dotted quad string of the destination TFTP.
Option 66 is TFTP-server _name_, no need to restrict yourself
to four dot seperated octets.

> >>>
> >>>
> >>> Hmm, actually, it doesn't resolve.
> >>>
> >>>>>              BF Option 67, length 11:
"pxelinux.0^@"
> >>>>>              Subnet-Mask Option 1, length 4:
255.255.0.0
> >>>>>              BR Option 28, length 4: 10.141.255.255
> >>>>>              Domain-Name-Server Option 6, length 4:
10.141.20.1
> >>>>>              Domain-Name Option 15, length 14:
"openstacklocal"
> >>>>>              Hostname Option 12, length 16:
"host-10-141-20-2"
> >>>>>              Default-Gateway Option 3, length 4:
10.141.255.250
> >>>>>              T209 Option 209, length 45:
> >>>>>
> >>>>>
112.120.101.108.105.110.117.120.46.99.102.103.47.99.97.116.101.103.111.114.121.46.118.105.114.116.117.97.108.45.110.111.100.101.115.46.111.112.101.110.115.116.97.99.107
  pxelinux.cfg/category.virtual-nodes.openstack

> >>>>>              MTU Option 26, length 2: 1450
> >>>>>              END Option 255, length 0
> >>>>>
> >>>>> Followed by this TFTP request:
> >>>>>
> >>>>> 10.141.20.2.49152 > 10.141.20.1.tftp: [udp sum ok] 
41 RRQ "ldlinux.c32"
> >>>>> octet tsize 0 blksize 1408
> >>>>>
> >>>>> It should be sending that TFTP request to
10.141.255.254.
> >>>>
> >>>>
> >>>> Depends on what master points at.  Otherwise, it sounds
like a broken
> >>>> interaction.
> >>>>
> >>>
> >>> So, master does not resolve. But as per your suggestion,
I've also tested
> >>> with setting that field to the string
"10.141.255.254". The behavior doesn't
> >>> change though. Also, in my packet capture, with both settings,
I don't see
> >>> any DNS requests, which suggests to me it's not trying to
use this field?
> >>>
> >>> My best guess would be that this is a bug that was introduced
by the commit
> >>> I mentioned earlier. What do you think?
> >>
> >> Sounds like a bug that was exposed by the commit (mild
difference).
> >> Let me know if you'd like me to file a bug on your behalf.
> >
> > As I suspected, exposed.  Someone thought we should parse DHCP option
> > 54 for the next server to use which is completely wrong. A DHCP,
> > proxyDHCP or PXE server could point clients to other addresses but
> > always record their own IP in option 54.  It should use BOOTP field
> > siaddr, resolve DHCP option 66 or resolve BOOTP field sname.  Since we
> > only pay attention to siaddr for now, this is a minimal change to fix.
> 
} This patch should help:
> 
> 
> diff --git a/core/fs/pxe/dhcp_option.c b/core/fs/pxe/dhcp_option.c
> index 8d93a6a..5cc0ef7 100644
> --- a/core/fs/pxe/dhcp_option.c
> +++ b/core/fs/pxe/dhcp_option.c
> @@ -143,7 +143,6 @@ static const struct dhcp_options dhcp_opts[] = {
>      {15,  local_domain},
>      {43,  vendor_encaps},
>      {52,  option_overload},
> -    {54,  server},
>      {61,  client_identifier},
>      {67,  bootfile_name},
>      {97,  uuid_client_identifier},
Euh, could this be reviewed: 

diff --git a/core/fs/pxe/dhcp_option.c b/core/fs/pxe/dhcp_option.c
index 8d93a6a..b82e944 100644
--- a/core/fs/pxe/dhcp_option.c
+++ b/core/fs/pxe/dhcp_option.c
@@ -143,8 +143,8 @@ static const struct dhcp_options dhcp_opts[] = {
     {15,  local_domain},
     {43,  vendor_encaps},
     {52,  option_overload},
-    {54,  server},
     {61,  client_identifier},
+    {66,  tftpserver_name},
     {67,  bootfile_name},
     {97,  uuid_client_identifier},
     {209, pxelinux_configfile},
@@ -215,7 +215,7 @@ void parse_dhcp_options(const void *option, int size,
uint8_t opt_filter)
  * information is present:
  *
  * MyIP		- client IP address
- * server_ip	- boot server IP address
+ * server_ip	- boot server IP address,  REVIEW ME   FIXME
  * net_mask	- network mask
  * gate_way	- default gateway router IP
  * boot_file	- boot file name
@@ -252,4 +252,6 @@ void parse_dhcp(const void *pkt, size_t pkt_len, int
pkt_type)
 
     if (over_load & 2)
         parse_dhcp_options(dhcp->sname, 64, 0);
+
+    /* What about option 66, tftserver_name ?   FIXME */
 }



Thing I want to say is that DHCP option 66 is 'next-server' in ISC DHCP
server.
Most likely is the 'next-server' in the subject-line (from original
poster)
also about that vary same option.


Another thing:
 commit 66105926f3d748de827eb863913171efb9624165
 Author: Erwan Velu <erwanaliasr1 at gmail.com>
 Date:   Wed Sep 2 18:26:37 2015 +0200

     tftp: Report server IP address on debug message
     
     When debugging, it is very useful to get the ip adress of the server
     that reponsded to that packet.

That commit might now help. 
At least it makes sense to enable TFTP debugging.
 (
http://www.syslinux.org/wiki/index.php/Development/Debugging#Syslinux_Dynamic_Debugger
)


Groeten
Geert Stappers
-- 
Leven en laten leven

On Sat, Sep 12, 2015 at 10:23 AM, Geert Stappers <stappers at stappers.nl>
wrote:
> On Sat, Sep 12, 2015 at 08:42:15AM -0400, Gene Cumm via Syslinux wrote:
>> On Sat, Sep 12, 2015 at 7:37 AM, Gene Cumm <gene.cumm at
gmail.com> wrote:
>> > On Sat, Sep 12, 2015 at 7:08 AM, Gene Cumm <gene.cumm at
gmail.com> wrote:
>> >> On Sat, Sep 12, 2015 at 5:54 AM, Teun Docter
>> >> <teun.docter at brightcomputing.com> wrote:
>> >>> On 2015-09-12 04:58, Gene Cumm wrote:
>> >>>>>
>> >>>>> I've captured the following DHCP ACK:
>> >>>>>
>> >>>>> 10.141.20.1.bootps > 10.141.20.2.bootpc: [udp
sum ok] BOOTP/DHCP, Reply,
>> >>>>> length 399, xid 0xf0eb955a, secs 14, Flags [none]
(0x0000)
>> >>>>>            Your-IP 10.141.20.2
>> >>>>>            Server-IP 10.141.255.254
>> >>>>>            Client-Ethernet-Address
fa:16:3e:08:31:b9
>> >>>>>            Vendor-rfc1048 Extensions
>> >>>>>              Magic Cookie 0x63825363
>> >>>>>              DHCP-Message Option 53, length 1: ACK
>> >>>>>              Server-ID Option 54, length 4:
10.141.20.1
>> >>>>>              Lease-Time Option 51, length 4: 86400
>> >>>>>              RN Option 58, length 4: 43200
>> >>>>>              RB Option 59, length 4: 75600
>> >>>>>              TFTP Option 66, length 7:
"master^@"
>> >>>>
>> >>>>
>> >>>> What does the name "master.openstacklocal"
point to?  I typically set
>> >>>> dhcp-66 to the dotted quad string of the destination
TFTP.
>
> Option 66 is TFTP-server _name_, no need to restrict yourself
> to four dot seperated octets.
True, but a dotted quad can be resolved without DNS dependence and
it's always good to know this method exists, especially as a step for
debugging.
>> >>> Hmm, actually, it doesn't resolve.
>> >>>
>> >>>>>              BF Option 67, length 11:
"pxelinux.0^@"
>> >>>>>              Subnet-Mask Option 1, length 4:
255.255.0.0
>> >>>>>              BR Option 28, length 4:
10.141.255.255
>> >>>>>              Domain-Name-Server Option 6, length
4: 10.141.20.1
>> >>>>>              Domain-Name Option 15, length 14:
"openstacklocal"
>> >>>>>              Hostname Option 12, length 16:
"host-10-141-20-2"
>> >>>>>              Default-Gateway Option 3, length 4:
10.141.255.250
>> >>>>>              T209 Option 209, length 45:
>> >>>>>
>> >>>>>
112.120.101.108.105.110.117.120.46.99.102.103.47.99.97.116.101.103.111.114.121.46.118.105.114.116.117.97.108.45.110.111.100.101.115.46.111.112.101.110.115.116.97.99.107
>
>   pxelinux.cfg/category.virtual-nodes.openstack
>
>
>> >>>>>              MTU Option 26, length 2: 1450
>> >>>>>              END Option 255, length 0
>> >>>>>
>> >>>>> Followed by this TFTP request:
>> >>>>>
>> >>>>> 10.141.20.2.49152 > 10.141.20.1.tftp: [udp sum
ok]  41 RRQ "ldlinux.c32"
>> >>>>> octet tsize 0 blksize 1408
>> >>>>>
>> >>>>> It should be sending that TFTP request to
10.141.255.254.
>> >>>>
>> >>>>
>> >>>> Depends on what master points at.  Otherwise, it
sounds like a broken
>> >>>> interaction.
>> >>>>
>> >>>
>> >>> So, master does not resolve. But as per your suggestion,
I've also tested
>> >>> with setting that field to the string
"10.141.255.254". The behavior doesn't
>> >>> change though. Also, in my packet capture, with both
settings, I don't see
>> >>> any DNS requests, which suggests to me it's not trying
to use this field?
>> >>>
>> >>> My best guess would be that this is a bug that was
introduced by the commit
>> >>> I mentioned earlier. What do you think?
>> >>
>> >> Sounds like a bug that was exposed by the commit (mild
difference).
>> >> Let me know if you'd like me to file a bug on your behalf.
>> >
>> > As I suspected, exposed.  Someone thought we should parse DHCP
option
>> > 54 for the next server to use which is completely wrong. A DHCP,
>> > proxyDHCP or PXE server could point clients to other addresses but
>> > always record their own IP in option 54.  It should use BOOTP
field
>> > siaddr, resolve DHCP option 66 or resolve BOOTP field sname. 
Since we
>> > only pay attention to siaddr for now, this is a minimal change to
fix.
>>
> } This patch should help:
>>
>>
>> diff --git a/core/fs/pxe/dhcp_option.c b/core/fs/pxe/dhcp_option.c
>> index 8d93a6a..5cc0ef7 100644
>> --- a/core/fs/pxe/dhcp_option.c
>> +++ b/core/fs/pxe/dhcp_option.c
>> @@ -143,7 +143,6 @@ static const struct dhcp_options dhcp_opts[] = {
>>      {15,  local_domain},
>>      {43,  vendor_encaps},
>>      {52,  option_overload},
>> -    {54,  server},
>>      {61,  client_identifier},
>>      {67,  bootfile_name},
>>      {97,  uuid_client_identifier},
>
> Euh, could this be reviewed:
>
> diff --git a/core/fs/pxe/dhcp_option.c b/core/fs/pxe/dhcp_option.c
> index 8d93a6a..b82e944 100644
> --- a/core/fs/pxe/dhcp_option.c
> +++ b/core/fs/pxe/dhcp_option.c
> @@ -143,8 +143,8 @@ static const struct dhcp_options dhcp_opts[] = {
>      {15,  local_domain},
>      {43,  vendor_encaps},
>      {52,  option_overload},
> -    {54,  server},
>      {61,  client_identifier},
> +    {66,  tftpserver_name},
>      {67,  bootfile_name},
>      {97,  uuid_client_identifier},
>      {209, pxelinux_configfile},
> @@ -215,7 +215,7 @@ void parse_dhcp_options(const void *option, int size,
uint8_t opt_filter)
>   * information is present:
>   *
>   * MyIP                - client IP address
> - * server_ip   - boot server IP address
> + * server_ip   - boot server IP address,  REVIEW ME   FIXME
>   * net_mask    - network mask
>   * gate_way    - default gateway router IP
>   * boot_file   - boot file name
> @@ -252,4 +252,6 @@ void parse_dhcp(const void *pkt, size_t pkt_len, int
pkt_type)
>
>      if (over_load & 2)
>          parse_dhcp_options(dhcp->sname, 64, 0);
> +
> +    /* What about option 66, tftserver_name ?   FIXME */
>  }
Something like this would be preferable except it's not quite so
simple.  We'd need to not store BOOTP siaddr before parsing.  Store a
pointer to the string during parsing if serverip is unset.  After all
parsing, if the pointer is set, attempt to resolve then set serverip.
Last, if serverip is not set, copy siaddr to serverip.
> Thing I want to say is that DHCP option 66 is 'next-server' in ISC
DHCP server.
> Most likely is the 'next-server' in the subject-line (from original
poster)
> also about that vary same option.
>
>
> Another thing:
>  commit 66105926f3d748de827eb863913171efb9624165
>  Author: Erwan Velu <erwanaliasr1 at gmail.com>
>  Date:   Wed Sep 2 18:26:37 2015 +0200
>
>      tftp: Report server IP address on debug message
>
>      When debugging, it is very useful to get the ip adress of the server
>      that reponsded to that packet.
>
> That commit might now help.
> At least it makes sense to enable TFTP debugging.
>  (
http://www.syslinux.org/wiki/index.php/Development/Debugging#Syslinux_Dynamic_Debugger
)
>
>
> Groeten
> Geert Stappers
> --
> Leven en laten leven
Can't enable dynamic debugging until after ldlinux.* is loaded

-- 
-Gene

On Sat, Sep 12, 2015 at 6:37 PM, Gene Cumm <gene.cumm at gmail.com>
wrote:
> On Sat, Sep 12, 2015 at 10:23 AM, Geert Stappers <stappers at
stappers.nl> wrote:
>> Euh, could this be reviewed:
>>
>> diff --git a/core/fs/pxe/dhcp_option.c b/core/fs/pxe/dhcp_option.c
>> index 8d93a6a..b82e944 100644
>> --- a/core/fs/pxe/dhcp_option.c
>> +++ b/core/fs/pxe/dhcp_option.c
>> @@ -143,8 +143,8 @@ static const struct dhcp_options dhcp_opts[] = {
>>      {15,  local_domain},
>>      {43,  vendor_encaps},
>>      {52,  option_overload},
>> -    {54,  server},
>>      {61,  client_identifier},
>> +    {66,  tftpserver_name},
>>      {67,  bootfile_name},
>>      {97,  uuid_client_identifier},
>>      {209, pxelinux_configfile},
>> @@ -215,7 +215,7 @@ void parse_dhcp_options(const void *option, int
size, uint8_t opt_filter)
>>   * information is present:
>>   *
>>   * MyIP                - client IP address
>> - * server_ip   - boot server IP address
>> + * server_ip   - boot server IP address,  REVIEW ME   FIXME
>>   * net_mask    - network mask
>>   * gate_way    - default gateway router IP
>>   * boot_file   - boot file name
>> @@ -252,4 +252,6 @@ void parse_dhcp(const void *pkt, size_t pkt_len,
int pkt_type)
>>
>>      if (over_load & 2)
>>          parse_dhcp_options(dhcp->sname, 64, 0);
>> +
>> +    /* What about option 66, tftserver_name ?   FIXME */
>>  }
>
> Something like this would be preferable except it's not quite so
> simple.  We'd need to not store BOOTP siaddr before parsing.  Store a
> pointer to the string during parsing if serverip is unset.  After all
> parsing, if the pointer is set, attempt to resolve then set serverip.
> Last, if serverip is not set, copy siaddr to serverip.
Perhaps a more important question is if ANY PXE implementation uses
DHCP option 66 over BOOTP field siaddr.

-- 
-Gene