Commit f7b61b26 authored by Milan Broz's avatar Milan Broz

Prevent compiler to optiize-out memset for on-stack variables.

Also see
https://cryptocoding.net/index.php/Coding_rules#Prevent_compiler_interference_with_security-critical_operations

The used code is inspired by the code in Blake2 implementation.
parent dc40b91c
......@@ -102,4 +102,11 @@ int crypt_storage_decrypt(struct crypt_storage *ctx, uint64_t sector,
int crypt_storage_encrypt(struct crypt_storage *ctx, uint64_t sector,
size_t count, char *buffer);
/* Memzero helper (memset on stack can be optimized out) */
static inline void crypt_backend_memzero(void *s, size_t n)
{
volatile uint8_t *p = (volatile uint8_t *)s;
while(n--) *p++ = 0;
}
#endif /* _CRYPTO_BACKEND_H */
......@@ -217,7 +217,7 @@ static int crypt_cipher_crypt(struct crypt_cipher *ctx,
if (len != (ssize_t)length)
r = -EIO;
bad:
memset(buffer, 0, sizeof(buffer));
crypt_backend_memzero(buffer, sizeof(buffer));
return r;
}
......
......@@ -164,7 +164,7 @@ int crypt_hash_final(struct crypt_hash *ctx, char *buffer, size_t length)
return -EINVAL;
memcpy(buffer, tmp, length);
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
if (tmp_len < length)
return -EINVAL;
......@@ -266,7 +266,7 @@ int crypt_hmac_final(struct crypt_hmac *ctx, char *buffer, size_t length)
return -EINVAL;
memcpy(buffer, tmp, length);
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
if (tmp_len < length)
return -EINVAL;
......
......@@ -133,7 +133,7 @@ int crypt_hash_final(struct crypt_hash *ctx, char *buffer, size_t length)
return -EINVAL;
memcpy(buffer, tmp, length);
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
if (tmp_len < length)
return -EINVAL;
......@@ -203,7 +203,7 @@ int crypt_hmac_final(struct crypt_hmac *ctx, char *buffer, size_t length)
HMAC_Final(&ctx->md, tmp, &tmp_len);
memcpy(buffer, tmp, length);
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
if (tmp_len < length)
return -EINVAL;
......
......@@ -103,13 +103,13 @@ static int crypt_sector_iv_init(struct crypt_sector_iv *ctx,
r = crypt_hash_final(h, tmp, hash_size);
crypt_hash_destroy(h);
if (r) {
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
return r;
}
r = crypt_cipher_init(&ctx->essiv_cipher, cipher_name, "ecb",
tmp, hash_size);
memset(tmp, 0, sizeof(tmp));
crypt_backend_memzero(tmp, sizeof(tmp));
if (r)
return r;
......
......@@ -188,7 +188,7 @@ int pkcs5_pbkdf2(const char *hash,
if (crypt_hmac_init(&hmac, hash, P_hash, hLen))
return -EINVAL;
memset(P_hash, 0, sizeof(P_hash));
crypt_backend_memzero(P_hash, sizeof(P_hash));
} else {
if (crypt_hmac_init(&hmac, hash, P, Plen))
return -EINVAL;
......@@ -224,9 +224,9 @@ int pkcs5_pbkdf2(const char *hash,
rc = 0;
out:
crypt_hmac_destroy(hmac);
memset(U, 0, sizeof(U));
memset(T, 0, sizeof(T));
memset(tmp, 0, tmplen);
crypt_backend_memzero(U, sizeof(U));
crypt_backend_memzero(T, sizeof(T));
crypt_backend_memzero(tmp, tmplen);
return rc;
}
......
......@@ -57,6 +57,7 @@ struct volume_key {
char key[];
};
void crypt_memzero(void *s, size_t n);
struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key);
struct volume_key *crypt_generate_volume_key(struct crypt_device *cd, unsigned keylength);
void crypt_free_volume_key(struct volume_key *vk);
......
......@@ -212,7 +212,7 @@ int LUKS_hdr_backup(const char *backup_file, struct crypt_device *ctx)
out:
if (devfd != -1)
close(devfd);
memset(&hdr, 0, sizeof(hdr));
crypt_memzero(&hdr, sizeof(hdr));
crypt_safe_free(buffer);
return r;
}
......@@ -398,7 +398,7 @@ static int _keyslot_repair(struct luks_phdr *phdr, struct crypt_device *ctx)
}
out:
crypt_free_volume_key(vk);
memset(&temp_phdr, 0, sizeof(temp_phdr));
crypt_memzero(&temp_phdr, sizeof(temp_phdr));
return r;
}
......@@ -618,7 +618,7 @@ static int LUKS_check_cipher(struct luks_phdr *hdr, struct crypt_device *ctx)
empty_key, 0, ctx);
crypt_free_volume_key(empty_key);
memset(buf, 0, sizeof(buf));
crypt_memzero(buf, sizeof(buf));
return r;
}
......
......@@ -1453,7 +1453,7 @@ int crypt_header_restore(struct crypt_device *cd,
r = LUKS_hdr_restore(backup_file, isLUKS(cd->type) ? &cd->u.luks1.hdr : &hdr, cd);
memset(&hdr, 0, sizeof(hdr));
crypt_memzero(&hdr, sizeof(hdr));
return r;
}
......@@ -1486,7 +1486,7 @@ void crypt_free(struct crypt_device *cd)
free(cd->type);
/* Some structures can contain keys (TCRYPT), wipe it */
memset(cd, 0, sizeof(*cd));
crypt_memzero(cd, sizeof(*cd));
free(cd);
}
}
......
......@@ -269,8 +269,8 @@ static int decrypt_blowfish_le_cbc(struct tcrypt_alg *alg,
}
crypt_cipher_destroy(cipher);
memset(iv, 0, bs);
memset(iv_old, 0, bs);
crypt_memzero(iv, bs);
crypt_memzero(iv_old, bs);
return r;
}
......@@ -336,8 +336,8 @@ static int TCRYPT_decrypt_hdr_one(struct tcrypt_alg *alg, const char *mode,
crypt_cipher_destroy(cipher);
}
memset(backend_key, 0, sizeof(backend_key));
memset(iv, 0, TCRYPT_HDR_IV_LEN);
crypt_memzero(backend_key, sizeof(backend_key));
crypt_memzero(iv, TCRYPT_HDR_IV_LEN);
return r;
}
......@@ -387,8 +387,8 @@ out:
if (cipher[j])
crypt_cipher_destroy(cipher[j]);
memset(iv, 0, bs);
memset(iv_old, 0, bs);
crypt_memzero(iv, bs);
crypt_memzero(iv_old, bs);
return r;
}
......@@ -434,7 +434,7 @@ static int TCRYPT_decrypt_hdr(struct crypt_device *cd, struct tcrypt_phdr *hdr,
r = -EPERM;
}
memset(&hdr2, 0, sizeof(hdr2));
crypt_memzero(&hdr2, sizeof(hdr2));
return r;
}
......@@ -471,8 +471,8 @@ static int TCRYPT_pool_keyfile(struct crypt_device *cd,
j %= TCRYPT_KEY_POOL_LEN;
}
memset(&crc, 0, sizeof(crc));
memset(data, 0, TCRYPT_KEYFILE_LEN);
crypt_memzero(&crc, sizeof(crc));
crypt_memzero(data, TCRYPT_KEYFILE_LEN);
return 0;
}
......@@ -562,9 +562,9 @@ static int TCRYPT_init_hdr(struct crypt_device *cd,
params->cipher, params->mode, params->key_size);
}
out:
memset(pwd, 0, TCRYPT_KEY_POOL_LEN);
crypt_memzero(pwd, TCRYPT_KEY_POOL_LEN);
if (key)
memset(key, 0, TCRYPT_HDR_KEY_LEN);
crypt_memzero(key, TCRYPT_HDR_KEY_LEN);
free(key);
return r;
}
......
......@@ -81,6 +81,18 @@ int crypt_parse_name_and_mode(const char *s, char *cipher, int *key_nums,
return -EINVAL;
}
/*
* Replacement for memset(s, 0, n) on stack that can be optimized out
* Also used in safe allocations for explicit memory wipe.
*/
void crypt_memzero(void *s, size_t n)
{
volatile uint8_t *p = (volatile uint8_t *)s;
while(n--)
*p++ = 0;
}
/* safe allocations */
void *crypt_safe_alloc(size_t size)
{
......@@ -94,7 +106,7 @@ void *crypt_safe_alloc(size_t size)
return NULL;
alloc->size = size;
memset(&alloc->data, 0, size);
crypt_memzero(&alloc->data, size);
/* coverity[leaked_storage] */
return &alloc->data;
......@@ -110,7 +122,7 @@ void crypt_safe_free(void *data)
alloc = (struct safe_allocation *)
((char *)data - offsetof(struct safe_allocation, data));
memset(data, 0, alloc->size);
crypt_memzero(data, alloc->size);
alloc->size = 0x55aa55aa;
free(alloc);
......
......@@ -35,7 +35,7 @@ struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key)
if (key)
memcpy(&vk->key, key, keylength);
else
memset(&vk->key, 0, keylength);
crypt_memzero(&vk->key, keylength);
return vk;
}
......@@ -43,7 +43,7 @@ struct volume_key *crypt_alloc_volume_key(unsigned keylength, const char *key)
void crypt_free_volume_key(struct volume_key *vk)
{
if (vk) {
memset(vk->key, 0, vk->keylength);
crypt_memzero(vk->key, vk->keylength);
vk->keylength = 0;
free(vk);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment