Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 64d107d3 authored by Eric Biggers's avatar Eric Biggers Committed by James Morris
Browse files

KEYS: encrypted: fix race causing incorrect HMAC calculations 



The encrypted-keys module was using a single global HMAC transform,
which could be rekeyed by multiple threads concurrently operating on
different keys, causing incorrect HMAC values to be calculated.  Fix
this by allocating a new HMAC transform whenever we need to calculate a
HMAC.  Also simplify things a bit by allocating the shash_desc's using
SHASH_DESC_ON_STACK() for both the HMAC and unkeyed hashes.

The following script reproduces the bug:

    keyctl new_session
    keyctl add user master "abcdefghijklmnop" @s
    for i in $(seq 2); do
        (
            set -e
            for j in $(seq 1000); do
                keyid=$(keyctl add encrypted desc$i "new user:master 25" @s)
                datablob="$(keyctl pipe $keyid)"
                keyctl unlink $keyid > /dev/null
                keyid=$(keyctl add encrypted desc$i "load $datablob" @s)
                keyctl unlink $keyid > /dev/null
            done
        ) &
    done

Output with bug:

    [  439.691094] encrypted_key: bad hmac (-22)
    add_key: Invalid argument
    add_key: Invalid argument

Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
parent 794b4bc2

security/keys/encrypted-keys/encrypted.c

+32 −83
Original line number Diff line number Diff line
@@ -54,13 +54,7 @@ static int blksize; 
#define MAX_DATA_SIZE 4096

#define MIN_DATA_SIZE  20



struct sdesc {

	struct shash_desc shash;

	char ctx[];

};



static struct crypto_shash *hashalg;

static struct crypto_shash *hmacalg;

static struct crypto_shash *hash_tfm;



enum {

	Opt_err = -1, Opt_new, Opt_load, Opt_update
@@ -320,53 +314,38 @@ static struct key *request_user_key(const char *master_desc, const u8 **master_k 
	return ukey;

}



static struct sdesc *alloc_sdesc(struct crypto_shash *alg)

{

	struct sdesc *sdesc;

	int size;



	size = sizeof(struct shash_desc) + crypto_shash_descsize(alg);

	sdesc = kmalloc(size, GFP_KERNEL);

	if (!sdesc)

		return ERR_PTR(-ENOMEM);

	sdesc->shash.tfm = alg;

	sdesc->shash.flags = 0x0;

	return sdesc;

}



static int calc_hmac(u8 *digest, const u8 *key, unsigned int keylen,

static int calc_hash(struct crypto_shash *tfm, u8 *digest,

		     const u8 *buf, unsigned int buflen)

{

	struct sdesc *sdesc;

	int ret;

	SHASH_DESC_ON_STACK(desc, tfm);

	int err;



	sdesc = alloc_sdesc(hmacalg);

	if (IS_ERR(sdesc)) {

		pr_info("encrypted_key: can't alloc %s\n", hmac_alg);

		return PTR_ERR(sdesc);

	}

	desc->tfm = tfm;

	desc->flags = 0;



	ret = crypto_shash_setkey(hmacalg, key, keylen);

	if (!ret)

		ret = crypto_shash_digest(&sdesc->shash, buf, buflen, digest);

	kfree(sdesc);

	return ret;

	err = crypto_shash_digest(desc, buf, buflen, digest);

	shash_desc_zero(desc);

	return err;

}



static int calc_hash(u8 *digest, const u8 *buf, unsigned int buflen)

static int calc_hmac(u8 *digest, const u8 *key, unsigned int keylen,

		     const u8 *buf, unsigned int buflen)

{

	struct sdesc *sdesc;

	int ret;

	struct crypto_shash *tfm;

	int err;



	sdesc = alloc_sdesc(hashalg);

	if (IS_ERR(sdesc)) {

		pr_info("encrypted_key: can't alloc %s\n", hash_alg);

		return PTR_ERR(sdesc);

	tfm = crypto_alloc_shash(hmac_alg, 0, CRYPTO_ALG_ASYNC);

	if (IS_ERR(tfm)) {

		pr_err("encrypted_key: can't alloc %s transform: %ld\n",

		       hmac_alg, PTR_ERR(tfm));

		return PTR_ERR(tfm);

	}



	ret = crypto_shash_digest(&sdesc->shash, buf, buflen, digest);

	kfree(sdesc);

	return ret;

	err = crypto_shash_setkey(tfm, key, keylen);

	if (!err)

		err = calc_hash(tfm, digest, buf, buflen);

	crypto_free_shash(tfm);

	return err;

}



enum derived_key_type { ENC_KEY, AUTH_KEY };
@@ -394,7 +373,7 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type, 


	memcpy(derived_buf + strlen(derived_buf) + 1, master_key,

	       master_keylen);

	ret = calc_hash(derived_key, derived_buf, derived_buf_len);

	ret = calc_hash(hash_tfm, derived_key, derived_buf, derived_buf_len);

	kfree(derived_buf);

	return ret;

}
@@ -998,47 +977,17 @@ struct key_type key_type_encrypted = { 
};

EXPORT_SYMBOL_GPL(key_type_encrypted);



static void encrypted_shash_release(void)

{

	if (hashalg)

		crypto_free_shash(hashalg);

	if (hmacalg)

		crypto_free_shash(hmacalg);

}



static int __init encrypted_shash_alloc(void)

static int __init init_encrypted(void)

{

	int ret;



	hmacalg = crypto_alloc_shash(hmac_alg, 0, CRYPTO_ALG_ASYNC);

	if (IS_ERR(hmacalg)) {

		pr_info("encrypted_key: could not allocate crypto %s\n",

			hmac_alg);

		return PTR_ERR(hmacalg);

	hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);

	if (IS_ERR(hash_tfm)) {

		pr_err("encrypted_key: can't allocate %s transform: %ld\n",

		       hash_alg, PTR_ERR(hash_tfm));

		return PTR_ERR(hash_tfm);

	}



	hashalg = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC);

	if (IS_ERR(hashalg)) {

		pr_info("encrypted_key: could not allocate crypto %s\n",

			hash_alg);

		ret = PTR_ERR(hashalg);

		goto hashalg_fail;

	}



	return 0;



hashalg_fail:

	crypto_free_shash(hmacalg);

	return ret;

}



static int __init init_encrypted(void)

{

	int ret;



	ret = encrypted_shash_alloc();

	if (ret < 0)

		return ret;

	ret = aes_get_sizes();

	if (ret < 0)

		goto out;
@@ -1047,14 +996,14 @@ static int __init init_encrypted(void) 
		goto out;

	return 0;

out:

	encrypted_shash_release();

	crypto_free_shash(hash_tfm);

	return ret;



}



static void __exit cleanup_encrypted(void)

{

	encrypted_shash_release();

	crypto_free_shash(hash_tfm);

	unregister_key_type(&key_type_encrypted);

}