| 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: <13986065.EbkBp8M36a@tachyon.chronox.de>
Date: Fri, 13 Mar 2015 22:09:21 +0100
From: Stephan Mueller <smueller@...onox.de>
To: 'Herbert Xu <herbert@...dor.apana.org.au>,
linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC PATCH] crypto: prevent helper ciphers from being allocated by users
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))
+ 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