wpa_supplicant-openipc: 2.11 with WPA3-SAE on mbedtls - #2449
Conversation
PR Summary by QodoAdd compact WPA3-SAE supplicant using mbedTLS
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you |
openipc-ai
left a comment
There was a problem hiding this comment.
Thanks — this is a well-made PR, and the provenance work in particular stands up to checking. I rebuilt it and verified a few things before reviewing:
src/crypto/crypto_mbedtls.candsrc/crypto/tls_mbedtls.cas added by0001are byte-identical to OpenWrt's110-mbedtls-TLS-crypto-option-initial-port.patch;0003is byte-identical to their135-;0002is their120-with the hostapd hunks trimmed and the offset adjusted. Nothing hidden in 7.6k lines.- Both sha256s match the real tarball from w1.fi.
ci-matrix.py --self-testpasses on the branch (137 packages), andmake BOARD=ssc377_lite defconfigresolves the way you intended.- Your stripped size is exact: I get 588672 for the new binary, against 438844 for the stock 2.10 in the shipped image.
The approach is also the right one, and it's worth saying why: only the openssl, wolfssl and mbedtls backends implement crypto_ec_*, so SAE genuinely cannot be built against the internal crypto every image ships, and Buildroot's own BR2_PACKAGE_WPA_SUPPLICANT_WPA3 pulls OpenSSL. An mbedtls-backed fork is the only option that fits, and keeping the two packages mutually exclusive with the stock one default is the correct default.
Two things need fixing before this can go in, and two more are worth discussing.
1. The hash file is never read
Buildroot looks for <RAWNAME>.hash, where RAWNAME is the package name — wpa_supplicant-openipc.hash (package/pkg-generic.mk:512). The file here is wpa_supplicant.hash, so it is never opened. From my build log:
WARNING: no hash file for wpa_supplicant-2.11.tar.gz
We don't set BR2_DOWNLOAD_FORCE_CHECK_HASHES, so that is a warning and nothing else: the tarball is downloaded over the network and used with no integrity check, and the build goes green. The contents of the file are correct — it just needs the rename:
git mv general/package/wpa_supplicant-openipc/wpa_supplicant.hash \
general/package/wpa_supplicant-openipc/wpa_supplicant-openipc.hash
(Not your fault that this is easy to miss — dropbear-openipc/dropbear.hash has had the same defect for years. We'll fix that separately.)
2. CI has not run
This is a cross-repo PR, so build, gcc-compat and qodo-gate are all sitting at action_required waiting for a maintainer to approve the workflows, and the branch is behind master. We can't merge anything here until the matrix is green — a maintainer will kick the workflows off; please rebase on master when you get a chance.
3. ssc377_lite is probably not the right board to flip
ssc377_lite ships no Wi-Fi driver at all — no rtl8188fu-openipc, no driver package in the defconfig, just CONFIG_CFG80211=m with nothing to bind to it. So on the generic board the new supplicant is flash spent on something nothing in the image can use, and anyone who wants Wi-Fi there has to add a driver that is an order of magnitude larger than the headroom.
I built both sides of this to get a real number (container build, base 49908b5a vs this branch):
| rootfs.squashfs | headroom (5120 KB cap) | |
|---|---|---|
| base | 5012 KB | 108 KB |
| this PR | 5080 KB | 40 KB |
+68 KB, which clears the cap and misses the 32 KB headroom warning by 8 KB. Three separate PRs in the last fortnight (#2433, #2440, #2442) existed only to pull boards back under that cap after ordinary shared-overlay growth, so 40 KB is not a lot of runway to spend on a board with no radio.
The Tapo C120 is a retail model, which puts it in OpenIPC/builder — and builder device defconfigs already select firmware packages exactly like this (ssc335_lite_tp-link-tapo-c310-v220 carries BR2_PACKAGE_RTL8192EU_OPENIPC=y alongside the supplicant selection). Once this package exists here, flipping it on a C120 device defconfig there is one line, next to the 8188fu driver that device actually needs.
So: keep the package here, drop the ssc377_lite hunk, and open the enablement against builder. If you'd rather keep a board in this tree building it so CI covers the package, say so and we'll work out which one — but it should be a board that ships a radio.
For context on why we're cautious about spreading it, measured against last night's nightly across 61 lite and 35 ultimate images:
- On lite the marginal cost is just the binary (+68 KB), because mbedtls is already in 93 of 125 defconfigs. At that price
hi3519v101_lite(20 KB free) andhi3518ev100_lite(48 KB free) go over cap, andgk7205v300_lite,hi3518ev300_lite,gm8135_lite,gm8136_liteall land inside the warning band. - On ultimate it is much worse, not better: only 1 of 27 ultimate defconfigs selects
BR2_PACKAGE_MBEDTLS_OPENIPC, soselect BR2_PACKAGE_MBEDTLS_OPENIPCalso drags in all three mbedtls shared libraries there — about 301 KB all told, which putshi3516ev300_ultimate(140 KB free) andssc338q_ultimate(188 KB free) over cap.
None of that is an argument against the package. It is an argument for exactly the opt-in design you already chose, and for every future board flip carrying its own measured size line.
One observation you may find useful if size ever becomes the blocker. I built three binaries with the same toolchain and flags:
| build | stripped |
|---|---|
| 2.10, Buildroot's stock config (internal crypto) | 438,844 B |
| 2.11, byte-identical config | 518,624 B |
| 2.11, your config (mbedtls, SAE, OWE, PMF) | 588,672 B |
+78 KB of the +146 KB is the 2.10 → 2.11 jump alone, before any of your changes. By symbol size, SAE is 10.3 KB, OWE 1.2 KB, and the entire mbedtls wrapper layer 5.2 KB — the feature is cheap, the version bump is not. A 2.10-pinned build with the same backend would cost roughly half. The OpenWrt series does not apply cleanly to 2.10, but only two hunks fail and both are mechanical context drift around the AESOBJS exclusions in wpa_supplicant/Makefile. Entirely your call, and 2.11 has five years of fixes in it — just wanted the number on the record.
4. Nothing shipped can actually produce an SAE config
The only Wi-Fi bring-up path in the tree is general/overlay/etc/network/interfaces.d/wlan0:
pre-up wpa_passphrase "$(fw_printenv -n wlanssid)" "$(fw_printenv -n wlanpass)" > /tmp/wpa_supplicant.conf
pre-up sed -i 's/#psk.*/scan_ssid=1/g' /tmp/wpa_supplicant.conf
pre-up wpa_supplicant -B -i wlan0 -D nl80211,wext -c /tmp/wpa_supplicant.conf
wpa_passphrase emits a WPA2-PSK block and nothing else, and /tmp does not survive a reboot — so a user on a WPA3-only AP still cannot associate after this lands. That makes the PR a prerequisite rather than the fix, which is fine, but it is worth saying in the description so nobody expects the capability to be reachable yet.
The follow-up is the interesting part, and it cannot simply be written into the shared overlay: your own before-evidence shows the stock supplicant rejects key_mgmt=SAE at parse time and drops the whole network block, so one shared wlan0 script cannot serve both packages. Whatever lands will need to key on which supplicant is present. Happy to discuss that on a separate PR.
5. 0004 drops upstream's constant-time comparison
Your mbedtls_mpi_random() stand-in rejection-samples correctly and is unbiased, and the 30-try cap matches upstream. But the loop condition uses mbedtls_mpi_cmp_mpi(), where upstream uses mbedtls_mpi_lt_mpi_ct() specifically so the comparison does not leak timing — their comment calls it out as "especially important for ECDSA". The value being compared here is the secret SAE scalar.
mbedtls_mpi_lt_mpi_ct() does exist in mbedtls 2.25 (include/mbedtls/bignum.h:610), so it is a small change — but not a straight substitution. It returns MBEDTLS_ERR_MPI_BAD_INPUT_DATA unless both operands have the same allocated limb count, and 2.25's public mbedtls_mpi_fill_random() calls mbedtls_mpi_resize_clear(X, CHARS_TO_LIMBS(n_bytes)) internally, which can leave X shorter than N when N has leading zero limbs. That is exactly why upstream resizes X to N->n up front and uses a non-resizing internal fill. Something like:
unsigned lt = 0;
do {
if (++count > 30 ||
mbedtls_mpi_fill_random(X, n_bytes, mbedtls_ctr_drbg_random,
crypto_mbedtls_ctr_drbg()) != 0 ||
mbedtls_mpi_shift_r(X, 8 * n_bytes - n_bits) != 0 ||
mbedtls_mpi_grow(X, N->n) != 0 || /* lt_mpi_ct wants equal limb counts */
mbedtls_mpi_lt_mpi_ct(X, N, <) != 0)
return -1;
} while (!lt);
return 0;n_bytes is derived from N's bit length, so CHARS_TO_LIMBS(n_bytes) <= N->n always holds and the grow only ever pads — but please confirm that against the curves SAE actually negotiates rather than taking my word for it.
While you are in that patch: please give it a real From: line rather than From: OpenIPC contributor. The other three carry proper authorship and this one should too, since it is the one piece of the series that is ours.
Minor
- The new package stops installing
/etc/wpa_supplicant.conf, which Buildroot's does. Nothing reads it (wlan0andsetnetworkboth use/tmp/wpa_supplicant.conf), so this is fine — just noting it so it is a decision rather than an accident. - Your reasoning for keeping
CONFIG_IEEE8021X_EAPOLon checks out; the call site that then needscrypto_rsa_key_read()is real, because OpenWrt's0001sets-DCRYPTO_RSA_OAEP_SHA256unconditionally for the mbedtls backend (with anXXX: create a config option?of their own). It is not worth trying to avoid: the RSA wrappers total 352 bytes in the linked binary, since mbedtls is a shared library. - The PR enables
CONFIG_DRIVER_WEXT, which the stock package does not build. That is an addition rather than a regression, but it is not mentioned in the description and the shippedwlan0script has been passing-D nl80211,wextagainst a binary without it.
Everything above is from container builds and source review — I don't have an ssc377 with that dongle, so your hardware evidence is the only hardware evidence on this PR.
The stock package is wpa_supplicant 2.10 with the internal crypto and no SAE, so a camera cannot join a WPA3-only network. Buildroot's WPA3 switch pulls in OpenSSL, which does not fit the lite images. This package builds 2.11 with CONFIG_TLS=mbedtls against mbedtls-openipc (already in every image), with SAE and OWE on. The mbedtls backend is the series OpenWrt carries for its hostapd (Glenn Strauss), trimmed to the wpa_supplicant tree, plus one patch for mbedtls 2.25: MBEDTLS_ERROR_ADD, a local crypto_bignum_rand() in place of mbedtls_mpi_random() (2.27+, same rejection sampling and constant-time compare as upstream), and the RSA helpers for wpa_supplicant's imsi_privacy code. Station only: nl80211 and wext, control interface, no EAP methods, no AP/P2P/mesh/WPS/DPP, no TDLS/802.11r. wext is on (the stock package builds without it, although the shipped wlan0 script asks for it). IEEE8021X_EAPOL stays on because the PMKSA cache lives behind it and drivers doing SAE through external auth (rtl8188fu) reject the association without the PMKID. No /etc/wpa_supplicant.conf is installed: nothing in the tree reads it. Stripped binary 589 KB against 439 KB for the stock 2.10, and no second copy of crypto code. Tested on a TP-Link Tapo C120 (ssc377, RTL8188FTV, WPA3-only AP): SAE association with PMF, DHCP, RTSP. Opt-in: no board in this tree selects it. Device defconfigs in OpenIPC/builder that ship a Wi-Fi driver can, one line next to it. This is a prerequisite, not the fix: the shipped wlan0 script still writes a WPA2-PSK block through wpa_passphrase, so a WPA3-only network needs a follow-up that emits key_mgmt=SAE when this supplicant is present.
SSC377, SmartSens SC430AI, 16 MB NOR, RTL8188FTV on USB. The generic ssc377_lite defconfig with the 16 MB layout, the rtl8188fu driver and the SAE-capable wpa_supplicant-openipc; the Wi-Fi module's power gate (GPIO 42) in the wireless/usb entry; IR-cut, backlight and IR LED pins; the SC430AI IQ file. Depends on OpenIPC/sensors#7 (driver), OpenIPC/firmware#2445 (loader entry) and OpenIPC/firmware#2449 (supplicant package).
1fc3fbd to
fa0d4c6
Compare
|
Thanks for the thorough look. v2 is pushed, rebased on master, still one commit:
The two minor items (WEXT on, no Rebuilt binary (same toolchain and flags) is 588680 B stripped, +8 B for the constant-time compare. Re-tested on the Tapo C120 against the same WPA3-only AP, both as a drop-in and inside the full builder image: cold boot, |
Dropping the ssc377_lite hunk during review left the package with no defconfig selecting it in this tree, which is deliberate but fails ci-matrix.py --self-test, a merge gate. The board that wants WPA3-SAE is OpenIPC/builder's ssc377_lite_tp-link-tapo-c120.
openipc-ai
left a comment
There was a problem hiding this comment.
All five items check out, and I verified each rather than taking the summary on trust:
- Hash —
wpa_supplicant-openipc.hash, which is the namepkg-generic.mkactually looks for, so the tarball is verified now instead of silently accepted. ssc377_litehunk dropped — the enablement is in the device profile where the radio is.0004— growsXtoN->nand compares withmbedtls_mpi_lt_mpi_ct(),while (!lt). Your reasoning on the limb count is right and the comment you added says why, which is the part that matters for whoever reads it next. RealFrom:line too.- Rebased, and the full board matrix is green.
- 2.10 vs 2.11 — agreed, staying on 2.11 is the right call. Hardware evidence beats 39 KB, and the backport stays available if a board ever needs it.
One thing you should know: I pushed a commit to your branch, eb98545d, ci: register wpa_supplicant-openipc as built by no board here. Dropping the defconfig hunk — which I asked for — left the package with nothing selecting it in this tree, and that fails ci-matrix.py --self-test, a merge gate. That was my omission when I asked, not yours, so I fixed it rather than send you round again. It is one entry in NOT_BUILT plus a paragraph saying why, in the style of the usb-dual-role note above it. Revert it if you would rather word it differently.
Approving. OpenIPC/sensors#7 and #2445 are both merged, so OpenIPC/builder#162 can come out of draft once this lands.
SSC377, SmartSens SC430AI, 16 MB NOR, RTL8188FTV on USB. The generic ssc377_lite defconfig with the 16 MB layout, the rtl8188fu driver and the SAE-capable wpa_supplicant-openipc; the Wi-Fi module's power gate (GPIO 42) in the wireless/usb entry; IR-cut, backlight and IR LED pins; the SC430AI IQ file. Depends on OpenIPC/sensors#7 (driver), OpenIPC/firmware#2445 (loader entry) and OpenIPC/firmware#2449 (supplicant package). Co-authored-by: nzzane <12163646+nzzane@users.noreply.github.com>
Problem
A camera cannot join a WPA3-only network. The stock package is wpa_supplicant 2.10 built with the internal crypto and no SAE, so a
key_mgmt=SAEnetwork is rejected at config parse time. Buildroot'sBR2_PACKAGE_WPA_SUPPLICANT_WPA3would fix it but pulls in OpenSSL, which does not fit the lite images.This adds
wpa_supplicant-openipc: 2.11 withCONFIG_TLS=mbedtlsagainstmbedtls-openipc, which every image already carries, with SAE and OWE on. The mbedtls backend is the patch series OpenWrt ships for its hostapd (by Glenn Strauss), trimmed to the wpa_supplicant tree, plus one patch so it builds against mbedtls 2.25 (details in0004-*.patch). Station use only: nl80211 + wext, control interface, no EAP methods, no AP/P2P/mesh/WPS/DPP, no TDLS/802.11r (same reasoning as #2320).CONFIG_IEEE8021X_EAPOLstays on with all EAP methods off: the PMKSA cache is compiled out without it, and drivers that do SAE via external auth (rtl8188fu) get the association rejected when no PMKID is installed after SAE. Found the hard way, comment in the .mk.Opt-in: no board in this tree selects it. Device defconfigs in OpenIPC/builder that ship a Wi-Fi driver can, one line next to the driver, each with its own measured size line; the Tapo C120 device does (OpenIPC/builder#162). The two packages are exclusive in Kconfig.
CONFIG_DRIVER_WEXTis on (the stock package builds without it, while the shippedwlan0script passes-D nl80211,wext). No/etc/wpa_supplicant.confis installed: nothing in the tree reads it.Hardware tested on
TP-Link Tapo C120 (SSC377, RTL8188FTV on USB,
8188fu), against a WPA3-only (SAE, PMF required) access point. Binary tested is the one Buildroot produced for this package, first as a drop-in on the running camera and then inside the full OpenIPC/builder#162 image.Evidence
Before (stock package, same config file):
After:
DHCP lease and RTSP stream over that link as usual. Stripped size 588680 vs 438844 for the stock 2.10 (+150 KB before squashfs), and no second copy of crypto code in the image.
Prerequisite, not the fix. The shipped
interfaces.d/wlan0still writes a WPA2-PSK block throughwpa_passphrase, so a WPA3-only network is not reachable from a stock config after this lands. The follow-up has to key on which supplicant is present (the stock one drops akey_mgmt=SAEblock at parse time); that will be a separate PR.ci-matrix.py --self-testpasses; the selector reports thegeneral/package/Config.inline reaches every board.Scope
general/package/all-patches/linux/general/overlay/or in a sharedload_<vendor>script hardcodes a value specific to my boardLD_PRELOAD, and no binaries that cannot be rebuilt from sourcessc377_lite_tp-link-tapo-c120device in ssc377_lite: add TP-Link Tapo C120 device profile builder#162, which builds it on that repo's CI