Nitrokey board cleaning+ unification cleanup (enable htop validated autoboot + tethering) #1640
Conversation
|
No regression discovered, all good for tests specified in OP. |
|
very good! testing ns50 asap... |
@alexgithublab not sure what this means. The instructions on screen have been followed? Connect phone when requested (phone in host mode needs to detect data not just power, requiring heads tethering drivers to be loaded prior of phone possibly permitting to activate USB network tethering, and then heads setups tethering against phone). If not, the behavior you see is normal? Different behavior then nv41? |
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
from
1b35b9f
to
35aff5c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
3 times, most recently
from
a68fe24
to
bfe67ab
Compare
I'm using a Pixel 5 with GrapheneOS and I'm not able to get tethering network working on heads. |
| @@ -1900,7 +1756,7 @@ CONFIG_FB_CFB_IMAGEBLIT=y | |||
| # CONFIG_FB_ASILIANT is not set | |||
| # CONFIG_FB_IMSTT is not set | |||
| # CONFIG_FB_VGA16 is not set | |||
| CONFIG_FB_VESA=y | |||
| # CONFIG_FB_VESA is not set | |||
There was a problem hiding this comment.
@nestire: might be the cause of #1641
Was enabled under nitrokey 2.4.1 https://github.com/Nitrokey/heads/blob/50e03c1cfe8ce2cb1d7ad2c43c15d17d05c82dc1/config/linux-nitropad-x.config#L1902
| @@ -1003,7 +1009,7 @@ CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK=y | |||
| # CONFIG_ISCSI_IBFT is not set | |||
| # CONFIG_FW_CFG_SYSFS is not set | |||
| CONFIG_SYSFB=y | |||
| # CONFIG_SYSFB_SIMPLEFB is not set | |||
| CONFIG_SYSFB_SIMPLEFB=y | |||
There was a problem hiding this comment.
@nestire: 2.4.1 was not pushing this compared to master https://github.com/Nitrokey/heads/blob/50e03c1cfe8ce2cb1d7ad2c43c15d17d05c82dc1/config/linux-nitropad-x.config#L1005
Might be cause for #1641?
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
3 times, most recently
from
fa70cba
to
ae5f9c5
Compare
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
from
521c0b0
to
16f1d07
Compare
|
So only relevant changes under https://github.com/linuxboot/heads/compare/521c0b039ea95c9133566944d2e4bc29a9772507..16f1d07867ddca9a5feee1f9541e2a7cc52d3b4a outside of rebasing on master and removing irrelevant changes here:
OP and comments here to be hidden/review resolved since splitted under #1642 and merged in master yesterday to ease debugging forward. |
What this unfortunately means for the Pixel 5, which is EOL from Google but in extended support under GrapheneOS, is as said in the warning when enabling tethering, that RNDIS (Microsoft tethering technology) enables tethering on those phones, not CDC. If CDC was enabling tethering, then tethering would work following on screen instructions there. I tested on Pixel 4a 5G and Pixel 6a, which both supports CDC tethering, and where, generally, USB-C snapdragon platform based SoC phones will support CDC for tethering. Unfortunately, there is not really good documentation on which phone supports CDC for tethering, so it's trial and error, where laptops having an Ethernet port can fallback to it to have on-demand connectivity. I also gathered a quick table under I will open an issue on documenting tethering support to track this better. This is also one of the reason why it's not currently enabled through GUI and hidden down from launching a script, to easy time synchronisation mostly, for the moment Tldr: the phones currently in Nitrokey shop (3a+ = Pixel 6+) should work. Librem phones work. For other LineageOS phones, experience will vary depending on what tethering technology is enforced. For Replicant, I highly doubt anything other then RNDIS is supported there, which heads won't include for discussed reasons (including security implications) on merged PR. iPhones won't be supported since support requires additional proprietary tooling and extended kernel modules as well which older devices won't have space for. Originally posted by @tlaurion in #1640 (comment) |
|
@tlaurion |
|
@alexgithublab @daringer : no regression here? I can merge? @daringer can you approve? I think #1641 (comment) should be considered independently for PR, we need stop mixing everything together preventing quick merges of PR. |
|
Putting in draft since I'm reviewing changes against defconfigs (coreboot) and need confirmation on values there prior of things to be physically tested. |
| # CONFIG_USB_OHCI_HCD is not set | ||
| # CONFIG_USB_UHCI_HCD is not set |
There was a problem hiding this comment.
@daringer: no usb1/usb2 ports right?
| @@ -2746,7 +2564,7 @@ CONFIG_HARDENED_USERCOPY=y | |||
| # CONFIG_STATIC_USERMODEHELPER is not set | |||
| # CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT is not set | |||
| CONFIG_DEFAULT_SECURITY_DAC=y | |||
| CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" | |||
| CONFIG_LSM="lockdown,yama,loadpin,safesetid,integrity,bpf" | |||
There was a problem hiding this comment.
@daringer not sure if this change is desired but landlock is for sandboxing. Nothing uses it under Heads as of now
JonathonHall-Purism
dismissed
their stale review
We moved all the things I commented on to a separate PR
|
@daringer @alexgithublab @JonathonHall-Purism ready for final review (and test on ns50 which I don't have) @JonathonHall-Purism commits touch librems but final files changes not relevant/not touching them. @alexgithublab @daringer see commit messages. I made those steps reproducible so that you can learn how to use current helpers properly to see changes. The master coreboot configurations were outdated and relevant to an older coreboot version, which makes it really difficult to understand which coreboot options are impacting or not something. As a reminder defconfig permits to check changes against default config options relevant to a certain coreboot version. After that, latest commit puts those configs back to oldconfig format which permits to compare boards against each other. @alexgithublab nothing really to report that could explain #1641 without providing logs there as requested. I think we should merge this ASAP. |
|
Looks good to me, will let the NK folks give the approval since they are most familiar with these boards 👍 |
config/coreboot-nitropad-ns50.config
Outdated
| @@ -114,23 +119,23 @@ CONFIG_DIMM_SPD_SIZE=512 | |||
| CONFIG_FMDFILE="" | |||
| # CONFIG_NO_POST is not set | |||
| CONFIG_MAINBOARD_VENDOR="Notebook" | |||
| CONFIG_CBFS_SIZE=0x1000000 | |||
There was a problem hiding this comment.
Definitely changing this was wrong as it makes the board build fail for ns50. Will revert that change later.
|
@daringer @alexgithublab it is to be noted that the bug related to #1641 might already have resolved itself if coreboot version and configs were bumped to latest dasaharo+heads 1.7.2 novacustom coreboot release where of course, there is no dasharo+heads release for the ns50, leading to more low level work required on your side to make it working. Changes since Line 94 in b2629f8 The dasharo+heads referred commit vs Heads used coreboot commit (more then 2 months of features + bugfixes there between Heads used coreboot commit and upstream which I have not skimmed through): @daringer @alexgithublab : I would of course recommend staying as close as possible from Novacustom's coreboot fork's commit to not duplicate downstream work (heads and nitrokey releases based forks related issues) related to firmware related bugs that may already be fixed or not, and collaborate upstream there. Heads uses coreboot and is not coreboot. I do not have the resources to debug things there, that is why 3mdeb are coreboot developers and provide dasharo subscription, and why I am not pretending to be a coreboot developer myself. |
|
Alternate testing branch at https://github.com/tlaurion/heads/tree/nitrokey_board_unification_clean-enable_htop_validated_autoboot-novacustom_coreboot_version_bump Note that both branches add board's config "KERNEL_ADD='debug'" so that more output can be provided from OS boot to help troubleshoot #1641. Great, required GPU blobs are now present under initramfs, but why they are not loaded is still not explainable. |
|
@daringer @alexgithublab both this PR and attempted novacustom's dasharo+head 1.7.2 pointed commit in their release is testable now. If Ubuntu was installed with firmware blobs, please test and report here results. Note that the initramfs content not lib directory content from installer should be reported, just like @nestire report above. @daringer cc |
|
Will rebase on master. |
… nitrokey boards: enable Automatic boot when HOTP valid after 5 seconds Signed-off-by: Thierry Laurion <[email protected]>
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
from
6da1ed2
to
0f3cd02
Compare
…nst each other - Add tethering in board configs - Add autoboot after 5 seconds if HOTP remote attestation is successful Signed-off-by: Thierry Laurion <[email protected]>
TODO: fix discrepencies in kernel config to limit technological debt in later commit in this PR Signed-off-by: Thierry Laurion <[email protected]>
tlaurion
force-pushed
the
nitrokey_board_unification_clean-enable_htop_validated_autoboot
branch
from
0f3cd02
to
0f412ed
Compare
Result of: make BOARD=nitropad-nv41 coreboot.save_in_oldconfig_format_in_place make BOARD=nitropad-ns50 coreboot.save_in_oldconfig_format_in_place No change, was applied like this anyway at compilation. Signed-off-by: Thierry Laurion <[email protected]>
… config (GOP compliant) TODO: next, readd what might have been pertinent Signed-off-by: Thierry Laurion <[email protected]>
git difftool -d HEAD^ to check config against previous version (librem shared config), noticed I2C options being maybe relevant, added them back in Then saved with make BOARD=nitropad-ns50 linux.modify_and_save_oldconfig_in_place Signed-off-by: Thierry Laurion <[email protected]>
|
@daringer @alex-nitrokey
We should try to reduce delays between PR and merge since upstream is used in forks... @daringer please approve. Bumping to newer version of coreboot will happen in other PR. As said #1640 (comment) another branch was made to ease your porting. Seal commits, adapt, and do PR in draft mode so we can iterate. This pr is about making board configuration as clean as possible, making current coreboot oldconfig files coherent to present version used and add tetheting and hotp autoboot in board configs and tethering requirements under shared linux config, based on librems, where librem 11 FB is GOP enabled. Only thing here to evaluated if goal of this PR is now mixed with #1641 being fixed would be to check librem11 config against ns50/nv41. I would appreciate collaboration here or under Matrix to make things fixed upstream as fast as possible, more for nv41 since otherwise double work for me with novacustom bug reports and support. Let's improve collaboration. |
|
This takes way too long without justifcations. Using my maintainer veto and merging. Hope I will never have to do this again. |
Edit: UX changes related to this PR state in pictures.
NS50/NV41:
qemu//nitropads:
@daringer : testing notes under commit. Running this on my nv41: all good.
Please approve and use master's commit for your tests/next version.
Todo: firmware upg from zip+ TPMTOTP/HOTP reseal + TPM DUK reseal + TPM DUK based boot + network-init-recovery(usb network tethering+ time sync [auto])