diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2017-06-11 16:17:29 -0700 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2017-06-11 16:17:29 -0700 |
commit | 32627645e9e89b58e0e2fcf6f5f6755b9bd046bb (patch) | |
tree | 7d1434ee7ce112933f9c69ddbb18e4454869d946 | |
parent | 6d53cefb18e4646fb4bf62ccb6098fb3808486df (diff) | |
parent | 92347cfd62c174ab91ad97dd4bfbaa1d4aa28e67 (diff) |
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security
Pull key subsystem fixes from James Morris:
"Here are a bunch of fixes for Linux keyrings, including:
- Fix up the refcount handling now that key structs use the
refcount_t type and the refcount_t ops don't allow a 0->1
transition.
- Fix a potential NULL deref after error in x509_cert_parse().
- Don't put data for the crypto algorithms to use on the stack.
- Fix the handling of a null payload being passed to add_key().
- Fix incorrect cleanup an uninitialised key_preparsed_payload in
key_update().
- Explicit sanitisation of potentially secure data before freeing.
- Fixes for the Diffie-Helman code"
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (23 commits)
KEYS: fix refcount_inc() on zero
KEYS: Convert KEYCTL_DH_COMPUTE to use the crypto KPP API
crypto : asymmetric_keys : verify_pefile:zero memory content before freeing
KEYS: DH: add __user annotations to keyctl_kdf_params
KEYS: DH: ensure the KDF counter is properly aligned
KEYS: DH: don't feed uninitialized "otherinfo" into KDF
KEYS: DH: forbid using digest_null as the KDF hash
KEYS: sanitize key structs before freeing
KEYS: trusted: sanitize all key material
KEYS: encrypted: sanitize all key material
KEYS: user_defined: sanitize key payloads
KEYS: sanitize add_key() and keyctl() key payloads
KEYS: fix freeing uninitialized memory in key_update()
KEYS: fix dereferencing NULL payload with nonzero length
KEYS: encrypted: use constant-time HMAC comparison
KEYS: encrypted: fix race causing incorrect HMAC calculations
KEYS: encrypted: fix buffer overread in valid_master_desc()
KEYS: encrypted: avoid encrypting/decrypting stack buffers
KEYS: put keyring if install_session_keyring_to_cred() fails
KEYS: Delete an error message for a failed memory allocation in get_derived_key()
...
-rw-r--r-- | arch/arm64/Kconfig | 4 | ||||
-rw-r--r-- | arch/powerpc/Kconfig | 5 | ||||
-rw-r--r-- | arch/s390/Kconfig | 3 | ||||
-rw-r--r-- | arch/sparc/Kconfig | 3 | ||||
-rw-r--r-- | arch/x86/Kconfig | 4 | ||||
-rw-r--r-- | crypto/asymmetric_keys/verify_pefile.c | 4 | ||||
-rw-r--r-- | crypto/asymmetric_keys/x509_cert_parser.c | 1 | ||||
-rw-r--r-- | include/linux/key.h | 1 | ||||
-rw-r--r-- | include/uapi/linux/keyctl.h | 4 | ||||
-rw-r--r-- | security/keys/Kconfig | 6 | ||||
-rw-r--r-- | security/keys/dh.c | 300 | ||||
-rw-r--r-- | security/keys/encrypted-keys/encrypted.c | 204 | ||||
-rw-r--r-- | security/keys/gc.c | 4 | ||||
-rw-r--r-- | security/keys/key.c | 16 | ||||
-rw-r--r-- | security/keys/keyctl.c | 16 | ||||
-rw-r--r-- | security/keys/keyring.c | 12 | ||||
-rw-r--r-- | security/keys/process_keys.c | 7 | ||||
-rw-r--r-- | security/keys/trusted.c | 50 | ||||
-rw-r--r-- | security/keys/user_defined.c | 16 |
19 files changed, 330 insertions, 330 deletions
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 3dcd7ec69bca..b2024db225a9 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1084,10 +1084,6 @@ config SYSVIPC_COMPAT def_bool y depends on COMPAT && SYSVIPC -config KEYS_COMPAT - def_bool y - depends on COMPAT && KEYS - endmenu menu "Power management options" diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 964da1891ea9..bf4391d18923 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -1199,11 +1199,6 @@ source "arch/powerpc/Kconfig.debug" source "security/Kconfig" -config KEYS_COMPAT - bool - depends on COMPAT && KEYS - default y - source "crypto/Kconfig" config PPC_LIB_RHEAP diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index e161fafb495b..6967addc6a89 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -363,9 +363,6 @@ config COMPAT config SYSVIPC_COMPAT def_bool y if COMPAT && SYSVIPC -config KEYS_COMPAT - def_bool y if COMPAT && KEYS - config SMP def_bool y prompt "Symmetric multi-processing support" diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index b558c9e29de3..5639c9fe5b55 100644 --- a/arch/sparc/Kconfig +++ b/arch/sparc/Kconfig @@ -577,9 +577,6 @@ config SYSVIPC_COMPAT depends on COMPAT && SYSVIPC default y -config KEYS_COMPAT - def_bool y if COMPAT && KEYS - endmenu source "net/Kconfig" diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 4ccfacc7232a..0efb4c9497bc 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2776,10 +2776,6 @@ config COMPAT_FOR_U64_ALIGNMENT config SYSVIPC_COMPAT def_bool y depends on SYSVIPC - -config KEYS_COMPAT - def_bool y - depends on KEYS endif endmenu diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c index 672a94c2c3ff..d178650fd524 100644 --- a/crypto/asymmetric_keys/verify_pefile.c +++ b/crypto/asymmetric_keys/verify_pefile.c @@ -381,7 +381,7 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen, } error: - kfree(desc); + kzfree(desc); error_no_desc: crypto_free_shash(tfm); kleave(" = %d", ret); @@ -450,6 +450,6 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen, ret = pefile_digest_pe(pebuf, pelen, &ctx); error: - kfree(ctx.digest); + kzfree(ctx.digest); return ret; } diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index c80765b211cf..dd03fead1ca3 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -102,6 +102,7 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen) } } + ret = -ENOMEM; cert->pub->key = kmemdup(ctx->key, ctx->key_size, GFP_KERNEL); if (!cert->pub->key) goto error_decode; diff --git a/include/linux/key.h b/include/linux/key.h index 0c9b93b0d1f7..78e25aabedaf 100644 --- a/include/linux/key.h +++ b/include/linux/key.h @@ -173,7 +173,6 @@ struct key { #ifdef KEY_DEBUGGING unsigned magic; #define KEY_DEBUG_MAGIC 0x18273645u -#define KEY_DEBUG_MAGIC_X 0xf8e9dacbu #endif unsigned long flags; /* status flags (change with bitops) */ diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h index 201c6644b237..ef16df06642a 100644 --- a/include/uapi/linux/keyctl.h +++ b/include/uapi/linux/keyctl.h @@ -70,8 +70,8 @@ struct keyctl_dh_params { }; struct keyctl_kdf_params { - char *hashname; - char *otherinfo; + char __user *hashname; + char __user *otherinfo; __u32 otherinfolen; __u32 __spare[8]; }; diff --git a/security/keys/Kconfig b/security/keys/Kconfig index 6fd95f76bfae..a7a23b5541f8 100644 --- a/security/keys/Kconfig +++ b/security/keys/Kconfig @@ -20,6 +20,10 @@ config KEYS If you are unsure as to whether this is required, answer N. +config KEYS_COMPAT + def_bool y + depends on COMPAT && KEYS + config PERSISTENT_KEYRINGS bool "Enable register of persistent per-UID keyrings" depends on KEYS @@ -89,9 +93,9 @@ config ENCRYPTED_KEYS config KEY_DH_OPERATIONS bool "Diffie-Hellman operations on retained keys" depends on KEYS - select MPILIB select CRYPTO select CRYPTO_HASH + select CRYPTO_DH help This option provides support for calculating Diffie-Hellman public keys and shared secrets using values stored as keys diff --git a/security/keys/dh.c b/security/keys/dh.c index e603bd912e4c..4755d4b4f945 100644 --- a/security/keys/dh.c +++ b/security/keys/dh.c @@ -8,34 +8,17 @@ * 2 of the License, or (at your option) any later version. */ -#include <linux/mpi.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/scatterlist.h> #include <linux/crypto.h> #include <crypto/hash.h> +#include <crypto/kpp.h> +#include <crypto/dh.h> #include <keys/user-type.h> #include "internal.h" -/* - * Public key or shared secret generation function [RFC2631 sec 2.1.1] - * - * ya = g^xa mod p; - * or - * ZZ = yb^xa mod p; - * - * where xa is the local private key, ya is the local public key, g is - * the generator, p is the prime, yb is the remote public key, and ZZ - * is the shared secret. - * - * Both are the same calculation, so g or yb are the "base" and ya or - * ZZ are the "result". - */ -static int do_dh(MPI result, MPI base, MPI xa, MPI p) -{ - return mpi_powm(result, base, xa, p); -} - -static ssize_t mpi_from_key(key_serial_t keyid, size_t maxlen, MPI *mpi) +static ssize_t dh_data_from_key(key_serial_t keyid, void **data) { struct key *key; key_ref_t key_ref; @@ -56,19 +39,17 @@ static ssize_t mpi_from_key(key_serial_t keyid, size_t maxlen, MPI *mpi) status = key_validate(key); if (status == 0) { const struct user_key_payload *payload; + uint8_t *duplicate; payload = user_key_payload_locked(key); - if (maxlen == 0) { - *mpi = NULL; + duplicate = kmemdup(payload->data, payload->datalen, + GFP_KERNEL); + if (duplicate) { + *data = duplicate; ret = payload->datalen; - } else if (payload->datalen <= maxlen) { - *mpi = mpi_read_raw_data(payload->data, - payload->datalen); - if (*mpi) - ret = payload->datalen; } else { - ret = -EINVAL; + ret = -ENOMEM; } } up_read(&key->sem); @@ -79,6 +60,29 @@ error: return ret; } +static void dh_free_data(struct dh *dh) +{ + kzfree(dh->key); + kzfree(dh->p); + kzfree(dh->g); +} + +struct dh_completion { + struct completion completion; + int err; +}; + +static void dh_crypto_done(struct crypto_async_request *req, int err) +{ + struct dh_completion *compl = req->data; + + if (err == -EINPROGRESS) + return; + + compl->err = err; + complete(&compl->completion); +} + struct kdf_sdesc { struct shash_desc shash; char ctx[]; @@ -89,6 +93,7 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname) struct crypto_shash *tfm; struct kdf_sdesc *sdesc; int size; + int err; /* allocate synchronous hash */ tfm = crypto_alloc_shash(hashname, 0, 0); @@ -97,16 +102,25 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname) return PTR_ERR(tfm); } + err = -EINVAL; + if (crypto_shash_digestsize(tfm) == 0) + goto out_free_tfm; + + err = -ENOMEM; size = sizeof(struct shash_desc) + crypto_shash_descsize(tfm); sdesc = kmalloc(size, GFP_KERNEL); if (!sdesc) - return -ENOMEM; + goto out_free_tfm; sdesc->shash.tfm = tfm; sdesc->shash.flags = 0x0; *sdesc_ret = sdesc; return 0; + +out_free_tfm: + crypto_free_shash(tfm); + return err; } static void kdf_dealloc(struct kdf_sdesc *sdesc) @@ -120,14 +134,6 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc) kzfree(sdesc); } -/* convert 32 bit integer into its string representation */ -static inline void crypto_kw_cpu_to_be32(u32 val, u8 *buf) -{ - __be32 *a = (__be32 *)buf; - - *a = cpu_to_be32(val); -} - /* * Implementation of the KDF in counter mode according to SP800-108 section 5.1 * as well as SP800-56A section 5.8.1 (Single-step KDF). @@ -138,25 +144,39 @@ static inline void crypto_kw_cpu_to_be32(u32 val, u8 *buf) * 5.8.1.2). */ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, - u8 *dst, unsigned int dlen) + u8 *dst, unsigned int dlen, unsigned int zlen) { struct shash_desc *desc = &sdesc->shash; unsigned int h = crypto_shash_digestsize(desc->tfm); int err = 0; u8 *dst_orig = dst; - u32 i = 1; - u8 iteration[sizeof(u32)]; + __be32 counter = cpu_to_be32(1); while (dlen) { err = crypto_shash_init(desc); if (err) goto err; - crypto_kw_cpu_to_be32(i, iteration); - err = crypto_shash_update(desc, iteration, sizeof(u32)); + err = crypto_shash_update(desc, (u8 *)&counter, sizeof(__be32)); if (err) goto err; + if (zlen && h) { + u8 tmpbuffer[h]; + size_t chunk = min_t(size_t, zlen, h); + memset(tmpbuffer, 0, chunk); + + do { + err = crypto_shash_update(desc, tmpbuffer, + chunk); + if (err) + goto err; + + zlen -= chunk; + chunk = min_t(size_t, zlen, h); + } while (zlen); + } + if (src && slen) { err = crypto_shash_update(desc, src, slen); if (err) @@ -179,7 +199,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, dlen -= h; dst += h; - i++; + counter = cpu_to_be32(be32_to_cpu(counter) + 1); } } @@ -192,7 +212,7 @@ err: static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, char __user *buffer, size_t buflen, - uint8_t *kbuf, size_t kbuflen) + uint8_t *kbuf, size_t kbuflen, size_t lzero) { uint8_t *outbuf = NULL; int ret; @@ -203,7 +223,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, goto err; } - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, buflen); + ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, buflen, lzero); if (ret) goto err; @@ -221,21 +241,26 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params, struct keyctl_kdf_params *kdfcopy) { long ret; - MPI base, private, prime, result; - unsigned nbytes; + ssize_t dlen; + int secretlen; + int outlen; struct keyctl_dh_params pcopy; - uint8_t *kbuf; - ssize_t keylen; - size_t resultlen; + struct dh dh_inputs; + struct scatterlist outsg; + struct dh_completion compl; + struct crypto_kpp *tfm; + struct kpp_request *req; + uint8_t *secret; + uint8_t *outbuf; struct kdf_sdesc *sdesc = NULL; if (!params || (!buffer && buflen)) { ret = -EINVAL; - goto out; + goto out1; } if (copy_from_user(&pcopy, params, sizeof(pcopy)) != 0) { ret = -EFAULT; - goto out; + goto out1; } if (kdfcopy) { @@ -244,104 +269,147 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params, if (buflen > KEYCTL_KDF_MAX_OUTPUT_LEN || kdfcopy->otherinfolen > KEYCTL_KDF_MAX_OI_LEN) { ret = -EMSGSIZE; - goto out; + goto out1; } /* get KDF name string */ hashname = strndup_user(kdfcopy->hashname, CRYPTO_MAX_ALG_NAME); if (IS_ERR(hashname)) { ret = PTR_ERR(hashname); - goto out; + goto out1; } /* allocate KDF from the kernel crypto API */ ret = kdf_alloc(&sdesc, hashname); kfree(hashname); if (ret) - goto out; + goto out1; } - /* - * If the caller requests postprocessing with a KDF, allow an - * arbitrary output buffer size since the KDF ensures proper truncation. - */ - keylen = mpi_from_key(pcopy.prime, kdfcopy ? SIZE_MAX : buflen, &prime); - if (keylen < 0 || !prime) { - /* buflen == 0 may be used to query the required buffer size, - * which is the prime key length. - */ - ret = keylen; - goto out; + memset(&dh_inputs, 0, sizeof(dh_inputs)); + + dlen = dh_data_from_key(pcopy.prime, &dh_inputs.p); + if (dlen < 0) { + ret = dlen; + goto out1; + } + dh_inputs.p_size = dlen; + + dlen = dh_data_from_key(pcopy.base, &dh_inputs.g); + if (dlen < 0) { + ret = dlen; + goto out2; } + dh_inputs.g_size = dlen; - /* The result is never longer than the prime */ - resultlen = keylen; + dlen = dh_data_from_key(pcopy.private, &dh_inputs.key); + if (dlen < 0) { + ret = dlen; + goto out2; + } + dh_inputs.key_size = dlen; - keylen = mpi_from_key(pcopy.base, SIZE_MAX, &base); - if (keylen < 0 || !base) { - ret = keylen; - goto error1; + secretlen = crypto_dh_key_len(&dh_inputs); + secret = kmalloc(secretlen, GFP_KERNEL); + if (!secret) { + ret = -ENOMEM; + goto out2; } + ret = crypto_dh_encode_key(secret, secretlen, &dh_inputs); + if (ret) + goto out3; - keylen = mpi_from_key(pcopy.private, SIZE_MAX, &private); - if (keylen < 0 || !private) { - ret = keylen; - goto error2; + tfm = crypto_alloc_kpp("dh", CRYPTO_ALG_TYPE_KPP, 0); + if (IS_ERR(tfm)) { + ret = PTR_ERR(tfm); + goto out3; + } + + ret = crypto_kpp_set_secret(tfm, secret, secretlen); + if (ret) + goto out4; + + outlen = crypto_kpp_maxsize(tfm); + + if (!kdfcopy) { + /* + * When not using a KDF, buflen 0 is used to read the + * required buffer length + */ + if (buflen == 0) { + ret = outlen; + goto out4; + } else if (outlen > buflen) { + ret = -EOVERFLOW; + goto out4; + } } - result = mpi_alloc(0); - if (!result) { + outbuf = kzalloc(kdfcopy ? (outlen + kdfcopy->otherinfolen) : outlen, + GFP_KERNEL); + if (!outbuf) { ret = -ENOMEM; - goto error3; + goto out4; } - /* allocate space for DH shared secret and SP800-56A otherinfo */ - kbuf = kmalloc(kdfcopy ? (resultlen + kdfcopy->otherinfolen) : resultlen, - GFP_KERNEL); - if (!kbuf) { + sg_init_one(&outsg, outbuf, outlen); + + req = kpp_request_alloc(tfm, GFP_KERNEL); + if (!req) { ret = -ENOMEM; - goto error4; + goto out5; } + kpp_request_set_input(req, NULL, 0); + kpp_request_set_output(req, &outsg, outlen); + init_completion(&compl.completion); + kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG | + CRYPTO_TFM_REQ_MAY_SLEEP, + dh_crypto_done, &compl); + /* - * Concatenate SP800-56A otherinfo past DH shared secret -- the - * input to the KDF is (DH shared secret || otherinfo) + * For DH, generate_public_key and generate_shared_secret are + * the same calculation */ - if (kdfcopy && kdfcopy->otherinfo && - copy_from_user(kbuf + resultlen, kdfcopy->otherinfo, - kdfcopy->otherinfolen) != 0) { - ret = -EFAULT; - goto error5; + ret = crypto_kpp_generate_public_key(req); + if (ret == -EINPROGRESS) { + wait_for_completion(&compl.completion); + ret = compl.err; + if (ret) + goto out6; } - ret = do_dh(result, base, private, prime); - if (ret) - goto error5; - - ret = mpi_read_buffer(result, kbuf, resultlen, &nbytes, NULL); - if (ret != 0) - goto error5; - if (kdfcopy) { - ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, kbuf, - resultlen + kdfcopy->otherinfolen); - } else { - ret = nbytes; - if (copy_to_user(buffer, kbuf, nbytes) != 0) + /* + * Concatenate SP800-56A otherinfo past DH shared secret -- the + * input to the KDF is (DH shared secret || otherinfo) + */ + if (copy_from_user(outbuf + req->dst_len, kdfcopy->otherinfo, + kdfcopy->otherinfolen) != 0) { ret = -EFAULT; + goto out6; + } + + ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf, + req->dst_len + kdfcopy->otherinfolen, + outlen - req->dst_len); + } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) { + ret = req->dst_len; + } else { + ret = -EFAULT; } -error5: - kzfree(kbuf); -error4: - mpi_free(result); -error3: - mpi_free(private); -error2: - mpi_free(base); -error1: - mpi_free(prime); -out: +out6: + kpp_request_free(req); +out5: + kzfree(outbuf); +out4: + crypto_free_kpp(tfm); +out3: + kzfree(secret); +out2: + dh_free_data(&dh_inputs); +out1: kdf_dealloc(sdesc); return ret; } diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 0010955d7876..bb6324d1ccec 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -30,6 +30,7 @@ #include <linux/scatterlist.h> #include <linux/ctype.h> #include <crypto/aes.h> +#include <crypto/algapi.h> #include <crypto/hash.h> #include <crypto/sha.h> #include <crypto/skcipher.h> @@ -54,13 +55,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 @@ -141,23 +136,22 @@ static int valid_ecryptfs_desc(const char *ecryptfs_desc) */ static int valid_master_desc(const char *new_desc, const char *orig_desc) { - if (!memcmp(new_desc, KEY_TRUSTED_PREFIX, KEY_TRUSTED_PREFIX_LEN)) { - if (strlen(new_desc) == KEY_TRUSTED_PREFIX_LEN) - goto out; - if (orig_desc) - if (memcmp(new_desc, orig_desc, KEY_TRUSTED_PREFIX_LEN)) - goto out; - } else if (!memcmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN)) { - if (strlen(new_desc) == KEY_USER_PREFIX_LEN) - goto out; - if (orig_desc) - if (memcmp(new_desc, orig_desc, KEY_USER_PREFIX_LEN)) - goto out; - } else - goto out; + int prefix_len; + + if (!strncmp(new_desc, KEY_TRUSTED_PREFIX, KEY_TRUSTED_PREFIX_LEN)) + prefix_len = KEY_TRUSTED_PREFIX_LEN; + else if (!strncmp(new_desc, KEY_USER_PREFIX, KEY_USER_PREFIX_LEN)) + prefix_len = KEY_USER_PREFIX_LEN; + else + return -EINVAL; + + if (!new_desc[prefix_len]) + return -EINVAL; + + if (orig_desc && strncmp(new_desc, orig_desc, prefix_len)) + return -EINVAL; + return 0; -out: - return -EINVAL; } /* @@ -321,53 +315,38 @@ error: 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 }; @@ -385,10 +364,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type, derived_buf_len = HASH_SIZE; derived_buf = kzalloc(derived_buf_len, GFP_KERNEL); - if (!derived_buf) { - pr_err("encrypted_key: out of memory\n"); + if (!derived_buf) return -ENOMEM; - } + if (key_type) strcpy(derived_buf, "AUTH_KEY"); else @@ -396,8 +374,8 @@ 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); - kfree(derived_buf); + ret = calc_hash(hash_tfm, derived_key, derived_buf, derived_buf_len); + kzfree(derived_buf); return ret; } @@ -480,12 +458,9 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, struct skcipher_request *req; unsigned int encrypted_datalen; u8 iv[AES_BLOCK_SIZE]; - unsigned int padlen; - char pad[16]; int ret; encrypted_datalen = roundup(epayload->decrypted_datalen, blksize); - padlen = encrypted_datalen - epayload->decrypted_datalen; req = init_skcipher_req(derived_key, derived_keylen); ret = PTR_ERR(req); @@ -493,11 +468,10 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload, goto out; dump_decrypted_data(epayload); - memset(pad, 0, sizeof pad); sg_init_table(sg_in, 2); sg_set_buf(&sg_in[0], epayload->decrypted_data, epayload->decrypted_datalen); - sg_set_buf(&sg_in[1], pad, padlen); + sg_set_page(&sg_in[1], ZERO_PAGE(0), AES_BLOCK_SIZE, 0); sg_init_table(sg_out, 1); sg_set_buf(sg_out, epayload->encrypted_data, encrypted_datalen); @@ -533,6 +507,7 @@ static int datablob_hmac_append(struct encrypted_key_payload *epayload, if (!ret) dump_hmac(NULL, digest, HASH_SIZE); out: + memzero_explicit(derived_key, sizeof(derived_key)); return ret; } @@ -561,8 +536,8 @@ static int datablob_hmac_verify(struct encrypted_key_payload *epayload, ret = calc_hmac(digest, derived_key, sizeof derived_key, p, len); if (ret < 0) goto out; - ret = memcmp(digest, epayload->format + epayload->datablob_len, - sizeof digest); + ret = crypto_memneq(digest, epayload->format + epayload->datablob_len, + sizeof(digest)); if (ret) { ret = -EINVAL; dump_hmac("datablob", @@ -571,6 +546,7 @@ static int datablob_hmac_verify(struct encrypted_key_payload *epayload, dump_hmac("calc", digest, HASH_SIZE); } out: + memzero_explicit(derived_key, sizeof(derived_key)); return ret; } @@ -584,9 +560,14 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, struct skcipher_request *req; unsigned int encrypted_datalen; u8 iv[AES_BLOCK_SIZE]; - char pad[16]; + u8 *pad; int ret; + /* Throwaway buffer to hold the unused zero padding at the end */ + pad = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL); + if (!pad) + return -ENOMEM; + encrypted_datalen = roundup(epayload->decrypted_datalen, blksize); req = init_skcipher_req(derived_key, derived_keylen); ret = PTR_ERR(req); @@ -594,13 +575,12 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, goto out; dump_encrypted_data(epayload, encrypted_datalen); - memset(pad, 0, sizeof pad); sg_init_table(sg_in, 1); sg_init_table(sg_out, 2); sg_set_buf(sg_in, epayload->encrypted_data, encrypted_datalen); sg_set_buf(&sg_out[0], epayload->decrypted_data, epayload->decrypted_datalen); - sg_set_buf(&sg_out[1], pad, sizeof pad); + sg_set_buf(&sg_out[1], pad, AES_BLOCK_SIZE); memcpy(iv, epayload->iv, sizeof(iv)); skcipher_request_set_crypt(req, sg_in, sg_out, encrypted_datalen, iv); @@ -612,6 +592,7 @@ static int derived_key_decrypt(struct encrypted_key_payload *epayload, goto out; dump_decrypted_data(epayload); out: + kfree(pad); return ret; } @@ -722,6 +703,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload, out: up_read(&mkey->sem); key_put(mkey); + memzero_explicit(derived_key, sizeof(derived_key)); return ret; } @@ -828,13 +810,13 @@ static int encrypted_instantiate(struct key *key, ret = encrypted_init(epayload, key->description, format, master_desc, decrypted_datalen, hex_encoded_iv); if (ret < 0) { - kfree(epayload); + kzfree(epayload); goto out; } rcu_assign_keypointer(key, epayload); out: - kfree(datablob); + kzfree(datablob); return ret; } @@ -843,8 +825,7 @@ static void encrypted_rcu_free(struct rcu_head *rcu) struct encrypted_key_payload *epayload; epayload = container_of(rcu, struct encrypted_key_payload, rcu); - memset(epayload->decrypted_data, 0, epayload->decrypted_datalen); - kfree(epayload); + kzfree(epayload); } /* @@ -902,7 +883,7 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep) rcu_assign_keypointer(key, new_epayload); call_rcu(&epayload->rcu, encrypted_rcu_free); out: - kfree(buf); + kzfree(buf); return ret; } @@ -960,33 +941,26 @@ static long encrypted_read(const struct key *key, char __user *buffer, up_read(&mkey->sem); key_put(mkey); + memzero_explicit(derived_key, sizeof(derived_key)); if (copy_to_user(buffer, ascii_buf, asciiblob_len) != 0) ret = -EFAULT; - kfree(ascii_buf); + kzfree(ascii_buf); return asciiblob_len; out: up_read(&mkey->sem); key_put(mkey); + memzero_explicit(derived_key, sizeof(derived_key)); return ret; } /* - * encrypted_destroy - before freeing the key, clear the decrypted data - * - * Before freeing the key, clear the memory containing the decrypted - * key data. + * encrypted_destroy - clear and free the key's payload */ static void encrypted_destroy(struct key *key) { - struct encrypted_key_payload *epayload = key->payload.data[0]; - - if (!epayload) - return; - - memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen); - kfree(key->payload.data[0]); + kzfree(key->payload.data[0]); } struct key_type key_type_encrypted = { @@ -999,47 +973,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); - } - - 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; + 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); } - 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; @@ -1048,14 +992,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); } diff --git a/security/keys/gc.c b/security/keys/gc.c index 595becc6d0d2..87cb260e4890 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -158,9 +158,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kfree(key->description); -#ifdef KEY_DEBUGGING - key->magic = KEY_DEBUG_MAGIC_X; -#endif + memzero_explicit(key, sizeof(*key)); kmem_cache_free(key_jar, key); } } diff --git a/security/keys/key.c b/security/keys/key.c index 455c04d80bbb..83da68d98b40 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -660,14 +660,11 @@ not_found: goto error; found: - /* pretend it doesn't exist if it is awaiting deletion */ - if (refcount_read(&key->usage) == 0) - goto not_found; - - /* this races with key_put(), but that doesn't matter since key_put() - * doesn't actually change the key + /* A key is allowed to be looked up only if someone still owns a + * reference to it - otherwise it's awaiting the gc. */ - __key_get(key); + if (!refcount_inc_not_zero(&key->usage)) + goto not_found; error: spin_unlock(&key_serial_lock); @@ -966,12 +963,11 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen) /* the key must be writable */ ret = key_permission(key_ref, KEY_NEED_WRITE); if (ret < 0) - goto error; + return ret; /* attempt to update it if supported */ - ret = -EOPNOTSUPP; if (!key->type->update) - goto error; + return -EOPNOTSUPP; memset(&prep, 0, sizeof(prep)); prep.data = payload; diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 447a7d5cee0f..ab0b337c84b4 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -99,7 +99,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, /* pull the payload in if one was supplied */ payload = NULL; - if (_payload) { + if (plen) { ret = -ENOMEM; payload = kvmalloc(plen, GFP_KERNEL); if (!payload) @@ -132,7 +132,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, key_ref_put(keyring_ref); error3: - kvfree(payload); + if (payload) { + memzero_explicit(payload, plen); + kvfree(payload); + } error2: kfree(description); error: @@ -324,7 +327,7 @@ long keyctl_update_key(key_serial_t id, /* pull the payload in if one was supplied */ payload = NULL; - if (_payload) { + if (plen) { ret = -ENOMEM; payload = kmalloc(plen, GFP_KERNEL); if (!payload) @@ -347,7 +350,7 @@ long keyctl_update_key(key_serial_t id, key_ref_put(key_ref); error2: - kfree(payload); + kzfree(payload); error: return ret; } @@ -1093,7 +1096,10 @@ long keyctl_instantiate_key_common(key_serial_t id, keyctl_change_reqkey_auth(NULL); error2: - kvfree(payload); + if (payload) { + memzero_explicit(payload, plen); + kvfree(payload); + } error: return ret; } diff --git a/security/keys/keyring.c b/security/keys/keyring.c index 4d1678e4586f..de81793f9920 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -706,7 +706,7 @@ descend_to_keyring: * Non-keyrings avoid the leftmost branch of the root entirely (root * slots 1-15). */ - ptr = ACCESS_ONCE(keyring->keys.root); + ptr = READ_ONCE(keyring->keys.root); if (!ptr) goto not_this_keyring; @@ -720,7 +720,7 @@ descend_to_keyring: if ((shortcut->index_key[0] & ASSOC_ARRAY_FAN_MASK) != 0) goto not_this_keyring; - ptr = ACCESS_ONCE(shortcut->next_node); + ptr = READ_ONCE(shortcut->next_node); node = assoc_array_ptr_to_node(ptr); goto begin_node; } @@ -740,7 +740,7 @@ descend_to_node: if (assoc_array_ptr_is_shortcut(ptr)) { shortcut = assoc_array_ptr_to_shortcut(ptr); smp_read_barrier_depends(); - ptr = ACCESS_ONCE(shortcut->next_node); + ptr = READ_ONCE(shortcut->next_node); BUG_ON(!assoc_array_ptr_is_node(ptr)); } node = assoc_array_ptr_to_node(ptr); @@ -752,7 +752,7 @@ begin_node: ascend_to_node: /* Go through the slots in a node */ for (; slot < ASSOC_ARRAY_FAN_OUT; slot++) { - ptr = ACCESS_ONCE(node->slots[slot]); + ptr = READ_ONCE(node->slots[slot]); if (assoc_array_ptr_is_meta(ptr) && node->back_pointer) goto descend_to_node; @@ -790,13 +790,13 @@ ascend_to_node: /* We've dealt with all the slots in the current node, so now we need * to ascend to the parent and continue processing there. */ - ptr = ACCESS_ONCE(node->back_pointer); + ptr = READ_ONCE(node->back_pointer); slot = node->parent_slot; if (ptr && assoc_array_ptr_is_shortcut(ptr)) { shortcut = assoc_array_ptr_to_shortcut(ptr); smp_read_barrier_depends(); - ptr = ACCESS_ONCE(shortcut->back_pointer); + ptr = READ_ONCE(shortcut->back_pointer); slot = shortcut->parent_slot; } if (!ptr) diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 2217dfec7996..86bced9fdbdf 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -809,15 +809,14 @@ long join_session_keyring(const char *name) ret = PTR_ERR(keyring); goto error2; } else if (keyring == new->session_keyring) { - key_put(keyring); ret = 0; - goto error2; + goto error3; } /* we've got a keyring - now to install it */ ret = install_session_keyring_to_cred(new, keyring); if (ret < 0) - goto error2; + goto error3; commit_creds(new); mutex_unlock(&key_session_mutex); @@ -827,6 +826,8 @@ long join_session_keyring(const char *name) okay: return ret; +error3: + key_put(keyring); error2: mutex_unlock(&key_session_mutex); error: diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 2ae31c5a87de..435e86e13879 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -70,7 +70,7 @@ static int TSS_sha1(const unsigned char *data, unsigned int datalen, } ret = crypto_shash_digest(&sdesc->shash, data, datalen, digest); - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -114,7 +114,7 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, if (!ret) ret = crypto_shash_final(&sdesc->shash, digest); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -165,7 +165,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key, paramdigest, TPM_NONCE_SIZE, h1, TPM_NONCE_SIZE, h2, 1, &c, 0, 0); out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -246,7 +246,7 @@ static int TSS_checkhmac1(unsigned char *buffer, if (memcmp(testhmac, authdata, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -347,7 +347,7 @@ static int TSS_checkhmac2(unsigned char *buffer, if (memcmp(testhmac2, authdata2, SHA1_DIGEST_SIZE)) ret = -EINVAL; out: - kfree(sdesc); + kzfree(sdesc); return ret; } @@ -564,7 +564,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, *bloblen = storedsize; } out: - kfree(td); + kzfree(td); return ret; } @@ -678,7 +678,7 @@ static int key_seal(struct trusted_key_payload *p, if (ret < 0) pr_info("trusted_key: srkseal failed (%d)\n", ret); - kfree(tb); + kzfree(tb); return ret; } @@ -703,7 +703,7 @@ static int key_unseal(struct trusted_key_payload *p, /* pull migratable flag out of sealed key */ p->migratable = p->key[--p->key_len]; - kfree(tb); + kzfree(tb); return ret; } @@ -1037,12 +1037,12 @@ static int trusted_instantiate(struct key *key, if (!ret && options->pcrlock) ret = pcrlock(options->pcrlock); out: - kfree(datablob); - kfree(options); + kzfree(datablob); + kzfree(options); if (!ret) rcu_assign_keypointer(key, payload); else - kfree(payload); + kzfree(payload); return ret; } @@ -1051,8 +1051,7 @@ static void trusted_rcu_free(struct rcu_head *rcu) struct trusted_key_payload *p; p = container_of(rcu, struct trusted_key_payload, rcu); - memset(p->key, 0, p->key_len); - kfree(p); + kzfree(p); } /* @@ -1094,13 +1093,13 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = datablob_parse(datablob, new_p, new_o); if (ret != Opt_update) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } if (!new_o->keyhandle) { ret = -EINVAL; - kfree(new_p); + kzfree(new_p); goto out; } @@ -1114,22 +1113,22 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) ret = key_seal(new_p, new_o); if (ret < 0) { pr_info("trusted_key: key_seal failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } if (new_o->pcrlock) { ret = pcrlock(new_o->pcrlock); if (ret < 0) { pr_info("trusted_key: pcrlock failed (%d)\n", ret); - kfree(new_p); + kzfree(new_p); goto out; } } rcu_assign_keypointer(key, new_p); call_rcu(&p->rcu, trusted_rcu_free); out: - kfree(datablob); - kfree(new_o); + kzfree(datablob); + kzfree(new_o); return ret; } @@ -1158,24 +1157,19 @@ static long trusted_read(const struct key *key, char __user *buffer, for (i = 0; i < p->blob_len; i++) bufp = hex_byte_pack(bufp, p->blob[i]); if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) { - kfree(ascii_buf); + kzfree(ascii_buf); return -EFAULT; } - kfree(ascii_buf); + kzfree(ascii_buf); return 2 * p->blob_len; } /* - * trusted_destroy - before freeing the key, clear the decrypted data + * trusted_destroy - clear and free the key's payload */ static void trusted_destroy(struct key *key) { - struct trusted_key_payload *p = key->payload.data[0]; - - if (!p) - return; - memset(p->key, 0, p->key_len); - kfree(key->payload.data[0]); + kzfree(key->payload.data[0]); } struct key_type key_type_trusted = { diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c index 26605134f17a..3d8c68eba516 100644 --- a/security/keys/user_defined.c +++ b/security/keys/user_defined.c @@ -86,10 +86,18 @@ EXPORT_SYMBOL_GPL(user_preparse); */ void user_free_preparse(struct key_preparsed_payload *prep) { - kfree(prep->payload.data[0]); + kzfree(prep->payload.data[0]); } EXPORT_SYMBOL_GPL(user_free_preparse); +static void user_free_payload_rcu(struct rcu_head *head) +{ + struct user_key_payload *payload; + + payload = container_of(head, struct user_key_payload, rcu); + kzfree(payload); +} + /* * update a user defined key * - the key's semaphore is write-locked @@ -112,7 +120,7 @@ int user_update(struct key *key, struct key_preparsed_payload *prep) prep->payload.data[0] = NULL; if (zap) - kfree_rcu(zap, rcu); + call_rcu(&zap->rcu, user_free_payload_rcu); return ret; } EXPORT_SYMBOL_GPL(user_update); @@ -130,7 +138,7 @@ void user_revoke(struct key *key) if (upayload) { rcu_assign_keypointer(key, NULL); - kfree_rcu(upayload, rcu); + call_rcu(&upayload->rcu, user_free_payload_rcu); } } @@ -143,7 +151,7 @@ void user_destroy(struct key *key) { struct user_key_payload *upayload = key->payload.data[0]; - kfree(upayload); + kzfree(upayload); } EXPORT_SYMBOL_GPL(user_destroy); |