* [PATCH] xfrm: redact SA secret with lockdown confidentiality [not found] <20200728154342.GA31835@moon.secunet.de> @ 2020-10-16 13:36 ` Antony Antony 2020-10-31 10:49 ` Steffen Klassert 2020-11-17 16:47 ` [PATCH ipsec-next v5] " Antony Antony 0 siblings, 2 replies; 5+ 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] 5+ 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 2020-11-17 16:47 ` [PATCH ipsec-next v5] " Antony Antony 1 sibling, 1 reply; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
* [PATCH ipsec-next v5] 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:47 ` Antony Antony 2020-11-23 6:42 ` Steffen Klassert 1 sibling, 1 reply; 5+ messages in thread From: Antony Antony @ 2020-11-17 16:47 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 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH ipsec-next v5] xfrm: redact SA secret with lockdown confidentiality 2020-11-17 16:47 ` [PATCH ipsec-next v5] " Antony Antony @ 2020-11-23 6:42 ` Steffen Klassert 0 siblings, 0 replies; 5+ messages in thread From: Steffen Klassert @ 2020-11-23 6:42 UTC (permalink / raw) To: Antony Antony Cc: Herbert Xu, David S. Miller, Jakub Kicinski, netdev, linux-security-module, Antony Antony, Stephan Mueller 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-23 13:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200728154342.GA31835@moon.secunet.de>
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
2020-11-17 16:47 ` [PATCH ipsec-next v5] " Antony Antony
2020-11-23 6:42 ` Steffen Klassert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).