init: assign CONFIG_TPM depending on /dev/tpm0 presence #1200

SergiiDmytruk · 2022-08-19T21:30:31Z

A small piece of #836 (this file) to avoid adding -tmp/-no-tpm board variants in #1002 and other cases.

Is it OK that after gui-init prints Unable to locate /boot files on any mounted disk message $skip_to_menu equals true, $TOTP isn't set and main menu doesn't display NO TPM until you press r?

Also, with these changes all CONFIG_TPM in board files is ignored, so maybe it needs to be removed from them or dynamic behaviour should be used only if CONFIG_TPM=y in config.

tlaurion · 2022-08-19T22:47:29Z

Is it OK that after gui-init prints Unable to locate /boot files on any mounted disk message $skip_to_menu equals true, $TOTP isn't set and main menu doesn't display NO TPM until you press r?

That doesn't sound right, no.

I guess I could emultate that behavior by building a TPM enabled board config and disable coreboot support...

tlaurion · 2022-08-19T22:51:08Z

Actually, first use case to test your PR on top of the Qemu PR by disabling coreboot support.

SergiiDmytruk · 2022-08-20T11:16:52Z

What Qemu PR? I've tested it on Talos board running inside PPC64 Qemu (coreboot build for emulation separately), which has no TPM.

tlaurion · 2022-08-20T18:15:23Z

What Qemu PR? I've tested it on Talos board running inside PPC64 Qemu (coreboot build for emulation separately), which has no TPM.

#1188

Tested with current QEMU setup configured with the following changes upon current PR

user@heads-tests:~/heads$ cat diff_qemu 
diff --git a/config/coreboot-qemu-fbwhiptail-tpm1-hotp.config b/config/coreboot-qemu-fbwhiptail-tpm1-hotp.config
index 6e16a6d8..48090413 100644
--- a/config/coreboot-qemu-fbwhiptail-tpm1-hotp.config
+++ b/config/coreboot-qemu-fbwhiptail-tpm1-hotp.config
@@ -11,8 +11,6 @@ CONFIG_PCIEXP_ASPM=y
 CONFIG_PCIEXP_COMMON_CLOCK=y
 CONFIG_UART_PCI_ADDR=0
 CONFIG_DRIVERS_PS2_KEYBOARD=y
-CONFIG_USER_TPM1=y
-CONFIG_TPM_MEASURED_BOOT=y
 CONFIG_DEFAULT_CONSOLE_LOGLEVEL_6=y
 CONFIG_PAYLOAD_LINUX=y
 CONFIG_PAYLOAD_FILE="../../build/qemu-coreboot-fbwhiptail-tpm1-hotp/bzImage"
diff --git a/initrd/init b/initrd/init
index cdc75543..fca85517 100755
--- a/initrd/init
+++ b/initrd/init
@@ -43,6 +43,13 @@ hwclock -l -s
 . /etc/functions
 . /etc/config
 
+# set CONFIG_TPM dynamically before init
+if [ -e /dev/tpm0 ]; then
+	export CONFIG_TPM='y'
+else
+	export CONFIG_TPM='n'
+fi
+
 #Specify whiptail background colors cues under FBWhiptail only
 if [ -x /bin/fbwhiptail ]; then
 	export BG_COLOR_WARNING="${CONFIG_WARNING_BG_COLOR:-"--background-gradient 0 0 0 150 125 0"}"
@@ -97,6 +104,12 @@ if [ "$boot_option" = "r" ]; then
 	exit
 fi
 
+# Override CONFIG_TPM from /etc/config with runtime value determined above.
+#
+# Values in user config have higher priority during combining thus effectively
+# changing the value for the rest of the scripts which source /tmp/config.
+echo "CONFIG_TPM=$CONFIG_TPM" >> /etc/config.user
+
 combine_configs
 . /tmp/config

That is, over x86 qemu.

make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp
make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp PUBKEY_ASC=~/QubesIncoming/Insurgo/Insurgo_2023_pub.asc inject_gpg
make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp USB_TOKEN=NitrokeyPro PUBKEY_ASC=~/QubesIncoming/Insurgo/Insurgo_2023_pub.asc run

Then killing swtpm prior of Heads booting up (to have a no-tpm config), with configured disk:

