Re: [PATCH] crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 5932 bytes --]

On 6/22/26 19:48, Eric Biggers wrote:
> AF_ALG is a frequent source of vulnerabilities and a maintenance
> nightmare.  It exposes far more functionality to userspace than ever
> should have been exposed, especially to unprivileged processes.  Recent
> exploits have targeted kernel internal implementation details like
> "authencesn" that have zero use case for userspace access.
> 
> Fortunately, AF_ALG is rarely used in practice, as userspace crypto
> libraries exist.  And when it is used, only some functionality is known
> to be used, and many users are known to hold capabilities already.
> iwd for example requires CAP_NET_ADMIN and has a known algorithm list
> (https://lore.kernel.org/linux-crypto/bcbbef00-5881-421b-8892-7be6c04b832d@gmail.com/).
> 
> Thus, let's restrict the set of allowed algorithms by default, depending
> on the capabilities held.
> 
> Add a sysctl /proc/sys/crypto/af_alg_restrict with meaning:
> 
>     0: unrestricted
>     1: limited functionality
>     2: completely disabled
> 
> Set the default value to 1, which enables an algorithm allowlist for
> unprivileged processes and a slightly longer allowlist for privileged
> processes.
> 
> Note that the list may be tweaked in the future.  However, the common
> use cases such as iwd and bluez are taken into account already.  I've
> tested that iwd still works with the default value of 1.
I think there is room for something in-between the allowlist provided
here and "no restrictions".  For instance, I think it makes sense
to have a mode that allows modern¸ widely-used algorithms (AES-GCM,
ChaCha20-Poly1305, SHA-3, HMAC, etc) to all users.

This makes it less likely someone turns off all restrictions.

XFRM allows providing an arbitrary algorithm name, and it appears to
be accessible in unprivileged user namespaces.  That also needs an
allowlist.
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 787aac8aeb24..b9217f9086aa 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -32,10 +32,15 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <net/sock.h>
>  
> +static const struct af_alg_allowlist_entry aead_allowlist[] = {
> +	{ "ccm(aes)", true }, /* bluez */
> +	{},
> +};
> +
>  static inline bool aead_sufficient_data(struct sock *sk)
>  {
>  	struct alg_sock *ask = alg_sk(sk);
>  	struct sock *psk = ask->parent;
>  	struct alg_sock *pask = alg_sk(psk);
> @@ -342,10 +347,16 @@ static struct proto_ops algif_aead_ops_nokey = {
>  	.poll		=	af_alg_poll,
>  };
>  
>  static void *aead_bind(const char *name)
>  {
> +	int err;
> +
> +	err = af_alg_check_restriction(name, aead_allowlist);
> +	if (err)
> +		return ERR_PTR(err);
> +
>  	return crypto_alloc_aead(name, 0, AF_ALG_CRYPTOAPI_MASK);
>  }
>  
>  static void aead_release(void *private)
>  {
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index 5452ad6c1506..a8d958d51ece 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -14,10 +14,28 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <net/sock.h>
>  
> +static const struct af_alg_allowlist_entry hash_allowlist[] = {
> +	{ "cmac(aes)", true }, /* iwd, bluez */
> +	{ "hmac(md5)", true }, /* iwd */
> +	{ "hmac(sha1)", true }, /* iwd */
> +	{ "hmac(sha224)", true }, /* iwd */
> +	{ "hmac(sha256)", true }, /* iwd */
> +	{ "hmac(sha384)", true }, /* iwd */
> +	{ "hmac(sha512)", true }, /* iwd, sha512hmac */

Should this entry have privileged = false?  sha512hmac doesn't
need privileges.

> +	{ "md4", true }, /* iwd */
> +	{ "md5", true }, /* iwd */
> +	{ "sha1", false }, /* iwd, iproute2 < 7.0 */
> +	{ "sha224", true }, /* iwd */
> +	{ "sha256", true }, /* iwd */
> +	{ "sha384", true }, /* iwd */
> +	{ "sha512", true }, /* iwd */
> +	{},
> +};
> +
>  struct hash_ctx {
>  	struct af_alg_sgl sgl;
>  
>  	u8 *result;
>  
> @@ -380,10 +398,16 @@ static struct proto_ops algif_hash_ops_nokey = {
>  	.accept		=	hash_accept_nokey,
>  };
>  
>  static void *hash_bind(const char *name)
>  {
> +	int err;
> +
> +	err = af_alg_check_restriction(name, hash_allowlist);
> +	if (err)
> +		return ERR_PTR(err);
> +
>  	return crypto_alloc_ahash(name, 0, AF_ALG_CRYPTOAPI_MASK);
>  }
>  
>  static void hash_release(void *private)
>  {
> diff --git a/crypto/algif_rng.c b/crypto/algif_rng.c
> index 4dfe7899f8fa..bd522915d56d 100644
> --- a/crypto/algif_rng.c
> +++ b/crypto/algif_rng.c
> @@ -48,10 +48,14 @@
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
>  MODULE_DESCRIPTION("User-space interface for random number generators");
>  
> +static const struct af_alg_allowlist_entry rng_allowlist[] = {
> +	{},
> +};

Can this whole file be deleted?  You wrote that it isn't actually used.

(snip)
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index df20bdfe1f1f..2b8069667974 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -32,10 +32,24 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <net/sock.h>
>  
> +static const struct af_alg_allowlist_entry skcipher_allowlist[] = {
> +	{ "adiantum(xchacha12,aes)", false }, /* cryptsetup */
> +	{ "adiantum(xchacha20,aes)", false }, /* cryptsetup */
> +	{ "cbc(aes)", true }, /* iwd */
> +	{ "cbc(des)", true }, /* iwd */
> +	{ "cbc(des3_ede)", true }, /* iwd */
> +	{ "ctr(aes)", true }, /* iwd */
> +	{ "ecb(aes)", true }, /* iwd, bluez */
> +	{ "ecb(des)", true }, /* iwd */
> +	{ "hctr2(aes)", false }, /* cryptsetup */
> +	{ "xts(aes)", false }, /* cryptsetup benchmark */
> +	{},
> +};

Do the cryptsetup ones really need to be accessible to unprivileged users?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]