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.

Sign up for GitHub

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

Jump to bottom

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

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

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

wants to merge 85 commits into from
+5,009 −2,976

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Dec 9, 2024 •
edited
Loading

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.

Log level changes:

  • Informational: prior default, gives output of measured boot and pertinent boot verification output to console. DO_WITH_DEBUG output is not gathered nor redirected. To enable this, user has to turn DEBUG output at runtime. Technical users targeted
  • Debug: outputs most of the output to console, tweaking kmsg output to level 8, somehwat equivalent of having linux kernel "debug" option passed. Limitation added to early init stating that stderr+stdout command output is under /tmp/debug.log, not on console. Debug/development/codebase understanding users targeted
  • quiet (none): Similar to informational output, but where output of measured boot (tpm extend calls justification and boot related verification) is redirected to /tmp/debug.log for user inspection from recovery console. Quiet mode is like Informational output, but informational console output is redirected to /tmp/debug.log for user inspection. Non-technical users targeted

@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 ...

This was referenced Dec 11, 2024
Closed
Merged
@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 mentioned this pull request Dec 16, 2024
Closed
@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 •
edited
Loading

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
JonathonHall-Purism
JonathonHall-Purism reviewed Dec 20, 2024
View reviewed changes
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.

initrd/bin/oem-factory-reset
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 •
edited
Loading

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.

initrd/bin/seal-hotpkey
@@ -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.

initrd/bin/seal-hotpkey
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.

