[PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

when enabled, 1, redact XFRM SA secret in the netlink response to
xfrm_get_sa() or dump all sa.

e.g
echo 1 > /proc/sys/net/core/xfrm_redact_secret
ip xfrm state
src 172.16.1.200 dst 172.16.1.100
	proto esp spi 0x00000002 reqid 2 mode tunnel
	replay-window 0
	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96

the aead secret is redacted.

/proc/sys/core/net/xfrm_redact_secret is a toggle.
Once enabled, either at compile or via proc, it can not be disabled.
Redacting secret is a FIPS 140-2 requirement.

Cc: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 Documentation/networking/xfrm_sysctl.rst |  7 +++
 include/net/netns/xfrm.h                 |  1 +
 net/xfrm/Kconfig                         | 10 ++++
 net/xfrm/xfrm_sysctl.c                   | 20 +++++++
 net/xfrm/xfrm_user.c                     | 76 +++++++++++++++++++++---
 5 files changed, 105 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/xfrm_sysctl.rst b/Documentation/networking/xfrm_sysctl.rst
index 47b9bbdd0179..26432b0ff3ac 100644
--- a/Documentation/networking/xfrm_sysctl.rst
+++ b/Documentation/networking/xfrm_sysctl.rst
@@ -9,3 +9,10 @@ XFRM Syscall
 
 xfrm_acq_expires - INTEGER
 	default 30 - hard timeout in seconds for acquire requests
+
+xfrm_redact_secret - INTEGER
+	A toggle to redact xfrm SA's secret to userspace.
+	When true the kernel, netlink message will redact SA secret
+	to userspace. This is part of FIPS 140-2 requirement.
+	Once the value is set to true, either at compile or at run time,
+	it can not be set to false.
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 59f45b1e9dac..0ca9328daad4 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -64,6 +64,7 @@ struct netns_xfrm {
 	u32			sysctl_aevent_rseqth;
 	int			sysctl_larval_drop;
 	u32			sysctl_acq_expires;
+	u32			sysctl_redact_secret;
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
 #endif
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 5b9a5ab48111..270a4e906a15 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -91,6 +91,16 @@ config XFRM_ESP
 	select CRYPTO_SEQIV
 	select CRYPTO_SHA256
 
+config XFRM_REDACT_SECRET
+	bool "Redact xfrm SA secret in netlink message"
+	depends on SYSCTL
+	default n
+	help
+	  Enable XFRM SA secret redact in the netlink message.
+	  Redacting secret is a FIPS 140-2 requirement.
+	  Once enabled at compile, the value can not be set to false on
+	  a running system.
+
 config XFRM_IPCOMP
 	tristate
 	select XFRM_ALGO
diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
index 0c6c5ef65f9d..a41aa325a478 100644
--- a/net/xfrm/xfrm_sysctl.c
+++ b/net/xfrm/xfrm_sysctl.c
@@ -4,15 +4,25 @@
 #include <net/net_namespace.h>
 #include <net/xfrm.h>
 
+#ifdef CONFIG_SYSCTL
+#ifdef CONFIG_XFRM_REDACT_SECRET
+#define XFRM_REDACT_SECRET  1
+#else
+#define XFRM_REDACT_SECRET  0
+#endif
+#endif
+
 static void __net_init __xfrm_sysctl_init(struct net *net)
 {
 	net->xfrm.sysctl_aevent_etime = XFRM_AE_ETIME;
 	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
 	net->xfrm.sysctl_larval_drop = 1;
 	net->xfrm.sysctl_acq_expires = 30;
+	net->xfrm.sysctl_redact_secret = XFRM_REDACT_SECRET;
 }
 
 #ifdef CONFIG_SYSCTL
+
 static struct ctl_table xfrm_table[] = {
 	{
 		.procname	= "xfrm_aevent_etime",
@@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "xfrm_redact_secret",
+		.maxlen		= sizeof(u32),
+		.mode		= 0644,
+		/* only handle a transition from "0" to "1" */
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1         = SYSCTL_ONE,
+		.extra2         = SYSCTL_ONE,
+	},
 	{}
 };
 
@@ -54,6 +73,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
 	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
 	table[2].data = &net->xfrm.sysctl_larval_drop;
 	table[3].data = &net->xfrm.sysctl_acq_expires;
+	table[4].data = &net->xfrm.sysctl_redact_secret;
 
 	/* Don't export sysctls to unprivileged users */
 	if (net->user_ns != &init_user_ns)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e6cfaa680ef3..a3e89dddea9d 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -848,21 +848,78 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 	return 0;
 }
 
-static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
+static int copy_to_user_auth(u32 redact_secret, struct xfrm_algo_auth *auth,
+			     struct sk_buff *skb)
 {
 	struct xfrm_algo *algo;
+	struct xfrm_algo_auth *ap;
 	struct nlattr *nla;
 
 	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
 			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
 	if (!nla)
 		return -EMSGSIZE;
-
 	algo = nla_data(nla);
 	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
-	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
+
+	if (redact_secret && auth->alg_key_len)
+		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(algo->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
 	algo->alg_key_len = auth->alg_key_len;
 
+	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
+	if (!nla)
+		return -EMSGSIZE;
+	ap = nla_data(nla);
+	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
+	if (redact_secret)
+		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_aead(u32 redact_secret,
+			     struct xfrm_algo_aead *aead, struct sk_buff *skb)
+{
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
+	struct xfrm_algo_aead *ap;
+
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, aead, sizeof(*aead));
+
+	if (redact_secret)
+		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, aead->alg_key,
+		       (aead->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_ealg(u32 redact_secret, struct xfrm_algo *ealg,
+			     struct sk_buff *skb)
+{
+	struct xfrm_algo *ap;
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
+					 xfrm_alg_len(ealg));
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, ealg, sizeof(*ealg));
+
+	if (redact_secret)
+		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, ealg->alg_key,
+		       (ealg->alg_key_len + 7) / 8);
+
 	return 0;
 }
 
@@ -884,6 +941,7 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 				    struct sk_buff *skb)
 {
 	int ret = 0;
+	struct net *net = xs_net(x);
 
 	copy_to_user_state(x, p);
 
@@ -906,20 +964,20 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 			goto out;
 	}
 	if (x->aead) {
-		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead);
+		ret = copy_to_user_aead(net->xfrm.sysctl_redact_secret,
+					x->aead, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->aalg) {
-		ret = copy_to_user_auth(x->aalg, skb);
-		if (!ret)
-			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
-				      xfrm_alg_auth_len(x->aalg), x->aalg);
+		ret = copy_to_user_auth(net->xfrm.sysctl_redact_secret,
+					x->aalg, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->ealg) {
-		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg);
+		ret = copy_to_user_ealg(net->xfrm.sysctl_redact_secret,
+					x->ealg, skb);
 		if (ret)
 			goto out;
 	}
-- 
2.20.1

Re: [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Herbert Xu]