Moving root.qcow2 to root.qcow2.bak, killing swtpm and
make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp USB_TOKEN=NitrokeyPro PUBKEY_ASC=~/QubesIncoming/Insurgo/Insurgo_2023_pub.asc run

Will redo without fbwhiptail in next reply

tlaurion · 2022-08-20T18:44:51Z

Recreated qemu setup without fbwhiptail with resulting patch on top of #1188


diff --git a/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config b/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
index f7130e0c..8aa84709 100644
--- a/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
+++ b/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
@@ -28,16 +28,16 @@ CONFIG_LVM2=y
 CONFIG_MBEDTLS=y
 CONFIG_DROPBEAR=y
 CONFIG_MSRTOOLS=y
-CONFIG_HOTPKEY=y
+#CONFIG_HOTPKEY=y
 
 #Uncomment only one of the following block
 #Required for graphical gui-init (FBWhiptail)
-CONFIG_CAIRO=y
-CONFIG_FBWHIPTAIL=y
+#CONFIG_CAIRO=y
+#CONFIG_FBWHIPTAIL=y
 #
 #text-based init (generic-init and gui-init)
-#CONFIG_NEWT=y
-#CONFIG_SLANG=y
+CONFIG_NEWT=y
+CONFIG_SLANG=y
 
 endif
 
diff --git a/initrd/init b/initrd/init
index cdc75543..fca85517 100755
--- a/initrd/init
+++ b/initrd/init
@@ -43,6 +43,13 @@ hwclock -l -s
 . /etc/functions
 . /etc/config
 
+# set CONFIG_TPM dynamically before init
+if [ -e /dev/tpm0 ]; then
+	export CONFIG_TPM='y'
+else
+	export CONFIG_TPM='n'
+fi
+
 #Specify whiptail background colors cues under FBWhiptail only
 if [ -x /bin/fbwhiptail ]; then
 	export BG_COLOR_WARNING="${CONFIG_WARNING_BG_COLOR:-"--background-gradient 0 0 0 150 125 0"}"
@@ -97,6 +104,12 @@ if [ "$boot_option" = "r" ]; then
 	exit
 fi
 
+# Override CONFIG_TPM from /etc/config with runtime value determined above.
+#
+# Values in user config have higher priority during combining thus effectively
+# changing the value for the rest of the scripts which source /tmp/config.
+echo "CONFIG_TPM=$CONFIG_TPM" >> /etc/config.user
+
 combine_configs
 . /tmp/config

Then

make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp
make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp PUBKEY_ASC=~/QubesIncoming/Insurgo/Insurgo_2023_pub.asc inject_gpg
make BOARD=qemu-coreboot-fbwhiptail-tpm1-hotp USB_TOKEN=NitrokeyPro PUBKEY_ASC=~/QubesIncoming/Insurgo/Insurgo_2023_pub.asc run

Swap terminal to host and killall swtpm after qemu is luanch to simulate tpm cut-off (without changing coreboot config)

I see what you mean:

Question being why gui-init is not showing TOTP: NO TPM on first main screen invocation.

@SergiiDmytruk : Right?

Otherwise, I think it is normal that you see no drive error if your QEMU instance doesn't have any disk attached, and if there was a disk attached without partitions, you should get something similar as above.

tlaurion · 2022-08-20T18:53:26Z

@SergiiDmytruk :

combine_configs is not merging CONFIG_TPM but duplicates it.

tlaurion · 2022-08-20T19:46:33Z

diff --git a/initrd/init b/initrd/init
index fca85517..4266a017 100755
--- a/initrd/init
+++ b/initrd/init
@@ -45,9 +45,9 @@ hwclock -l -s
 
 # set CONFIG_TPM dynamically before init
 if [ -e /dev/tpm0 ]; then
-       export CONFIG_TPM='y'
+       CONFIG_TPM='"y"'
 else
-       export CONFIG_TPM='n'
+       CONFIG_TPM='"n"'
 fi
 
 #Specify whiptail background colors cues under FBWhiptail only
@@ -108,7 +108,7 @@ fi
 #
 # Values in user config have higher priority during combining thus effectively
 # changing the value for the rest of the scripts which source /tmp/config.
-echo "CONFIG_TPM=$CONFIG_TPM" >> /etc/config.user
+echo "export CONFIG_TPM="$CONFIG_TPM"" >> /etc/config.user
 
 combine_configs
 . /tmp/config

