dhcp icon indicating copy to clipboard operation
dhcp copied to clipboard

DHCPv6 client ignores messages if they contain any error (even a minor one)

Open xaionaro opened this issue 5 years ago • 3 comments

The problem if for example any DHCPv6 option was encoded incorrectly then the whole DHCP message is just silently ignored. It makes diagnostics much more difficult. So an end user may think that DHCP server does work or anything else.

An example case: If you encode bootfile param option as just a single string like "root=/dev/sda1 rw" instead of "\0x00\0x10root=/dev/sda rw" then the whole DHCPv6 message will be just silently ignored.

See also: https://github.com/insomniacslk/dhcp/pull/340/files#r356170985

xaionaro avatar Dec 11 '19 11:12 xaionaro

Thanks for raising the issue.

the whole DHCP message is just silently ignored

If the packet is not RFC compliant we will return/log an error. It is not silently ignored.

However, I get what you mean. In DHCPv4 we do lazy parsing of the options and if an option is not properly encoded we will not return an error.

I think this is the same issue as https://github.com/insomniacslk/dhcp/issues/22 we need to uniform it.

pmazzini avatar Dec 11 '19 13:12 pmazzini

@pmazzini :

If the packet is not RFC compliant we will return/log an error. It is not silently ignored.

I'm talking about continue here: https://github.com/insomniacslk/dhcp/blob/3997b8a58c66233275ad9b26612ef733a63bd9d3/dhcpv6/client6/client.go#L179

Unfortunately the err of the if is returned by dhcpv6.FromBytes(buf[:n]) which in turn returns an error if any error occurred, including if an option is parsed with an error.

xaionaro avatar Dec 11 '19 13:12 xaionaro

Oh sorry, I misread it :s. Ok that is the old client. Yes makes sense to fill an issue for it. At least the new one doesn't silently ignore them.

pmazzini avatar Dec 11 '19 13:12 pmazzini