From 187170ec51082ee9c510d654299e1a821e250d6e Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Fri, 6 Apr 2018 12:57:58 +0200 Subject: [PATCH] Check cipher before writing metadata (LUKS2). Some ciphers and key sizes created on-disk metadata that cannot be used. Use the same test for length-preserving cipher as LUKS1. Also check if key for integrity algorithm is not too small. Fixes #373. --- lib/integrity/integrity.c | 4 +--- lib/integrity/integrity.h | 3 ++- lib/luks1/keymanage.c | 17 ++++++++--------- lib/luks1/luks.h | 5 +++++ lib/luks2/luks2.h | 2 ++ lib/luks2/luks2_keyslot.c | 2 +- lib/setup.c | 21 ++++++++++++++++++--- src/cryptsetup.c | 4 +++- tests/api-test-2.c | 5 +++++ 9 files changed, 45 insertions(+), 18 deletions(-) diff --git a/lib/integrity/integrity.c b/lib/integrity/integrity.c index 186300de..03634bc0 100644 --- a/lib/integrity/integrity.c +++ b/lib/integrity/integrity.c @@ -106,10 +106,8 @@ int INTEGRITY_data_sectors(struct crypt_device *cd, return 0; } -int INTEGRITY_key_size(struct crypt_device *cd) +int INTEGRITY_key_size(struct crypt_device *cd, const char *integrity) { - const char *integrity = crypt_get_integrity(cd); - if (!integrity) return 0; diff --git a/lib/integrity/integrity.h b/lib/integrity/integrity.h index b11d88d1..5629255e 100644 --- a/lib/integrity/integrity.h +++ b/lib/integrity/integrity.h @@ -50,7 +50,8 @@ int INTEGRITY_dump(struct crypt_device *cd, struct device *device, uint64_t offs int INTEGRITY_data_sectors(struct crypt_device *cd, struct device *device, uint64_t offset, uint64_t *data_sectors); -int INTEGRITY_key_size(struct crypt_device *cd); +int INTEGRITY_key_size(struct crypt_device *cd, + const char *integrity); int INTEGRITY_tag_size(struct crypt_device *cd, const char *integrity, const char *cipher, diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c index 8eb666e8..5591270b 100644 --- a/lib/luks1/keymanage.c +++ b/lib/luks1/keymanage.c @@ -402,6 +402,10 @@ static int _keyslot_repair(struct luks_phdr *phdr, struct crypt_device *ctx) return -EINVAL; } + r = LUKS_check_cipher(ctx, phdr->keyBytes, phdr->cipherName, phdr->cipherMode); + if (r < 0) + return -EINVAL; + vk = crypt_alloc_volume_key(phdr->keyBytes, NULL); log_verbose(ctx, _("Repairing keyslots.\n")); @@ -691,23 +695,22 @@ int LUKS_write_phdr(struct luks_phdr *hdr, } /* Check that kernel supports requested cipher by decryption of one sector */ -static int LUKS_check_cipher(struct luks_phdr *hdr, struct crypt_device *ctx) +int LUKS_check_cipher(struct crypt_device *ctx, size_t keylength, const char *cipher, const char *cipher_mode) { int r; struct volume_key *empty_key; char buf[SECTOR_SIZE]; - log_dbg("Checking if cipher %s-%s is usable.", hdr->cipherName, hdr->cipherMode); + log_dbg("Checking if cipher %s-%s is usable.", cipher, cipher_mode); - empty_key = crypt_alloc_volume_key(hdr->keyBytes, NULL); + empty_key = crypt_alloc_volume_key(keylength, NULL); if (!empty_key) return -ENOMEM; /* No need to get KEY quality random but it must avoid known weak keys. */ r = crypt_random_get(ctx, empty_key->key, empty_key->keylength, CRYPT_RND_NORMAL); if (!r) - r = LUKS_decrypt_from_storage(buf, sizeof(buf), hdr->cipherName, - hdr->cipherMode, empty_key, 0, ctx); + r = LUKS_decrypt_from_storage(buf, sizeof(buf), cipher, cipher_mode, empty_key, 0, ctx); crypt_free_volume_key(empty_key); crypt_memzero(buf, sizeof(buf)); @@ -767,10 +770,6 @@ int LUKS_generate_phdr(struct luks_phdr *header, LUKS_fix_header_compatible(header); - r = LUKS_check_cipher(header, ctx); - if (r < 0) - return r; - log_dbg("Generating LUKS header version %d using hash %s, %s, %s, MK %d bytes", header->version, header->hashSpec ,header->cipherName, header->cipherMode, header->keyBytes); diff --git a/lib/luks1/luks.h b/lib/luks1/luks.h index f7b3a325..5a434123 100644 --- a/lib/luks1/luks.h +++ b/lib/luks1/luks.h @@ -99,6 +99,11 @@ struct luks_phdr { int LUKS_verify_volume_key(const struct luks_phdr *hdr, const struct volume_key *vk); +int LUKS_check_cipher(struct crypt_device *ctx, + size_t keylength, + const char *cipher, + const char *cipher_mode); + int LUKS_generate_phdr( struct luks_phdr *header, const struct volume_key *vk, diff --git a/lib/luks2/luks2.h b/lib/luks2/luks2.h index 7852b5b6..09f26593 100644 --- a/lib/luks2/luks2.h +++ b/lib/luks2/luks2.h @@ -155,6 +155,8 @@ int LUKS2_hdr_restore(struct crypt_device *cd, uint64_t LUKS2_hdr_and_areas_size(json_object *jobj); uint64_t LUKS2_keyslots_size(json_object *jobj); +int LUKS2_keyslot_cipher_incompatible(struct crypt_device *cd); + /* * Generic LUKS2 keyslot */ diff --git a/lib/luks2/luks2_keyslot.c b/lib/luks2/luks2_keyslot.c index 76b1d73d..b1a2168b 100644 --- a/lib/luks2/luks2_keyslot.c +++ b/lib/luks2/luks2_keyslot.c @@ -117,7 +117,7 @@ int LUKS2_keyslot_active_count(struct luks2_hdr *hdr, int segment) return num; } -static int LUKS2_keyslot_cipher_incompatible(struct crypt_device *cd) +int LUKS2_keyslot_cipher_incompatible(struct crypt_device *cd) { const char *cipher = crypt_get_cipher(cd); diff --git a/lib/setup.c b/lib/setup.c index d2e7247a..9fb5ad26 100644 --- a/lib/setup.c +++ b/lib/setup.c @@ -1438,6 +1438,10 @@ static int _crypt_format_luks1(struct crypt_device *cd, &required_alignment, &alignment_offset, DEFAULT_DISK_ALIGNMENT); + r = LUKS_check_cipher(cd, volume_key_size, cipher, cipher_mode); + if (r < 0) + return r; + r = LUKS_generate_phdr(&cd->u.luks1.hdr, cd->volume_key, cipher, cipher_mode, cd->pbkdf.hash, uuid, LUKS_STRIPES, required_alignment / SECTOR_SIZE, @@ -1511,7 +1515,10 @@ static int _crypt_format_luks2(struct crypt_device *cd, else return -EINVAL; } - + if (INTEGRITY_key_size(cd, integrity) >= volume_key_size) { + log_err(cd, _("Volume key is too small for encryption with integrity extensions.\n")); + return -EINVAL; + } } r = device_check_access(cd, crypt_metadata_device(cd), DEV_EXCL); @@ -1563,6 +1570,14 @@ static int _crypt_format_luks2(struct crypt_device *cd, goto out; } + /* FIXME: we have no way how to check AEAD ciphers, + * only length preserving mode or authenc() composed modes */ + if (!LUKS2_keyslot_cipher_incompatible(cd)) { + r = LUKS_check_cipher(cd, volume_key_size, cipher, cipher_mode); + if (r < 0) + goto out; + } + r = LUKS2_generate_hdr(cd, &cd->u.luks2.hdr, cd->volume_key, cipher, cipher_mode, integrity, uuid, @@ -3618,10 +3633,10 @@ const char *crypt_get_integrity(struct crypt_device *cd) int crypt_get_integrity_key_size(struct crypt_device *cd) { if (isINTEGRITY(cd->type)) - return INTEGRITY_key_size(cd); + return INTEGRITY_key_size(cd, crypt_get_integrity(cd)); if (isLUKS2(cd->type)) - return INTEGRITY_key_size(cd); + return INTEGRITY_key_size(cd, crypt_get_integrity(cd)); return 0; } diff --git a/src/cryptsetup.c b/src/cryptsetup.c index e6af8716..8ec3cca8 100644 --- a/src/cryptsetup.c +++ b/src/cryptsetup.c @@ -1008,7 +1008,9 @@ static int action_luksFormat(void) if (luks_version != 2 && opt_integrity) { log_err(_("Integrity option can be used only for LUKS2 format.\n")); goto out; - } if (opt_integrity) { + } + + if (opt_integrity) { r = crypt_parse_integrity_mode(opt_integrity, integrity, &integrity_keysize); if (r < 0) { log_err(_("No known integrity specification pattern detected.\n")); diff --git a/tests/api-test-2.c b/tests/api-test-2.c index 8a8ebaaf..91e3613f 100644 --- a/tests/api-test-2.c +++ b/tests/api-test-2.c @@ -2748,6 +2748,11 @@ static void Luks2Integrity(void) EQ_(32+16, ip.tag_size); OK_(crypt_deactivate(cd, CDEVICE_2)); crypt_free(cd); + + OK_(crypt_init(&cd, DEVICE_2)); + FAIL_(crypt_format(cd, CRYPT_LUKS2, cipher, cipher_mode, NULL, NULL, key_size - 32, ¶ms), "Wrong key size."); + FAIL_(crypt_format(cd, CRYPT_LUKS2, cipher, "xts-plainx", NULL, NULL, key_size, ¶ms), "Wrong cipher."); + crypt_free(cd); } static void Luks2Flags(void)