On Tue, Jul 28, 2020 at 05:47:30PM +0200, Antony Antony wrote:
> when enabled, 1, redact XFRM SA secret in the netlink response to
> xfrm_get_sa() or dump all sa.
> 
> e.g
> echo 1 > /proc/sys/net/core/xfrm_redact_secret
> ip xfrm state
> src 172.16.1.200 dst 172.16.1.100
> 	proto esp spi 0x00000002 reqid 2 mode tunnel
> 	replay-window 0
> 	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96
> 
> the aead secret is redacted.
> 
> /proc/sys/core/net/xfrm_redact_secret is a toggle.
> Once enabled, either at compile or via proc, it can not be disabled.
> Redacting secret is a FIPS 140-2 requirement.

Couldn't you use the existing fips_enabled sysctl?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

On Wed, Jul 29, 2020 at 02:22:52 +1000, Herbert Xu wrote:
> On Tue, Jul 28, 2020 at 05:47:30PM +0200, Antony Antony wrote:
> > when enabled, 1, redact XFRM SA secret in the netlink response to
> > xfrm_get_sa() or dump all sa.
> > 
> > e.g
> > echo 1 > /proc/sys/net/core/xfrm_redact_secret
> > ip xfrm state
> > src 172.16.1.200 dst 172.16.1.100
> > 	proto esp spi 0x00000002 reqid 2 mode tunnel
> > 	replay-window 0
> > 	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96
> > 
> > the aead secret is redacted.
> > 
> > /proc/sys/core/net/xfrm_redact_secret is a toggle.
> > Once enabled, either at compile or via proc, it can not be disabled.
> > Redacting secret is a FIPS 140-2 requirement.
> 
> Couldn't you use the existing fips_enabled sysctl?

that could be a step, however, not yet.

Libreswan in FIPS mode with xfrm_redact_secret enabled would work fine, however, enabling xfrm_redact_secret would break Strongswan in FIPS mode. We can add this option fips_enabled once Strongswan does not need SA secret, child_sa->update().

Also there was interest to able to use xfrm_redact_secret independent of FIPS.

I thik for now it best to be ouside fips_enabled.

Re: [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Stephan Mueller]

Am Dienstag, 28. Juli 2020, 17:47:30 CEST schrieb Antony Antony:

Hi Antony,

> when enabled, 1, redact XFRM SA secret in the netlink response to
> xfrm_get_sa() or dump all sa.
> 
> e.g
> echo 1 > /proc/sys/net/core/xfrm_redact_secret
> ip xfrm state
> src 172.16.1.200 dst 172.16.1.100
> 	proto esp spi 0x00000002 reqid 2 mode tunnel
> 	replay-window 0
> 	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96
> 
> the aead secret is redacted.
> 
> /proc/sys/core/net/xfrm_redact_secret is a toggle.
> Once enabled, either at compile or via proc, it can not be disabled.
> Redacting secret is a FIPS 140-2 requirement.
> 
> Cc: Stephan Mueller <smueller@chronox.de>
> Signed-off-by: Antony Antony <antony.antony@secunet.com>
> ---
>  Documentation/networking/xfrm_sysctl.rst |  7 +++
>  include/net/netns/xfrm.h                 |  1 +
>  net/xfrm/Kconfig                         | 10 ++++
>  net/xfrm/xfrm_sysctl.c                   | 20 +++++++
>  net/xfrm/xfrm_user.c                     | 76 +++++++++++++++++++++---
>  5 files changed, 105 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/networking/xfrm_sysctl.rst
> b/Documentation/networking/xfrm_sysctl.rst index 47b9bbdd0179..26432b0ff3ac
> 100644
> --- a/Documentation/networking/xfrm_sysctl.rst
> +++ b/Documentation/networking/xfrm_sysctl.rst
> @@ -9,3 +9,10 @@ XFRM Syscall
> 
>  xfrm_acq_expires - INTEGER
>  	default 30 - hard timeout in seconds for acquire requests
> +
> +xfrm_redact_secret - INTEGER
> +	A toggle to redact xfrm SA's secret to userspace.
> +	When true the kernel, netlink message will redact SA secret
> +	to userspace. This is part of FIPS 140-2 requirement.
> +	Once the value is set to true, either at compile or at run time,
> +	it can not be set to false.
> diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> index 59f45b1e9dac..0ca9328daad4 100644
> --- a/include/net/netns/xfrm.h
> +++ b/include/net/netns/xfrm.h
> @@ -64,6 +64,7 @@ struct netns_xfrm {
>  	u32			sysctl_aevent_rseqth;
>  	int			sysctl_larval_drop;
>  	u32			sysctl_acq_expires;
> +	u32			sysctl_redact_secret;
>  #ifdef CONFIG_SYSCTL
>  	struct ctl_table_header	*sysctl_hdr;
>  #endif
> diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
> index 5b9a5ab48111..270a4e906a15 100644
> --- a/net/xfrm/Kconfig
> +++ b/net/xfrm/Kconfig
> @@ -91,6 +91,16 @@ config XFRM_ESP
>  	select CRYPTO_SEQIV
>  	select CRYPTO_SHA256
> 
> +config XFRM_REDACT_SECRET
> +	bool "Redact xfrm SA secret in netlink message"
> +	depends on SYSCTL
> +	default n
> +	help
> +	  Enable XFRM SA secret redact in the netlink message.
> +	  Redacting secret is a FIPS 140-2 requirement.
> +	  Once enabled at compile, the value can not be set to false on
> +	  a running system.
> +
>  config XFRM_IPCOMP
>  	tristate
>  	select XFRM_ALGO
> diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
> index 0c6c5ef65f9d..a41aa325a478 100644
> --- a/net/xfrm/xfrm_sysctl.c
> +++ b/net/xfrm/xfrm_sysctl.c
> @@ -4,15 +4,25 @@
>  #include <net/net_namespace.h>
>  #include <net/xfrm.h>
> 
> +#ifdef CONFIG_SYSCTL
> +#ifdef CONFIG_XFRM_REDACT_SECRET
> +#define XFRM_REDACT_SECRET  1
> +#else
> +#define XFRM_REDACT_SECRET  0
> +#endif
> +#endif
> +
>  static void __net_init __xfrm_sysctl_init(struct net *net)
>  {
>  	net->xfrm.sysctl_aevent_etime = XFRM_AE_ETIME;
>  	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
>  	net->xfrm.sysctl_larval_drop = 1;
>  	net->xfrm.sysctl_acq_expires = 30;
> +	net->xfrm.sysctl_redact_secret = XFRM_REDACT_SECRET;
>  }
> 
>  #ifdef CONFIG_SYSCTL
> +
>  static struct ctl_table xfrm_table[] = {
>  	{
>  		.procname	= "xfrm_aevent_etime",
> @@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec
>  	},
> +	{
> +		.procname	= "xfrm_redact_secret",
> +		.maxlen		= sizeof(u32),
> +		.mode		= 0644,
> +		/* only handle a transition from "0" to "1" */
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1         = SYSCTL_ONE,
> +		.extra2         = SYSCTL_ONE,
> +	},
>  	{}
>  };
> 
> @@ -54,6 +73,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
>  	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
>  	table[2].data = &net->xfrm.sysctl_larval_drop;
>  	table[3].data = &net->xfrm.sysctl_acq_expires;
> +	table[4].data = &net->xfrm.sysctl_redact_secret;
> 
>  	/* Don't export sysctls to unprivileged users */
>  	if (net->user_ns != &init_user_ns)
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index e6cfaa680ef3..a3e89dddea9d 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -848,21 +848,78 @@ static int copy_user_offload(struct xfrm_state_offload
> *xso, struct sk_buff *skb return 0;
>  }
> 
> -static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff
> *skb) +static int copy_to_user_auth(u32 redact_secret, struct
> xfrm_algo_auth *auth, +			     struct sk_buff *skb)
>  {
>  	struct xfrm_algo *algo;
> +	struct xfrm_algo_auth *ap;
>  	struct nlattr *nla;
> 
>  	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
>  			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
>  	if (!nla)
>  		return -EMSGSIZE;
> -
>  	algo = nla_data(nla);
>  	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
> -	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
> +
> +	if (redact_secret && auth->alg_key_len)
> +		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
> +	else
> +		memcpy(algo->alg_key, auth->alg_key,
> +		       (auth->alg_key_len + 7) / 8);
>  	algo->alg_key_len = auth->alg_key_len;
> 
> +	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
> +	if (!nla)
> +		return -EMSGSIZE;
> +	ap = nla_data(nla);
> +	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
> +	if (redact_secret)

You test for auth->alg_key_len above. Shouldn't there such a check here too?

> +		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
> +	else
> +		memcpy(ap->alg_key, auth->alg_key,
> +		       (auth->alg_key_len + 7) / 8);
> +	return 0;
> +}
> +
> +static int copy_to_user_aead(u32 redact_secret,
> +			     struct xfrm_algo_aead *aead, struct sk_buff *skb)
> +{
> +	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
> +	struct xfrm_algo_aead *ap;
> +
> +	if (!nla)
> +		return -EMSGSIZE;
> +
> +	ap = nla_data(nla);
> +	memcpy(ap, aead, sizeof(*aead));
> +
> +	if (redact_secret)

And here?

> +		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
> +	else
> +		memcpy(ap->alg_key, aead->alg_key,
> +		       (aead->alg_key_len + 7) / 8);
> +	return 0;
> +}
> +
> +static int copy_to_user_ealg(u32 redact_secret, struct xfrm_algo *ealg,
> +			     struct sk_buff *skb)
> +{
> +	struct xfrm_algo *ap;
> +	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
> +					 xfrm_alg_len(ealg));
> +	if (!nla)
> +		return -EMSGSIZE;
> +
> +	ap = nla_data(nla);
> +	memcpy(ap, ealg, sizeof(*ealg));
> +
> +	if (redact_secret)

