From ace015a3e5d0940ae00c309ed53097f5a695dee5 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Tue, 31 Jan 2023 20:15:58 +0100 Subject: [PATCH] Fix OpenSSL < 2 crypto backend PBKDF2 possible iteration count overflow. For OpenSSL2, we use PKCS5_PBKDF2_HMAC() function. Unfortunately, the iteration count is defined as signed integer (unlike unsigned in OpenSSL3 PARAMS KDF API). This can lead to overflow and decreasing of actual iterations count. In reality this can happen only if pbkdf-force-iterations is used. This patch add check to INT_MAX if linked to older OpenSSL and disallows such setting. Note, this is misconception in OpenSSL2 API, cryptsetup internally use uint32_t for iterations count. Reported by wangzhiqiang in cryptsetup list. Conflict: context adaptation --- lib/crypto_backend/crypto_backend.h | 3 ++- lib/crypto_backend/crypto_openssl.c | 10 ++++++++++ lib/luks1/keymanage.c | 7 ++++++- lib/luks2/luks2_keyslot_luks2.c | 4 ++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/crypto_backend/crypto_backend.h b/lib/crypto_backend/crypto_backend.h index 88cc2d5..47e5186 100644 --- a/lib/crypto_backend/crypto_backend.h +++ b/lib/crypto_backend/crypto_backend.h @@ -34,7 +34,8 @@ struct crypt_storage; int crypt_backend_init(bool fips); void crypt_backend_destroy(void); -#define CRYPT_BACKEND_KERNEL (1 << 0) /* Crypto uses kernel part, for benchmark */ +#define CRYPT_BACKEND_KERNEL (1 << 0) /* Crypto uses kernel part, for benchmark */ +#define CRYPT_BACKEND_PBKDF2_INT (1 << 1) /* Iteration in PBKDF2 is signed int and can overflow */ uint32_t crypt_backend_flags(void); const char *crypt_backend_version(void); diff --git a/lib/crypto_backend/crypto_openssl.c b/lib/crypto_backend/crypto_openssl.c index 0dbcb75..3a58dcd 100644 --- a/lib/crypto_backend/crypto_openssl.c +++ b/lib/crypto_backend/crypto_openssl.c @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -228,7 +229,11 @@ void crypt_backend_destroy(void) uint32_t crypt_backend_flags(void) { +#if OPENSSL_VERSION_MAJOR >= 3 return 0; +#else + return CRYPT_BACKEND_PBKDF2_INT; +#endif } const char *crypt_backend_version(void) @@ -527,6 +532,11 @@ static int pbkdf2(const char *password, size_t password_length, if (!hash_id) return 0; + /* OpenSSL2 has iteration as signed int, avoid overflow */ + if (iterations > INT_MAX) + return 0; + + return PKCS5_PBKDF2_HMAC(password, (int)password_length, (const unsigned char *)salt, (int)salt_length, iterations, hash_id, (int)key_length, key); diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c index 244b0d5..792088c 100644 --- a/lib/luks1/keymanage.c +++ b/lib/luks1/keymanage.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "luks.h" #include "af.h" @@ -922,8 +923,12 @@ int LUKS_set_key(unsigned int keyIndex, hdr->keyblock[keyIndex].passwordSalt, LUKS_SALTSIZE, derived_key->key, hdr->keyBytes, hdr->keyblock[keyIndex].passwordIterations, 0, 0); - if (r < 0) + if (r < 0) { + if ((crypt_backend_flags() & CRYPT_BACKEND_PBKDF2_INT) && + hdr->keyblock[keyIndex].passwordIterations > INT_MAX) + log_err(ctx, _("PBKDF2 iteration value overflow.")); goto out; + } /* * AF splitting, the masterkey stored in vk->key is split to AfKey diff --git a/lib/luks2/luks2_keyslot_luks2.c b/lib/luks2/luks2_keyslot_luks2.c index ea58112..098f6c6 100644 --- a/lib/luks2/luks2_keyslot_luks2.c +++ b/lib/luks2/luks2_keyslot_luks2.c @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include "luks2_internal.h" /* FIXME: move keyslot encryption to crypto backend */ @@ -254,6 +255,9 @@ static int luks2_keyslot_set_key(struct crypt_device *cd, pbkdf.iterations, pbkdf.max_memory_kb, pbkdf.parallel_threads); if (r < 0) { + if ((crypt_backend_flags() & CRYPT_BACKEND_PBKDF2_INT) && + pbkdf.iterations > INT_MAX) + log_err(cd, _("PBKDF2 iteration value overflow.")); crypt_free_volume_key(derived_key); return r; } -- 2.33.0