feat(esp_netfi): Add DHCPS option for captive portal identification (IDFGH-11885)

[jkingsman]

RFC 8910 provides a DHCP option to notify clients of a captive portal to be visited prior to using the network.

This is supported by vendors (see Apple since iOS 14 and macOS Big Sur, Android since Android 11), and provides a more standards-compliant mechanism than DNS-based redirection.

This is a simple addition and will be useful in scenarios where the ESP32 broadcasts a network and needs to direct clients to a portal or served content in a modern, standards-compliant way.

This would also address #2723.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

	Messages
📖	🎉 Good Job! All checks are passing!

👋 Hello jkingsman, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 593194d

[mickeyl]

Looks like a nice, additive, patch. I like it!

[david-cermak]

Hi @jkingsman

Thanks for the PR, but is it really all that's needed for this option to work? Haven't you forgotten to add other files to your commit?

I mean, it could work from the dhcpserver API using the recently introduced hooks, but since you added the option to esp_netif layer, we probably need some more. I would be also great to update the captive portal example.

[jkingsman]

Hm, adding to the enum so that you can serve that DHCP option seems all that's necessary to me -- it's up to a user to actually write the captive portal, of course; we have no way of knowing what they want for that. Is there something obvious I've missed?

I will do up a demo this evening if I can find some time. I'm happy to overwrite that captive portal example if desired, but it uses DNS redirection which is probably a reasonable demo to keep around -- DHCP opt is the better, more modern way to accomplish a redirect but DNS is still a valid approach. Do you have any preferences on overwrite vs. add (and I a suppose perhaps annotate each example to emphasize the different approaches and when they're appropriate for use)?

Thanks for the tips!

[david-cermak]

adding to the enum so that you can serve that DHCP option seems all that's necessary to me

This is probably enough for the DHCP client to parse and use it, that is if the ESP32 is connecting to a captive portal.
If ESP32 as acting as a captive portal, it needs to add this option to DHCP server's messages. Our DHCP server is very simple, it just uses these few (hardcoded) options:

esp-idf/components/lwip/apps/dhcpserver/dhcpserver.c

Lines 407 to 456 in 6b9c2fc

	*optptr++ = DHCP_OPTION_SUBNET_MASK;
	*optptr++ = 4;
	*optptr++ = ip4_addr1(&dhcps->dhcps_mask);
	*optptr++ = ip4_addr2(&dhcps->dhcps_mask);
	*optptr++ = ip4_addr3(&dhcps->dhcps_mask);
	*optptr++ = ip4_addr4(&dhcps->dhcps_mask);
	*optptr++ = DHCP_OPTION_LEASE_TIME;
	*optptr++ = 4;
	*optptr++ = ((dhcps->dhcps_lease_time * DHCPS_LEASE_UNIT) >> 24) & 0xFF;
	*optptr++ = ((dhcps->dhcps_lease_time * DHCPS_LEASE_UNIT) >> 16) & 0xFF;
	*optptr++ = ((dhcps->dhcps_lease_time * DHCPS_LEASE_UNIT) >> 8) & 0xFF;
	*optptr++ = ((dhcps->dhcps_lease_time * DHCPS_LEASE_UNIT) >> 0) & 0xFF;
	*optptr++ = DHCP_OPTION_SERVER_ID;
	*optptr++ = 4;
	*optptr++ = ip4_addr1(&ipadd);
	*optptr++ = ip4_addr2(&ipadd);
	*optptr++ = ip4_addr3(&ipadd);
	*optptr++ = ip4_addr4(&ipadd);
	if (dhcps_router_enabled(dhcps->dhcps_offer)) {
	ip_info_t if_ip = { 0 };
	get_ip_info(dhcps->dhcps_netif, &if_ip);
	ip4_addr_t* gw_ip = (ip4_addr_t*)&if_ip.gw;
	if (!ip4_addr_isany_val(*gw_ip)) {
	*optptr++ = DHCP_OPTION_ROUTER;
	*optptr++ = 4;
	*optptr++ = ip4_addr1(gw_ip);
	*optptr++ = ip4_addr2(gw_ip);
	*optptr++ = ip4_addr3(gw_ip);
	*optptr++ = ip4_addr4(gw_ip);
	}
	}
	*optptr++ = DHCP_OPTION_DNS_SERVER;
	*optptr++ = 4;
	if (dhcps_dns_enabled(dhcps->dhcps_dns)) {
	*optptr++ = ip4_addr1(&dhcps->dns_server);
	*optptr++ = ip4_addr2(&dhcps->dns_server);
	*optptr++ = ip4_addr3(&dhcps->dns_server);
	*optptr++ = ip4_addr4(&dhcps->dns_server);
	}else {
	*optptr++ = ip4_addr1(&ipadd);
	*optptr++ = ip4_addr2(&ipadd);
	*optptr++ = ip4_addr3(&ipadd);
	*optptr++ = ip4_addr4(&ipadd);
	}

