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

WiP: Generate diceware passphrases in oem-factory-reset, output qr code of configured secrets prior of reboot #1850

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Nov 19, 2024

WiP: will change

size increase of firmware space used: ~5120 bytes

New Functionality:

  • initrd/etc/functions:
    • Added a new helper function generate_passphrase to generate a Diceware passphrase, including subfunctions for parsing parameters, generating dice rolls, and retrieving words from a dictionary.
    • QR code generation at the end of OEM/user mode and setting defaults in oem-factory-reset for user/oem (TODO: improve)

Minor Changes:

  • initrd/init:
    • Updated the oem-factory-reset command to include a --mode oem parameter for better clarity in OEM Factory Reset mode which enters early if 'o' key pressed around Heads asciiart, just after bootsplash vanishing

As of now:

  • oem-factory-reset called from GUI doesn't change behavior, its still provisioning 12345678 and 123456 weak PINs
    • This could be changed under gui-nit to call oem-factory-reset --ode user, which would generate random PINs replacing the default weak ones, and Qr code contains them, including Secrets app PIN output per PR change

TODO:

  • @JonathonHall-Purism do we want to switch default call to oem-factory-reset to oem-factory-reset --mode user from gui-jnit ?
    • Can be tested calling oem-factory-reset --mode user from recovery shell

Next steps:

@tlaurion tlaurion force-pushed the generate_passphrase-reownership_qr_code branch from 486b52f to b681574 Compare November 21, 2024 17:06
@tlaurion tlaurion marked this pull request as ready for review November 21, 2024 17:08
@tlaurion
Copy link
Collaborator Author

tlaurion commented Nov 28, 2024

Will try hotp-verification reset from Nitrokey/nitrokey-hotp-verification#46 even though segfaults on nk2/lk

@tlaurion
Copy link
Collaborator Author

Will try hotp-verification reset from Nitrokey/nitrokey-hotp-verification#46 even though segfaults on nk2/lk

Works, asks physical presence though, but this is Nitrokey/nitrokey-hotp-verification#41 and won't be part of feature freeze.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Nov 29, 2024

There seems to still be some confusion on hotp_verification reset SECRET_APP_PIN being needed per oem-factory-reset/re-ownership to only ask physical presence once, which is Nitrokey/nitrokey-hotp-verification#42

EDIT: Asked and got acknowledgment of understanding at Nitrokey/nitrokey-hotp-verification#46 (comment) since Nitrokey/nitrokey-hotp-verification#42 (comment) wasn't acknowledged yet.

Waiting for fix prior of testing and fixing oem-factory-reset further more.

Comment on lines 141 to 154
reset_nk3_secret_app() {
TRACE_FUNC
# Reset Nitrokey 3 secret app
if lsusb | grep -q "20a0:42b2"; then
echo
echo "Resetting Nitrokey 3 secret app"
# Reset Nitrokey 3 secret app
/bin/hotp_verification reset
fi
}
Copy link
Collaborator Author

@tlaurion tlaurion Nov 30, 2024

Choose a reason for hiding this comment

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

a SECRET_APP_PIN is definitely needed here, since reset doesn't set default SECRET_APP_PIN to be changed (no previous PIN default set as per GPG User/Admin PINs previously used for <nk3 dongles which rest of oem-factory-reset policy script acts upon) : we consequently need to create a SECRET_APP_PIN, ideally with same value as GPG Admin PIN (generated, single shared PIN chosen, or setting a OEM 12345678=GPG Admin PIN as per current (bad) default for end users.
We do not want to confuse users, nor complexify UX even more.

Blocked by Nitrokey/nitrokey-hotp-verification#46 final implementation resulting in merged hotp_verification changes.

