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

TESTING NEEDED: STAGING PR (quiet mode + diceware + nk3 fixes) #1875

Open
wants to merge 73 commits into
base: master
Choose a base branch
from

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Dec 9, 2024

Latest demos for Request For Comment (RFC) at #1875 (comment)
This changes massively the User eXperience an is an important change for feedback and testing

know board owners, testing would be better, but commenting on the demo videos would be as much appreciated (tested in qemu and nv41 on my side for both tpm1 and tpm2, with total hosnesty of it all being more tested under tpm2 boards though)
@natterangell @alexmaloteaux @akfhasodh @doob85 @srgrint @Thrilleratplay @nestire @lsafd @bwachter @shamen123 @eganonoa @nitrosimon @jans23 @icequbes1 @weyounsix @zifxify @jnscmns @computer-user123 @tlaurion @osresearch @merge @MrChromebox @n4ru @Tonux599 @househead @pcm720 @fhvyhjriur @3hhh @ThePlexus @akunterkontrolle @rbreslow @ResendeGHF @gaspar-ilom @JonathonHall-Purism @daringer @arhabd @d-wid


Summary:
Staging PR including diceware automatic passphrase generation for early 'o' OEM factory reset mode, quiet mode and hotp-verification changes so that secrets app PIN is finally setup per oem-factory-reset prior of use, info output consistent so nk3 is no regression compared to <nk3 security dongles under Heads use case for remote attestation.
This pull request introduces a new configuration option to enable quiet mode across various board configuration files. The quiet mode ensures that technical information is logged under /tmp/debug.log instead of being displayed.