Here, too?

> +		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
> +	else
> +		memcpy(ap->alg_key, ealg->alg_key,
> +		       (ealg->alg_key_len + 7) / 8);
> +
>  	return 0;
>  }
> 
> @@ -884,6 +941,7 @@ static int copy_to_user_state_extra(struct xfrm_state
> *x, struct sk_buff *skb)
>  {
>  	int ret = 0;
> +	struct net *net = xs_net(x);
> 
>  	copy_to_user_state(x, p);
> 
> @@ -906,20 +964,20 @@ static int copy_to_user_state_extra(struct xfrm_state
> *x, goto out;
>  	}
>  	if (x->aead) {
> -		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x-
>aead);
> +		ret = copy_to_user_aead(net->xfrm.sysctl_redact_secret,
> +					x->aead, skb);
>  		if (ret)
>  			goto out;
>  	}
>  	if (x->aalg) {
> -		ret = copy_to_user_auth(x->aalg, skb);
> -		if (!ret)
> -			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
> -				      xfrm_alg_auth_len(x->aalg), x->aalg);
> +		ret = copy_to_user_auth(net->xfrm.sysctl_redact_secret,
> +					x->aalg, skb);
>  		if (ret)
>  			goto out;
>  	}
>  	if (x->ealg) {
> -		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x-
>ealg);
> +		ret = copy_to_user_ealg(net->xfrm.sysctl_redact_secret,
> +					x->ealg, skb);
>  		if (ret)
>  			goto out;
>  	}


Ciao
Stephan

