initrd/gpg: final review fixes for PR #2158

- Add local rc declarations in gpg_card_factory_reset,
  gpg_card_change_pin, and gpg_keytocard_subkeys to prevent
  global scope leaks
- Add _luks_cleanup helper to close LUKS mappings on all error
  paths (removed broken EXIT trap that does not fire on function
  return)
- Re-run NK3 Secrets app reset with custom PIN when factory
  reset is retried from default to custom PIN
- Add chmod 600 on /tmp/secret/gpg_pin to match existing
  cache_gpg_signing_pin convention
- Fix recovery-shell.md auth scope: USB/external media boot
  is also gated by gpg_auth via media-scan.sh

Signed-off-by: Thierry Laurion <insurgo@riseup.net>

Author: tlaurion

doc/recovery-shell.md

@@ -27,7 +27,8 @@ with the in-memory backup path, or by running the reprovision flow from the GPG
 Management Menu), the recovery shell requires GPG smartcard authentication before
 the bash prompt opens.
 
-**Scope:** This ONLY guards recovery shell entry.  USB boot, TPM operations,
+**Scope:** This guards recovery shell and external media/USB boot entry
+(`media-scan.sh` also calls `gpg_auth` before scanning USB).  TPM operations,
 flash/update, GPG management, and all other GUI menu functions are NOT gated
 by this check — they remain accessible from the main menu.
 

initrd/etc/gpg_functions.sh

@@ -172,6 +172,7 @@ gpg_reset_nk3_secret_app() {
 			fi
 		done
 	fi
+	return 0
 }
 
 # Factory-reset the OpenPGP smartcard and set key attributes for the given
