Check for bad device-mapper name early

There is no need to unlock keyslot if the provided name has wrong format. Let's check for length and '/' in name early. Note that other commands could accept path to the device as libdevmapper translate it to the name (status /dev/mapper/xxx). Add early check only to activate commands. It still can fail later because of mangled characters. Fixes: #893
mbroz · Jul 16, 2024 · 021e5c3 · 021e5c3
1 parent 624b708
commit 021e5c3
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 0 deletions.
diff --git a/lib/libcryptsetup_macros.h b/lib/libcryptsetup_macros.h
@@ -49,6 +49,7 @@
 #define DEFAULT_MEM_ALIGNMENT	4096
 
 #define DM_UUID_LEN		129
+#define DM_NAME_LEN		128
 #define DM_BY_ID_PREFIX		"dm-uuid-"
 #define DM_BY_ID_PREFIX_LEN	8
 #define DM_UUID_PREFIX		"CRYPT-"

diff --git a/src/cryptsetup.c b/src/cryptsetup.c
@@ -323,6 +323,9 @@ static int action_open_plain(void)
 		pmode = cipher_mode;
 	}
 
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
+
 	if (ARG_SET(OPT_DEVICE_SIZE_ID))
 		params.size = ARG_UINT64(OPT_DEVICE_SIZE_ID) / SECTOR_SIZE;
 	else if (ARG_SET(OPT_SIZE_ID))
@@ -407,6 +410,9 @@ static int action_open_loopaes(void)
 			goto out;
 	}
 
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
+
 	set_activation_flags(&activate_flags);
 
 	r = crypt_activate_by_keyfile_device_offset(cd, activated_name, CRYPT_ANY_SLOT,
@@ -511,6 +517,8 @@ static int action_open_tcrypt(void)
 	int r;
 
 	activated_name = ARG_SET(OPT_TEST_PASSPHRASE_ID) ? NULL : action_argv[1];
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
 
 	r = crypt_init_data_device(&cd, ARG_STR(OPT_HEADER_ID) ?: action_argv[0], action_argv[0]);
 	if (r < 0)
@@ -542,6 +550,8 @@ static int action_open_bitlk(void)
 	size_t passwordLen;
 
 	activated_name = ARG_SET(OPT_TEST_PASSPHRASE_ID) ? NULL : action_argv[1];
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
 
 	if ((r = crypt_init(&cd, action_argv[0])))
 		goto out;
@@ -825,6 +835,8 @@ static int action_open_fvault2(void)
 	size_t passwordLen;
 
 	activated_name = ARG_SET(OPT_TEST_PASSPHRASE_ID) ? NULL : action_argv[1];
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
 
 	if ((r = crypt_init(&cd, action_argv[0])))
 		goto out;
@@ -1950,6 +1962,9 @@ static int action_open_luks(void)
 		}
 	}
 
+	if ((r = tools_check_newname(activated_name)))
+		goto out;
+
 	set_activation_flags(&activate_flags);
 
 	if (ARG_SET(OPT_EXTERNAL_TOKENS_PATH_ID)) {

diff --git a/src/cryptsetup.h b/src/cryptsetup.h
@@ -90,6 +90,7 @@ struct tools_progress_params {
 
 int tools_progress(uint64_t size, uint64_t offset, void *usrptr);
 const char *tools_get_device_name(const char *device, char **r_backing_file);
+int tools_check_newname(const char *name);
 
 int tools_read_vk(const char *file, char **key, int keysize);
 int tools_write_mk(const char *file, const char *key, int keysize);

diff --git a/src/integritysetup.c b/src/integritysetup.c
@@ -348,6 +348,9 @@ static int action_open(void)
 	if (ARG_SET(OPT_ALLOW_DISCARDS_ID))
 		activate_flags |= CRYPT_ACTIVATE_ALLOW_DISCARDS;
 
+	if ((r = tools_check_newname(action_argv[1])))
+		goto out;
+
 	r = _read_keys(&integrity_key, &params);
 	if (r)
 		goto out;

diff --git a/src/utils_tools.c b/src/utils_tools.c
@@ -458,3 +458,19 @@ void tools_package_version(const char *name, bool use_pwlibs)
 		passwdqc && use_pwlibs ? "PASSWDQC " : "",
 		hw_opal ? "HW_OPAL " : "");
 }