Re: [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

On Tue, Jul 28, 2020 at 21:09:10 +0200, Stephan Mueller wrote:
> Am Dienstag, 28. Juli 2020, 17:47:30 CEST schrieb Antony Antony:
> 
> Hi Antony,
> 
> > when enabled, 1, redact XFRM SA secret in the netlink response to
> > xfrm_get_sa() or dump all sa.
> > 
> > e.g
> > echo 1 > /proc/sys/net/core/xfrm_redact_secret
> > ip xfrm state
> > src 172.16.1.200 dst 172.16.1.100
> > 	proto esp spi 0x00000002 reqid 2 mode tunnel
> > 	replay-window 0
> > 	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96
> > 
> > the aead secret is redacted.
> > 
> > /proc/sys/core/net/xfrm_redact_secret is a toggle.
> > Once enabled, either at compile or via proc, it can not be disabled.
> > Redacting secret is a FIPS 140-2 requirement.
> > 
> > Cc: Stephan Mueller <smueller@chronox.de>
> > Signed-off-by: Antony Antony <antony.antony@secunet.com>
> > ---
> >  Documentation/networking/xfrm_sysctl.rst |  7 +++
> >  include/net/netns/xfrm.h                 |  1 +
> >  net/xfrm/Kconfig                         | 10 ++++
> >  net/xfrm/xfrm_sysctl.c                   | 20 +++++++
> >  net/xfrm/xfrm_user.c                     | 76 +++++++++++++++++++++---
> >  5 files changed, 105 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/networking/xfrm_sysctl.rst
> > b/Documentation/networking/xfrm_sysctl.rst index 47b9bbdd0179..26432b0ff3ac
> > 100644
> > --- a/Documentation/networking/xfrm_sysctl.rst
> > +++ b/Documentation/networking/xfrm_sysctl.rst
> > @@ -9,3 +9,10 @@ XFRM Syscall
> > 
> >  xfrm_acq_expires - INTEGER
> >  	default 30 - hard timeout in seconds for acquire requests
> > +
> > +xfrm_redact_secret - INTEGER
> > +	A toggle to redact xfrm SA's secret to userspace.
> > +	When true the kernel, netlink message will redact SA secret
> > +	to userspace. This is part of FIPS 140-2 requirement.
> > +	Once the value is set to true, either at compile or at run time,
> > +	it can not be set to false.
> > diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> > index 59f45b1e9dac..0ca9328daad4 100644
> > --- a/include/net/netns/xfrm.h
> > +++ b/include/net/netns/xfrm.h
> > @@ -64,6 +64,7 @@ struct netns_xfrm {
> >  	u32			sysctl_aevent_rseqth;
> >  	int			sysctl_larval_drop;
> >  	u32			sysctl_acq_expires;
> > +	u32			sysctl_redact_secret;
> >  #ifdef CONFIG_SYSCTL
> >  	struct ctl_table_header	*sysctl_hdr;
> >  #endif
> > diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
> > index 5b9a5ab48111..270a4e906a15 100644
> > --- a/net/xfrm/Kconfig
> > +++ b/net/xfrm/Kconfig
> > @@ -91,6 +91,16 @@ config XFRM_ESP
> >  	select CRYPTO_SEQIV
> >  	select CRYPTO_SHA256
> > 
> > +config XFRM_REDACT_SECRET
> > +	bool "Redact xfrm SA secret in netlink message"
> > +	depends on SYSCTL
> > +	default n
> > +	help
> > +	  Enable XFRM SA secret redact in the netlink message.
> > +	  Redacting secret is a FIPS 140-2 requirement.
> > +	  Once enabled at compile, the value can not be set to false on
> > +	  a running system.
> > +
> >  config XFRM_IPCOMP
> >  	tristate
> >  	select XFRM_ALGO
> > diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
> > index 0c6c5ef65f9d..a41aa325a478 100644
> > --- a/net/xfrm/xfrm_sysctl.c
> > +++ b/net/xfrm/xfrm_sysctl.c
> > @@ -4,15 +4,25 @@
> >  #include <net/net_namespace.h>
> >  #include <net/xfrm.h>
> > 
> > +#ifdef CONFIG_SYSCTL
> > +#ifdef CONFIG_XFRM_REDACT_SECRET
> > +#define XFRM_REDACT_SECRET  1
> > +#else
> > +#define XFRM_REDACT_SECRET  0
> > +#endif
> > +#endif
> > +
> >  static void __net_init __xfrm_sysctl_init(struct net *net)
> >  {
> >  	net->xfrm.sysctl_aevent_etime = XFRM_AE_ETIME;
> >  	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
> >  	net->xfrm.sysctl_larval_drop = 1;
> >  	net->xfrm.sysctl_acq_expires = 30;
> > +	net->xfrm.sysctl_redact_secret = XFRM_REDACT_SECRET;
> >  }
> > 
> >  #ifdef CONFIG_SYSCTL
> > +
> >  static struct ctl_table xfrm_table[] = {
> >  	{
> >  		.procname	= "xfrm_aevent_etime",
> > @@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
> >  		.mode		= 0644,
> >  		.proc_handler	= proc_dointvec
> >  	},
> > +	{
> > +		.procname	= "xfrm_redact_secret",
> > +		.maxlen		= sizeof(u32),
> > +		.mode		= 0644,
> > +		/* only handle a transition from "0" to "1" */
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1         = SYSCTL_ONE,
> > +		.extra2         = SYSCTL_ONE,
> > +	},
> >  	{}
> >  };
> > 
> > @@ -54,6 +73,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
> >  	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
> >  	table[2].data = &net->xfrm.sysctl_larval_drop;
> >  	table[3].data = &net->xfrm.sysctl_acq_expires;
> > +	table[4].data = &net->xfrm.sysctl_redact_secret;
> > 
> >  	/* Don't export sysctls to unprivileged users */
> >  	if (net->user_ns != &init_user_ns)
> > diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> > index e6cfaa680ef3..a3e89dddea9d 100644
> > --- a/net/xfrm/xfrm_user.c
> > +++ b/net/xfrm/xfrm_user.c
> > @@ -848,21 +848,78 @@ static int copy_user_offload(struct xfrm_state_offload
> > *xso, struct sk_buff *skb return 0;
> >  }
> > 
> > -static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff
> > *skb) +static int copy_to_user_auth(u32 redact_secret, struct
> > xfrm_algo_auth *auth, +			     struct sk_buff *skb)
> >  {
> >  	struct xfrm_algo *algo;
> > +	struct xfrm_algo_auth *ap;
> >  	struct nlattr *nla;
> > 
> >  	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
> >  			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
> >  	if (!nla)
> >  		return -EMSGSIZE;
> > -
> >  	algo = nla_data(nla);
> >  	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
> > -	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
> > +
> > +	if (redact_secret && auth->alg_key_len)
> > +		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
> > +	else
> > +		memcpy(algo->alg_key, auth->alg_key,
> > +		       (auth->alg_key_len + 7) / 8);
> >  	algo->alg_key_len = auth->alg_key_len;
> > 
> > +	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
> > +	if (!nla)
> > +		return -EMSGSIZE;
> > +	ap = nla_data(nla);
> > +	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
> > +	if (redact_secret)
> 
> You test for auth->alg_key_len above. Shouldn't there such a check here too?

It is a good idea add checks before all memset calls.
I will send a new version out soon.

thanks,
-antony

[PATCH ipsec-next v2] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

when enabled, 1, redact XFRM SA secret in the netlink response to
xfrm_get_sa() or dump all sa.

e.g
echo 1 > /proc/sys/net/core/xfrm_redact_secret
ip xfrm state
src 172.16.1.200 dst 172.16.1.100
	proto esp spi 0x00000002 reqid 2 mode tunnel
	replay-window 0
	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96

the aead secret is redacted.

/proc/sys/core/net/xfrm_redact_secret is a toggle.
Once enabled, either at compile or via proc, it can not be disabled.
Redacting secret is a FIPS 140-2 requirement.

---

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
v1->v2
 - add size checks before memset calls

 Documentation/networking/xfrm_sysctl.rst |  7 +++
 include/net/netns/xfrm.h                 |  1 +
 net/xfrm/Kconfig                         | 10 ++++
 net/xfrm/xfrm_sysctl.c                   | 20 +++++++
 net/xfrm/xfrm_user.c                     | 76 +++++++++++++++++++++---
 5 files changed, 105 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/xfrm_sysctl.rst b/Documentation/networking/xfrm_sysctl.rst
index 47b9bbdd0179..26432b0ff3ac 100644
--- a/Documentation/networking/xfrm_sysctl.rst
+++ b/Documentation/networking/xfrm_sysctl.rst
@@ -9,3 +9,10 @@ XFRM Syscall
 
 xfrm_acq_expires - INTEGER
 	default 30 - hard timeout in seconds for acquire requests
+
+xfrm_redact_secret - INTEGER
+	A toggle to redact xfrm SA's secret to userspace.
+	When true the kernel, netlink message will redact SA secret
+	to userspace. This is part of FIPS 140-2 requirement.
+	Once the value is set to true, either at compile or at run time,
+	it can not be set to false.
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 59f45b1e9dac..0ca9328daad4 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -64,6 +64,7 @@ struct netns_xfrm {
 	u32			sysctl_aevent_rseqth;
 	int			sysctl_larval_drop;
 	u32			sysctl_acq_expires;
+	u32			sysctl_redact_secret;
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
 #endif
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 5b9a5ab48111..270a4e906a15 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -91,6 +91,16 @@ config XFRM_ESP
 	select CRYPTO_SEQIV
 	select CRYPTO_SHA256
 
+config XFRM_REDACT_SECRET
+	bool "Redact xfrm SA secret in netlink message"
+	depends on SYSCTL
+	default n
+	help
+	  Enable XFRM SA secret redact in the netlink message.
+	  Redacting secret is a FIPS 140-2 requirement.
+	  Once enabled at compile, the value can not be set to false on
+	  a running system.
+
 config XFRM_IPCOMP
 	tristate
 	select XFRM_ALGO
diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
index 0c6c5ef65f9d..a41aa325a478 100644
--- a/net/xfrm/xfrm_sysctl.c
+++ b/net/xfrm/xfrm_sysctl.c
@@ -4,15 +4,25 @@
 #include <net/net_namespace.h>
 #include <net/xfrm.h>
 
+#ifdef CONFIG_SYSCTL
+#ifdef CONFIG_XFRM_REDACT_SECRET
+#define XFRM_REDACT_SECRET  1
+#else
+#define XFRM_REDACT_SECRET  0
+#endif
+#endif
+
 static void __net_init __xfrm_sysctl_init(struct net *net)
 {
 	net->xfrm.sysctl_aevent_etime = XFRM_AE_ETIME;
 	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
 	net->xfrm.sysctl_larval_drop = 1;
 	net->xfrm.sysctl_acq_expires = 30;
+	net->xfrm.sysctl_redact_secret = XFRM_REDACT_SECRET;
 }
 
 #ifdef CONFIG_SYSCTL
+
 static struct ctl_table xfrm_table[] = {
 	{
 		.procname	= "xfrm_aevent_etime",
@@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "xfrm_redact_secret",
+		.maxlen		= sizeof(u32),
+		.mode		= 0644,
+		/* only handle a transition from "0" to "1" */
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1         = SYSCTL_ONE,
+		.extra2         = SYSCTL_ONE,
+	},
 	{}
 };
 
@@ -54,6 +73,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
 	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
 	table[2].data = &net->xfrm.sysctl_larval_drop;
 	table[3].data = &net->xfrm.sysctl_acq_expires;
+	table[4].data = &net->xfrm.sysctl_redact_secret;
 
 	/* Don't export sysctls to unprivileged users */
 	if (net->user_ns != &init_user_ns)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index fbb7d9d06478..c33ebc166e04 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -848,21 +848,78 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 	return 0;
 }
 
-static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
+static int copy_to_user_auth(u32 redact_secret, struct xfrm_algo_auth *auth,
+			     struct sk_buff *skb)
 {
 	struct xfrm_algo *algo;
+	struct xfrm_algo_auth *ap;
 	struct nlattr *nla;
 
 	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
 			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
 	if (!nla)
 		return -EMSGSIZE;
-
 	algo = nla_data(nla);
 	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
-	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
+
+	if (redact_secret && auth->alg_key_len)
+		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(algo->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
 	algo->alg_key_len = auth->alg_key_len;
 
+	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
+	if (!nla)
+		return -EMSGSIZE;
+	ap = nla_data(nla);
+	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
+	if (redact_secret && auth->alg_key_len)
+		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_aead(u32 redact_secret,
+			     struct xfrm_algo_aead *aead, struct sk_buff *skb)
+{
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
+	struct xfrm_algo_aead *ap;
+
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, aead, sizeof(*aead));
+
+	if (redact_secret && aead->alg_key_len)
+		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, aead->alg_key,
+		       (aead->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_ealg(u32 redact_secret, struct xfrm_algo *ealg,
+			     struct sk_buff *skb)
+{
+	struct xfrm_algo *ap;
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
+					 xfrm_alg_len(ealg));
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, ealg, sizeof(*ealg));
+
+	if (redact_secret && ealg->alg_key_len)
+		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, ealg->alg_key,
+		       (ealg->alg_key_len + 7) / 8);
+
 	return 0;
 }
 
@@ -884,6 +941,7 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 				    struct sk_buff *skb)
 {
 	int ret = 0;
+	struct net *net = xs_net(x);
 
 	copy_to_user_state(x, p);
 
@@ -906,20 +964,20 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 			goto out;
 	}
 	if (x->aead) {
-		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead);
+		ret = copy_to_user_aead(net->xfrm.sysctl_redact_secret,
+					x->aead, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->aalg) {
-		ret = copy_to_user_auth(x->aalg, skb);
-		if (!ret)
-			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
-				      xfrm_alg_auth_len(x->aalg), x->aalg);
+		ret = copy_to_user_auth(net->xfrm.sysctl_redact_secret,
+					x->aalg, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->ealg) {
-		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg);
+		ret = copy_to_user_ealg(net->xfrm.sysctl_redact_secret,
+					x->ealg, skb);
 		if (ret)
 			goto out;
 	}
