Skip to content

crypto: Add a function to check for label and subsystem validity #1133

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.

Sign up for GitHub

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

Merged
vojtechtrefny merged 1 commit into from
Oct 10, 2025
+50 −0
Merged

crypto: Add a function to check for label and subsystem validity #1133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/libblockdev-sections.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ bd_crypto_luks_kill_slot
bd_crypto_luks_header_backup
bd_crypto_luks_header_restore
bd_crypto_luks_set_label
bd_crypto_luks_check_label
bd_crypto_luks_set_uuid
bd_crypto_luks_convert
BDCryptoLUKSPersistentFlags
Expand Down
13 changes: 13 additions & 0 deletions src/lib/plugin_apis/crypto.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,19 @@ gboolean bd_crypto_luks_header_restore (const gchar *device, const gchar *backup
*/
gboolean bd_crypto_luks_set_label (const gchar *device, const gchar *label, const gchar *subsystem, GError **error);

/**
* bd_crypto_luks_check_label:
* @label: (nullable): label to check
* @subsystem: (nullable): subsystem to check
* @error: (out) (optional): place to store error
*
* Returns: whether @label and @subsystem are valid for LUKS2 or not
* (reason is provided in @error)
*
* Tech category: always available
*/
gboolean bd_crypto_luks_check_label (const gchar *label, const gchar *subsystem, GError **error);

/**
* bd_crypto_luks_set_uuid:
* @device: device to set UUID on
Expand Down
27 changes: 27 additions & 0 deletions src/plugins/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2159,6 +2159,33 @@ gboolean bd_crypto_luks_header_restore (const gchar *device, const gchar *backup
return TRUE;
}

/**
* bd_crypto_luks_check_label:
* @label: (nullable): label to check
* @subsystem: (nullable): subsystem to check
* @error: (out) (optional): place to store error
*
* Returns: whether @label and @subsystem are valid for LUKS2 or not
* (reason is provided in @error)
*
* Tech category: always available
*/
gboolean bd_crypto_luks_check_label (const gchar *label, const gchar *subsystem, GError **error) {
if (label && strlen (label) > 47) {
g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_INVALID_PARAMS,
"Label for LUKS must be at most 47 characters long");
return FALSE;
}

if (subsystem && strlen (subsystem) > 47) {
g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_INVALID_PARAMS,
"Subsystem for LUKS must be at most 47 characters long");
return FALSE;
}

return TRUE;
}
Comment on lines +2173 to +2187
Copy link

@gemini-code-assist gemini-code-assist bot Oct 9, 2025

Choose a reason for hiding this comment

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

critical

The length checks for both label and subsystem are incorrect. According to the cryptsetup source, the on-disk format for these fields is a 48-byte character array, which includes the null terminator. This means the maximum string length is 47 characters.

The current checks (> 48) allow strings of 48 characters, which would lead to a buffer overflow when used by cryptsetup.

The checks should be changed to > 47, and the error messages should be updated to reflect the correct maximum length.

gboolean bd_crypto_luks_check_label (const gchar *label, const gchar *subsystem, GError **error) {
    if (label && strlen (label) > 47) {
        g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_INVALID_PARAMS,
                     "Label for LUKS must be at most 47 characters long");
        return FALSE;
    }

    if (subsystem && strlen (subsystem) > 47) {
        g_set_error (error, BD_CRYPTO_ERROR, BD_CRYPTO_ERROR_INVALID_PARAMS,
                     "Subsystem for LUKS must be at most 47 characters long");
        return FALSE;
    }

    return TRUE;
}


/**
* bd_crypto_luks_set_label:
* @device: device to set label on
Expand Down
1 change: 1 addition & 0 deletions src/plugins/crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ gboolean bd_crypto_luks_kill_slot (const gchar *device, gint slot, GError **erro
gboolean bd_crypto_luks_header_backup (const gchar *device, const gchar *backup_file, GError **error);
gboolean bd_crypto_luks_header_restore (const gchar *device, const gchar *backup_file, GError **error);
gboolean bd_crypto_luks_set_label (const gchar *device, const gchar *label, const gchar *subsystem, GError **error);
gboolean bd_crypto_luks_check_label (const gchar *label, const gchar *subsystem, GError **error);
gboolean bd_crypto_luks_set_uuid (const gchar *device, const gchar *uuid, GError **error);
gboolean bd_crypto_luks_convert (const gchar *device, BDCryptoLUKSVersion target_version, GError **error);
gboolean bd_crypto_luks_set_persistent_flags (const gchar *device, BDCryptoLUKSPersistentFlags flags, GError **error);
Expand Down
8 changes: 8 additions & 0 deletions tests/crypto_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,14 @@ class CryptoTestSetLabelUuid(CryptoTestCase):
test_uuid = "4d7086c4-a4d3-432f-819e-73da03870df9"

def _test_set_label(self):
succ = BlockDev.crypto_luks_check_label(self.label, self.subsystem)
self.assertTrue(succ)

with self.assertRaisesRegex(GLib.GError, "Label for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(label="a" * 48)
with self.assertRaisesRegex(GLib.GError, "Subsystem for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(subsystem="a" * 48)
Comment on lines +1090 to +1093
Copy link

@gemini-code-assist gemini-code-assist bot Oct 9, 2025

Choose a reason for hiding this comment

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

medium

The tests for the new function are good, but they are missing a check for the boundary condition. A string of length 48 should be invalid.

Please add tests for strings of exactly 48 characters for both label and subsystem to ensure the boundary is handled correctly. These tests should expect a GLib.GError. I've also updated the existing tests to check for the corrected error message.

Suggested change
with self.assertRaisesRegex(GLib.GError, "Label for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(label="a" * 48)
with self.assertRaisesRegex(GLib.GError, "Subsystem for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(subsystem="a" * 48)
with self.assertRaisesRegex(GLib.GError, "Label for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(label="a" * 48)
with self.assertRaisesRegex(GLib.GError, "Label for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(label="a" * 49)
with self.assertRaisesRegex(GLib.GError, "Subsystem for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(subsystem="a" * 48)
with self.assertRaisesRegex(GLib.GError, "Subsystem for LUKS must be at most 47 characters long"):
BlockDev.crypto_luks_check_label(subsystem="a" * 49)


succ = BlockDev.crypto_luks_set_label(self.loop_devs[0], self.label, self.subsystem)
self.assertTrue(succ)

Expand Down