+
+int tools_check_newname(const char *name)
+{
+	if (!name)
+		return 0;
+
+	if (strlen(name) >= DM_NAME_LEN) {
+		log_err(_("Name is too long."));
+		return -EINVAL;
+	} else if (strchr(name, '/')) {
+		log_err(_("Name must not contain '/' character."));
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/src/veritysetup.c b/src/veritysetup.c
@@ -267,6 +267,9 @@ static int action_open(void)
 		return -EINVAL;
 	}
 
+	if (tools_check_newname(action_argv[1]))
+		return -EINVAL;
+
 	return _activate(action_argv[1],
 			 action_argv[0],
 			 action_argv[2],

diff --git a/tests/compat-test b/tests/compat-test
@@ -1148,5 +1148,14 @@ echo $PWD3 | $CRYPTSETUP open -q --test-passphrase -S5 $IMG || fail
 $CRYPTSETUP luksAddKey -q $FAST_PBKDF_OPT -S6 --volume-key-file $VK_FILE --new-keyfile $KEY5 $IMG || fail
 $CRYPTSETUP open --test-passphrase -q -S6 -d $KEY5 $IMG || fail
 
+prepare "[40] Early check for active name." wipe
+DM_BAD_NAME=x/x
+DM_LONG_NAME=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+echo $PWD1 | $CRYPTSETUP open --type plain $LOOPDEV $DM_BAD_NAME --hash sha256 --cipher aes-cbc-essiv:sha256 --key-size 256 2>/dev/null && fail
+echo $PWD1 | $CRYPTSETUP open --type plain $LOOPDEV $DM_LONG_NAME --hash sha256 --cipher aes-cbc-essiv:sha256 --key-size 256 2>/dev/null && fail
+echo $PWD1 | $CRYPTSETUP luksFormat --type luks1 $FAST_PBKDF_OPT $LOOPDEV || fail
+echo $PWD1 | $CRYPTSETUP luksOpen $LOOPDEV $DM_BAD_NAME 2>/dev/null && fail
+echo $PWD1 | $CRYPTSETUP luksOpen $LOOPDEV $DM_LONG_NAME 2>/dev/null && fail
+
 remove_mapping
 exit 0
diff --git a/tests/compat-test2 b/tests/compat-test2
@@ -1684,5 +1684,12 @@ echo $PWD2 | $CRYPTSETUP -q luksAddKey $FAST_PBKDF_OPT --unbound --key-size 256
 expect_retried_unlocked_keyslot 0 $PWD1 "open -v --test-passphrase --tries 2 $LOOPDEV" || fail
 expect_retried_unlocked_keyslot 1 $PWD2 "open -v --test-passphrase --tries 2 -S1 $LOOPDEV" || fail
 
+prepare "[51] Early check for active name." wipe
+DM_BAD_NAME=x/x
+DM_LONG_NAME=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+echo $PWD1 | $CRYPTSETUP luksFormat -q $FAST_PBKDF_OPT --type luks2 $LOOPDEV || fail
+echo $PWD1 | $CRYPTSETUP luksOpen $LOOPDEV $DM_BAD_NAME 2>/dev/null && fail
+echo $PWD1 | $CRYPTSETUP luksOpen $LOOPDEV $DM_LONG_NAME 2>/dev/null && fail
+
 remove_mapping
 exit 0
diff --git a/tests/integrity-compat-test b/tests/integrity-compat-test
@@ -655,4 +655,13 @@ if [ -n "$DM_INTEGRITY_HMAC_FIX" ] ; then
 	test_resize "[INTEGRITY RESIZE KEY DETACHED]" 1 1 "--integrity hmac-sha256 --integrity-key-size 128 --integrity-key-file $KEY_FILE --journal-integrity hmac-sha256 --journal-integrity-key-file $KEY_FILE --journal-integrity-key-size 128 --journal-crypt ctr-aes --journal-crypt-key-size 16 --journal-crypt-key-file $KEY_FILE"
 fi
 
+echo -n "Early check for active name:"
+add_device
+DM_BAD_NAME=x/x
+DM_LONG_NAME=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+$INTSETUP format -q $DEV --no-wipe || fail "Cannot format device."
+$INTSETUP open $DEV $DM_BAD_NAME 2>/dev/null && fail "Cannot activate device."
+$INTSETUP open $DEV $DM_LONG_NAME 2>/dev/null && fail "Cannot activate device."
+echo "[OK]"
+
 cleanup
diff --git a/tests/verity-compat-test b/tests/verity-compat-test
@@ -563,5 +563,14 @@ else
 	echo "[N/A]"
 fi
 
+echo -n "Early check for active name:"
+prepare 8192 1024
+DM_BAD_NAME=x/x
+DM_LONG_NAME=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+$VERITYSETUP format $LOOPDEV1 $IMG_HASH --format=1 --data-block-size=512 --hash-block-size=512 --hash=sha256 --salt=$SALT >/dev/null 2>&1 || fail "Cannot format device."
+$VERITYSETUP open $LOOPDEV1 $DM_BAD_NAME $DEV $IMG_HASH 9de18652fe74edfb9b805aaed72ae2aa48f94333f1ba5c452ac33b1c39325174 2>/dev/null && fail
+$VERITYSETUP open $LOOPDEV1 $DM_LONG_NAME $DEV $IMG_HASH 9de18652fe74edfb9b805aaed72ae2aa48f94333f1ba5c452ac33b1c39325174 2>/dev/null && fail
+echo "[OK]"
+
 remove_mapping
 exit 0