newAM / w5500-rs

Embedded rust support for the Wiznet W5500 SPI internet offload chip
MIT License
36 stars 14 forks source link

dhcp: fix server identifier in REQUEST message #268

Closed zer0-droids closed 1 year ago

zer0-droids commented 1 year ago

Currently, the server identifier added in the REQUEST message corresponds to the siaddr retrieved in the OFFER. This is probably correct in some situations, but I noticed this does not work in my case: the REQUEST is NACK'ed by the server.

The DHCP server of my Internet box sends an OFFER containing 0.0.0.0 as next server IP, and its address (192.168.1.254) in the DHCP server identifier option (54).

From RFC2131, the next server IP (siaddr) is indeed not supposed to be used like this:

DHCP clarifies the interpretation of the 'siaddr' field as the address of the server to use in the next step of the client's bootstrap process. A DHCP server may return its own address in the 'siaddr' field, if the server is prepared to supply the next bootstrap service (e.g., delivery of an operating system executable image). A DHCP server always returns its own address in the 'server identifier' option.

Additionally, in dhclient sources (see the 2nd link), we can see that next_srv_addr (filled from siaddr) is just passed to the script. On the other hand, the server identifier option is added in the REQUEST if it was present in the OFFER.

This patch tries to imitate this behavior: if the option was present in the OFFER, it will be added in the REQUEST.

Fixes: 3cb6a20f8556 ("DHCP: add ServerId in REQUEST message") Link: https://www.rfc-editor.org/rfc/rfc2131.txt Link: https://github.com/isc-projects/dhcp/blob/master/client/dhclient.c

zer0-droids commented 1 year ago

Hello,

This pull request fixes the server id option (54) sent in the DHCP request message. It is related to Issue #249.

Thanks!

zer0-droids commented 1 year ago

Thanks @newAM for the review and merge. Yes please, a release with the fix would be nice!

newAM commented 1 year ago

Done! v0.6.0 contains the fix.