Improve TPM extend ops output in normal and DEBUG mode

[tlaurion]

• cbfs-init: remove temp files, measure direct cbfs output, extend PCR with proper introspection tracing
• flash.sh: do not die but go to recovery if flashrom fails, cosmetic fix for warning given to user
• kexec-insert-key: extend PCR with proper introspection tracing
• kexec-select-boot: extend PCR with proper introspection tracing
• kexec-measure-luks: extend PCR with proper introspection tracing
• tpmr: Add missing TRACE_FUNC, fix comments, extend give hash that was extended to tpm call in DEBUG, fix early startsession output that was still present
• ash_functions: extend PCR with proper introspection tracing
• insmod: DEBUG info more pertinent, extend PCR with proper introspection tracing

--

Change of DEBUG UX, After firmware upgrade/user config chanfe/tampering where tpm nvram cannot be unsealed with sealed measurements in last commit log message 024e199

Notes:

• content hashes used to extend TPM PCR[X] are reported in debug mode
• One can replay those measurements (hashes reported to extend PCR[x] to get final PCR bank final extended values reported, manually doing introspection in debug mode only).
• PCR[2] is extended by coreboot measured boot and is exposed by cbmem -L, not covered here. Heads uses those measurements to seal unseal, but does not interfere nor validate the content measured: only final extended PCR[2] is used by Heads for sealing/unsealing ops.

---