-- 
2.20.1

Re: [PATCH ipsec-next v2] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Jakub Kicinski]

On Thu, 20 Aug 2020 14:04:53 +0200 Antony Antony wrote:
> ---
> 
> Signed-off-by: Antony Antony <antony.antony@secunet.com>

A warning here - anything after --- will be cut off by git when
applying the patch. Perhaps you could resend the patch without it 
to save Steffen manual work?

Re: [PATCH ipsec-next v2] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Nicolas Dichtel]

Le 20/08/2020 à 14:04, Antony Antony a écrit :
[snip]
> @@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec
>  	},
> +	{
> +		.procname	= "xfrm_redact_secret",
> +		.maxlen		= sizeof(u32),
> +		.mode		= 0644,
> +		/* only handle a transition from "0" to "1" */
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1         = SYSCTL_ONE,
> +		.extra2         = SYSCTL_ONE,
nit for the v3: the '=' of the last two lines is aligned with spaces while the
first lines use tabs.

[PATCH ipsec-next v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

when enabled, 1, redact XFRM SA secret in the netlink response to
xfrm_get_sa() or dump all sa.

e.g
echo 1 > /proc/sys/net/core/xfrm_redact_secret
ip xfrm state
src 172.16.1.200 dst 172.16.1.100
	proto esp spi 0x00000002 reqid 2 mode tunnel
	replay-window 0
	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96

the aead secret is redacted.

/proc/sys/core/net/xfrm_redact_secret is a toggle.
Once enabled, either at compile or via proc, it can not be disabled.
Redacting secret is a FIPS 140-2 requirement.

v1->v2
 - add size checks before memset calls
v1->v3
 - replace spaces with tabs for consistancy

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 Documentation/networking/xfrm_sysctl.rst |  7 +++
 include/net/netns/xfrm.h                 |  1 +
 net/xfrm/Kconfig                         | 10 ++++
 net/xfrm/xfrm_sysctl.c                   | 20 +++++++
 net/xfrm/xfrm_user.c                     | 76 +++++++++++++++++++++---
 5 files changed, 105 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/xfrm_sysctl.rst b/Documentation/networking/xfrm_sysctl.rst
index 47b9bbdd0179..26432b0ff3ac 100644
--- a/Documentation/networking/xfrm_sysctl.rst
+++ b/Documentation/networking/xfrm_sysctl.rst
@@ -9,3 +9,10 @@ XFRM Syscall
 
 xfrm_acq_expires - INTEGER
 	default 30 - hard timeout in seconds for acquire requests
+
+xfrm_redact_secret - INTEGER
+	A toggle to redact xfrm SA's secret to userspace.
+	When true the kernel, netlink message will redact SA secret
+	to userspace. This is part of FIPS 140-2 requirement.
+	Once the value is set to true, either at compile or at run time,
+	it can not be set to false.
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 59f45b1e9dac..0ca9328daad4 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -64,6 +64,7 @@ struct netns_xfrm {
 	u32			sysctl_aevent_rseqth;
 	int			sysctl_larval_drop;
 	u32			sysctl_acq_expires;
+	u32			sysctl_redact_secret;
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*sysctl_hdr;
 #endif
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 5b9a5ab48111..270a4e906a15 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -91,6 +91,16 @@ config XFRM_ESP
 	select CRYPTO_SEQIV
 	select CRYPTO_SHA256
 
+config XFRM_REDACT_SECRET
+	bool "Redact xfrm SA secret in netlink message"
+	depends on SYSCTL
+	default n
+	help
+	  Enable XFRM SA secret redact in the netlink message.
+	  Redacting secret is a FIPS 140-2 requirement.
+	  Once enabled at compile, the value can not be set to false on
+	  a running system.
+
 config XFRM_IPCOMP
 	tristate
 	select XFRM_ALGO
diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
index 0c6c5ef65f9d..bff1f55b198e 100644
--- a/net/xfrm/xfrm_sysctl.c
+++ b/net/xfrm/xfrm_sysctl.c
@@ -4,15 +4,25 @@
 #include <net/net_namespace.h>
 #include <net/xfrm.h>
 
+#ifdef CONFIG_SYSCTL
+#ifdef CONFIG_XFRM_REDACT_SECRET
+#define XFRM_REDACT_SECRET  1
+#else
+#define XFRM_REDACT_SECRET  0
+#endif
+#endif
+
 static void __net_init __xfrm_sysctl_init(struct net *net)
 {
 	net->xfrm.sysctl_aevent_etime = XFRM_AE_ETIME;
 	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
 	net->xfrm.sysctl_larval_drop = 1;
 	net->xfrm.sysctl_acq_expires = 30;
+	net->xfrm.sysctl_redact_secret = XFRM_REDACT_SECRET;
 }
 
 #ifdef CONFIG_SYSCTL
+
 static struct ctl_table xfrm_table[] = {
 	{
 		.procname	= "xfrm_aevent_etime",
@@ -38,6 +48,15 @@ static struct ctl_table xfrm_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "xfrm_redact_secret",
+		.maxlen		= sizeof(u32),
+		.mode		= 0644,
+		/* only handle a transition from "0" to "1" */
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ONE,
+		.extra2		= SYSCTL_ONE,
+	},
 	{}
 };
 
@@ -54,6 +73,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
 	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
 	table[2].data = &net->xfrm.sysctl_larval_drop;
 	table[3].data = &net->xfrm.sysctl_acq_expires;
+	table[4].data = &net->xfrm.sysctl_redact_secret;
 
 	/* Don't export sysctls to unprivileged users */
 	if (net->user_ns != &init_user_ns)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index fbb7d9d06478..c33ebc166e04 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -848,21 +848,78 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 	return 0;
 }
 
-static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
+static int copy_to_user_auth(u32 redact_secret, struct xfrm_algo_auth *auth,
+			     struct sk_buff *skb)
 {
 	struct xfrm_algo *algo;
+	struct xfrm_algo_auth *ap;
 	struct nlattr *nla;
 
 	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
 			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
 	if (!nla)
 		return -EMSGSIZE;
-
 	algo = nla_data(nla);
 	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
-	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
+
+	if (redact_secret && auth->alg_key_len)
+		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(algo->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
 	algo->alg_key_len = auth->alg_key_len;
 
+	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
+	if (!nla)
+		return -EMSGSIZE;
+	ap = nla_data(nla);
+	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
+	if (redact_secret && auth->alg_key_len)
+		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_aead(u32 redact_secret,
+			     struct xfrm_algo_aead *aead, struct sk_buff *skb)
+{
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
+	struct xfrm_algo_aead *ap;
+
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, aead, sizeof(*aead));
+
+	if (redact_secret && aead->alg_key_len)
+		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, aead->alg_key,
+		       (aead->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_ealg(u32 redact_secret, struct xfrm_algo *ealg,
+			     struct sk_buff *skb)
+{
+	struct xfrm_algo *ap;
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
+					 xfrm_alg_len(ealg));
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, ealg, sizeof(*ealg));
+
+	if (redact_secret && ealg->alg_key_len)
+		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, ealg->alg_key,
+		       (ealg->alg_key_len + 7) / 8);
+
 	return 0;
 }
 
@@ -884,6 +941,7 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 				    struct sk_buff *skb)
 {
 	int ret = 0;
+	struct net *net = xs_net(x);
 
 	copy_to_user_state(x, p);
 
@@ -906,20 +964,20 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 			goto out;
 	}
 	if (x->aead) {
-		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead);
+		ret = copy_to_user_aead(net->xfrm.sysctl_redact_secret,
+					x->aead, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->aalg) {
-		ret = copy_to_user_auth(x->aalg, skb);
-		if (!ret)
-			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
-				      xfrm_alg_auth_len(x->aalg), x->aalg);
+		ret = copy_to_user_auth(net->xfrm.sysctl_redact_secret,
+					x->aalg, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->ealg) {
-		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg);
+		ret = copy_to_user_ealg(net->xfrm.sysctl_redact_secret,
+					x->ealg, skb);
 		if (ret)
 			goto out;
 	}
