GPG User Authentication: In-memory gpg keygen + keytocard and GPG key material backup enabling (plus a lot of code cleanup and UX improvements) #1515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As you can see in this screenshot, some guidelines were added in warnings so that the user is made aware of signing errors that can happen under Heads if TPM reset is needed to re-create TPM counter which otherwise sealing operations fail. As of now, the user faces a loop when doing TPM Disk Unlock Key resealing and is now informed that he should do a TPM reset in such corner case. |
|
As of now, after having re-owned/oem factory reset secrets accepting to create a GPG thumb drive backup, the user can use either the backup or the USB security dongle, interchangeably: On recovery/usb boot, we now have maybe too verbose output but those access are authenticated with either USB Security dongle or the Thumb drive backup: |
|
Some unrelated changes in vocabulary for consistency in current state of PR when resealing TPM Disk Unlock Key |
tlaurion
force-pushed
the
inmemory_keygen-gpg_backup_usable_for_RSA_only-copy_to_card_working_for_RSA_only-gpg_auth_for_recovery_and_sub_boot
branch
from
a4ab0dd
to
5468082
Compare
|
Latest commit fixes output and forces dmesg output pushed in console to be of warning level and lower if no debugging mode set in board config/through configuration menu. Accessing the recovery shell gives something like this in production environement: |
|
Last commit removes unnecessary output. Just tracing here, will hide messages later on |
tlaurion
force-pushed
the
inmemory_keygen-gpg_backup_usable_for_RSA_only-copy_to_card_working_for_RSA_only-gpg_auth_for_recovery_and_sub_boot
branch
from
66c03b7
to
b7c4f1a
Compare
|
With last commit b7c4f1a NK3 is supported with ECC p256 in-memory keygen and keytocard (as well as non-officially supported RSA2048 subkeys). It looks like this when asking recovery shell access or USB boot (non-debug qemu tpm2 whiptail): Normal behavior (default) is to authenticate against USB Security dongle (NK3 here): Otherwise GPG Thumb drive key material backup looks like this: |
|
Next step is to reprovision USB Security dongle (keytocard) from GPG key material backup thumb drive. |
|
As current state:
A lot of code cleanup has been made along the way. @JonathonHall-Purism Please check left TODO in code. The check for TPM2 primary handle has been removed from now since if causes a lot of issues linked to default boot and it not being signed. That would need to be fixed on master separately and would need some advice/coop.
Here is a quick preview of what to expect: Questionnaire changes:
|
tlaurion
commented
Oct 30, 2023
tlaurion
commented
Oct 30, 2023
tlaurion
commented
Oct 30, 2023
tlaurion
commented
Oct 30, 2023
There was a problem hiding this comment.
Thanks @tlaurion. I think the ideas here and general structure are good. I haven't tested any of it yet, but I did review it thoroughly keeping in mind it's still WIP.
Specific comments above, but as some key highlights:
- I suggest moving the "boot authentication" (gpg_auth) to a separate PR so we can more easily discuss the goals and behavior of that feature (but I think it is a great idea and on a good path).
- I suggested wording improvements for the new prompts in OEM reset but I think the strategy makes sense
- I'll look into the issues with the TPM2 primary handle hash as there are some clear gaps there as we've discussed before
I looked over the new GPG functionality and don't have any problems with it, with that said I'm not really a GPG expert at this point so testing will be pretty important there IMO.
boards/qemu-coreboot-fbwhiptail-tpm1/qemu-coreboot-fbwhiptail-tpm1.config
Show resolved
Hide resolved
tlaurion
commented
Oct 31, 2023
tlaurion
commented
Oct 31, 2023
tlaurion
commented
Oct 31, 2023
tlaurion
commented
Oct 31, 2023
…up exists. Might want to discuss that implementation. Some functions needed to be moved from functions to ash_functions so that gpg_auth can be called from recovery function. That might need to be discussed as well, recovery could be moved from ash_functions to functions instead. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…s linked to detach signing errors Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…not so and adapt further troubleshooting notes in code when keys cannot be accessed on media for whatever cause so user can understand what is happening when accessing GPG material on backup thumb drive Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…used with GPG key material thumb drive backup Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…should be written to file and reused since not all in same functions/files for TPM2 Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…o to recovery shell instead of rebooting Signed-off-by: Thierry Laurion <insurgo@riseup.net>
|
So with latest commit Testing extended, normal (without in-memory GPG keygen nor move to card) but with different passphrases for each security components, the workflow is as expected (tpm2-whiptail-non-hotp, with debug on). It is to note that old Nitrokey Pro (non-upgradeable firmware outside of external flashing) takes more then 3 minutes to generate RSA 3072 subkeys. I do not expect a lot of users to have such keys, but this is what it is. Taking notes to compare with next test which moves sybkeys to card. I love to test with whiptail because the output is directly on console and can be copy pasted. Analysis:
After injecting public key in next build, GPG User Authentication against USB Security dongle works as expected with GPG User PIN configured. Then redoing with in-memory keygen, backup to asked to be prepared USB Thumb drive backup: Analysis:
After injecting public key to rom and enabling
or
|
Fixed under 160367d
Fixed under 3787293 |
tlaurion
force-pushed
the
inmemory_keygen-gpg_backup_usable_for_RSA_only-copy_to_card_working_for_RSA_only-gpg_auth_for_recovery_and_sub_boot
branch
from
dcc3b0f
to
9f4a521
Compare
…logic. Also move USB Security dongle capability detection under code already checking for USB Security Dongle's smartcard presence. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion
force-pushed
the
inmemory_keygen-gpg_backup_usable_for_RSA_only-copy_to_card_working_for_RSA_only-gpg_auth_for_recovery_and_sub_boot
branch
from
9f4a521
to
3787293
Compare
tlaurion
changed the title
tlaurion
changed the title
|
The refactors all look great, confirmed everything reported above was fixed. Testing the new functionality now on L1UM v2 (TPM2). @tlaurion I didn't unplug my flash drive when it asked me to remove all external USB devices (to see what happens). It still selected that flash drive automatically when I continued. Isn't this supposed to exclude USB devices that remained connected? I figured this was to exclude internal USB devices, like the Librem 14's SD card reader. edit: Then when I looked back after posting this, it had errored out and sent me back to the main menu (no error prompt). This was on the console:
750 MB is right, it's an "8GB" flash drive and I selected 10%. So it had the right device, I'll pull it and see what it put on there. |
config/linux-librem_common-6.1.8.config: passed to oldconfig format through 'make BOARD=librem_14 linux.modify_and_save_oldconfig_in_place' Signed-off-by: Thierry Laurion <insurgo@riseup.net>
@JonathonHall-Purism I'll rework the removal/insert prompt to list all usb connected block devices and list capacity for user to select disk prior of confirming once after selection for wiping.
newer librems based on config/linux-librem_common-6.1.8.config didn't have EXFAT support in the kernel. That was fixed under 23c967f (Note that CircleCI is having temporarily difficulties fetching tarballs on coreboot build as of now, and also flash boards seem to have race condition (what? Why!) so I might disable the builds in CircleCI here if they fail on clean cache. Nitrokey deprecated them and I will do the same, moving them to boards/UNSUPPORTED_ boardname if they cause problem on a rerun. No time for that anymore. Was told before, might be the time for it now.) |
|
Thanks @tlaurion. Kconfig changes look good, saved as defconfig locally to compare, we'll stick to oldconfig going forward. Let me know when USB device selection is ready for review again 👍 |
@JonathonHall-Purism that would be that moment :) |
…storage size and loop until none is connected to exit loop. Warn user if connected usb block device is less then 128mb, since creating LUKS container of less then 8mb might cause issues. Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion
force-pushed
the
inmemory_keygen-gpg_backup_usable_for_RSA_only-copy_to_card_working_for_RSA_only-gpg_auth_for_recovery_and_sub_boot
branch
from
8bf507f
to
e924a8a
Compare
tlaurion
changed the title
Combine prompt to disconnect other devices with prompt to connect the desired device. Show block device sizes in MB/GB when selecting device so it is easier to select. file_selector now supports --show-size to include block device sizes in menu. Rework file_selector so menu options can contain spaces (use bash array) and to simplify logic. Prompt to select flash drive and LUKS percentage in OEM reset before actually taking any actions, so aborting doesn't half-reset the system. Abort OEM reset if user aborts the flash drive selection instead of looping forever. (Canceling the confirmation still loops to retry but it is possible to exit by aborting the repeated menu.) Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Move confirmation of formatting flash drive with LUKS percentage selection before any reset actions have been taken, so aborting does not result in a half-reset system. Combine with the more basic "confirm" prompt that existed after selecting the device (but did not include the LUKS size information). Split up prepare_flash_drive into interactive_prepare_flash_drive (both prompts and formats as before), confirm_thumb_drive_format (just confirms the selections), and prepare_thumb_drive (now noninteractive). Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Don't repeat this message if the user says "no" to the confirmation prompt. Go directly to the menu. Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
|
@tlaurion I tested this a lot and made some changes to smooth out the OEM reset flow with flash drive format: https://github.com/JonathonHall-Purism/heads/commits/oem_reset_fd_sel_improvement Changes: This is the resulting sequence. All prompts occur before OEM reset starts to reset anything, so aborting will not leave a half-reset system. You can now abort by aborting the flash drive selection (no longer loops in this case). Deduplicated some prompts. Put some of the better-size-display enhancements in the more specific confirmation prompt and used that instead of the more basic one.
|
|
Wow @JonathonHall-Purism . Merging disk sizes with prompts is a really good flow improvement, and your changes to parametrize functions makes it easier to understand, follow and improve further as well. Wow! I think it's a merge day! Doing one last whole test case on real hardware and as discussed I will merge (reviewing your changes on top of the changes you reviewed and applied changes you saw fit better.) I really love the graphics and your work on this! Awesome collaboration! I'm in awwwwwwwe! :) |
JonathonHall-Purism
approved these changes
Nov 13, 2023
There was a problem hiding this comment.
Thanks @tlaurion ! I think this is a great result. Merge when ready! 💯
This was referenced Nov 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal of this PR: Permit User Authentication prior of going to Recovery Shell or USB boot. But prior of enabling that functionality, we need to make sure that the user has a backup of his key material. And to have a backup of key material, we need to generate master key and subkeys in memory and backup over encrypted partition and then keytocard the subkeys to the USB Security dongle. This PR does that and clean the code that was reviewed along the way.
EDIT: this comment from @JonathonHall-Purism shows the new flow graphically: #1515 (comment)
Ok! from #1515 (comment):
Unrelated work under this PR:
Discussions on how GPG Authentication and OEM Factory Reset/Re-Ownership features should evolve are happening under #1520 and #1521 respectively.
Please participate, your voice is important to be considered!
Old:
Creating WiP PR to track changes.
WARNING: At current stage, prior merged PR #1476 makes it possible (warning with Intel 3rd gen I3 laptops without RDRAND: crng is not ready early on boot, will need to update board configs for notice even though those CPUs are not widely used).
Raw notes:
usb_security_token_capabilities_check(My only testing key is NK3 and I wanted to support RSA first.... so stay tuned here for latest changes and squashing prior of testing this if you have NK3 and not testing pre-releases, this is not ready to use).CONFIG_HAVE_GPG_KEY_BACKUPand saves it in config.user, injects it and then checks against in in codebase to automatically activate authentication on recovery shell and usb boot codepathCreating PR since I came across changes needed and wanted to be able to discuss them with stakeholders prior of proposing final version. Will comment on those in next comments, and will come back to OP and modify it with current state as I progress up to proposing final version. Will set PR as draft after creating PR.