Normal changes without DEBUG (hotp: I can't use usb dongle under qemu...):

After firmware upgrade/user config change/tampering, where TPM nvram region cannot be unsealed with prior sealed measurements at #1758 (comment)

Oem-factory-reset/re-ownership (kernel modules UX changes) :

Setting TPM DUK:

Normal boot:

[tlaurion]

Helpful waiting for #1568

[JonathonHall-Purism]

This was supposed to extend the PCR with the module arguments if any were given, but now it's extending a second time with the module itself. So inserting the module with different arguments won't yield a different PCR digest.

I think you need to revert this change. If you don't want a temp file, maybe you could do something with -if /dev/stdin?

[JonathonHall-Purism]

I think you could use `tpmr extend -ix "$MODULE_PCR" -ic "$*"

-ic extends with a literal value

[tlaurion]

This was supposed to extend the PCR with the module arguments if any were given, but now it's extending a second time with the module itself. So inserting the module with different arguments won't yield a different PCR digest.

I think you need to revert this change. If you don't want a temp file, maybe you could do something with -if /dev/stdin?

@JonathonHall-Purism : I'm not sure I follow. Traces better then words.
Called mount-usb to simulate HTOP auto-loading usb modules prior of HOTP related code, with same PCR[5] extend ops. Of course here in DEBUG so traces, so that hashes of files are outputted under /tmp/debug.log (which is the improvement here toward introspection):

~ # grep PCR /tmp/debug.log 
DEBUG: TPM: Extending PCR[7] with /.gnupg/pubring.kbx
DEBUG: TPM: Extended PCR[7] with 9cbb20eb7c942f83e25cba463d949ef2ac3b01ff092ffa9ebf74b7ce004e4368
DEBUG: TPM: Extending PCR[7] with /.gnupg/trustdb.gpg
DEBUG: TPM: Extended PCR[7] with 0f1a689ad3d6e75d26ab1ba9a99cb3b412f59f80e9e982b3cabdf175242e1d0b
DEBUG: TPM: Extending PCR[4] with recovery
DEBUG: TPM: Extended PCR[4] with 8c585378513f5f7a2e1456ee54042605fdb890392becefadd2ab180fd02fb341
DEBUG: TPM: Extending PCR[5] with /lib/modules/ehci-hcd.ko
DEBUG: TPM: Extended PCR[5] with 40c5206f06702e45d8e6632632255258af433be0641c96f514ea75ac14523a30
DEBUG: TPM: Extending PCR[5] with /lib/modules/uhci-hcd.ko
DEBUG: TPM: Extended PCR[5] with 1877332107fb8737a5636da26d4db2c10ffe4d1db2bcbde30b47774cdf05e02f
DEBUG: TPM: Extending PCR[5] with /lib/modules/ohci-hcd.ko
DEBUG: TPM: Extended PCR[5] with 8a12ce4abfc87f11a023d4f1c26c225f5cffae248f9dad1fd30e78022996df02
DEBUG: TPM: Extending PCR[5] with /lib/modules/ohci-pci.ko
DEBUG: TPM: Extended PCR[5] with 2065ee6544d78a5d31e67983166a9b8cf60dbe61bf0ee99c39e92816cc3a98db
DEBUG: TPM: Extending PCR[5] with /lib/modules/ehci-pci.ko
DEBUG: TPM: Extended PCR[5] with 116145df2c495dfd58354025799fe5bb9b4d8e078960e8d0d7ceda746e4f2d06
DEBUG: TPM: Extending PCR[5] with /lib/modules/xhci-hcd.ko
DEBUG: TPM: Extended PCR[5] with 7f5a6bd0f7de6104e49374e1e5ce421e11795fcc4f53014ef9259d630d7876bc
DEBUG: TPM: Extending PCR[5] with /lib/modules/xhci-pci.ko
DEBUG: TPM: Extended PCR[5] with 5502fa8c101f7e509145b9826094f06dd0e225c2311a14edc9ae9c812518a250
DEBUG: TPM: Extending PCR[5] with /lib/modules/usb-storage.ko
DEBUG: TPM: Extended PCR[5] with e01a3fbe9510da3ebb83f70126652edc99b8acac89d05ed0a4b6b8f35aafdf4c

~ # sha256sum /.gnupg/pubring.kbx
9cbb20eb7c942f83e25cba463d949ef2ac3b01ff092ffa9ebf74b7ce004e4368  /.gnupg/pubring.kbx
~ # sha256sum /.gnupg/trustdb.gpg
0f1a689ad3d6e75d26ab1ba9a99cb3b412f59f80e9e982b3cabdf175242e1d0b  /.gnupg/trustdb.gpg
~ # sha256sum /lib/modules/ehci-hcd.ko
40c5206f06702e45d8e6632632255258af433be0641c96f514ea75ac14523a30  /lib/modules/ehci-hcd.ko
~ # sha256sum /lib/modules/uhci-hcd.ko
1877332107fb8737a5636da26d4db2c10ffe4d1db2bcbde30b47774cdf05e02f  /lib/modules/uhci-hcd.ko
~ # sha256sum /lib/modules/ohci-hcd.ko
8a12ce4abfc87f11a023d4f1c26c225f5cffae248f9dad1fd30e78022996df02  /lib/modules/ohci-hcd.ko
~ # sha256sum /lib/modules/ohci-pci.ko
2065ee6544d78a5d31e67983166a9b8cf60dbe61bf0ee99c39e92816cc3a98db  /lib/modules/ohci-pci.ko
~ # sha256sum /lib/modules/ehci-pci.ko
116145df2c495dfd58354025799fe5bb9b4d8e078960e8d0d7ceda746e4f2d06  /lib/modules/ehci-pci.ko
~ # sha256sum /lib/modules/xhci-hcd.ko
7f5a6bd0f7de6104e49374e1e5ce421e11795fcc4f53014ef9259d630d7876bc  /lib/modules/xhci-hcd.ko
~ # sha256sum /lib/modules/xhci-pci.ko
5502fa8c101f7e509145b9826094f06dd0e225c2311a14edc9ae9c812518a250  /lib/modules/xhci-pci.ko
~ # sha256sum /lib/modules/usb-storage.ko
e01a3fbe9510da3ebb83f70126652edc99b8acac89d05ed0a4b6b8f35aafdf4c  /lib/modules/usb-storage.ko

So basically, I only changed removal of tmp file altogether in pointed code.

[tlaurion]

See also #1758 (comment)

[JonathonHall-Purism]

So basically, I only changed removal of tmp file altogether in pointed code.

No, you changed what the PCR is extended with. It was not just extended with the module file content before, it was extended with the module parameters and the module file content.

In the PR, changing the module parameters will no longer affect the resulting PCR measurement, but it does affect the module behavior.

Granted, today we don't insert any modules with parameters. So I guess a change to actually include a module parameter would also necessarily change the initrd. But I think it's an unnecessary risk to introduce this possible vulnerability down the road, since it was there before, since it's easy to address.

In other words - we currently do the following: insmod /lib/modules/usbhid.ko

You could instead insert it with a module parameter, like quirks: insmod /lib/modules/usbhid.ko quirks=0x1234:0x5678:0x55AA

Prior to this PR, those resulted in different PCR values, which I think is important since this changes the behavior of the module in ways unknown to insmod. The PR no longer extends with the module parameters, so those two insmods now result in the same PCR but different kernel behavior.

So IMO add tpmr extend -ix "$MODULE_PCR" -ic "$*" before extending with the module file content.

[JonathonHall-Purism]

I think you could use `tpmr extend -ix "$MODULE_PCR" -ic "$*"

-ic extends with a literal value

[JonathonHall-Purism]

This used to also extend with the CBFS file name. I think we should continue to do that:

tpmr extend -ix "$CONFIG_PCR" -ic "$filename"

While it's hard to reason about whether an attack could be possible by moving the entire content of one file to another in some way that doesn't invalidate the PCRs, it's easy to just extend with the file name and be sure there's no such attack.

[tlaurion]

@JonathonHall-Purism same comment #1758 (comment)

[tlaurion]

@JonathonHall-Purism We do not to hash the filenames; we want to hash with file content.

Only occurrences in code of TPM PCR exttends -ic ops are to extend PCR[4] with hashes of literals:

user@heads-tests-deb12-nix:~/heads$ grep -Rn "\-ic" ./initrd/
./initrd/bin/kexec-select-boot:388:			tpmr extend -ix 4 -ic generic ||
./initrd/bin/usb-init:11:	tpmr extend -ix 4 -ic usb
./initrd/bin/tpmr:241:		-ic)
./initrd/bin/kexec-insert-key:69:tpmr extend -ix 4 -ic generic ||
grep: ./initrd/etc/mtab: No such file or directory
./initrd/etc/ash_functions:245:		tpmr extend -ix 4 -ic recovery