@@ -185,6 +186,7 @@ gpg_card_factory_reset() {
 	local algo="$1"
 	local rsa_key_length="$2"
 	local card_admin_pin="${3:-12345678}"
+	local rc
 
 	STATUS "Factory resetting $DONGLE_BRAND OpenPGP smartcard"
 	{
@@ -359,6 +361,7 @@ gpg_card_change_pin() {
 	local pin_type="$1"
 	local old_pin="$2"
 	local new_pin="$3"
+	local rc
 	{
 		echo admin       # admin menu
 		echo passwd      # change PIN
@@ -389,6 +392,7 @@ gpg_keytocard_subkeys() {
 	local key_id="$1"
 	local subkey_pin="$2"
 	local card_pin="${3:-12345678}"
+	local rc
 
 	# Ensure USB and smartcard are accessible
 	enable_usb
@@ -435,6 +439,15 @@ gpg_keytocard_subkeys() {
 	TRACE_FUNC
 }
 
+# Unmount /media and close any LUKS usb_mount mappings.
+# Called by reprovision_smartcard_from_backup() on error paths.
+_luks_cleanup() {
+	umount /media 2>/dev/null || true
+	for d in /dev/mapper/usb_mount_*; do
+		[ -e "$d" ] && cryptsetup close "$(basename "$d")" 2>/dev/null || true
+	done
+}
+
 # Reprovision an OpenPGP smartcard from a GPG key backup on a LUKS-encrypted
 # USB drive.  Detects key type (RSA vs ECC) from the imported key, mounts the
 # LUKS private partition to import privkey.sec, mounts the public partition to
@@ -448,9 +461,6 @@ reprovision_smartcard_from_backup() {
 	local mapper_dev parent_disk pub_partition
 	local algo_code bit_len
 
-	# Ensure LUKS mappings are cleaned up on any exit path (error or normal)
-	trap 'umount /media 2>/dev/null || true; for d in /dev/mapper/usb_mount_*; do [ -e "$d" ] && cryptsetup close "$(basename "$d")" 2>/dev/null || true; done' EXIT
-
 	# Detect dongle branding early -- needed for whiptail messages and
 	# Nitrokey Storage AES key reset in factory_reset_and_configure.
 	enable_usb
@@ -505,7 +515,7 @@ reprovision_smartcard_from_backup() {
 
 	# Verify the private key backup file exists
 	if [ ! -f /media/privkey.sec ]; then
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		WARN "privkey.sec not found on backup drive -- not a valid GPG key backup"
 		whiptail_error --title 'ERROR: No Backup Found' \
 			--msgbox "No privkey.sec found on this drive.\n\nThis does not appear to be a valid\nGPG key backup drive." 0 80
@@ -517,7 +527,7 @@ reprovision_smartcard_from_backup() {
 	STATUS "Importing GPG keys from backup"
 	if ! gpg --pinentry-mode=loopback --passphrase-fd 3 3< <(echo -n "$admin_pin") \
 		--import-options restore --import /media/privkey.sec >/dev/null 2>/tmp/gpg_import_err; then
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		ERROR="$(cat /tmp/gpg_import_err)"
 		WARN "GPG key import from backup failed: $(head -3 /tmp/gpg_import_err 2>/dev/null)"
 		whiptail_error --title 'ERROR: Key Import Failed' \
@@ -546,7 +556,7 @@ reprovision_smartcard_from_backup() {
 		DEBUG "Detected ECC P-256 key from backup"
 		;;
 	*)
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		WARN "Unrecognized GPG algorithm code $algo_code from backup key"
 		whiptail_error --title 'ERROR: Unknown Key Type' \
 			--msgbox "Could not detect the key type from the backup\n(algorithm $algo_code).\n\nThe backup file may be corrupted." 0 80
@@ -571,7 +581,7 @@ reprovision_smartcard_from_backup() {
 	else
 		key_id="$(gpg --list-secret-keys --with-colons 2>/dev/null | grep '^sec:' | cut -d: -f5)"
 		[ -z "$key_id" ] && {
-			umount /media 2>/dev/null || true
+			_luks_cleanup
 			DIE "Could not determine key ID from imported backup"
 		}
 	fi
@@ -596,7 +606,7 @@ reprovision_smartcard_from_backup() {
 			if ! whiptail_warning --title 'Dongle Compatibility Warning' \
 				--yesno "The backed-up key is ECC P-256, but your $DONGLE_BRAND\nmay have limited ECC support.\n\nProceeding may fail.\n\nDo you want to continue?" 0 80; then
 				DEBUG "User aborted: ECC key incompatible with dongle"
-				umount /media 2>/dev/null || true
+				_luks_cleanup
 				return 1
 			fi
 			DEBUG "User accepted ECC compatibility risk"
@@ -611,15 +621,15 @@ reprovision_smartcard_from_backup() {
 	if ! whiptail_warning --title 'Reprovision Smartcard' \
 		--yesno "This will:\n\n  * ERASE all keys on your $DONGLE_BRAND\n  * Import GPG key: $identity_summary\n    (${key_algo}$([ "$key_algo" = "RSA" ] && echo " ${rsa_key_length}-bit"))\n  * Copy subkeys to the smartcard\n\nDo you want to continue?" 0 80; then
 		DEBUG "User declined reprovision via confirmation dialog"
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		return 1
 	fi
 	DEBUG "User confirmed reprovision; proceeding with factory reset"
 
 	# Re-verify the smartcard is still present before wiping it
 	STATUS "Verifying $DONGLE_BRAND smartcard is still present"
 	if ! gpg --card-status >/dev/null 2>&1; then
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		WARN "$DONGLE_BRAND smartcard disappeared after user confirmation"
 		whiptail_error --title 'ERROR: Smartcard Not Found' \
 			--msgbox "The $DONGLE_BRAND smartcard is no longer detected.\n\nCheck the connection and try again." 0 80
@@ -653,10 +663,14 @@ reprovision_smartcard_from_backup() {
 			while [ -z "$card_admin_pin" ]; do
 				INPUT "Enter the current $DONGLE_BRAND admin PIN:" -r -s card_admin_pin
 			done
+			# Re-run NK3 Secrets app reset with the custom PIN
+			release_scdaemon
+			gpg_reset_nk3_secret_app "$card_admin_pin" || \
+				DEBUG "NK3 Secrets app reset with custom PIN also failed (non-fatal)"
 		fi
 	done
 	if [ "$factory_reset_ok" != "y" ]; then
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		ERROR="$(tail -n 3 /tmp/gpg_card_edit_output 2>/dev/null | fold -s)"
 		WARN "Smartcard factory reset failed after retry with correct admin PIN"
 		whiptail_error --title 'ERROR: Factory Reset Failed' \
@@ -670,7 +684,7 @@ reprovision_smartcard_from_backup() {
 	# Phase 6: move subkeys from the local keyring to the smartcard.
 	DEBUG "Starting keytocard with key_id=$key_id, admin_pin=${#admin_pin} chars, card_admin_pin=${#card_admin_pin} chars"
 	if ! gpg_keytocard_subkeys "$key_id" "$admin_pin" "$card_admin_pin"; then
-		umount /media 2>/dev/null || true
+		_luks_cleanup
 		ERROR="$(cat /tmp/gpg_card_edit_output)"
 		WARN "GPG keytocard operation failed: $(head -3 /tmp/gpg_card_edit_output 2>/dev/null)"
 		whiptail_error --title 'ERROR: Keytocard Failed' \
@@ -698,7 +712,7 @@ reprovision_smartcard_from_backup() {
 			ERROR="$(cat /tmp/gpg_card_edit_output | fold -s)"
 			whiptail_error --title 'ERROR: Admin PIN Change Failed' \
 				--msgbox "Could not change the Admin PIN.\n\n${ERROR}" 0 80
-			umount /media 2>/dev/null || true
+			_luks_cleanup
 			return 1
 		fi
 		STATUS_OK "${pin_label_admin} changed"
@@ -717,16 +731,18 @@ reprovision_smartcard_from_backup() {
 			ERROR="$(cat /tmp/gpg_card_edit_output | fold -s)"
 			whiptail_error --title 'ERROR: User PIN Change Failed' \
 				--msgbox "Could not change the User PIN.\n\n${ERROR}" 0 80
-			umount /media 2>/dev/null || true
+			_luks_cleanup
 			return 1
 		fi
 		STATUS_OK "GPG User PIN changed"
 		# Cache the User PIN for /boot signing after reprovision
 		printf '%s' "$new_user_pin" >/tmp/secret/gpg_pin
+		chmod 600 /tmp/secret/gpg_pin 2>/dev/null || true
 	else
 		DEBUG "User declined custom PINs; keeping factory defaults"
 		# Cache default User PIN for /boot signing after reprovision
 		printf '%s' "123456" >/tmp/secret/gpg_pin
+		chmod 600 /tmp/secret/gpg_pin 2>/dev/null || true
 	fi
 
 	# Sign /boot so hashes exist on next boot.
@@ -779,7 +795,7 @@ reprovision_smartcard_from_backup() {
 	# Phase 8: save the LUKS mapper name so we can derive the public
 	# partition device, then unmount the LUKS partition.
 	mapper_dev="$(ls /dev/mapper/usb_mount_* 2>/dev/null | head -1)"
-	umount /media 2>/dev/null || true
+	_luks_cleanup
 	if [ -n "$mapper_dev" ]; then
 		# Extract partition name from mapper (e.g. usb_mount_sdb1 -> sdb1)
 		# and derive parent disk (e.g. /dev/sdb) + public partition (/dev/sdb2).
@@ -823,7 +839,7 @@ reprovision_smartcard_from_backup() {
 		else
 			DEBUG "pubkey.asc import failed; exporting from imported keyring"
 			gpg --export --armor "$key_id" >/tmp/reprovision_pubkey.asc 2>/dev/null || {
-				umount /media 2>/dev/null || true
+				_luks_cleanup
 				DIE "Failed to export public key for ROM flash"
 			}
 			PUBKEY=/tmp/reprovision_pubkey.asc
@@ -832,7 +848,7 @@ reprovision_smartcard_from_backup() {
 		# Fallback: export from the keyring (public key is already there
 		# from the privkey.sec import)
 		gpg --export --armor "$key_id" >/tmp/reprovision_pubkey.asc 2>/dev/null || {
-			umount /media 2>/dev/null || true
+			_luks_cleanup
 			DIE "Failed to export public key for ROM flash"
 		}
 		PUBKEY=/tmp/reprovision_pubkey.asc
@@ -882,7 +898,7 @@ reprovision_smartcard_from_backup() {
 
 	DEBUG "Running cleanup: unmounting partitions and closing LUKS mappings"
 	# Cleanup: unmount public partition and close any orphaned LUKS mappings
-	umount /media 2>/dev/null || true
+	_luks_cleanup
 	for dev in /dev/mapper/usb_mount_*; do
 		[ -e "$dev" ] && cryptsetup close "$(basename "$dev")" 2>/dev/null || true
 	done