Also note that physical presence needed here (touch dongle) until possibly removed later in next nk3 firmware version (issue Nitrokey/nitrokey-hotp-verification#41, unplanned for current/late #1821)

# v1.6
hotp-verification_version := e9050e0c914e7a8ffef5d1c82a014e0e2bf79346
# v1.6 + patch from https://github.com/Nitrokey/nitrokey-hotp-verification/pull/46/commits/de355ed93ba50280bf377772082b76b7a2285185
hotp-verification_version := de355ed93ba50280bf377772082b76b7a2285185
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is Nitrokey/nitrokey-hotp-verification#46 (comment) original PR state commit, not implementing argument for single SECRET_APP_PIN to be set since no default PIN set as gpg counterpart.

This is commit Nitrokey/nitrokey-hotp-verification@de355ed for the curious eyes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now using 1.6 commit + patches per a9d3d96

only testing PR 46 because conflits with PR 43.

…ords then short list v1 for easier to remember passphrases

This lists comes from https://www.eff.org/files/2016/09/08/eff_short_wordlist_2_0.txt
Refered in article: https://www.eff.org/dice

Signed-off-by: Thierry Laurion <[email protected]>
Nothing uses it for the moment, needs to be called from recovery shell: bash, source /etc/functions. generate_passphrase

- parses dictionary to check how many dice rolls needed on first entry, defaults to EFF short list v2 (bigger words easier to remember, 4 dices roll instead of 5)
  - defaults to using initrd/etc/diceware_dictionnaries/eff_short_wordlist_2_0.txt, parametrable
  - make sure format of dictionary is 'digit word' and fail early otherwise: we expect EFF diceware format dictionaries
- enforces max length of 256 chars, parametrable, reduces number of words to fit if not override
- enforces default 3 words passphrase, parametrable
- enforces captialization of first letter, lowercase parametrable
- read multiple bytes from /dev/urandom to fit number of dice rolls

Unrelated: uniformize format of file

Signed-off-by: Thierry Laurion <[email protected]>
…ount /etc/fstab existing /boot partition (otherwise early 'o' to enter oem mode of oem-factory-reset

Signed-off-by: Thierry Laurion <[email protected]>
…user press y (end of reownership wizard secret output)

Signed-off-by: Thierry Laurion <[email protected]>

works:
- oem and user mode passphrase generation
- qrcode

missing:
- unattended
  - luks reencryption + passphrase change for OEM mode (only input to be provided) with SINGLE passphrase when in unattended mode
    - same for user reownership when previously OEM reset unattended

Signed-off-by: Thierry Laurion <[email protected]>
…p, make sure defaults are set for all modes, including default which uses current defaults being DEF pins (12345678 and 123456 as master)

Signed-off-by: Thierry Laurion <[email protected]>
…for this PR (43 conflicts when applied atop 46. 46 is needed here)

Signed-off-by: Thierry Laurion <[email protected]>
…e current defaults being DEF pins (12345678 and 123456 as master)

Signed-off-by: Thierry Laurion <[email protected]>
…N as text and in Qr code

Signed-off-by: Thierry Laurion <[email protected]>
…n that physical presence is needed

Signed-off-by: Thierry Laurion <[email protected]>
…ctory Reset Mode', 'Re-Ownership Mode' or 'OEM Factory Reset / Re-Ownership'

TODO: further specialize warning prompt to tell what is going to happen (randomized PIN, signle custom randomized PIN etc)

Signed-off-by: Thierry Laurion <[email protected]>
…cal presence, put nk3 secure APP PIN after TPM but before GPG PINS in output for consistency

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion tlaurion force-pushed the generate_passphrase-reownership_qr_code branch from 07f3710 to 444ff3e Compare December 5, 2024 19:44
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 5, 2024

So in 444ff3e state

  • master's behavior is conserved: meaning a user going through menu option to launch re-ownership will present same options, leading to choosing defaults generating PINs for
    • TPM,Admin PIN and Secure App PIN=12345678, User PIN=12345678
  • Selecting 'o' early at boot jumps in OEM Factory Reset Mode and
    • generates a single diceware single PIN of 2 words userd for TPM Owner, Secure Element PIN, GPG Admin/User PIN

It is to define if we want gui-init (main menu) to change the defaults. If so, code is ready to switch to user mode from GUI options, and randomize unique PINs, which today can only be launched from Recovery shell through oem-factory-reset --mode user

Output (no quiet mode)

gui->oem factory reset/re-ownership (Secure App PIN added, Qr code added, otherwise as in master)

2024-12-05-144130
2024-12-05-144145

oem mode ( 'o' key pressed around Heads asciiart):

2024-12-05-140158
2024-12-05-140231

user mode (all ~Admin PINs=same, different GPG User PIN (daily usage) [only called from Recovery shell with oem-factory-reset --mode user as per this commit]:

2024-12-05-151944
2024-12-05-151931

TODO: check logic in this file because assumptions on PINs retry count are wrong and will depend on Nitrokey/nitrokey-hotp-verification#43 not tested here

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 5, 2024

With 295935f, HOTP sealing prompts for Secure App PIN / GPG Admin PIN depending if nk3 or not.

2024-12-05-165413

@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 6, 2024

1f51267 tested both Nitrokey/nitrokey-hotp-verification#43 Nitrokey/nitrokey-hotp-verification#46

Last commit removes all patches to hotp_verification. Expectig a hotp_verification commit that integrates both this PR under modules/hotp_verification matching upstream repo merge for next commit of this PR and make it ready to review

@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 6, 2024

Put to draft under next commit changes modules/hotp_verification module upstream commit when Nitrokey/nitrokey-hotp-verification#43 Nitrokey/nitrokey-hotp-verification#46 merged.

Related output of seal-hotp, showing hotp_verification info and hotp_initialize output related changes
2024-12-06-120927
2024-12-06-121013

@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 6, 2024

@JonathonHall-Purism I would love if you could review code early (hotp_verification commit needs changes, one cannot test both Nitrokey/nitrokey-hotp-verification#43 Nitrokey/nitrokey-hotp-verification#46 patches since they conflit)

…instead of Nitrokey/nitrokey-hotp-verification#46 for hotp-verification info parsing and validation of oem-factory-reset and seal-hotp

Signed-off-by: Thierry Laurion <[email protected]>
- oem-factory-reset: fix strings for nk3 is from Nitrokey/nitrokey-hotp-verification#43 is Secrets app, not Secret App singular, not App capitalized
- initrd/bin/seal-hotpkey: adapt to check nk3 Secrets App PIN counter if nk3, keep Card counters for <nk3 from Nitrokey/nitrokey-hotp-verification#43
  - Unattended hotp_initialize output removed since we need physical presence to seal HOTP until Nitrokey/nitrokey-hotp-verification#41 is fixed
  - Finally make seal_hotp use logic to detect if public key <1m old, use HOTP related PIN by default if counter is not <3, warn that re-ownership needs to be ran to change it since no security offered at all otherwise with HOTP
- unify format with linting tool

Tested in local tree against https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/43.patch, removing https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch
 - will revert the change above in PR once testing is over

Signed-off-by: Thierry Laurion <[email protected]>
…fef5d1c82a014e0e2bf79346 directory: waiting for Nitrokey/nitrokey-hotp-verification#43 and Nitrokey/nitrokey-hotp-verification#46 to be merged to change modules/hotp-verification commit

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion tlaurion force-pushed the generate_passphrase-reownership_qr_code branch from 1f51267 to 6591f26 Compare December 6, 2024 17:05
The dice-rolls method was relatively complex and somewhat biased
(~2.4% biased toward 1-4 on each roll due to modulo bias).

Just pick a line from the dictionary at random.  Using all 32 bits of
entropy to pick a line once distributes the modulo bias so it is only
0.000003% biased toward the first 1263 words.

Signed-off-by: Jonathon Hall <[email protected]>
We're adding leading blank lines, which makes the prompt look odd and
now have to be removed later.  Just stop adding the leading blank
lines.

Signed-off-by: Jonathon Hall <[email protected]>
@JonathonHall-Purism
Copy link
Collaborator

@tlaurion I started reviewing/testing this today and made a few commits to clean up/fix a few things: https://github.com/JonathonHall-Purism/heads/commits/generate_passphrase-reownership_qr_code/

Have a look at the commit messages there 👌 I still need to go through your comments and review more, will have more next week.

…erification#46 so that this PR can be tested and reviewed from OEM Factory Reset/User Re-Ownership perspective

(PR 43 not in which fixes hotp_verification info, needed to reuse default PINs under seal-hotp if pubkey age <1 month and if Secret app PIN/GPG Admin PIN count >=3 )

Repro:
mkdir patches/hotp-verification-e9050e0c914e7a8ffef5d1c82a014e0e2bf79346
wget https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch -O patches/hotp-verification-e9050e0c914e7a8ffef5d1c82a014e0e2bf79346/46.patch
sudo rm -rf  build/x86/hotp-verification-e9050e0c914e7a8ffef5d1c82a014e0e2bf79346/
./docker_repro.sh make BOARD=qemu-coreboot-whiptail-tpm2-hotp USB_TOKEN=Nitrokey3NFC PUBKEY_ASC=pubkey.asc inject_gpg run

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion tlaurion force-pushed the generate_passphrase-reownership_qr_code branch from 957a1b9 to 4b4ac60 Compare December 7, 2024 16:38
@tlaurion
Copy link
Collaborator Author

tlaurion commented Dec 9, 2024

@tlaurion I started reviewing/testing this today and made a few commits to clean up/fix a few things: https://github.com/JonathonHall-Purism/heads/commits/generate_passphrase-reownership_qr_code/

Have a look at the commit messages there 👌 I still need to go through your comments and review more, will have more next week.

@JonathonHall-Purism thank you so much for that!

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.

2 participants