-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
Add CDC based tethering over USB (CDC NCM/ECM compatible phones only) #1384
Add CDC based tethering over USB (CDC NCM/ECM compatible phones only) #1384
Conversation
bd228c5
to
fcc6d33
Compare
For comparison, Librem 5 uses CDC-NCM, not RNDIS. The Gentoo wiki notes that RNDIS is deprectated and may be removed in the future. The actual LKML thread linked from the wiki sounds like there are different opinions about the host and gadget side modules (here we need the host, phone is the gadget), not sure what will end up happening. The Gentoo wiki and the LKML thread both sound like most Android phones still require RNDIS though. |
@JonathonHall-Purism had a little insightful adventure with android recently and can tell this implementation is not going away on android-13 or anytime soon. Want to propose PR for librem phone? |
Rebasing minimal changes on top of master |
fcc6d33
to
f7bbeab
Compare
Btw, time sync was failing because bad dns name. Its pool.ntp.org, not ntp.pool.org.... @JonathonHall-Purism network-init-recovery is mostly unused as of now, but can now be used with eth0 cable connected without tethering, or when tethering active on phone, be used to sync time and do network related things without the need of wifi networking stack etc. As we saw before, iphone stack is nearly impossible to integrate here because too heavy. But Librem5 could most probably be! Let me know if you would want something cleaner as of now. I will amend past commit to add to all maximized boards. As of now, it can only be activated by calling network-init-recovery from recovery shell, but could be integrated in any way we see fit later on to sync time, where this fixes ntp sync over eth0 to pool.ntp.org if local dns provided by dhcp server is not also a ntp server. |
To test:
QubesOS
|
f7bbeab
to
efbe44f
Compare
efbe44f adds additional requirements to all maximized boards and associated linux configs. |
Analysis of additional required modules consumption will follow successful build for all boards. |
@daringer: should I add this to nv41/ns50 upstream board configs/linux configs? TLDR: This permits Android 11+ (GrapheneOS 6a) usb tethering support, permitting to sync time over ntp when launching |
Oups. Rebasing on master to compare only between 1 commit and changes related to added commit here. |
efbe44f
to
a70db6d
Compare
Comparing new size requirements of this PR. x230-hotp-maximized's size.txt artifact compared between master and 1384
Raw changes analysis:
Important changes for ROM size constraints: |
@JonathonHall-Purism @daringer:
|
Testing Github integration with matrix. Putting back from draft -> ready for review. |
PR itself lgtm
no need to add to them right now, we don't would like them activated as default for our boards. |
For info, newer phones (pixel 6a+) are also using cdc-ncm instead of rndis. But that is not the case for older phones. If we wanted to have on-demand usb tethering more granular, we could insmod CDC-NCM first and only if usb0 doesn't appear, load rndis which is unneeded in case of CDC-NCM is sufficient to bring up usb0 network interface. It's more a note for future improvements then nothing else. @JonathonHall-Purism thoughts? Otherwise please approve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tlaurion. Just a couple of minor comments, just based on review so let me know if I overlooked something. Otherwise with those minor fixes I think this looks good to merge.
CDC-NCM would be great at some point, but there's no sense holding up this feature for a second feature.
Thanks for adding the compile-time config for the feature as well. You might want to consider locking this out if Restricted Boot is enabled, since RNDIS can (apparently) compromise the host kernel - it'd offer a Restricted Boot bypass if an attacker can enable network recovery, plug in a malicious USB device, and then take over the Heads kernel.
But since it's guarded by a config and not enabled for my boards, I think that's up to you, so I'm OK merging with the two minor fixes if you don't want to add that.
initrd/bin/network-init-recovery
Outdated
insmod /lib/modules/$module.ko | ||
fi | ||
done | ||
#other prep needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we delete this #other prep needed
comment or is this a placeholder for something?
@daringer i don't understand what you meant here. This is on-demand only from recovery shell as of now. Meaning totp related time drift problems could be resolved by asking users to plug nitrokey phone/GrapheneOS/android 11+ burner phone, activate USB tethering on phone and then launch network-recovery-init from recovery shell from now to ntp sync and be done. While also providing some kind of unified networking possibilities from people wanting network without Heads having to add wifi support or use Ethernet and a Ethernet cable. As said in OP, Android phone also conveniently compartmentalize privacy/security needs here from advanced needs where users can configure VPN always on if needed on phone and not care about details. Next step would be to add option from option menu and refactor this into additional functions and have network-recovery-init call those refactored functions if compiled as modules in kernel config and packed under modules.cpio added under initrd per board config settings. |
|
@JonathonHall-Purism Time has passed and AFAIK, we could push only for Android 13+ supported devices which RNDIS is not a requirement? This PR could only support phone modles not requiring RNDIS (CDC-NCM) in the goal of crafting an automatic NTP sync option on HOTP/TOTP mismatch option as first step here and network-recovery-init left alone but reusing code here exposed on /etc/functions instead to enable what is configured per board options? |
@tlaurion I'd absolutely support omitting RNDIS and shipping CDC drivers only. I'd also humbly suggest that we could call this CONFIG_MOBILE_TETHERING or something not Android-specific, since there are non-Android phones out there that support these protocols 🙂 |
Decided against. This is will be CDC NCM only PR with fixes in. Refactoring to be reused into TOTP/HOTP out of sync will happen next so that code here is put into functions and reused. |
f9f91de
to
ff9c78d
Compare
So as of kernel 5.10.x CDC ECM/ECM host tethering is supported and included. Waiting for build of x230-hotp-maximized to complete to make size comparison of compressed initrd for modules.cpio changes and will update. This PR is ready for review. User experience is as follow once on recovery shell, Tethering with supported phones (fails NTP sync to returned DNS by DHCP request)
Tethering with unsupported phone + Ethernet fallback (NTP sync against NTP server returned by DHCP request)
|
Here is a small table giving some examples of models supported by different kernel modules. A reminder that this PR supports NCM and ECM only, while rejecting RNDIS altogether.
|
082fd58
to
4407a44
Compare
…neOS Pixel 6a, no more RNDIS support) - Add additional requirements to linux config - Add additional CONFIG_MOBILE_TETHERING=y to all maximized board configs - Fix issue under network-recovery-init to NTP sync against NTP server pool - Extend network-recovery-init to first try NTP sync against DNS server returned by DHCP answer - Remove network-recovery-init earlytty and tty0 redirection (console should be setuped properly by init in all cases) - If CONFIG_MOBILE_TETHERING=y added to board config and network-recovery-init called, wait to user input on instructions and warning 30 secs before proceeding (non-blocking) - Machines having STATIC_IP under board config won't benefit of autoatic NTP sync Since network-recovery-init can only be called from recovery shell now, and recovery shell can be guarded by GPG auth, this is PoC code to be used to complement TOTP being out of sync TODO(Future PR): - Refactor into functions and reuse into TOTP/HOTP being out of sync automatically. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…unify with x230-hotp-maximized Signed-off-by: Thierry Laurion <insurgo@riseup.net>
4407a44
to
9b69f1b
Compare
Size comparison. Master's 4af7808 vs this PR's 9b69f1b
Analysis:
Additional ROM consumed compressed space: (4691968-4666368)+(2489216-2488800)= 26016 bytes (~25Kb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works with Librem 5! It's easy too, just change the wired network's IPv4 mode to Shared with Other Computers, same as for Network Manager on any other device.
I gave a lot of thought to the messages/prompts and commented inline. Also unsure how this interacts with Ethernet tethering and commented on that too.
Overall, great feature and great work. Looking forward to polishing for merge and future features leveraging this functionality 💯
@tlaurion If you want you can cherry-pick this commit enabling mobile tethering for Librems, or I can PR it after this is merged if you'd rather: JonathonHall-Purism@92346b4 I haven't gone through to enable Ethernet network recovery yet, but I've noted that for the next PB release (can test the relevant devices then when doing the release testing). Couple of Kconfig FYI notes:
|
Enable mobile tethering on all Librem boards. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…vation in functions and ask user prior of using each mode Also remove output of attempted module loading since DEBUG will show if needed Remove timeout after 30 seconds to unify UX and block Change UX wording Should address all PR review comments Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… module already insmodded (ehci-hcd.ko module name is ehci_hcd...) Signed-off-by: Thierry Laurion <insurgo@riseup.net>
c065fb7
to
a5ab32b
Compare
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
All boards with CONFIG_LINUX_USB=y ship ehci-* and xhci-*, they are not controlled by CONFIG_LINUX_USB_COMPANION_CONTROLLER. Always insert them when initializing USB. Fixes commit 35de234 Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
@tlaurion Thanks, the fixes look great, but xhci is broken on boards that don't ship USB companion controller modules due to having moved those under CONFIG_LINUX_USB_COMPANION_CONTROLLER. Fixup here and a trivial indentation fix (2 commits): https://github.com/JonathonHall-Purism/heads/tree/mobile-tethering-fixup-20240223 |
… of starting it Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@JonathonHall-Purism Thanks for that (and sorry, last testing were done on qemu and not real hardware, which hid modules loading problems...)! Added one last commit to make sure dropbear is not running prior of trying to start it. Note that in case of tethering, only the phone can easily connect to the Heads SSH server otherwise further config is needed on the phone itself. This also opens interesting possibilities for the future; ie the phone could be used for attestation if it was the client of Heads... Food for thoughts here (#1307 and other). Phone could push updates etc. Anyway, not in scope for this first step toward on-demand, low firmware footprint networking capabilities (personal desires highlighted under #1384 (comment)) @daringer a reminder that this PR adds ~25Kb of drivers and if you desire adding this feature, you will have to do a seperate PR based on this one to enable the feature both in linux and board configs to have users depend on your nitro phones which should support this out of the box (to be tested of course on your side). To do so, simply take a diff for a linux config and apply to your board linux config, do same for board config and test and create PR. A reminder that activating either ethernet/mobile tethering is currently only offered through calling network-recovery-init per this PR, whic also syncs time from DNS server provided by DHCP answer first and then tried to NTP against ntp pool (to correct TPMTOTP behavior linked to hwclock having fell out of sync because time drift). That feature is guarded behind GPG based authentication when OEM Factory Reset/Re-Ownership is ran in non-default config, accepting to generate key in ram and preparing a USB Thumb drive with key material backup, which enables authentication prior of USB booting and Recovery Shell access. As of now, a user having access to Recovery Shell can call network-recovery-init to temporarily activate Ethernet/Mobile tethering. A reminder that accessing Recovery Shell invalidates unsealing of secrets until the machine is rebooted and the machine is booted in default boot. I think this is ready for merge! |
Oh! @JonathonHall-Purism : you might want to reconsider the removal of linux kernel config option:
Generally speaking, when one enables networking (even on-demand), it is suggested to enable syn flood protection since there is no drawback:
SRC: https://www.kernelconfig.io/CONFIG_SYN_COOKIES?q=CONFIG_SYN_COOKIES&kernelversion=6.7.5&arch=x86 |
This PR adds on-demand network connectivity through CDC NCM/ECM compatible phones tethering over USB cable (and therefore, permits phone to tether its actual network config (let it be what is configured on the phone: VPN/Tor + Mobile/Wifi -> USB).
It also fixes a bug where DNS name was wrong for NTP sync over ntp.pool.org.
CONFIG_MOBILE_TETHERING=y
(here on all maximized and Librem boards )