From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 74B18C433E0 for ; Tue, 28 Jul 2020 19:15:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A2EC3207FC for ; Tue, 28 Jul 2020 19:15:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=chronox.de header.i=@chronox.de header.b="fFqylUzZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732620AbgG1TPP (ORCPT ); Tue, 28 Jul 2020 15:15:15 -0400 Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.220]:17209 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728561AbgG1TPP (ORCPT ); Tue, 28 Jul 2020 15:15:15 -0400 X-Greylist: delayed 348 seconds by postgrey-1.27 at vger.kernel.org; Tue, 28 Jul 2020 15:15:12 EDT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1595963711; s=strato-dkim-0002; d=chronox.de; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=XUUr7VRRoINftMeP06Rutnv9csE5aGgHrwnaN1iS2+g=; b=fFqylUzZ5rb4jePe40czdXiL9CyXktEx6yWtZXZmECsqBlM2CC/h50+rWiKkpm6O/t W7hQqlajjWath9lm6D5l5ltnJeU/O1upMk0v9mV6R5Cb0CWI3gqHuxbRb6UqXeB1LvaU oygdDHCqkImx0jAqUmRlHosjRKCYo0Gst6itQPI4Jl/rcriSXeMIf8gKxyv9ZrwOTutW 4oXGsShN8UI8lzKuffwrmGA8a/mE0T0pmkqgaHQS/kCPjxR2+j+oZlDF3qX1iMc3DgBl Gc6LLl+uzjA0ycpQRQPx6L3xMhz3JCzkqyQ0CvNDWSU5BDwwHjKHCfbel4j0zOiofIJh YsSA== X-RZG-AUTH: ":P2ERcEykfu11Y98lp/T7+hdri+uKZK8TKWEqNyiHySGSa9k9xmwdNnzGHXPZJ/Sf9P0=" X-RZG-CLASS-ID: mo00 Received: from tauon.chronox.de by smtp.strato.de (RZmta 46.10.5 DYNA|AUTH) with ESMTPSA id y0546bw6SJ9B9jg (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Tue, 28 Jul 2020 21:09:11 +0200 (CEST) From: Stephan Mueller To: Steffen Klassert , netdev@vger.kernel.org, antony.antony@secunet.com Cc: Herbert Xu , Antony Antony Subject: Re: [PATCH ipsec-next] xfrm: add /proc/sys/core/net/xfrm_redact_secret Date: Tue, 28 Jul 2020 21:09:10 +0200 Message-ID: <3322274.jE0xQCEvom@tauon.chronox.de> In-Reply-To: <20200728154342.GA31835@moon.secunet.de> References: <20200728154342.GA31835@moon.secunet.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 > Signed-off-by: Antony Antony > --- > 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 > #include > > +#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