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-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html