[JonathonHall-Purism]

We do not to hash the filenames; we want to hash with file content.

It used to hash both. Now it only hashes the content.

This appears to open up an attack by pivoting file contents or renaming files.

For example, say there were only two such files:

• heads/initrd/etc/config.user: echo CONFIG_RESTRICTED_BOOT=y....
• heads/initrd/.gnupg/pubring.gpg: some GPG key data

An attacker could manipulate CBFS to the following without affecting the measured PCR state.

• heads/initrd/etc/config.user.ignore_me_haha: echo CONFIG_RESTRICTED_BOOT=y...
• heads/initrd/.gnupg/pubring.gpg.whoops_no_more_gpg: some GPG key data

Heads would construct the same PCR measurements in this PR because it is only measuring the file content. But the restricted boot config and GPG keyring will be gone, since they've been renamed in such a way that they'll be ignored.

[JonathonHall-Purism]

(should spell this out 🙂 ) - So my point is you do need to hash both the filename and the content.

It used to do this by putting both the filename and content in a temp file, and hashing that.

A better strategy would be to extend once with the filename, then extend a second time with the file content. (That way the filename and content are clearly separated, you couldn't move the last few letters of the filename to the content, or something like that.)

So include tpmr extend -ix "$CONFIG_PCR" -ic "$filename" before hashing the file content.

[JonathonHall-Purism]

We're discussing this out of band too, but TBH I thought the "Extending PCR[n] with blah blah" messages made the most sense as debug output. You'd want to see that in debug logs, and I'm not sure why somebody who isn't debugging Heads would want to see them. They also contribute a lot of clutter to the console and will tend to bury more important things 🤔

The PCR hashes still appear on the console too ("New value of PCR[5]: blahblah"), I think that comes from tpm / tpm2 itself.

[tlaurion]

We're discussing this out of band too, but TBH I thought the "Extending PCR[n] with blah blah" messages made the most sense as debug output. You'd want to see that in debug logs, and I'm not sure why somebody who isn't debugging Heads would want to see them. They also contribute a lot of clutter to the console and will tend to bury more important things 🤔

The PCR hashes still appear on the console too ("New value of PCR[5]: blahblah"), I think that comes from tpm / tpm2 itself.

The goal of this PR is to contextualize output in normal mode, and output hashes of files measured under debug output, as shown under screenshots and detailed under comment #1758 (comment)

If we want to remove TPM[x] extend operations under master, it will have to be refactored to "hide" things, which should be subject to simplifying UX.

To me this is an sensical improvement over master, where UX noise reduction should be addressed separately. Now, the user is told why there is TPM related output on screen where none was given prior :)

[tlaurion]

The PCR hashes still appear on the console too ("New value of PCR[5]: blahblah"), I think that comes from tpm / tpm2 itself.

Yes. the functions over tpmr call tpm toolstack and uses output of functions directly, which is outputted to console. This PR wraps around that so that content is given prior of those calls. If we want to silencethat output, tpmr will need refactoring.

As of this PR, callers give context, and tpmr was modified to add extend op for tpm1 so we hae unified output for both TPM1/TPM2.