-- 
2.20.1

Re: [PATCH ipsec-next v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[David Miller]

From: Antony Antony <antony.antony@secunet.com>
Date: Thu, 20 Aug 2020 20:35:49 +0200

> Redacting secret is a FIPS 140-2 requirement.

Why not control this via the kernel lockdown mode rather than making
an ad-hoc API for this?

Re: [PATCH ipsec-next v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

On Thu, Aug 20, 2020 at 15:42:22 -0700, David Miller wrote:
> From: Antony Antony <antony.antony@secunet.com>
> Date: Thu, 20 Aug 2020 20:35:49 +0200
> 
> > Redacting secret is a FIPS 140-2 requirement.
> 
> Why not control this via the kernel lockdown mode rather than making
> an ad-hoc API for this? 

Let me try to use kernel lockdown mode. thanks for the idea. 

From a quick googling I guess it would be part of "lockdown= confidentiality".
I wonder if kernel lockdown would allow disabling just this one feature independent of other lockdowns.

-antony

Re: [PATCH ipsec-next v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[Antony Antony]

Hi David,

On Mon, Aug 24, 2020 at 08:00:38 +0200, Antony Antony wrote:
> On Thu, Aug 20, 2020 at 15:42:22 -0700, David Miller wrote:
> > From: Antony Antony <antony.antony@secunet.com>
> > Date: Thu, 20 Aug 2020 20:35:49 +0200
> > 
> > > Redacting secret is a FIPS 140-2 requirement.
> > 
> > Why not control this via the kernel lockdown mode rather than making
> > an ad-hoc API for this? 
> 
> Let me try to use kernel lockdown mode. thanks for the idea. 
> 
> From a quick googling I guess it would be part of "lockdown= confidentiality".
> I wonder if kernel lockdown would allow disabling just this one feature independent of other lockdowns.

I looked at kernel lockdown mode code and documentation. I am thinking xfrm_redact is probably not a kernel lockdown mode feature. There is no kernel lockdown setting per net namespace.

During an initial discussions of xfrm_redact we thought per namespace would be useful in some use cases.

If there is a way to set lockdown per net namespace it would be better than /proc/sys/core/net/xfrm_redact_secret.

Re: [PATCH ipsec-next v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret

[David Miller]

From: Antony Antony <antony.antony@secunet.com>
Date: Thu, 27 Aug 2020 22:15:36 +0200

> If there is a way to set lockdown per net namespace it would be
> better than /proc/sys/core/net/xfrm_redact_secret.

Lockmode is a whole system attribute.

As should any facility that restricts access to keying information
stored inside of the kernel.

[PATCH] xfrm: redact SA secret with lockdown confidentiality

[Antony Antony]

redact XFRM SA secret in the netlink response to xfrm_get_sa()
or dumpall sa.
Enable this at build time and set kernel lockdown to confidentiality.

e.g.
cat /sys/kernel/security/lockdown
none integrity [confidentiality]

ip xfrm state
src 172.16.1.200 dst 172.16.1.100
	proto esp spi 0x00000002 reqid 2 mode tunnel
	replay-window 0
	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96

note: the aead secret is redacted.
Redacting secret is also a FIPS 140-2 requirement.

v1->v2
 - add size checks before memset calls
v2->v3
 - replace spaces with tabs for consistency
v3->v4
 - use kernel lockdown instead of a /proc setting

Reviewed-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/linux/security.h |  1 +
 net/xfrm/Kconfig         |  9 +++++
 net/xfrm/xfrm_user.c     | 76 ++++++++++++++++++++++++++++++++++++----
 security/security.c      |  1 +
 4 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 0a0a03b36a3b..8438970473b1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -126,6 +126,7 @@ enum lockdown_reason {
 	LOCKDOWN_PERF,
 	LOCKDOWN_TRACEFS,
 	LOCKDOWN_XMON_RW,
+	LOCKDOWN_XFRM_SECRET,
 	LOCKDOWN_CONFIDENTIALITY_MAX,
 };
 
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 5b9a5ab48111..cb592524701d 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -91,6 +91,15 @@ config XFRM_ESP
 	select CRYPTO_SEQIV
 	select CRYPTO_SHA256
 
+config XFRM_REDACT_SECRET
+	bool "Redact xfrm SA secret"
+	depends on XFRM && SECURITY_LOCKDOWN_LSM
+	default n
+	help
+	  Redats XFRM SA secret in the netlink message to user space.
+	  Redacting secret is also a FIPS 140-2 requirement.
+	  e.g. ip xfrm state; will show redacted the SA secret.
+
 config XFRM_IPCOMP
 	tristate
 	select XFRM_ALGO
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index fbb7d9d06478..b57599d050dc 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include <linux/fips.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -848,21 +849,85 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 	return 0;
 }
 
+static bool xfrm_redact(void)
+{
+	return IS_ENABLED(CONFIG_SECURITY) &&
+		IS_ENABLED(CONFIG_XFRM_REDACT_SECRET) &&
+		security_locked_down(LOCKDOWN_XFRM_SECRET);
+}
+
 static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
 {
 	struct xfrm_algo *algo;
+	struct xfrm_algo_auth *ap;
 	struct nlattr *nla;
+	bool redact_secret = xfrm_redact();
 
 	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
 			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
 	if (!nla)
 		return -EMSGSIZE;
-
 	algo = nla_data(nla);
 	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
-	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
+
+	if (redact_secret && auth->alg_key_len)
+		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(algo->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
 	algo->alg_key_len = auth->alg_key_len;
 
+	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
+	if (!nla)
+		return -EMSGSIZE;
+	ap = nla_data(nla);
+	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
+	if (redact_secret && auth->alg_key_len)
+		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_aead(struct xfrm_algo_aead *aead, struct sk_buff *skb)
+{
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
+	struct xfrm_algo_aead *ap;
+	bool redact_secret = xfrm_redact();
+
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, aead, sizeof(*aead));
+
+	if (redact_secret && aead->alg_key_len)
+		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, aead->alg_key,
+		       (aead->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_ealg(struct xfrm_algo *ealg, struct sk_buff *skb)
+{
+	struct xfrm_algo *ap;
+	bool redact_secret = xfrm_redact();
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
+					 xfrm_alg_len(ealg));
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, ealg, sizeof(*ealg));
+
+	if (redact_secret && ealg->alg_key_len)
+		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, ealg->alg_key,
+		       (ealg->alg_key_len + 7) / 8);
+
 	return 0;
 }
 
@@ -906,20 +971,17 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 			goto out;
 	}
 	if (x->aead) {
-		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead);
+		ret = copy_to_user_aead(x->aead, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->aalg) {
 		ret = copy_to_user_auth(x->aalg, skb);
-		if (!ret)
-			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
-				      xfrm_alg_auth_len(x->aalg), x->aalg);
 		if (ret)
 			goto out;
 	}
 	if (x->ealg) {
-		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg);
+		ret = copy_to_user_ealg(x->ealg, skb);
 		if (ret)
 			goto out;
 	}
diff --git a/security/security.c b/security/security.c
index 70a7ad357bc6..72d9aac7178a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -64,6 +64,7 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
 	[LOCKDOWN_PERF] = "unsafe use of perf",
 	[LOCKDOWN_TRACEFS] = "use of tracefs",
 	[LOCKDOWN_XMON_RW] = "xmon read and write access",
+	[LOCKDOWN_XFRM_SECRET] = "xfrm SA secret",
 	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
 };
 
-- 
2.20.1

Re: [PATCH] xfrm: redact SA secret with lockdown confidentiality

[Steffen Klassert]

On Fri, Oct 16, 2020 at 03:36:12PM +0200, Antony Antony wrote:
> redact XFRM SA secret in the netlink response to xfrm_get_sa()
> or dumpall sa.
> Enable this at build time and set kernel lockdown to confidentiality.

Wouldn't it be better to enable is at boot or runtime? This defaults
to 'No' at build time, so distibutions will not compile it in. That
means that noone who uses a kernel that comes with a Linux distribution
can use that.

Re: [PATCH] xfrm: redact SA secret with lockdown confidentiality

[Antony Antony]

On Sat, Oct 31, 2020 at 11:49:11 +0100, Steffen Klassert wrote:
> On Fri, Oct 16, 2020 at 03:36:12PM +0200, Antony Antony wrote:
> > redact XFRM SA secret in the netlink response to xfrm_get_sa()
> > or dumpall sa.
> > Enable this at build time and set kernel lockdown to confidentiality.
> 
> Wouldn't it be better to enable is at boot or runtime? This defaults
> to 'No' at build time, so distibutions will not compile it in. That
> means that noone who uses a kernel that comes with a Linux distribution
> can use that.

It is a good idea. I will send new version soon.

thanks,
-antony

[PATCH ipsec-next v5] xfrm: redact SA secret with lockdown confidentiality

[Antony Antony]

redact XFRM SA secret in the netlink response to xfrm_get_sa()
or dumpall sa.
Enable lockdown, confidentiality mode, at boot or at run time.

e.g. when enabled:
cat /sys/kernel/security/lockdown
none integrity [confidentiality]

ip xfrm state
src 172.16.1.200 dst 172.16.1.100
	proto esp spi 0x00000002 reqid 2 mode tunnel
	replay-window 0
	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96

note: the aead secret is redacted.
Redacting secret is also a FIPS 140-2 requirement.

v1->v2
 - add size checks before memset calls
v2->v3
 - replace spaces with tabs for consistency
v3->v4
 - use kernel lockdown instead of a /proc setting
v4->v5
 - remove kconfig option

Reviewed-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/linux/security.h |  1 +
 net/xfrm/xfrm_user.c     | 74 ++++++++++++++++++++++++++++++++++++----
 security/security.c      |  1 +
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index bc2725491560..1112a79a7dba 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -127,6 +127,7 @@ enum lockdown_reason {
 	LOCKDOWN_PERF,
 	LOCKDOWN_TRACEFS,
 	LOCKDOWN_XMON_RW,
+	LOCKDOWN_XFRM_SECRET,
 	LOCKDOWN_CONFIDENTIALITY_MAX,
 };
 
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d0c32a8fcc4a..0727ac853b55 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -848,21 +848,84 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb
 	return 0;
 }
 
+static bool xfrm_redact(void)
+{
+	return IS_ENABLED(CONFIG_SECURITY) &&
+		security_locked_down(LOCKDOWN_XFRM_SECRET);
+}
+
 static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb)
 {
 	struct xfrm_algo *algo;
+	struct xfrm_algo_auth *ap;
 	struct nlattr *nla;
+	bool redact_secret = xfrm_redact();
 
 	nla = nla_reserve(skb, XFRMA_ALG_AUTH,
 			  sizeof(*algo) + (auth->alg_key_len + 7) / 8);
 	if (!nla)
 		return -EMSGSIZE;
-
 	algo = nla_data(nla);
 	strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name));
-	memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8);
+
+	if (redact_secret && auth->alg_key_len)
+		memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(algo->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
 	algo->alg_key_len = auth->alg_key_len;
 
+	nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth));
+	if (!nla)
+		return -EMSGSIZE;
+	ap = nla_data(nla);
+	memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
+	if (redact_secret && auth->alg_key_len)
+		memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, auth->alg_key,
+		       (auth->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_aead(struct xfrm_algo_aead *aead, struct sk_buff *skb)
+{
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead));
+	struct xfrm_algo_aead *ap;
+	bool redact_secret = xfrm_redact();
+
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, aead, sizeof(*aead));
+
+	if (redact_secret && aead->alg_key_len)
+		memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, aead->alg_key,
+		       (aead->alg_key_len + 7) / 8);
+	return 0;
+}
+
+static int copy_to_user_ealg(struct xfrm_algo *ealg, struct sk_buff *skb)
+{
+	struct xfrm_algo *ap;
+	bool redact_secret = xfrm_redact();
+	struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT,
+					 xfrm_alg_len(ealg));
+	if (!nla)
+		return -EMSGSIZE;
+
+	ap = nla_data(nla);
+	memcpy(ap, ealg, sizeof(*ealg));
+
+	if (redact_secret && ealg->alg_key_len)
+		memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8);
+	else
+		memcpy(ap->alg_key, ealg->alg_key,
+		       (ealg->alg_key_len + 7) / 8);
+
 	return 0;
 }
 
@@ -906,20 +969,17 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 			goto out;
 	}
 	if (x->aead) {
-		ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead);
+		ret = copy_to_user_aead(x->aead, skb);
 		if (ret)
 			goto out;
 	}
 	if (x->aalg) {
 		ret = copy_to_user_auth(x->aalg, skb);
-		if (!ret)
-			ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC,
-				      xfrm_alg_auth_len(x->aalg), x->aalg);
 		if (ret)
 			goto out;
 	}
 	if (x->ealg) {
-		ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg);
+		ret = copy_to_user_ealg(x->ealg, skb);
 		if (ret)
 			goto out;
 	}
diff --git a/security/security.c b/security/security.c
index a28045dc9e7f..abff77c1c8a7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -65,6 +65,7 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
 	[LOCKDOWN_PERF] = "unsafe use of perf",
 	[LOCKDOWN_TRACEFS] = "use of tracefs",
 	[LOCKDOWN_XMON_RW] = "xmon read and write access",
+	[LOCKDOWN_XFRM_SECRET] = "xfrm SA secret",
 	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
 };
 
-- 
2.20.1

Re: [PATCH ipsec-next v5] xfrm: redact SA secret with lockdown confidentiality

[Steffen Klassert]

On Tue, Nov 17, 2020 at 05:47:23PM +0100, Antony Antony wrote:
> redact XFRM SA secret in the netlink response to xfrm_get_sa()
> or dumpall sa.
> Enable lockdown, confidentiality mode, at boot or at run time.
> 
> e.g. when enabled:
> cat /sys/kernel/security/lockdown
> none integrity [confidentiality]
> 
> ip xfrm state
> src 172.16.1.200 dst 172.16.1.100
> 	proto esp spi 0x00000002 reqid 2 mode tunnel
> 	replay-window 0
> 	aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96
> 
> note: the aead secret is redacted.
> Redacting secret is also a FIPS 140-2 requirement.
> 
> v1->v2
>  - add size checks before memset calls
> v2->v3
>  - replace spaces with tabs for consistency
> v3->v4
>  - use kernel lockdown instead of a /proc setting
> v4->v5
>  - remove kconfig option
> 
> Reviewed-by: Stephan Mueller <smueller@chronox.de>
> Signed-off-by: Antony Antony <antony.antony@secunet.com>
> ---
>  include/linux/security.h |  1 +
>  net/xfrm/xfrm_user.c     | 74 ++++++++++++++++++++++++++++++++++++----
>  security/security.c      |  1 +
>  3 files changed, 69 insertions(+), 7 deletions(-)

I'm ok with this and I plan to apply it to ipsec-next if I do not see
objections from the LSM people.