It's easy to append more options using the LWIP_HOOK_DHCPS_POST_APPEND_OPTS() macro (as I mentioned ^^):

esp-idf/components/lwip/apps/dhcpserver/dhcpserver.c

Line 609 in 6b9c2fc

LWIP_HOOK_DHCPS_POST_APPEND_OPTS(dhcps->dhcps_netif, dhcps, DHCPOFFER, &end)

But that's again, just the lower layer. If you're adding this enum to esp_netif you'd like to make the netif API aware of this and allow users to configure this option.

Do you have any preferences on overwrite vs. add

It would be great if you use some local Kconfig.projbuld configuration to enable/disable this method in the example (perhaps you can also make the current DNS method configurable)

[jkingsman]

Ah, that's helpful context; thank you. Seems like I misunderstood how the DHCP system fit together in that regard -- I did indeed intend this as an offer value and not a client value.

I'll look into the funcs you linked and get my head around the least invasive way to do this.

[jkingsman]

I've got a draft but have just moved states so am waiting for my collection of devices to arrive with our moving van in a few days or so for real testing + sniffing the offer packet for validation. Thanks for your patience!

[jkingsman]

Alrighty, sorry for the force push noise but I've got this working now. Thank you again for your patience and such a complete explanation to remedy my super surface level understanding -- I've deepened that a bit now, haha.

I've also got a demo "done" in that it's code complete but I still want to make it its own little demo folder with a README etc. so I will have that ready for you tomorrow evening.

The gist is

index 4eba17b50b..4d14061864 100644
--- a/examples/wifi/getting_started/softAP/main/softap_example_main.c
+++ b/examples/wifi/getting_started/softAP/main/softap_example_main.c
@@ -85,6 +85,14 @@ void wifi_init_softap(void)
     ESP_ERROR_CHECK(esp_wifi_set_config(WIFI_IF_AP, &wifi_config));
     ESP_ERROR_CHECK(esp_wifi_start());

+    esp_netif_t* netif = esp_netif_get_handle_from_ifkey("WIFI_AP_DEF");
+
+    char* captive="http://captiveportal.local";
+
+    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_netif_dhcps_stop(netif));
+    ESP_ERROR_CHECK(esp_netif_dhcps_option(netif, ESP_NETIF_OP_SET, ESP_NETIF_CAPTIVEPORTAL_URI, captive, strlen(captive)));
+    ESP_ERROR_CHECK_WITHOUT_ABORT(esp_netif_dhcps_start(netif));
+
     ESP_LOGI(TAG, "wifi_init_softap finished. SSID:%s password:%s channel:%d",
              EXAMPLE_ESP_WIFI_SSID, EXAMPLE_ESP_WIFI_PASS, EXAMPLE_ESP_WIFI_CHANNEL);
 }

which updates the DHCP OFFER with the option:

Also wanted to add that I haven't written any C since college and had to muddle through with some help from my partner who is an embedded dev, so if I've made any boneheaded decisions or used any blindingly bad antipatterns, please call me out -- I won't be offended 🙂

[jkingsman]

Alrighty @david-cermak I think this is ready to go. I've validated this with an example, including tcpdump output of the result as a demo in the example file, and tested everything on real hardware.

Per the contributing docs, I also did an astyle run which moved some bits and bobs around in the files I worked on, but all seem like good changes.

Please let me know your feedback and if you want any changes. Thank you again for your patience!

[david-cermak]

Thanks for the update @jkingsman ! The changes look very good to me. The only thing is the allocation in the netif layers. Could we let users "own" that buffer, so we just copy the pointer? This would usually be a const char* to the program memory, so no allocation would be needed in most cases.

[jkingsman]

Sounds good! I've moved that malloc out.

[david-cermak]

Thanks for the PR and the updates. The changes look good to me!
We're moving this PR to the internal pipeline of reviews and tests before merging to master.
This process usually takes some time...

I'd only like to ask you to squash those 4 commits into a single one. (we'd get back to you if there're some issues or any other request for changes)

[jkingsman]

Woohoo! Glad to hear it's made it to the next round.

I've squashed down my commits to one, and I'll sit tight. If you have a rough order of magnitude, please, is 'some time' for review usually weeks? Months? No worries no matter what it is; just curious.

Thanks again!

[david-cermak]

is 'some time' for review usually weeks? Months?

:-) I'd say ~ weeks (1 month on average, but the standard deviation is 'big')

feat(esp_netfi): add support for captive portal DHCP Option 114

just a typo with your last commit message -> feat(esp_netif)

[jkingsman]

Whoops, fixed.

Thanks for the info! I will wait to hear more from you when things progress.