| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <19820211.DIXnfZpKhF@tachyon.chronox.de>
Date: Sun, 15 Mar 2015 12:49:09 +0100
From: Stephan Mueller <smueller@...onox.de>
To: 'Herbert Xu <herbert@...dor.apana.org.au>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] crypto: prevent helper ciphers from being allocated by users
Am Freitag, 13. März 2015, 22:09:21 schrieb Stephan Mueller:
Hi Stephan,
> Hi,
>
> Several hardware related cipher implementations are implemented as follows:
> a "helper" cipher implementation is registered with the kernel crypto API.
>
> Such helper ciphers are never intended to be called by normal users. In some
> cases, calling them via the normal crypto API may even cause failures
> including kernel crashes. In a normal case, the "wrapping" ciphers that use
> the helpers ensure that these helpers are invoked such that they cannot
> cause any calamity.
>
> Also, with kernel code, we can be reasonably sure that such helper ciphers
> are never called directly as the kernel code is under our control.
>
> But I am getting very uneasy when the AF_ALG user space interface comes into
> play. With that, unprivileged users can call all ciphers registered with
> the crypto API, including these helper ciphers that are not intended to be
> called directly. That means, with AF_ALG user space may invoke these helper
> ciphers and may cause undefined states or side effects.
>
> For example, without the commit 81e397d937a8e9f46f024a9f876cf14d8e2b45a7 the
> AES-NI GCM implementation could be used to crash the kernel with the
> AF_ALG(aead) interface. But without the patch, using the AES-NI GCM
> implementation through the regular cipher types was no problem at all.
>
> To avoid any potential side effects with such helpers, I propose a change to
> the kernel crypto API to prevent the helpers to be called directly. These
> helpers have the following properties:
>
> - they are all marked with a cra_priority of 0 and can therefore be easily
> identified
>
> - they are never intended to be instantiated via the regular crypto_alloc_*
> routines, but always via the crypto_*_spawn API. That API is separate from
> the regular allocation API of crypto_alloc_*
>
> Therefore, a guard to prevent the instantiation of helper ciphers by normal
> users can be done by preventing successful instances of helper ciphers in
> crypto_alloc_*. To make life easy, I would recommend to simply use the
> cra_priority as a flag that shall trigger an error in crypto_alloc_*.
>
> The following code is tested and confirmed to work (i.e. preventing the use
> of helper ciphers by callers, but allowing helper ciphers to be used to
> serve other ciphers). This patch searched for all invocations of
> __crypto_alloc_tfm and added the check for cra_priority except in the
> crypto_spawn_tfm call. Specifically, I tested __driver-gcm-aes-aesni vs
> rfc4106-gcm-aesni. In addition, I tested a large array of other ciphers
> where none were affected by the change.
>
> diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
> index db201bca..2cd83ad 100644
> --- a/crypto/ablkcipher.c
> +++ b/crypto/ablkcipher.c
> @@ -688,7 +688,7 @@ struct crypto_ablkcipher *crypto_alloc_ablkcipher(const
> char *alg_name,
> goto err;
> }
>
> - tfm = __crypto_alloc_tfm(alg, type, mask);
> + tfm = __crypto_alloc_tfm_safe(alg, type, mask);
> if (!IS_ERR(tfm))
> return __crypto_ablkcipher_cast(tfm);
>
> diff --git a/crypto/aead.c b/crypto/aead.c
> index 2222710..9ae3aa9 100644
> --- a/crypto/aead.c
> +++ b/crypto/aead.c
> @@ -542,7 +542,7 @@ struct crypto_aead *crypto_alloc_aead(const char
> *alg_name, u32 type, u32 mask)
> goto err;
> }
>
> - tfm = __crypto_alloc_tfm(alg, type, mask);
> + tfm = __crypto_alloc_tfm_safe(alg, type, mask);
> if (!IS_ERR(tfm))
> return __crypto_aead_cast(tfm);
>
> diff --git a/crypto/api.c b/crypto/api.c
> index 2a81e98..8b1bb2d 100644
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -389,6 +389,27 @@ out:
> }
> EXPORT_SYMBOL_GPL(__crypto_alloc_tfm);
>
> +struct crypto_tfm *__crypto_alloc_tfm_safe(struct crypto_alg *alg, u32
> type, + u32 mask)
> +{
> + /*
> + * Prevent all ciphers from being loaded which have a cra_priority
> + * of 0. Those cipher implementations are helper ciphers and
> + * are not intended for general consumption.
> + *
> + * The only exceptions are the compression algorithms which
> + * have no priority.
> + */
> + if (!alg->cra_priority &&
> + ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) !=
> + CRYPTO_ALG_TYPE_PCOMPRESS) &&
> + ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) !=
> + CRYPTO_ALG_TYPE_COMPRESS))
&& !(mask & CRYPTO_ALG_TESTED))
is missing here to allow ciphers to be self tested
> + return ERR_PTR(-ENOENT);
> +
> + return __crypto_alloc_tfm(alg, type, mask);
> +}
> +EXPORT_SYMBOL_GPL(__crypto_alloc_tfm_safe);
> /*
> * crypto_alloc_base - Locate algorithm and allocate transform
> * @alg_name: Name of algorithm
> @@ -425,7 +446,7 @@ struct crypto_tfm *crypto_alloc_base(const char
> *alg_name, u32 type, u32 mask)
> goto err;
> }
>
> - tfm = __crypto_alloc_tfm(alg, type, mask);
> + tfm = __crypto_alloc_tfm_safe(alg, type, mask);
> if (!IS_ERR(tfm))
> return tfm;
>
> diff --git a/crypto/internal.h b/crypto/internal.h
> index bd39bfc..8526a37 100644
> --- a/crypto/internal.h
> +++ b/crypto/internal.h
> @@ -91,6 +91,8 @@ void crypto_remove_final(struct list_head *list);
> void crypto_shoot_alg(struct crypto_alg *alg);
> struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
> u32 mask);
> +struct crypto_tfm *__crypto_alloc_tfm_safe(struct crypto_alg *alg, u32
> type, + u32 mask);
> void *crypto_create_tfm(struct crypto_alg *alg,
> const struct crypto_type *frontend);
> struct crypto_alg *crypto_find_alg(const char *alg_name,
--
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists