Add constant time memcmp and use it for comparing keys.

There is perhaps no problem now, but it is a good practise to use
constant time for key comaprison to avoid possible side channel
issues.
This commit is contained in:
Milan Broz
2022-04-27 23:05:02 +02:00
parent 4f44bb40b7
commit 2bf0f537f6
12 changed files with 65 additions and 5 deletions

View File

@@ -149,4 +149,7 @@ static inline void crypt_backend_memzero(void *s, size_t n)
#endif
}
/* Memcmp helper (memcmp in constant time) */
int crypt_backend_memeq(const void *m1, const void *m2, size_t n);
#endif /* _CRYPTO_BACKEND_H */

View File

@@ -58,4 +58,18 @@ int crypt_bitlk_decrypt_key_kernel(const void *key, size_t key_length,
const char *iv, size_t iv_length,
const char *tag, size_t tag_length);
/* Internal implementation for constant time memory comparison */
static inline int crypt_internal_memeq(const void *m1, const void *m2, size_t n)
{
const unsigned char *_m1 = (const unsigned char *) m1;
const unsigned char *_m2 = (const unsigned char *) m2;
unsigned char result = 0;
size_t i;
for (i = 0; i < n; i++)
result |= _m1[i] ^ _m2[i];
return result;
}
#endif /* _CRYPTO_BACKEND_INTERNAL_H */

View File

@@ -550,3 +550,8 @@ out:
return -ENOTSUP;
#endif
}
int crypt_backend_memeq(const void *m1, const void *m2, size_t n)
{
return crypt_internal_memeq(m1, m2, n);
}

View File

@@ -416,3 +416,8 @@ int crypt_bitlk_decrypt_key(const void *key, size_t key_length,
return crypt_bitlk_decrypt_key_kernel(key, key_length, in, out, length,
iv, iv_length, tag, tag_length);
}
int crypt_backend_memeq(const void *m1, const void *m2, size_t n)
{
return crypt_internal_memeq(m1, m2, n);
}

View File

@@ -26,6 +26,7 @@
#include <nettle/sha3.h>
#include <nettle/hmac.h>
#include <nettle/pbkdf2.h>
#include <nettle/memops.h>
#include "crypto_backend_internal.h"
#if HAVE_NETTLE_VERSION_H
@@ -446,3 +447,9 @@ int crypt_bitlk_decrypt_key(const void *key, size_t key_length,
return crypt_bitlk_decrypt_key_kernel(key, key_length, in, out, length,
iv, iv_length, tag, tag_length);
}
int crypt_backend_memeq(const void *m1, const void *m2, size_t n)
{
/* The logic is inverse to memcmp... */
return !memeql_sec(m1, m2, n);
}

View File

@@ -395,3 +395,8 @@ int crypt_bitlk_decrypt_key(const void *key, size_t key_length,
return crypt_bitlk_decrypt_key_kernel(key, key_length, in, out, length,
iv, iv_length, tag, tag_length);
}
int crypt_backend_memeq(const void *m1, const void *m2, size_t n)
{
return NSS_SecureMemcmp(m1, m2, n);
}

View File

@@ -30,6 +30,7 @@
#include <string.h>
#include <errno.h>
#include <openssl/crypto.h>
#include <openssl/evp.h>
#include <openssl/hmac.h>
#include <openssl/rand.h>
@@ -806,3 +807,8 @@ out:
return -ENOTSUP;
#endif
}
int crypt_backend_memeq(const void *m1, const void *m2, size_t n)
{
return CRYPTO_memcmp(m1, m2, n);
}

View File

@@ -982,7 +982,7 @@ int LUKS_verify_volume_key(const struct luks_phdr *hdr,
hdr->mkDigestIterations, 0, 0) < 0)
return -EINVAL;
if (memcmp(checkHashBuf, hdr->mkDigest, LUKS_DIGESTSIZE))
if (crypt_backend_memeq(checkHashBuf, hdr->mkDigest, LUKS_DIGESTSIZE))
return -EPERM;
return 0;

View File

@@ -78,7 +78,7 @@ static int PBKDF2_digest_verify(struct crypt_device *cd,
mkDigestIterations, 0, 0) < 0) {
r = -EINVAL;
} else {
if (memcmp(checkHashBuf, mkDigest, len) == 0)
if (crypt_backend_memeq(checkHashBuf, mkDigest, len) == 0)
r = 0;
}
out:

View File

@@ -2401,7 +2401,7 @@ static int _compare_volume_keys(struct volume_key *svk, unsigned skeyring_only,
return 1;
if (!skeyring_only && !tkeyring_only)
return memcmp(svk->key, tvk->key, svk->keylength);
return crypt_backend_memeq(svk->key, tvk->key, svk->keylength);
if (svk->key_description && tvk->key_description)
return strcmp(svk->key_description, tvk->key_description);

View File

@@ -214,7 +214,7 @@ static int create_or_verify(struct crypt_device *cd, FILE *rd, FILE *wr,
r = -EIO;
goto out;
}
if (memcmp(read_digest, calculated_digest, digest_size)) {
if (crypt_backend_memeq(read_digest, calculated_digest, digest_size)) {
log_err(cd, _("Verification failed at position %" PRIu64 "."),
ftello(rd) - data_block_size);
r = -EPERM;
@@ -380,7 +380,7 @@ out:
log_err(cd, _("Verification of data area failed."));
else {
log_dbg(cd, "Verification of data area succeeded.");
r = memcmp(root_hash, calculated_digest, digest_size) ? -EFAULT : 0;
r = crypt_backend_memeq(root_hash, calculated_digest, digest_size) ? -EFAULT : 0;
if (r)
log_err(cd, _("Verification of root hash failed."));
else

View File

@@ -1429,6 +1429,18 @@ static int default_alg_test(void)
return EXIT_SUCCESS;
}
static int memcmp_test(void)
{
printf("MEMEQ ");
if (!crypt_backend_memeq("aaaaaaaa", "bbbbbbbb", 8))
return EXIT_FAILURE;
if (crypt_backend_memeq("aaaaaaaa", "aaaaaaaa", 8))
return EXIT_FAILURE;
printf("[OK]\n");
return EXIT_SUCCESS;
}
static void __attribute__((noreturn)) exit_test(const char *msg, int r)
{
if (msg)
@@ -1469,6 +1481,9 @@ int main(__attribute__ ((unused)) int argc, __attribute__ ((unused))char *argv[]
if (base64_test())
exit_test("BASE64 test failed.", EXIT_FAILURE);
if (memcmp_test())
exit_test("Memcmp test failed.", EXIT_FAILURE);
if (default_alg_test()) {
if (fips_mode())
printf("\nDefault compiled-in algorithms test ignored (FIPS mode on).\n");