Produces CONFIG_TPM="n" correctly, but combine_configs is simply concatenating /etc/config* into /tmp/config and sourcing it.

Basically, the last redefinition of the variable is the one valid, whatever if they are strings being written differently, this is not the source of the problem.

When an OS is installed, the behavior is normal:

But without OS/Disk:

tlaurion · 2022-08-20T19:59:39Z

@SergiiDmytruk : the following part was missing to produce TOTP:NO TPM and HOTP: N/A

diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index afcf28eb..ede11561 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -46,6 +46,7 @@ mount_boot()
         ;;
       m )
         skip_to_menu="true"
+        update_totp && update_hotp
         break
         ;;
       * )

The following is cleaner, while combine_configs is still really dumb, while working:

diff --git a/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config b/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
index f7130e0c..8aa84709 100644
--- a/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
+++ b/boards/qemu-coreboot-fbwhiptail-tpm1-hotp/qemu-coreboot-fbwhiptail-tpm1-hotp.config
@@ -28,16 +28,16 @@ CONFIG_LVM2=y
 CONFIG_MBEDTLS=y
 CONFIG_DROPBEAR=y
 CONFIG_MSRTOOLS=y
-CONFIG_HOTPKEY=y
+#CONFIG_HOTPKEY=y
 
 #Uncomment only one of the following block
 #Required for graphical gui-init (FBWhiptail)
-CONFIG_CAIRO=y
-CONFIG_FBWHIPTAIL=y
+#CONFIG_CAIRO=y
+#CONFIG_FBWHIPTAIL=y
 #
 #text-based init (generic-init and gui-init)
-#CONFIG_NEWT=y
-#CONFIG_SLANG=y
+CONFIG_NEWT=y
+CONFIG_SLANG=y
 
 endif
 
diff --git a/initrd/bin/gui-init b/initrd/bin/gui-init
index afcf28eb..ede11561 100755
--- a/initrd/bin/gui-init
+++ b/initrd/bin/gui-init
@@ -46,6 +46,7 @@ mount_boot()
         ;;
       m )
         skip_to_menu="true"
+        update_totp && update_hotp
         break
         ;;
       * )
diff --git a/initrd/init b/initrd/init
index fca85517..4266a017 100755
--- a/initrd/init
+++ b/initrd/init
@@ -45,9 +45,9 @@ hwclock -l -s
 
 # set CONFIG_TPM dynamically before init
 if [ -e /dev/tpm0 ]; then
-	export CONFIG_TPM='y'
+	CONFIG_TPM='"y"'
 else
-	export CONFIG_TPM='n'
+	CONFIG_TPM='"n"'
 fi
 
 #Specify whiptail background colors cues under FBWhiptail only
@@ -108,7 +108,7 @@ fi
 #
 # Values in user config have higher priority during combining thus effectively
 # changing the value for the rest of the scripts which source /tmp/config.
-echo "CONFIG_TPM=$CONFIG_TPM" >> /etc/config.user
+echo "export CONFIG_TPM="$CONFIG_TPM"" >> /etc/config.user
 
 combine_configs
 . /tmp/config

And produces expected behavior (if swtpm is killed before linux creates tpm0 device)

Without disk but with a TPM should bring us at:

Resetting TPM generates Qr code after ownership (if TPM is not owned; otherwise gnerating HOTP/TOTP works) and then TOTP shows TPMTOTP code as expected, matching phone:

Signed-off-by: Sergii Dmytruk <[email protected]>

Skip only GPG key check, but always init TOTP and HOTP. Signed-off-by: Sergii Dmytruk <[email protected]>

SergiiDmytruk · 2022-08-20T21:12:37Z

+ CONFIG_TPM='"y"'
+ CONFIG_TPM='"n"'

Those extra quotes shouldn't be there or checks like [ "$CONFIG_TPM" = "y" ] will fail, moved quotes into echo command.

+ update_totp && update_hotp

That was the problem, I just didn't know why GPG, TOTP and HOTP are initialized together. If we're actually only want to skip GPG initialization, the variable should say that and initialization of TOTP and HOTP should be unconditional, which is how I commited it.

tlaurion · 2022-08-20T21:28:15Z

CONFIG_TPM='"y"'

CONFIG_TPM='"n"'

Those extra quotes shouldn't be there or checks like [ "$CONFIG_TPM" = "y" ] will fail, moved quotes into echo command.