initrd/bin/seal-hotpkey Outdated
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 •
edited
Loading

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
@tlaurion
initrd/bin/seal-hotpkey: fix regression of hotp_verification 1.7+ ver…
29b2108 
…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 •
edited
Loading

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
@tlaurion
initrd/bin/seal-hotpkey: fix regression of hotp_verification 1.7+ ver…
0f5c3a0 
…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
qemu-coreboot-fbwhiptail-tpm2-hotp-prod_quiet board: addition of boar…
496d930 
…d containing 'export CONFIG_QUIET_MODE=y' for output comparison between debug, prod and quiet mode

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
initrd bin/* sbin/insmod + /etc/ash_functions: TPM extend operations …
1f02912 
…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>
@tlaurion
init: suppress /etc/config.user not existing on grep calls
9cd4757 
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
init: inform user that running in quiet mode, tell user that technica…
e03a790 
…l information can be seen running 'cat /tmp/debug.log' from Recovery Shell

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
codebase: silence dd output while capturing output in variables when …
48807de 
…needed

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
initrd/bin/tmpr: silence tpm reset console output, LOG instead
3726e90 
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion tlaurion marked this pull request as draft January 13, 2025 16:02
@tlaurion tlaurion marked this pull request as ready for review January 13, 2025 18:13
@tlaurion
Copy link
Collaborator Author

@JonathonHall-Purism #1875 (comment) documents 930d3e6 which IMOH is a proper fix.

@tlaurion tlaurion mentioned this pull request Jan 15, 2025
Merged
@tlaurion
Merge remote-tracking branch 'osresearch/master' into introduce_quiet…
b1690ce 
…_mode-diceware_STAGING
@tlaurion
BOARD_TESTERS.md: revise board names, add v560tu, add testers expecte…
69037fc 
…d to answer testing calls

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion
novacustom-v560tu board config: set board to have quiet mode enabled …
bab46bc 
…by default

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

tlaurion commented Jan 15, 2025 •
edited
Loading

  • nv41 works as expected in quiet (none per config settings), informational and debug

@tlaurion
typo: s01x -> s0ix
392d456 
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@mkopec
Copy link
Contributor

mkopec commented Jan 16, 2025 •
edited
Loading

Does not seem to be working correctly yet on V560TU, kernel messages are still visible:
image

@JonathonHall-Purism JonathonHall-Purism mentioned this pull request Jan 16, 2025
Open
3 tasks
@JonathonHall-Purism
Copy link
Collaborator

@tlaurion Per our DM, I suggest only doing the "did you really scan the QR code and loop unless you say you really did" if we actually generated diceware passphrases. There's no need to do this to the user if they entered their own passphrase or are using defaults to change later.

Could you take a look at this commit: JonathonHall-Purism@23d4543

Otherwise this works for me on L14 👍

I also started documenting config variables and the updated logging behavior in #1888 (not blocking this PR, we can do that separately).

@tlaurion
v560tu: unify board config, remove debug cmdline passed from coreboot…
2872f44 
… to linux kernel

Note: qemu coreboot config still pass debug (non quiet, non prod board = debug)
config/coreboot-qemu-tpm1.config:173:CONFIG_LINUX_COMMAND_LINE="debug console=ttyS0,115200 console=tty"
config/coreboot-qemu-tpm2.config:170:CONFIG_LINUX_COMMAND_LINE="debug console=ttyS0,115200 console=tty"

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@tlaurion tlaurion force-pushed the introduce_quiet_mode-diceware_STAGING branch from 99f3c25 to 2872f44 Compare January 16, 2025 16:24
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 16, 2025 •
edited
Loading

Does not seem to be working correctly yet on V560TU, kernel messages are still visible: image

@mkopec debug was still under coreboot LINUX_COMMAND_LINE config. Fixed by 2872f44 thanks for testing.

@JonathonHall-Purism @tlaurion
oem-factory-reset: Only badger user to record passphrases if generated
22a86e6 
There are many flows through oem-factory-reset that use passwords
provided by the user or basic defaults to be changed later.  We don't
need to badger the user to record those passwords.

Still do this if we generated diceware passwords though, as the user
does not know them yet.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 16, 2025 •
edited
Loading

@tlaurion Per our DM, I suggest only doing the "did you really scan the QR code and loop unless you say you really did" if we actually generated diceware passphrases. There's no need to do this to the user if they entered their own passphrase or are using defaults to change later.

Could you take a look at this commit: JonathonHall-Purism@23d4543

Otherwise this works for me on L14 👍

I also started documenting config variables and the updated logging behavior in #1888 (not blocking this PR, we can do that separately).

Thanks @JonathonHall-Purism : makes sense, with bemol that QrCode not given to end user now for master's default, and a lot of user read too quick and don't notice that 123456 is different then 12345678 :)

So as of 22a86e6, QR code and loop forcing end user/OEM to confirm passphrases are noted down currently only happens if OEM presses 'o' early at boot (Around Heads asciiart) since only that codepath generates diceware passphrases, which are the only ones unknown/non defaults. Note that default OEM mode still gives 12345678 and 123456 without QR and user might have misnoted them, since the only difference between them is "78" @JonathonHall-Purism :) But let's not hit this too hard in this PR, after all this is behavior of master and if we want to change it later, we can.

@mkopec : take this into consideration in your testing as well :) this PR doesn't change the default behavior from GUI.

@mkopec
Copy link
Contributor

mkopec commented Jan 16, 2025 •
edited
Loading

debug was still under coreboot LINUX_COMMAND_LINE config. Fixed by 2872f44 thanks for testing.

Ah, derp :) Thanks for fixing, I just tested it and all seems to be working well now 👍

Output on the normal boot path now exactly matches Default boot, with TPM Disk Unlock Key from #1875 (comment) . OEM factory reset in OEM mode also works.

This was referenced Jan 16, 2025
Open
Open
Merged
Merged
This was referenced Jan 17, 2025
Closed
Merged
@tlaurion
Copy link
Collaborator Author

2d19fa9 reverted #1889

@mkc, testing of that branch behavior can still be tested by artifacts of 0cdd441

@tlaurion
Copy link
Collaborator Author

tlaurion commented Jan 17, 2025 •
edited
Loading

61e6cf6 added bugfix of #1875 (comment) under master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonathonHall-Purism JonathonHall-Purism JonathonHall-Purism approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tlaurion @wessel-novacustom @d-wid @srgrint @3hhh @mkopec @JonathonHall-Purism