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

fix receicing of DHCP vendor info (IDFGH-10591) #11828

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

ljakob
Copy link
Contributor

@ljakob ljakob commented Jul 7, 2023

Test Setup

DHCP-Server sends forced vendor option

host devbrd-dhcptest    { 
 hardware ethernet cc:db:a7:1d:XX:XX; fixed-address 10.X.X.X;
 option dhcp-parameter-request-list 1,3,28,6,43; # default + vendor
 option vendor-encapsulated-options "hallo test 1234";
}

with current master

this source

 char vendor_buf[128];
 bzero(&vendor_buf[0], sizeof(vendor_buf));
 esp_netif_dhcpc_option(wifi_device, ESP_NETIF_OP_GET, ESP_NETIF_VENDOR_SPECIFIC_INFO, &vendor_buf[0], sizeof(vendor_buf)-1);
 ESP_LOGI(TAG, "vendor string:%s:", &vendor_buf[0]);

logs

vendor string:llahllahllahllah:

what is obviously wrong

Bugs in lwip_default_hooks.c

  • the vendor option of the DHCP response is treated as multiple of 32-bit (what is an invalid assumption, see RFC 2132)
  • the response is converted from network to host byte order (for no reason)
  • the loop will always duplicate the first 4 bytes because offset is not adjusted
  • not checked: the 32-bit handling will produce garbage when the last read is shorter than 4 bytes

Hints for everyone who would like to use vendor stuff in DHCP

  • query for vendor response in DHCP request: CMakeLists.txt -> add_definitions(-DLWIP_HOOK_DHCP_EXTRA_REQUEST_OPTIONS=,43)

  • send custom vendor with request: after WIFI_EVENT_STA_START -> esp_netif_dhcpc_option(wifi_device, ESP_NETIF_OP_SET, ESP_NETIF_VENDOR_CLASS_IDENTIFIER, vendor_string, strlen(vendor_string))

@CLAassistant
Copy link

CLAassistant commented Jul 7, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 7, 2023
@github-actions github-actions bot changed the title fix receicing of DHCP vendor info fix receicing of DHCP vendor info (IDFGH-10591) Jul 7, 2023
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jul 28, 2023
@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: In Progress Work is in progress labels Oct 26, 2023
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Reviewing Issue is being reviewed labels Nov 14, 2023
@espressif-bot espressif-bot merged commit 0d12732 into espressif:master Nov 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants