All of lore.kernel.org
 help / color / mirror / Atom feed
* pull request (net-next): ipsec-next 2020-12-12
@ 2020-12-12  8:57 Steffen Klassert
  2020-12-12  8:57 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Steffen Klassert
  2020-12-12 21:40 ` pull request (net-next): ipsec-next 2020-12-12 patchwork-bot+netdevbpf
  0 siblings, 2 replies; 7+ messages in thread
From: Steffen Klassert @ 2020-12-12  8:57 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

Just one patch this time:

1) Redact the SA keys with kernel lockdown confidentiality.
   If enabled, no secret keys are sent to uuserspace.
   From Antony Antony.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 8be33ecfc1ffd2da20cc29e957e4cb6eb99310cb:

  net: skb_vlan_untag(): don't reset transport offset if set by GRO layer (2020-11-09 20:03:55 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master

for you to fetch changes up to c7a5899eb26e2a4d516d53f65b6dd67be2228041:

  xfrm: redact SA secret with lockdown confidentiality (2020-11-27 11:03:06 +0100)

----------------------------------------------------------------
Antony Antony (1):
      xfrm: redact SA secret with lockdown confidentiality

 include/linux/security.h |  1 +
 net/xfrm/xfrm_user.c     | 74 +++++++++++++++++++++++++++++++++++++++++++-----
 security/security.c      |  1 +
 3 files changed, 69 insertions(+), 7 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] xfrm: redact SA secret with lockdown confidentiality
  2020-12-12  8:57 pull request (net-next): ipsec-next 2020-12-12 Steffen Klassert
@ 2020-12-12  8:57 ` Steffen Klassert
  2020-12-12 21:40   ` patchwork-bot+netdevbpf
  2020-12-12 21:40 ` pull request (net-next): ipsec-next 2020-12-12 patchwork-bot+netdevbpf
  1 sibling, 1 reply; 7+ messages in thread
From: Steffen Klassert @ 2020-12-12  8:57 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Antony Antony <antony.antony@secunet.com>

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>
Signed-off-by: Steffen Klassert <steffen.klassert@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.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfrm: redact SA secret with lockdown confidentiality
  2020-12-12  8:57 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Steffen Klassert
@ 2020-12-12 21:40   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-12 21:40 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, kuba, herbert, netdev

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Sat, 12 Dec 2020 09:57:37 +0100 you wrote:
> From: Antony Antony <antony.antony@secunet.com>
> 
> 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]
> 
> [...]

Here is the summary with links:
  - xfrm: redact SA secret with lockdown confidentiality
    https://git.kernel.org/netdev/net-next/c/c7a5899eb26e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: pull request (net-next): ipsec-next 2020-12-12
  2020-12-12  8:57 pull request (net-next): ipsec-next 2020-12-12 Steffen Klassert
  2020-12-12  8:57 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Steffen Klassert
@ 2020-12-12 21:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-12 21:40 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, kuba, herbert, netdev

Hello:

This pull request was applied to netdev/net-next.git (refs/heads/master):

On Sat, 12 Dec 2020 09:57:36 +0100 you wrote:
> Just one patch this time:
> 
> 1) Redact the SA keys with kernel lockdown confidentiality.
>    If enabled, no secret keys are sent to uuserspace.
>    From Antony Antony.
> 
> Please pull or let me know if there are problems.
> 
> [...]

Here is the summary with links:
  - pull request (net-next): ipsec-next 2020-12-12
    https://git.kernel.org/netdev/net-next/c/e2437ac2f59d

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfrm: redact SA secret with lockdown confidentiality
  2020-10-31 10:49   ` Steffen Klassert
@ 2020-11-17 16:46     ` Antony Antony
  0 siblings, 0 replies; 7+ messages in thread
From: Antony Antony @ 2020-11-17 16:46 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Antony Antony, Herbert Xu, David S. Miller, Jakub Kicinski,
	netdev, linux-security-module, Antony Antony, Stephan Mueller

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfrm: redact SA secret with lockdown confidentiality
  2020-10-16 13:36 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Antony Antony
@ 2020-10-31 10:49   ` Steffen Klassert
  2020-11-17 16:46     ` Antony Antony
  0 siblings, 1 reply; 7+ messages in thread
From: Steffen Klassert @ 2020-10-31 10:49 UTC (permalink / raw)
  To: Antony Antony
  Cc: Herbert Xu, David S. Miller, Jakub Kicinski, netdev,
	linux-security-module, Antony Antony, Stephan Mueller

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] xfrm: redact SA secret with lockdown confidentiality
  2020-07-28 15:47 [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret Antony Antony
@ 2020-10-16 13:36 ` Antony Antony
  2020-10-31 10:49   ` Steffen Klassert
  0 siblings, 1 reply; 7+ messages in thread
From: Antony Antony @ 2020-10-16 13:36 UTC (permalink / raw)
  To: Steffen Klassert, Herbert Xu, David S. Miller, Jakub Kicinski
  Cc: netdev, linux-security-module, Antony Antony, Antony Antony,
	Stephan Mueller

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-12-12 21:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12  8:57 pull request (net-next): ipsec-next 2020-12-12 Steffen Klassert
2020-12-12  8:57 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Steffen Klassert
2020-12-12 21:40   ` patchwork-bot+netdevbpf
2020-12-12 21:40 ` pull request (net-next): ipsec-next 2020-12-12 patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2020-07-28 15:47 [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret Antony Antony
2020-10-16 13:36 ` [PATCH] xfrm: redact SA secret with lockdown confidentiality Antony Antony
2020-10-31 10:49   ` Steffen Klassert
2020-11-17 16:46     ` Antony Antony

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.