@JonathonHall-Purism Does that answer/recontextual the why of this PR? In DEBUG mode, someone can actually replay tpm extend ops and arrive to the same PCR bank final has value manually; what is measured in PCR[5] PCR[7] can be confirmed to be hashes of files as showed previously, and irrelevant and confusing temporary files have been removed to clarify and be clear of what is actually measured per extend ops.

[tlaurion]

@JonathonHall-Purism ping. Can be tested under #1773

[JonathonHall-Purism]

We're discussing this out of band too, but TBH I thought the "Extending PCR[n] with blah blah" messages made the most sense as debug output. You'd want to see that in debug logs, and I'm not sure why somebody who isn't debugging Heads would want to see them. They also contribute a lot of clutter to the console and will tend to bury more important things 🤔
The PCR hashes still appear on the console too ("New value of PCR[5]: blahblah"), I think that comes from tpm / tpm2 itself.

The goal of this PR is to contextualize output in normal mode, and output hashes of files measured under debug output, as shown under screenshots and detailed under comment #1758 (comment)

If we want to remove TPM[x] extend operations under master, it will have to be refactored to "hide" things, which should be subject to simplifying UX.

To me this is an sensical improvement over master, where UX noise reduction should be addressed separately. Now, the user is told why there is TPM related output on screen where none was given prior :)

Alright, I see what you're going for. PCR digests were spewing to stdout, now we at least say what they mean. Thanks for clarifying.

[tlaurion]

We do not to hash the filenames; we want to hash with file content.

It used to hash both. Now it only hashes the content.

This appears to open up an attack by pivoting file contents or renaming files.

For example, say there were only two such files:

• heads/initrd/etc/config.user: echo CONFIG_RESTRICTED_BOOT=y....
• heads/initrd/.gnupg/pubring.gpg: some GPG key data

An attacker could manipulate CBFS to the following without affecting the measured PCR state.

• heads/initrd/etc/config.user.ignore_me_haha: echo CONFIG_RESTRICTED_BOOT=y...
• heads/initrd/.gnupg/pubring.gpg.whoops_no_more_gpg: some GPG key data

Heads would construct the same PCR measurements in this PR because it is only measuring the file content. But the restricted boot config and GPG keyring will be gone, since they've been renamed in such a way that they'll be ignored.

Okok, now I get it, somehow.

We would need to parse the number of arguments then, and also display the quirks or additional parameters to a file, but only showing parameters and extending when used. This should be visually present to the end user, even more when unsealing fails.

@JonathonHall-Purism you have a code suggestion for that? I'm still confused on how I changed that behavior. Passing string instead of array or something?

[JonathonHall-Purism]

@tlaurion To restore the measurements of CBFS file names / module parameters: 72feade

I'm still confused on how I changed that behavior. Passing string instead of array or something?

Here's the old code from cbfs-init:

			echo "$filename" > $TMPFILE
			cat $filename >> $TMPFILE
			DEBUG "Extending TPM PCR $CONFIG_PCR with $filename"
			tpmr extend -ix "$CONFIG_PCR" -if $TMPFILE \
			|| die "$filename: tpm extend failed"

See how it wrote both the filename and the file content to the temp file? So it'd look something like this:

heads/initrd/etc/config.user
CONFIG_RESTRICTED_BOOT=y
CONFIG_BASIC_MODE=n
...

So if the file was renamed in CBFS, the measured data would change too.

