[Dnsmasq-discuss] Bug in 2.31 and dhcp relay

Dirk Schenkewitz schenkewitz at docomolab-euro.com
Mon Jun 26 16:19:45 BST 2006


On Monday, 26. June 2006 15:11, Simon Kelley wrote:
> Daniel Hamlin wrote:
> > There appears to be a bug in 2.31 (and earlier), that mishandles requests
> > forwarded by a dhcp relay.  In this packet capture, 192.168.1.196 is the
> > DHCP server, and 192.168.0.1 is the router (Nortel Passport 8600). 
> > Notice the response is sent back to the wrong port on the relay. 
> > According to "The DHCP Handbook", the response to a relay should go back
> > to port 67, not 68 as dnsmasq is currently doing.
>
> Your analysis is spot on: this code was changed around 2.28 to honour
> the source port in the request from the relay, rather then always send
> to port 68. The aim was to allow use of non-standard ports by relays,
> but it breaks if the relay sets port 67 as the source for the relayed
> packets, as in this case. It's probably best just to revert this change,
> and always send to port 68.

Although I have no idea what this really is about, I have an opinion. ;-)

Just to be sure: The change was made around 2.28 to deal with a certain issue 
(and it really solved the problem, isn't it?) and now it turned out that it 
breaks another thing - is that correct?

If that's correct, then my idea/suggestion is: Simon, please don't revert it 
completely, make it a compile-time option instead. This would still have the 
disadvantage that one cannot have a solution for both problems at the same 
time, but this will hopefully happen very rarely.

> Cheers,
>
> Simon.
>
> > It appears that the bug is in dhcp.c line 229:
> >
> > if (mess->giaddr.s_addr)
> > {
> >   /* Send to BOOTP relay  */
> >   if (!dest.sin_port)
> > dest.sin_port = htons(DHCP_SERVER_PORT);
> >   dest.sin_addr = mess->giaddr;
> > }

E.g. like so: ;-)


if (mess->giaddr.s_addr)
{
 /* Send to BOOTP relay  */
#ifdef ALLOW_NONSTD_RELAY_PORTS
 if (!dest.sin_port)
#endif
     dest.sin_port = htons(DHCP_SERVER_PORT);
   dest.sin_addr = mess->giaddr;
}


Then put an option into the configure defintions, 
like "--allow-nonstd-relay-ports" and explain somewhere that this breaks the 
standard behaviour. 
Or name it  e.g. "--allow-nonstd-relay-ports_breaks-std-behaviour". ;-)

Otherwise, reverting the change could cause a regression for those whose 
relays use non-standard ports...

Cheers
Dirk




More information about the Dnsmasq-discuss mailing list