Right. I focused on trying to produce exports that would match under /tmp/config after combine_config

But since combine_configs is just concatenating /etc/config and /etc/config.user under /tmp/config and sourcing the later, we don't really care about producing a perfect match.

Edit: https://github.com/osresearch/heads/blob/75748e86b7373b5becb5f51e35b07c20d3b43ca5/initrd/init#L111

does it right

tlaurion · 2022-08-20T21:29:34Z

update_totp && update_hotp

That was the problem, I just didn't know why GPG, TOTP and HOTP are initialized together. If we're actually only want to skip GPG initialization, the variable should say that and initialization of TOTP and HOTP should be unconditional, which is how I commited it.

Makes sense but will have to retest.
Edit: LGTM!

tlaurion · 2022-08-21T18:24:06Z

@JonathonHall-Purism @jans23: Will leave this open for you to review until let's say wednesday?

I would appreciate a quick review/testing on your side/use cases and a note here saying it works for you as well (should), but as said before LGTM and ready for merge to me.

JonathonHall-Purism · 2022-08-23T21:03:48Z

Looks good to me - I did not quite have time to test today but will tomorrow, it's very similar to the patches we're currently carrying.

JonathonHall-Purism · 2022-08-24T13:16:16Z

Tested 13v2 (TPM) and mini v2 (no TPM). LGTM 👍

tlaurion mentioned this pull request Aug 20, 2022

QEMU testing with OS, TPM, USB token #1188

Merged

SergiiDmytruk added 2 commits August 20, 2022 23:56

init: assign CONFIG_TPM depending on /dev/tpm0 presence

b989889

Signed-off-by: Sergii Dmytruk <[email protected]>

gui-init: fix TOTP/HOTP initialization on missing OS

75748e8

Skip only GPG key check, but always init TOTP and HOTP. Signed-off-by: Sergii Dmytruk <[email protected]>

SergiiDmytruk force-pushed the optional-otp branch from e56e1a1 to 75748e8 Compare August 20, 2022 21:08

tlaurion merged commit f7facf0 into linuxboot:master Aug 24, 2022

SergiiDmytruk deleted the optional-otp branch August 24, 2022 15:04

JonathonHall-Purism mentioned this pull request Nov 23, 2022

gui-init: Always go to main menu when selecting "continue to main menu" #1246

Merged

SergiiDmytruk mentioned this pull request Jul 25, 2023

Talos II - 0.6 Release - Cannot boot if no TPM - TPM module cannot be expected (cannot be bought) Dasharo/dasharo-issues#191

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

init: assign CONFIG_TPM depending on /dev/tpm0 presence #1200

init: assign CONFIG_TPM depending on /dev/tpm0 presence #1200

SergiiDmytruk commented Aug 19, 2022

tlaurion commented Aug 19, 2022

tlaurion commented Aug 19, 2022

SergiiDmytruk commented Aug 20, 2022

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022

tlaurion commented Aug 20, 2022 •

edited

Loading

SergiiDmytruk commented Aug 20, 2022

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 21, 2022 •

edited

Loading

JonathonHall-Purism commented Aug 23, 2022

JonathonHall-Purism commented Aug 24, 2022

init: assign CONFIG_TPM depending on /dev/tpm0 presence #1200

init: assign CONFIG_TPM depending on /dev/tpm0 presence #1200

Conversation

SergiiDmytruk commented Aug 19, 2022

tlaurion commented Aug 19, 2022

tlaurion commented Aug 19, 2022

SergiiDmytruk commented Aug 20, 2022

tlaurion commented Aug 20, 2022 • edited Loading

tlaurion commented Aug 20, 2022 • edited Loading

tlaurion commented Aug 20, 2022 • edited Loading

tlaurion commented Aug 20, 2022

tlaurion commented Aug 20, 2022 • edited Loading

SergiiDmytruk commented Aug 20, 2022

tlaurion commented Aug 20, 2022 • edited Loading

tlaurion commented Aug 20, 2022 • edited Loading

tlaurion commented Aug 21, 2022 • edited Loading

JonathonHall-Purism commented Aug 23, 2022

JonathonHall-Purism commented Aug 24, 2022

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 20, 2022 •

edited

Loading

tlaurion commented Aug 21, 2022 •

edited

Loading