Details:

  • early 'o' launches oem-factory-reset in oem mode (menu option could be splitted into User Re-Ownership doing individual secret provisioning. Here oem mode randomizes one single diceware passphrase for all secrets: TPM Owner/GPG Admin/GPG User/Secrets app PIN(nk3).
  • Quiet mode enalbed on all boards per 65d6fc4 (Q: do we want this to be the default? Toherwise will be reverted prior of merge and optional, turned on by Options->Configuration Settings menu, where quiet disable DEBUG and vice versa))
  • nk3 hotp-verification changes bettering info output (number of retries before locking on GPG Admin/User PIN/Secrets app PIN (nk3 only)
  • oem-factory-reset sets a Secret App PINs on orem-factory-reset (master doesn't and silently sets the first PIN typed after end user received his dongle when HOTP sealing of remote attestation challenge (nk3 firmware < 1.7.1 neglected PINs entirely and relied only on physical presence. This brings nk3 par with nk2 and permit users to reset Secret app as per re-ownership
  • Configuration Settings -> Enable quiet mode disables DEBUG/TRACE mode and vice versa
  • Quiet mode activated in board config (like here) sliences all technical unnecessary verbiage and outputs it through LOG call under /tmp/debug.log
  • Some important bugfixe related to primary handle hash for TPM2
  • some indentation fix (I stopped fighting with code changes and rely to formatting of IDE which is with TAB for files I review for now on)
  • Some literals changed and unified I came across on reviewing code

RFC and testing required!

  • Do we want Quiet mode to become the default?
  • Do we want oem-factory-mode (activated here solely by 'o' early on boot around Heads asciiart showed) to generate unique secrets for all GPG Admin/User PINs/TPM Owner/Secrets App?

This is staging of #1822 and #1850 with fixes picked up after hotp-verification version bump 1.7, next fixes coming as PR from Nitrokey under Heads for review from this moment on.

@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch from fd24b6b to 66829e9 Compare December 10, 2024 22:22
@tlaurion
Copy link
Collaborator Author

Default boot @wessel-novacustom : additional quiet iterations are harder and harder.

2024-12-10_19-53-39.mp4

@wessel-novacustom
Copy link

This is a great improvement!

Strings that should be muted as well from my point of view are:

From ***** Normat boot ...
Till (but not included): HOTP code is correct.

I can imagine that trying to mute those strings is hard since it's part of the HOT code verification application? If it's not hard to mute, please mute for Quiet Mode. Otherwise, it's OK to leave those strings as they are.

I think the following strings should be muted:

  • Found verified kexec ...
  • /tmp/secret/primary ...
  • Found verified kexec ... (second time)
  • Scanning for unsigned ...
  • WARN: Reading full size ...
  • 101515a: 000 ...
  • /tmp/counter-101 ...

@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch 3 times, most recently from f08b552 to 3a04195 Compare December 13, 2024 22:23
@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch from 1ac5229 to 78f17b1 Compare December 18, 2024 20:52
@tlaurion tlaurion marked this pull request as draft December 18, 2024 21:29
@tlaurion tlaurion changed the title STAGING PR (quiet mode + diceware + nk3 fixes) TESTING NEEDED: STAGING PR (quiet mode + diceware + nk3 fixes) Dec 18, 2024
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 18, 2024

So here are the demos from 65d6fc4 with qemu fbwhiptail tpm2 hotp prod quiet

Firmware upgrade simulation (or tampering if not user initiated!)

(injecting pubkey from command line in firmware image generates the tampering simulation) in quiet mode. Since TPM Disk Unlock Key was sealed before, it is renewed as part of the resealing of secrets and /boto is asked to be renewed

2024-12-18_16-50-24.mp4

Default boot, with TPM Disk Unlock Key

2024-12-18_16-55-42.mp4

OEM Factory reset mode initiated by early 'o' keypress, generating one shared secret for all provisioned components

2024-12-18_17-04-37.mp4

@tlaurion tlaurion marked this pull request as ready for review December 18, 2024 22:19
@d-wid
Copy link
Contributor

d-wid commented Dec 20, 2024 via email

@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch 2 times, most recently from 7c49fde to 9b8b815 Compare December 20, 2024 22:18
Copy link
Collaborator

@JonathonHall-Purism JonathonHall-Purism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good 👍

I made a handful of commits, have a look over those: tlaurion/heads@introduce_quiet_mode-diceware_STAGING...JonathonHall-Purism:heads:introduce_quiet_mode-diceware_STAGING

Also posted some review comments.

I'll be away for EOY after this. I can help address the remaining issues after holidays, but if you figure them out and need to merge I think that is reasonable.

Comment on lines +1439 to +1441
if [ "$prompt_output" == "y" -o "$prompt_output" == "Y" ]; then
break
fi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to loop here rather than our usual "press enter"? If somebody forgets to scan the QR code, you can always OEM reset again

Copy link
Collaborator Author

@tlaurion tlaurion Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonathonHall-Purism to make sure they understand that qr code here contains all providioned secrets and have to type Y (forced input)

If somebody forgets to scan the QR code, you can always OEM reset again.

The goal here would be for oem-factory-reset OEM mode to prompt as well for OS installation luks disk unlock key and, since unique, passphrase change it to the same diceware passphrase.

The request from OEM here was to be able to use a Qr code scanner/keyboard emulator to enter keypresses here, but I considered it too much changes on initial PR. The ideal workflow would be to load usb controllers + hid kernel modules and have a single prompt or two: either hardcode ISO defined OEM PINs per config (12345678/PleaseChangeMe known to be used by oem repacked ISOs) and prompt for oem actual/desired Disk Recovery Key passphrase.

Since physical presence is still needed for nk3 today, unattended workflow is not possible and I decided to postpone this to later, regression testing, as you pointed out for librem keys/nk2 might need more work and each iteration needs testing under Heads.

Tldr:So as of now, we force the user to acknowledge the secrets provisioned. Cause that involves DRK even today (if defaults are not accepted and user reencrypts + passphrase change luks containers) where a re-ownership alone won't fix a DRK passphrase change: réinstallation would be needed.


Security considerations

A security reminder that OEM OS pre-installation with publicly available OS luks key passphrases is convenient for unattended OS installation only, with severe implications: it eases in transit interdiction. Someone could not only implant something in installed OS if that default luks passphrase is publicly known, and only provides security if not shared with end user until past delivery for sole scope of the user reencrypting luks + passphrase changing upon reception of delivery.

End use's data at rest (encrypted user data, files, configs) requires luks containers reencryption+passphrase change, otherwise a passphrase change alone would not prevent in-transit luks header having been backup to be restored, and attacker to type PleaseChangeMe to access data at rest later on. This puts undesired liability on OEM if for whatever reason, luks passphrase happened to leak until end user reencrypts+passphrase change luks containers through re-ownership.

Luks containers reencryption+passphrase change is needed to protect user of oem and in-transit interception and defeat possible usage of luks header backup to be restored and data at rest to be accessed from pre-installed OEM OSes.

@@ -121,38 +130,50 @@ if [ "$((now_date - gpg_key_create_time))" -gt "$month_secs" ]; then
elif [ "$admin_pin_retries" -lt 3 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something's not right here on Librem Key, I'm getting:

/bin/seal-hotpkey: line 130: [: Admin3,User3: integer expression expected

(it's getting Admin3,User3 in admin_pin_retries)

I can help figure this out but it'll have to be in January

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed under 696ecf5 and tested with undusted old 0.10 fw based librem key I had in toolbox. <Nk3 works.

Comment on lines +139 to +142
#TODO: silence the output of hotp_initialize once https://github.com/Nitrokey/nitrokey-hotp-verification/issues/41 is fixed
#hotp_initialize "$admin_pin" $HOTP_SECRET $counter_value "$HOTPKEY_BRANDING" >/dev/null 2>&1
hotp_initialize "$admin_pin" $HOTP_SECRET $counter_value "$HOTPKEY_BRANDING"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably know this but there's a TODO here and it's producing some output for other keys now. Just leaving this open so we fix it before merge. Or you could use DO_WITH_DEBUG if capturing stdout/stderr to the log is sufficient for working on this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonathonHall-Purism agreed. This is regression caused by Nitrokey with their nk3 introduction without testing Heads for regression since nk3 is sold. If we silence output as before, then user doesn't receive prompt from hotp_initialize for nk3 keys.

@JonathonHall-Purism Feel free to open issue under https://github.com/Nitrokey/nitrokey-hotp-verification/ for @daringer and @daringer (cc @jans23) to be aware of time lost by ecosystem, so its not just me complaining. Further more that their nk3 regressions have impact for the whole ecosystem which they don't seem to acknowledge, living in their Nitrokey bubble.

I'm sorry, but i've lost 60h up to now which won't be paid and there is no compensation for my time applying mitigations/testing fixes etc that happened under #1866.

So from now on, regressions for heads will need issues opened under https://github.com/Nitrokey/nitrokey-hotp-verification and pr will be proposed by Nitrokey team under Heads since they refuse to compensate for work done. Todos in code for them to fix, as noted, depending on Nitrokey/nitrokey-hotp-verification#41 (requiring nk3 firmware upgrade) which was delayed from this feature freeze, even if @jans23 said it would be present for feature freeze per business related discussions.

Comment on lines 168 to 173
# remind user to change admin password
warn "Weak OEM default PINs are under use to enforce remote attestation/encryption/signature operations"
warn "$CONFIG_BRAND_NAME security is compromised until the ownership of this device is re-established by changing secrets by non-default values"
warn "You must change current default secrets through 'Options -> OEM Factory Reset/Re-Ownership' menu and not accept the default options"
warn "You will be asked to answer a questionnaire to re-own your device and USB security dongles with new secrets"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the amount of text in this warning will cause fewer people to read it, not more. (I am guilty of this.) I know I have read research on this, sadly I don't have enough time today to look it up. Above a sentence or two, users very frequently ignore the message entirely.

It is a great improvement to suggest where to go rather than leaving users totally confused. There is no sense re-explaining what OEM factory reset will explain anyway though.

Suggested change
# remind user to change admin password
warn "Weak OEM default PINs are under use to enforce remote attestation/encryption/signature operations"
warn "$CONFIG_BRAND_NAME security is compromised until the ownership of this device is re-established by changing secrets by non-default values"
warn "You must change current default secrets through 'Options -> OEM Factory Reset/Re-Ownership' menu and not accept the default options"
warn "You will be asked to answer a questionnaire to re-own your device and USB security dongles with new secrets"
# remind user to change admin password
warn "Default admin PIN detected. Please change this as soon as possible with Options > OEM Factory Reset / Re-Ownership"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed under 94dd788 which shows Secrets app PIN/ GPG Admin PIN depending if nk3/<nk3 (tested on 0.10 fw librem key: ok with branding info ok as well.

initrd/bin/tpmr Outdated
@@ -630,6 +651,7 @@ tpm1_unseal() {
-sz "$sealed_size" \
-of "$sealed_file" ||
die "Unable to read sealed file from TPM NVRAM"
# TODO: Cannot log + exit instead of dying!?!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifacts of quiet modes TODOs that were still in code but not relevant anymore. Fixed by af59704

All other TODOs in code considered relevant per quick review.

initrd/bin/tpmr Outdated
Comment on lines 675 to 683
tpm2 clear -c platform >/dev/null 2>&1 || LOG "Unable to clear TPM on platform hierarchy"
tpm2 changeauth -c owner "$(tpm2_password_hex "$tpm_owner_password")" >/dev/null 2>&1 || LOG "Unable to change owner password"
tpm2 changeauth -c endorsement "$(tpm2_password_hex "$tpm_owner_password")" >/dev/null 2>&1 || LOG "Unable to change endorsement password"
tpm2 createprimary -C owner -g sha256 -G "${CONFIG_PRIMARY_KEY_TYPE:-rsa}" \
-c "$SECRET_DIR/primary.ctx" -P "$(tpm2_password_hex "$tpm_owner_password")"
-c "$SECRET_DIR/primary.ctx" -P "$(tpm2_password_hex "$tpm_owner_password")" >/dev/null 2>&1 || LOG "Unable to create primary key"
tpm2 evictcontrol -C owner -c "$SECRET_DIR/primary.ctx" "$PRIMARY_HANDLE" \
-P "$(tpm2_password_hex "$tpm_owner_password")"
shred -u "$SECRET_DIR/primary.ctx"
tpm2_startsession
-P "$(tpm2_password_hex "$tpm_owner_password")" >/dev/null 2>&1 || LOG "Unable to evict primary key"
shred -u "$SECRET_DIR/primary.ctx" >/dev/null 2>&1
tpm2_startsession >/dev/null 2>&1 || LOG "Unable to start session"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TPM2 reset is not working on L1UM v2 🤔 Tried to do OEM reset and just get an error about TPM reset failed with blank output.

Trying to grab a debug log (not much time left for today but I think it'll be done in time). I haven't tried master on L1UM v2 lately either, not sure it's from this branch.

(Also not sure the problem is here, just putting this somewhere relevant 🙂 )

Copy link
Collaborator Author

@tlaurion tlaurion Dec 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot replicate neither on tpm2 qemu boards or nv4x :/ (but with your fixes in though)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hypothesis is that you haven't tested your proposed changes merged under this pr under a06ead6 (maybe old code artifact under initrd when you tested?)

All is good on my side here and your comment block doesn't reflect a06ead6

@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch from c66b5b6 to c41c923 Compare December 21, 2024 00:53
@tlaurion
Copy link
Collaborator Author

All proposed changes by @JonathonHall-Purism tested, good improvements, specifically the Config Settings menu changes under single menu and sink log hacks that are more streamlined.

tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…sion bump output parsing for <nk3

As tested working with old librem key fw 0.10: works
Log entry of additioanl 30 minutes for linuxboot#1875 (I cannot not fix with my time @jans23 linuxboot#1866, since nk3 is not the only dongle support by Heads)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 21, 2024

Houla. https://github.com/linuxboot/heads/pull/1875/checks?check_run_id=34750406698 says there is unsigned commit at HEAD~72 :/

Trying to fix but this pr has 67 commits. Oh well.

Edit: DCO error points to commit tlaurion@853541c
Cannot fi the past without rewriting git history: https://github.com/tlaurion/heads/tree/introduce_quiet_mode-diceware_STAGING-DCO_fix

Will rebase on master and move on.

tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…sion bump output parsing for <nk3

As tested working with old librem key fw 0.10: works
Log entry of additioanl 30 minutes for linuxboot#1875 (I cannot not fix with my time @jans23 linuxboot#1866, since nk3 is not the only dongle support by Heads)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…d containing 'export CONFIG_QUIET_MODE=y' for output comparison between debug, prod and quiet mode

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…now all passed to LOG (quiet mode doesn't show them and logs them to /tmp/debug.log)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…l information can be seen running 'cat /tmp/debug.log' from Recovery Shell

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…needed

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion and others added 19 commits December 21, 2024 13:20
…an't wait we get rid of this... file must exist and not be empty, and hash output to console must not be silenced

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…r qemu so only prod_quiet boards have quiet upon revert

repro
user@localhost:~/heads$ sed -i 's|export CONFIG_BOOTSCRIPT=/bin/gui-init|#Enable quiet mode: technical information logged under /tmp/debug.log\nexport CONFIG_QUIET_MODE=y\nexport CONFIG_BOOTSCRIPT=/bin/gui-init|' boards/*/*.config
user@localhost:~/heads$ git restore boards/*qemu*/*.config

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…ell access

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… dongle...'

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…s created upon setting default boot (was not clear)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
… at build time but disabled through Configuration Settings applied override, early measurement output got suppressed

Also tell user that those early suppressed messages can be seen under /tmp/debug.txt

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Add examples for capturing stderr or both stdout+stderr.

Trace blank lines with LOG like non-blank lines.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Use SINK_LOG to capture tpm2 unseal rather than a temp file.

Don't double up output from tpm "$@" to log; DO_WITH_DEBUG already
captures it.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
If a TPM reset step fails, don't blindly continue onto the other
steps.  Use DO_WITH_DEBUG to trace failures, so they're visible in the
log but we still exit due to set -e.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Don't print the URL and then explain how to get the secret out of it,
just print the secret.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
These two settings are exclusive, so they would disable each other if
enabled.  Present them as one setting with three output levels.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…olution

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…evert for qemu so only prod_quiet boards have quiet upon revert"

This reverts commit 65d6fc4.

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…et(y) just prior of gui-init to attempt to unify to all boards

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…prior of bootscript to unify to all boards

with exception of
- qemu boards not being *quiet: quiet=n
- qemu boards not being *prod* having pcap=y
- qemy boards not being *prod* have debug+tracing=y
- qemu tpm1 boards have '#pcap=n'

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…/tmp/tpm0.pcap (not just check for existence of CONFIG_TPM2_CAPTURE_PCAP under env)

So that export CONFIG_TPM2_CAPTURE_PCAP=n across all boards doesn't break and so that its easy for auditors to just toggle on in board configs

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…sion bump output parsing for <nk3

As tested working with old librem key fw 0.10: works
Log entry of additioanl 30 minutes for linuxboot#1875 (I cannot not fix with my time @jans23 linuxboot#1866, since nk3 is not the only dongle support by Heads)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch from 29b2108 to 696ecf5 Compare December 21, 2024 18:26
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 21, 2024

Ha right. Sorry for the noise of rebasing: https://github.com/linuxboot/heads/compare/29b210800a99de24bef8519363a3bdfbc2a82bd7..696ecf54cd38d278495a3119ca8afcd627d6d2bb shows last master unsigned commit which was used for testing unsigned commit (commit signed by github for #1794)

…IN is detected

Additional 0.5h for applying changes linked to code review under linuxboot#1875
Linked to Nitrokey unacknowledged RfP linuxboot#1866 that continues to grow past the 40h (now near 42... but unpaid because 'unplanned'... As if this was planned on my side.)

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion tlaurion marked this pull request as draft December 21, 2024 20:59
@tlaurion
Copy link
Collaborator Author

Ready for testing for all, with fixes per @JonathonHall-Purism and additional commits to address his review. Switching from draft to ready to review so github bot updates in channel

@tlaurion tlaurion marked this pull request as ready for review December 21, 2024 20:59
@srgrint
Copy link
Contributor

srgrint commented Dec 23, 2024

Excellent work - thanks

I have flashed heads-x220-maximized-v0.2.0-2537-gaf59704.zip

Seems to work fine so far. Will test more extensively over next few days.

In my opinion, quiet mode is a sensible default - can easily switch to more debugging if needed

@3hhh
Copy link
Contributor

3hhh commented Dec 24, 2024

I tested 82e6635de61e9507564fee00a4934ce753f38b96b235a78240394a973e084ab2f813cd4a767b02786e1c269747874390054e369b4a64b85cb8d72422b296929c heads-t530-hotp-maximized-v0.2.0-2537-gaf59704.rom.

So far it mostly seems to do its job, thanks!

Some things I noticed:

  • I encountered a TOTP fail after an OEM reset and had to do a manual TPM reset to fix it. This happened after the below and some configuration change (informational output).
  • I had to do two OEM resets because my keyboard layout doesn't have "y" where the US keyboard layout has it. I know that this issue is around for a long time, but it should be fairly straightforward for heads to ignore all characters except for y/n/return (=default) and repeat the same prompt question, if an unexpected character is pressed. Currently it always assumes the default on unexpected character press. That lead to the default password 12345678 for me and possibly also to the aforementioned TPM reset issue, idk.
  • The boot options looked weird, namely a lot of kexec-parse-boot-stdout: _Qubes ... along with some regular boot options.

Merry Christmas btw.

@tlaurion
Copy link
Collaborator Author

I tested 82e6635de61e9507564fee00a4934ce753f38b96b235a78240394a973e084ab2f813cd4a767b02786e1c269747874390054e369b4a64b85cb8d72422b296929c heads-t530-hotp-maximized-v0.2.0-2537-gaf59704.rom.

So far it mostly seems to do its job, thanks!

Some things I noticed:

  • I encountered a TOTP fail after an OEM reset and had to do a manual TPM reset to fix it. This happened after the below and some configuration change (informational output).

This is weird unless you changed the informational output option, which saves back under cbfs config.user overrides for debug output which you seemed to have tested here. This would modify flash as warned and? What was unexpected?

  • I had to do two OEM resets because my keyboard layout doesn't have "y" where the US keyboard layout has it. I know that this issue is around for a long time, but it should be fairly straightforward for heads to ignore all characters except for y/n/return (=default) and repeat the same prompt question, if an unexpected character is pressed. Currently it always assumes the default on unexpected character press. That lead to the default password 12345678 for me and possibly also to the aforementioned TPM reset issue, idk.

Please open an issue here. This is important issue and not sure one is opened, would not be trivial to change but not complicated either.

  • The boot options looked weird, namely a lot of kexec-parse-boot-stdout: _Qubes ... along with some regular boot options.

That would be part of debug output and is considered normal unless thought otherwise?

Merry Christmas btw.

You too and thanks for the feedback, means a lot.

@3hhh
Copy link
Contributor

3hhh commented Dec 25, 2024

  • I had to do two OEM resets because my keyboard layout doesn't have "y" where the US keyboard layout has it. I know that this issue is around for a long time, but it should be fairly straightforward for heads to ignore all characters except for y/n/return (=default) and repeat the same prompt question, if an unexpected character is pressed. Currently it always assumes the default on unexpected character press. That lead to the default password 12345678 for me and possibly also to the aforementioned TPM reset issue, idk.

Please open an issue here. This is important issue and not sure one is opened, would not be trivial to change but not complicated either.

Ok, I reported it at #1880.

  • The boot options looked weird, namely a lot of kexec-parse-boot-stdout: _Qubes ... along with some regular boot options.

That would be part of debug output and is considered normal unless thought otherwise?

I had enabled informational output only and didn't expect that the option affects the list of boot options which I can select from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants