diff mbox series

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

Message ID 20200820183549.GA823@moon.secunet.de
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series [ipsec-next,v3] xfrm: add /proc/sys/core/net/xfrm_redact_secret | expand

Commit Message

Antony Antony Aug. 20, 2020, 6:35 p.m. UTC 
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(-)

Comments

David Miller Aug. 20, 2020, 10:42 p.m. UTC | #1 
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?
Antony Antony Aug. 24, 2020, 6 a.m. UTC | #2 
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
Antony Antony Aug. 27, 2020, 8:15 p.m. UTC | #3 
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.
David Miller Aug. 27, 2020, 8:20 p.m. UTC | #4 
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.
diff mbox series

Patch

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;
 	}