I just brought it back by measuring the file name, then measuring the file content. This is simpler and also solves a similar problem from the old scheme. (What if somebody renamed the file to heads/initrd/etc/config.user\nCONFIG_RESTRICTED_BOOT=y (with an actual newline in the file name, not \n). If you also deleted the first line from the file....the measured data would have been unchanged, but the config is now gone. That's fixed now by measuring the filename and content separately.)

[tlaurion]

@JonathonHall-Purism this PR just lost its sexiness, as well as easy and usefull replay capabilities for next steps under #1568 :(

[    4.556024] DEBUG: Debug output enabled from board CONFIG_DEBUG_OUTPUT=y option (/etc/config)
[    4.575593] TRACE: Under init
[    4.610405] DEBUG: Applying panic_on_oom setting to sysctl
[    4.788295] TRACE: /bin/tpmr(339): tpm2_startsession
[    5.476555] TRACE: /bin/cbfs-init(5): main
[    5.678822] DEBUG: TPM: Extending PCR[7] with /.gnupg/pubring.kbx
[    5.706624] TRACE: /bin/tpmr(234): tpm2_extend
[    5.879455] DEBUG: TPM: Extended PCR[7] with 96ab5053e4630a040d55549ba73cff2178d401d763147776771f9774597b86a1
[    5.994666] DEBUG: TPM: Extending PCR[7] with /.gnupg/pubring.kbx
[    6.015103] TRACE: /bin/tpmr(234): tpm2_extend
[    6.206430] DEBUG: TPM: Extended PCR[7] with 8f7d984f622fabb570be39567713382b1fe01436ab9d8c8136e4c181f388659b
[    6.394573] DEBUG: TPM: Extending PCR[7] with /.gnupg/trustdb.gpg
[    6.423905] TRACE: /bin/tpmr(234): tpm2_extend
[    6.634872] DEBUG: TPM: Extended PCR[7] with 53b843fe9bb52894d3a7d00197c776d56f3059f6a285124c7916724cd5013b0b
[    6.743655] DEBUG: TPM: Extending PCR[7] with /.gnupg/trustdb.gpg
[    6.766740] TRACE: /bin/tpmr(234): tpm2_extend
[    6.925740] DEBUG: TPM: Extended PCR[7] with 99706538cc0075f494c73a8e493d564570d4b07df50212e5cdc1ec64b70531a4
[    7.024578] TRACE: /bin/key-init(6): main
[    8.211107] TRACE: Under /etc/ash_functions:combine_configs
[    8.267536] TRACE: Under /etc/ash_functions:pause_recovery
!!! Hit enter to proceed to recovery shell !!!
[    8.492658] TRACE: /bin/setconsolefont.sh(6): main
[    8.527251] DEBUG: Board does not ship setfont, not checking console font
[    8.719963] TRACE: /bin/gui-init(641): main
[    8.739053] TRACE: Under /etc/ash_functions:enable_usb
[    8.809612] TRACE: /sbin/insmod(9): main
[    9.028215] DEBUG: TPM: Extending PCR[5] with
[    9.056142] TRACE: /bin/tpmr(234): tpm2_extend
[    9.228729] DEBUG: TPM: Extended PCR[5] with e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
[    9.344195] DEBUG: TPM: Extending PCR[5] with /lib/modules/ehci-hcd.ko

So time to challenge why and how we do things a little bit more? A step back. Everything we do is hardcoded somehow, part of measured boot contract measuring payload as of today. Dive in:

• kernel options, for which quirks are applied to kernel modules compiled in, would be passed by coreboot config linux command line and part of measured boot ie

  heads/config/coreboot-qemu-fbwhiptail-tpm1-hotp.config

  Line 8 in b28c257

  CONFIG_LINUX_COMMAND_LINE="debug console=ttyS0,115200 console=tty"

• insmod calls, for drivers compiled as modules, are measured boot as part of payload (modules.cpio). No place to place quirks as of today:

  heads/initrd/etc/ash_functions

  Lines 331 to 356 in b28c257

  	enable_usb()
  	{
  	TRACE "Under /etc/ash_functions:enable_usb"
  	#insmod ehci_hcd prior of uhdc_hcd and ohci_hcd to suppress dmesg warning
  	insmod /lib/modules/ehci-hcd.ko || die "ehci_hcd: module load failed"
  	if [ "$CONFIG_LINUX_USB_COMPANION_CONTROLLER" = y ]; then
  	insmod /lib/modules/uhci-hcd.ko || die "uhci_hcd: module load failed"
  	insmod /lib/modules/ohci-hcd.ko || die "ohci_hcd: module load failed"
  	insmod /lib/modules/ohci-pci.ko || die "ohci_pci: module load failed"
  	fi
  	insmod /lib/modules/ehci-pci.ko || die "ehci_pci: module load failed"
  	insmod /lib/modules/xhci-hcd.ko || die "xhci_hcd: module load failed"
  	insmod /lib/modules/xhci-pci.ko || die "xhci_pci: module load failed"
  	sleep 2
  	# For resiliency, test CONFIG_USB_KEYBOARD_REQUIRED explicitly rather
  	# than having it imply CONFIG_USER_USB_KEYBOARD at build time.
  	# Otherwise, if a user got CONFIG_USER_USB_KEYBOARD=n in their
  	# config.user by mistake (say, by copying config.user from a laptop to a
  	# desktop/server), they could lock themselves out, only recoverable by
  	# hardware flash.
  	if [ "$CONFIG_USB_KEYBOARD_REQUIRED" = y ] || [ "$CONFIG_USER_USB_KEYBOARD" = y ]; then
  	insmod /lib/modules/usbhid.ko || die "usbhid: module load failed"
  	fi
  	}

• insmod dynamic calls are part of network scripts, no place for quirks, hardcoded in code under

  heads/initrd/bin/network-init-recovery

  Lines 64 to 69 in b28c257

  	network_modules="e1000 e1000e igb sfc mdio mlx4_core mlx4_en"
  	for module in $(echo $network_modules); do
  	if [ -f /lib/modules/$module.ko ]; then
  	insmod /lib/modules/$module.ko
  	fi
  	done

• cbfs extract and usage leaves no place for quirks, hardcoded under cbfs-init ans measured boot as part of payload once more

  heads/initrd/bin/cbfs-init

  Lines 13 to 32 in b28c257

  	cbfsfiles=`cbfs -t 50 -l 2>/dev/null | grep "^heads/initrd/"`
  	for cbfsname in `echo $cbfsfiles`; do
  	filename=${cbfsname:12}
  	if [ ! -z "$filename" ]; then
  	echo "Loading $filename from CBFS"
  	mkdir -p `dirname $filename` \
  	|| die "$filename: mkdir failed"
  	cbfs -t 50 -r $cbfsname > "$filename" \
  	|| die "$filename: cbfs file read failed"
  	if [ "$CONFIG_TPM" = "y" ]; then
  	TMPFILE=/tmp/cbfs.$$
  	echo "$filename" > $TMPFILE
  	cat $filename >> $TMPFILE
  	DEBUG "Extending TPM PCR $CONFIG_PCR with $filename"
  	tpmr extend -ix "$CONFIG_PCR" -if $TMPFILE \
  	|| die "$filename: tpm extend failed"
  	fi
  	fi
  	done

@JonathonHall-Purism why do we need something more then file content, once again?
If you insist on this, this means tprm would need refactoring so that -ic and "-ix" inform of measuring filenames and filename content.

We are back at outputting not only useless cryptic information to the user but also extending things that are also useless and preventing easy introspection (think forward sealing from flash.sh in the future, think fwupd deployed rom under /boot/firmware/*.zip and resealing pcr0-7)

TLDR: do we really want to extend filenames? As of today PCR4 is cryptic too without outputting -ic for normal/recovery string.

Thoughts?

[tlaurion]

@JonathonHall-Purism this is non-debug output in screenshot, debug output under 024e199

As can be seen, drivers do not have their filename+content hash extended; only file hash is extended as opposed to cbfs content; if no arguments passed to modules, then no double extend for non-existing modules args. Thoughts?

TPM1:

TPM2:

@JonathonHall-Purism fine with that?

[JonathonHall-Purism]

I don't see any need to have a second code path through here, just more code to test/break. You can extend a PCR with an empty string, so the above code path works even if the module parameters are empty.

Can we delete this path again?

[tlaurion]

@JonathonHall-Purism then end users/auditors will see a call twice with nothing. If you insist, I can remove, yes.

[JonathonHall-Purism]

As can be seen, drivers do not have their filename+content hash extended; only file hash is extended as opposed to cbfs content

I did think about this and I feel OK with it. If you (say) replace the content of usbhid.ko with nvme.ko, then insert the replaced module, you're just getting nvme.ko.

So I feel OK with that. That said, it's so easy to extend with the name too, it wouldn't really harm anything, if you want to 🙂

if no arguments passed to modules, then no double extend for non-existing modules args.

Is an additional extend with an empty string harmful in any way? It seems unnecessary to me to have two slightly-different code paths for this (more room for error). Extending with an empty string is fine, so we could have less code to maintain.

But I don't care that much 😛 So we could go with this if you want.

[tlaurion]

@JonathonHall-Purism I want the debug to say exactly what the code is doing today. Having an extend with nothing extendeddoesn't make sense to me.

If we want to add filename extend to module drivers loaded, that will need to be done under another PR.'m happy with the codebase saying what it says it does. I'm happy with the codebase being auditable and understandable for anyone putting things in DEBUG+TRACE mode, and other changes done on master after this PR is done with at least be understandable from a tracing perspective. Refactoring will have to be done later. This is an improvement to me, not a regression, no more black magic for people wanting to understand the codebase.

@JonathonHall-Purism I'm ok with code suggestion, otherwise i'm ok merging this as is and other changes being in other PR to be reviewed later on.

Please approve if you're ok with this.

[JonathonHall-Purism]

@tlaurion Alrighty, thanks, let's merge it, approving 👍