Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
xaionaro opened this issue Dec 11, 2019 · 3 comments
Labels

Comments

@xaionaro
Copy link
Contributor

xaionaro commented Dec 11, 2019

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

@pmazzini
Copy link
Collaborator

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 #22 we need to uniform it.

@xaionaro
Copy link
Contributor Author

xaionaro commented Dec 11, 2019

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

// skip non-DHCP packets

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.

@pmazzini
Copy link
Collaborator

pmazzini commented Dec 11, 2019

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 pmazzini added the bug label Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants