All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns
@ 2019-07-09 11:11 Ondrej Mosnacek
  2019-07-09 14:38 ` Herbert Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ondrej Mosnacek @ 2019-07-09 11:11 UTC (permalink / raw)
  To: linux-crypto, Herbert Xu
  Cc: netdev, David S . Miller, Stephan Mueller, Steffen Klassert, Don Zickus

Currently, NETLINK_CRYPTO works only in the init network namespace. It
doesn't make much sense to cut it out of the other network namespaces,
so do the minor plumbing work necessary to make it work in any network
namespace. Code inspired by net/core/sock_diag.c.

Tested using kcapi-dgst from libkcapi [1]:
Before:
    # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
    libkcapi - Error: Netlink error: sendmsg failed
    libkcapi - Error: Netlink error: sendmsg failed
    libkcapi - Error: NETLINK_CRYPTO: cannot obtain cipher information for hmac(sha512) (is required crypto_user.c patch missing? see documentation)
    0

After:
    # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
    32

[1] https://github.com/smuellerDD/libkcapi

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 crypto/crypto_user_base.c            | 37 +++++++++++++++++++---------
 crypto/crypto_user_stat.c            |  4 ++-
 include/crypto/internal/cryptouser.h |  2 --
 include/net/net_namespace.h          |  3 +++
 4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/crypto/crypto_user_base.c b/crypto/crypto_user_base.c
index e48da3b75c71..c92d415eaf82 100644
--- a/crypto/crypto_user_base.c
+++ b/crypto/crypto_user_base.c
@@ -22,9 +22,10 @@
 #include <linux/crypto.h>
 #include <linux/cryptouser.h>
 #include <linux/sched.h>
-#include <net/netlink.h>
 #include <linux/security.h>
+#include <net/netlink.h>
 #include <net/net_namespace.h>
+#include <net/sock.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/internal/rng.h>
 #include <crypto/akcipher.h>
@@ -37,9 +38,6 @@
 
 static DEFINE_MUTEX(crypto_cfg_mutex);
 
-/* The crypto netlink socket */
-struct sock *crypto_nlsk;
-
 struct crypto_dump_info {
 	struct sk_buff *in_skb;
 	struct sk_buff *out_skb;
@@ -195,6 +193,7 @@ out:
 static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 			 struct nlattr **attrs)
 {
+	struct net *net = sock_net(in_skb->sk);
 	struct crypto_user_alg *p = nlmsg_data(in_nlh);
 	struct crypto_alg *alg;
 	struct sk_buff *skb;
@@ -226,7 +225,7 @@ drop_alg:
 	if (err)
 		return err;
 
-	return nlmsg_unicast(crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
+	return nlmsg_unicast(net->crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
 }
 
 static int crypto_dump_report(struct sk_buff *skb, struct netlink_callback *cb)
@@ -429,6 +428,7 @@ static const struct crypto_link {
 static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 			       struct netlink_ext_ack *extack)
 {
+	struct net *net = sock_net(skb->sk);
 	struct nlattr *attrs[CRYPTOCFGA_MAX+1];
 	const struct crypto_link *link;
 	int type, err;
@@ -459,7 +459,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 				.done = link->done,
 				.min_dump_alloc = min(dump_alloc, 65535UL),
 			};
-			err = netlink_dump_start(crypto_nlsk, skb, nlh, &c);
+			err = netlink_dump_start(net->crypto_nlsk, skb, nlh, &c);
 		}
 
 		return err;
@@ -483,22 +483,35 @@ static void crypto_netlink_rcv(struct sk_buff *skb)
 	mutex_unlock(&crypto_cfg_mutex);
 }
 
-static int __init crypto_user_init(void)
+static int __net_init crypto_netlink_init(struct net *net)
 {
 	struct netlink_kernel_cfg cfg = {
 		.input	= crypto_netlink_rcv,
 	};
 
-	crypto_nlsk = netlink_kernel_create(&init_net, NETLINK_CRYPTO, &cfg);
-	if (!crypto_nlsk)
-		return -ENOMEM;
+	net->crypto_nlsk = netlink_kernel_create(net, NETLINK_CRYPTO, &cfg);
+	return net->crypto_nlsk == NULL ? -ENOMEM : 0;
+}
 
-	return 0;
+static void __net_exit crypto_netlink_exit(struct net *net)
+{
+	netlink_kernel_release(net->crypto_nlsk);
+	net->crypto_nlsk = NULL;
+}
+
+static struct pernet_operations crypto_netlink_net_ops = {
+	.init = crypto_netlink_init,
+	.exit = crypto_netlink_exit,
+};
+
+static int __init crypto_user_init(void)
+{
+	return register_pernet_subsys(&crypto_netlink_net_ops);
 }
 
 static void __exit crypto_user_exit(void)
 {
-	netlink_kernel_release(crypto_nlsk);
+	unregister_pernet_subsys(&crypto_netlink_net_ops);
 }
 
 module_init(crypto_user_init);
diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
index a03f326a63d3..8bad88413de1 100644
--- a/crypto/crypto_user_stat.c
+++ b/crypto/crypto_user_stat.c
@@ -10,6 +10,7 @@
 #include <linux/cryptouser.h>
 #include <linux/sched.h>
 #include <net/netlink.h>
+#include <net/sock.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/internal/rng.h>
 #include <crypto/akcipher.h>
@@ -298,6 +299,7 @@ out:
 int crypto_reportstat(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 		      struct nlattr **attrs)
 {
+	struct net *net = sock_net(in_skb->sk);
 	struct crypto_user_alg *p = nlmsg_data(in_nlh);
 	struct crypto_alg *alg;
 	struct sk_buff *skb;
@@ -329,7 +331,7 @@ drop_alg:
 	if (err)
 		return err;
 
-	return nlmsg_unicast(crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
+	return nlmsg_unicast(net->crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
 }
 
 MODULE_LICENSE("GPL");
diff --git a/include/crypto/internal/cryptouser.h b/include/crypto/internal/cryptouser.h
index 8c602b187c58..40623f4457df 100644
--- a/include/crypto/internal/cryptouser.h
+++ b/include/crypto/internal/cryptouser.h
@@ -1,8 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #include <net/netlink.h>
 
-extern struct sock *crypto_nlsk;
-
 struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact);
 
 #ifdef CONFIG_CRYPTO_STATS
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 12689ddfc24c..610e40eaea52 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -165,6 +165,9 @@ struct net {
 #endif
 #ifdef CONFIG_XDP_SOCKETS
 	struct netns_xdp	xdp;
+#endif
+#if IS_ENABLED(CONFIG_CRYPTO_USER)
+	struct sock		*crypto_nlsk;
 #endif
 	struct sock		*diag_nlsk;
 	atomic_t		fnhe_genid;
-- 
2.21.0


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

* Re: [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns
  2019-07-09 11:11 [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns Ondrej Mosnacek
@ 2019-07-09 14:38 ` Herbert Xu
  2019-07-09 15:28   ` Ondrej Mosnacek
  2019-07-26 12:32 ` Herbert Xu
  2021-04-05 13:55 ` [PATCH] backports: crypto " Jianmin Wang
  2 siblings, 1 reply; 10+ messages in thread
From: Herbert Xu @ 2019-07-09 14:38 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: linux-crypto, netdev, David S . Miller, Stephan Mueller,
	Steffen Klassert, Don Zickus

On Tue, Jul 09, 2019 at 01:11:24PM +0200, Ondrej Mosnacek wrote:
> Currently, NETLINK_CRYPTO works only in the init network namespace. It
> doesn't make much sense to cut it out of the other network namespaces,
> so do the minor plumbing work necessary to make it work in any network
> namespace. Code inspired by net/core/sock_diag.c.
> 
> Tested using kcapi-dgst from libkcapi [1]:
> Before:
>     # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
>     libkcapi - Error: Netlink error: sendmsg failed
>     libkcapi - Error: Netlink error: sendmsg failed
>     libkcapi - Error: NETLINK_CRYPTO: cannot obtain cipher information for hmac(sha512) (is required crypto_user.c patch missing? see documentation)
>     0
> 
> After:
>     # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
>     32
> 
> [1] https://github.com/smuellerDD/libkcapi
> 
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>

Should we really let root inside a namespace manipulate crypto
algorithms which are global?

I think we should only allow the query operations without deeper
surgery.

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

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

* Re: [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns
  2019-07-09 14:38 ` Herbert Xu
@ 2019-07-09 15:28   ` Ondrej Mosnacek
  2019-07-09 16:14     ` Herbert Xu
  0 siblings, 1 reply; 10+ messages in thread
From: Ondrej Mosnacek @ 2019-07-09 15:28 UTC (permalink / raw)
  To: Herbert Xu
  Cc: linux-crypto, netdev, David S . Miller, Stephan Mueller,
	Steffen Klassert, Don Zickus

On Tue, Jul 9, 2019 at 4:38 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Jul 09, 2019 at 01:11:24PM +0200, Ondrej Mosnacek wrote:
> > Currently, NETLINK_CRYPTO works only in the init network namespace. It
> > doesn't make much sense to cut it out of the other network namespaces,
> > so do the minor plumbing work necessary to make it work in any network
> > namespace. Code inspired by net/core/sock_diag.c.
> >
> > Tested using kcapi-dgst from libkcapi [1]:
> > Before:
> >     # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
> >     libkcapi - Error: Netlink error: sendmsg failed
> >     libkcapi - Error: Netlink error: sendmsg failed
> >     libkcapi - Error: NETLINK_CRYPTO: cannot obtain cipher information for hmac(sha512) (is required crypto_user.c patch missing? see documentation)
> >     0
> >
> > After:
> >     # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
> >     32
> >
> > [1] https://github.com/smuellerDD/libkcapi
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Should we really let root inside a namespace manipulate crypto
> algorithms which are global?

I admit I'm not an expert on Linux namespaces, but aren't you
confusing network and user namespaces? Unless I'm mistaken, these
changes only affect _network_ namespaces (which only isolate the
network stuff itself) and the semantics of the netlink_capable(skb,
CAP_NET_ADMIN) calls remain unchanged - they check if the opener of
the socket has the CAP_NET_ADMIN capability within the global _user_
namespace.

>
> I think we should only allow the query operations without deeper
> surgery.
>
> 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

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

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

* Re: [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns
  2019-07-09 15:28   ` Ondrej Mosnacek
@ 2019-07-09 16:14     ` Herbert Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2019-07-09 16:14 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: linux-crypto, netdev, David S . Miller, Stephan Mueller,
	Steffen Klassert, Don Zickus

On Tue, Jul 09, 2019 at 05:28:35PM +0200, Ondrej Mosnacek wrote:
>
> I admit I'm not an expert on Linux namespaces, but aren't you
> confusing network and user namespaces? Unless I'm mistaken, these
> changes only affect _network_ namespaces (which only isolate the
> network stuff itself) and the semantics of the netlink_capable(skb,
> CAP_NET_ADMIN) calls remain unchanged - they check if the opener of
> the socket has the CAP_NET_ADMIN capability within the global _user_
> namespace.

Good point.  I think your patch should be OK then.

Thanks,
-- 
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

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

* Re: [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns
  2019-07-09 11:11 [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns Ondrej Mosnacek
  2019-07-09 14:38 ` Herbert Xu
@ 2019-07-26 12:32 ` Herbert Xu
  2021-04-05 13:55 ` [PATCH] backports: crypto " Jianmin Wang
  2 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2019-07-26 12:32 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: linux-crypto, netdev, davem, smueller, steffen.klassert, dzickus

Ondrej Mosnacek <omosnace@redhat.com> wrote:
> Currently, NETLINK_CRYPTO works only in the init network namespace. It
> doesn't make much sense to cut it out of the other network namespaces,
> so do the minor plumbing work necessary to make it work in any network
> namespace. Code inspired by net/core/sock_diag.c.
> 
> Tested using kcapi-dgst from libkcapi [1]:
> Before:
>    # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
>    libkcapi - Error: Netlink error: sendmsg failed
>    libkcapi - Error: Netlink error: sendmsg failed
>    libkcapi - Error: NETLINK_CRYPTO: cannot obtain cipher information for hmac(sha512) (is required crypto_user.c patch missing? see documentation)
>    0
> 
> After:
>    # unshare -n kcapi-dgst -c sha256 </dev/null | wc -c
>    32
> 
> [1] https://github.com/smuellerDD/libkcapi
> 
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
> crypto/crypto_user_base.c            | 37 +++++++++++++++++++---------
> crypto/crypto_user_stat.c            |  4 ++-
> include/crypto/internal/cryptouser.h |  2 --
> include/net/net_namespace.h          |  3 +++
> 4 files changed, 31 insertions(+), 15 deletions(-)

Patch applied.  Thanks.
-- 
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

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

* [PATCH] backports: crypto user - make NETLINK_CRYPTO work inside netns
  2019-07-09 11:11 [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns Ondrej Mosnacek
  2019-07-09 14:38 ` Herbert Xu
  2019-07-26 12:32 ` Herbert Xu
@ 2021-04-05 13:55 ` Jianmin Wang
  2021-04-05 16:14   ` Greg KH
  2 siblings, 1 reply; 10+ messages in thread
From: Jianmin Wang @ 2021-04-05 13:55 UTC (permalink / raw)
  To: stable
  Cc: omosnace, davem, dzickus, herbert, linux-crypto, netdev,
	smueller, steffen.klassert, Jianmin Wang

There is same problem found in linux 4.19.y as upstream commit. The 
changes of crypto_user_* and cryptouser.h files from upstream patch are merged into 
crypto/crypto_user.c for backporting.

Upstream commit:
    commit 91b05a7e7d8033a90a64f5fc0e3808db423e420a
    Author: Ondrej Mosnacek <omosnace@redhat.com>
    Date:   Tue,  9 Jul 2019 13:11:24 +0200

    Currently, NETLINK_CRYPTO works only in the init network namespace. It
    doesn't make much sense to cut it out of the other network namespaces,
    so do the minor plumbing work necessary to make it work in any network
    namespace. Code inspired by net/core/sock_diag.c.

    Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
    Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>

Signed-off-by: Jianmin Wang <jianmin@iscas.ac.cn>
---
 crypto/crypto_user.c        | 37 +++++++++++++++++++++++++------------
 include/net/net_namespace.h |  3 +++
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index f847c181a39c..3f9e8e6e96f2 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -22,8 +22,9 @@
 #include <linux/crypto.h>
 #include <linux/cryptouser.h>
 #include <linux/sched.h>
-#include <net/netlink.h>
 #include <linux/security.h>
+#include <net/netlink.h>
+#include <net/sock.h>
 #include <net/net_namespace.h>
 #include <crypto/internal/skcipher.h>
 #include <crypto/internal/rng.h>
@@ -36,9 +37,6 @@
 
 static DEFINE_MUTEX(crypto_cfg_mutex);
 
-/* The crypto netlink socket */
-static struct sock *crypto_nlsk;
-
 struct crypto_dump_info {
 	struct sk_buff *in_skb;
 	struct sk_buff *out_skb;
@@ -260,6 +258,7 @@ static int crypto_report_alg(struct crypto_alg *alg,
 static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 			 struct nlattr **attrs)
 {
+	struct net *net = sock_net(in_skb->sk);
 	struct crypto_user_alg *p = nlmsg_data(in_nlh);
 	struct crypto_alg *alg;
 	struct sk_buff *skb;
@@ -293,7 +292,7 @@ static int crypto_report(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 		return err;
 	}
 
-	return nlmsg_unicast(crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
+	return nlmsg_unicast(net->crypto_nlsk, skb, NETLINK_CB(in_skb).portid);
 }
 
 static int crypto_dump_report(struct sk_buff *skb, struct netlink_callback *cb)
@@ -494,6 +493,7 @@ static const struct crypto_link {
 static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 			       struct netlink_ext_ack *extack)
 {
+	struct net *net = sock_net(skb->sk);
 	struct nlattr *attrs[CRYPTOCFGA_MAX+1];
 	const struct crypto_link *link;
 	int type, err;
@@ -524,7 +524,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 				.done = link->done,
 				.min_dump_alloc = min(dump_alloc, 65535UL),
 			};
-			err = netlink_dump_start(crypto_nlsk, skb, nlh, &c);
+			err = netlink_dump_start(net->crypto_nlsk, skb, nlh, &c);
 		}
 
 		return err;
@@ -548,22 +548,35 @@ static void crypto_netlink_rcv(struct sk_buff *skb)
 	mutex_unlock(&crypto_cfg_mutex);
 }
 
-static int __init crypto_user_init(void)
+static int __net_init crypto_netlink_init(struct net *net)
 {
 	struct netlink_kernel_cfg cfg = {
 		.input	= crypto_netlink_rcv,
 	};
 
-	crypto_nlsk = netlink_kernel_create(&init_net, NETLINK_CRYPTO, &cfg);
-	if (!crypto_nlsk)
-		return -ENOMEM;
+	net->crypto_nlsk = netlink_kernel_create(net, NETLINK_CRYPTO, &cfg);
+	return net->crypto_nlsk == NULL ? -ENOMEM : 0;
+}
 
-	return 0;
+static void __net_exit crypto_netlink_exit(struct net *net)
+{
+	netlink_kernel_release(net->crypto_nlsk);
+	net->crypto_nlsk = NULL;
+}
+
+static struct pernet_operations crypto_netlink_net_ops = {
+	.init = crypto_netlink_init,
+	.exit = crypto_netlink_exit,
+};
+
+static int __init crypto_user_init(void)
+{
+	return register_pernet_subsys(&crypto_netlink_net_ops);
 }
 
 static void __exit crypto_user_exit(void)
 {
-	netlink_kernel_release(crypto_nlsk);
+	unregister_pernet_subsys(&crypto_netlink_net_ops);
 }
 
 module_init(crypto_user_init);
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 5007eaba207d..ab5e8fd011f9 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -158,6 +158,9 @@ struct net {
 #endif
 #if IS_ENABLED(CONFIG_CAN)
 	struct netns_can	can;
+#endif
+#if IS_ENABLED(CONFIG_CRYPTO_USER)
+	struct sock		*crypto_nlsk;
 #endif
 	struct sock		*diag_nlsk;
 	atomic_t		fnhe_genid;
-- 
2.27.0


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

* Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work inside netns
  2021-04-05 13:55 ` [PATCH] backports: crypto " Jianmin Wang
@ 2021-04-05 16:14   ` Greg KH
  2021-04-08 19:11     ` Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work Jianmin Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2021-04-05 16:14 UTC (permalink / raw)
  To: Jianmin Wang
  Cc: stable, omosnace, davem, dzickus, herbert, linux-crypto, netdev,
	smueller, steffen.klassert

On Mon, Apr 05, 2021 at 01:55:15PM +0000, Jianmin Wang wrote:
> There is same problem found in linux 4.19.y as upstream commit. The 
> changes of crypto_user_* and cryptouser.h files from upstream patch are merged into 
> crypto/crypto_user.c for backporting.
> 
> Upstream commit:
>     commit 91b05a7e7d8033a90a64f5fc0e3808db423e420a
>     Author: Ondrej Mosnacek <omosnace@redhat.com>
>     Date:   Tue,  9 Jul 2019 13:11:24 +0200
> 
>     Currently, NETLINK_CRYPTO works only in the init network namespace. It
>     doesn't make much sense to cut it out of the other network namespaces,
>     so do the minor plumbing work necessary to make it work in any network
>     namespace. Code inspired by net/core/sock_diag.c.
> 
>     Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>     Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
> 
> Signed-off-by: Jianmin Wang <jianmin@iscas.ac.cn>
> ---
>  crypto/crypto_user.c        | 37 +++++++++++++++++++++++++------------
>  include/net/net_namespace.h |  3 +++
>  2 files changed, 28 insertions(+), 12 deletions(-)

How does this change fit with the stable kernel rules?  It looks to be a
new feature, if you need this, why not just use a newer kernel version?
What is preventing you from doing that?

thanks,

greg k-h

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

* Re: Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work
  2021-04-05 16:14   ` Greg KH
@ 2021-04-08 19:11     ` Jianmin Wang
  2021-04-09  6:36       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Jianmin Wang @ 2021-04-08 19:11 UTC (permalink / raw)
  To: gregkh
  Cc: davem, dzickus, herbert, jianmin, linux-crypto, netdev, omosnace,
	smueller, stable, steffen.klassert

On Mon, Apr 05, 2021 at 16:14 UTC, Greg KH wrote:
> On Mon, Apr 05, 2021 at 01:55:15PM +0000, Jianmin Wang wrote:
> > There is same problem found in linux 4.19.y as upstream commit. The 
> > changes of crypto_user_* and cryptouser.h files from upstream patch are merged into 
> > crypto/crypto_user.c for backporting.
> > 
> > Upstream commit:
> >     commit 91b05a7e7d8033a90a64f5fc0e3808db423e420a
> >     Author: Ondrej Mosnacek <omosnace@redhat.com>
> >     Date:   Tue,  9 Jul 2019 13:11:24 +0200
> > 
> >     Currently, NETLINK_CRYPTO works only in the init network namespace. It
> >     doesn't make much sense to cut it out of the other network namespaces,
> >     so do the minor plumbing work necessary to make it work in any network
> >     namespace. Code inspired by net/core/sock_diag.c.
> > 
> >     Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> >     Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
> > 
> > Signed-off-by: Jianmin Wang <jianmin@iscas.ac.cn>
> > ---
> >  crypto/crypto_user.c        | 37 +++++++++++++++++++++++++------------
> >  include/net/net_namespace.h |  3 +++
> >  2 files changed, 28 insertions(+), 12 deletions(-)
> 
> How does this change fit with the stable kernel rules?  It looks to be a
> new feature, if you need this, why not just use a newer kernel version?
> What is preventing you from doing that?
> 

This problem was found when we deployed new services on our container cluster, 
while the new services need to invoke libkcapi in the container environment.

We have verified that the problem doesn't exist on newer kernel version. 
However, due to many services and the cluster running on many server machines 
whose host os are long-term linux distribution with linux 4.19 kernel, it will 
cost too much to migrate them to newer os with newer kernel version. This is 
why we need to fix the problem on linux 4.19.

Only when we run docker with param --net=host, the libkcapi can be invoked 
properly. Otherwise, almost all test cases in smuellerDD/libkcapi [1] will 
failed with same error as below:

    libkcapi - Error: Netlink error: sendmsg failed
    libkcapi - Error: Netlink error: sendmsg failed
    libkcapi - Error: NETLINK_CRYPTO: cannot obtain cipher information for 
      hmac(sha1) (is required crypto_user.c patch missing? see documentation)

The cause is same as statement in upstream commit 91b05a7e, which is that 
NETLINK_CRYPTO works only in the init network namespace.

In my opinion, there are still many linux distribution running with linux 4.19 
or similar version, such as Debian 10 with linux 4.19, CentOS 8 with linux 4.18
and also their derivatives. If other people want to use libkcapi in container 
environment, they will also be bothered by this problem. [2]

So I think this patch meet two rules in stable kernel rules: It must fix a real
bug that bothers people and the upstream commit 91b05a7e exists in Linus's tree
from linux 5.4.

Thanks for your review and reply.

--
Email: Jianmin Wang <jianmin@iscas.ac.cn>


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

* Re: Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work
  2021-04-08 19:11     ` Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work Jianmin Wang
@ 2021-04-09  6:36       ` Greg KH
  2021-04-09 13:14         ` Jianmin Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2021-04-09  6:36 UTC (permalink / raw)
  To: Jianmin Wang
  Cc: davem, dzickus, herbert, linux-crypto, netdev, omosnace,
	smueller, stable, steffen.klassert

On Thu, Apr 08, 2021 at 07:11:48PM +0000, Jianmin Wang wrote:
> On Mon, Apr 05, 2021 at 16:14 UTC, Greg KH wrote:
> > On Mon, Apr 05, 2021 at 01:55:15PM +0000, Jianmin Wang wrote:
> > > There is same problem found in linux 4.19.y as upstream commit. The 
> > > changes of crypto_user_* and cryptouser.h files from upstream patch are merged into 
> > > crypto/crypto_user.c for backporting.
> > > 
> > > Upstream commit:
> > >     commit 91b05a7e7d8033a90a64f5fc0e3808db423e420a
> > >     Author: Ondrej Mosnacek <omosnace@redhat.com>
> > >     Date:   Tue,  9 Jul 2019 13:11:24 +0200
> > > 
> > >     Currently, NETLINK_CRYPTO works only in the init network namespace. It
> > >     doesn't make much sense to cut it out of the other network namespaces,
> > >     so do the minor plumbing work necessary to make it work in any network
> > >     namespace. Code inspired by net/core/sock_diag.c.
> > > 
> > >     Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > >     Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
> > > 
> > > Signed-off-by: Jianmin Wang <jianmin@iscas.ac.cn>
> > > ---
> > >  crypto/crypto_user.c        | 37 +++++++++++++++++++++++++------------
> > >  include/net/net_namespace.h |  3 +++
> > >  2 files changed, 28 insertions(+), 12 deletions(-)
> > 
> > How does this change fit with the stable kernel rules?  It looks to be a
> > new feature, if you need this, why not just use a newer kernel version?
> > What is preventing you from doing that?
> > 
> 
> This problem was found when we deployed new services on our container cluster, 
> while the new services need to invoke libkcapi in the container environment.
> 
> We have verified that the problem doesn't exist on newer kernel version. 
> However, due to many services and the cluster running on many server machines 
> whose host os are long-term linux distribution with linux 4.19 kernel, it will 
> cost too much to migrate them to newer os with newer kernel version. This is 
> why we need to fix the problem on linux 4.19.

But this is not a regression, but rather a "resolve an issue that has
never worked for new hardware", right?

And for that, moving to a new kernel seems like a wise thing to do to
me because we do not like backporting new features.  Distro kernel are
of course, free to do that if they wish.

thanks,

greg k-h

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

* Re: Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work
  2021-04-09  6:36       ` Greg KH
@ 2021-04-09 13:14         ` Jianmin Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Jianmin Wang @ 2021-04-09 13:14 UTC (permalink / raw)
  To: gregkh
  Cc: davem, dzickus, herbert, jianmin, linux-crypto, netdev, omosnace,
	smueller, stable, steffen.klassert

On Fri, Apr 09, 2021 at 08:36:07 +0200, Greg KH
> On Thu, Apr 08, 2021 at 07:11:48PM +0000, Jianmin Wang wrote:
> > while the new services need to invoke libkcapi in the container environment.
> > 
> > We have verified that the problem doesn't exist on newer kernel version. 
> > However, due to many services and the cluster running on many server machines 
> > whose host os are long-term linux distribution with linux 4.19 kernel, it will 
> > cost too much to migrate them to newer os with newer kernel version. This is 
> > why we need to fix the problem on linux 4.19.
>
> But this is not a regression, but rather a "resolve an issue that has
> never worked for new hardware", right?
> 
> And for that, moving to a new kernel seems like a wise thing to do to
> me because we do not like backporting new features.  Distro kernel are
> of course, free to do that if they wish.
> 
> thanks,
> 
> greg k-h

I understand. Thank you for your review and response.
--
Email: Jianmin Wang <jianmin@iscas.ac.cn>


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

end of thread, other threads:[~2021-04-09 13:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-09 11:11 [PATCH] crypto: user - make NETLINK_CRYPTO work inside netns Ondrej Mosnacek
2019-07-09 14:38 ` Herbert Xu
2019-07-09 15:28   ` Ondrej Mosnacek
2019-07-09 16:14     ` Herbert Xu
2019-07-26 12:32 ` Herbert Xu
2021-04-05 13:55 ` [PATCH] backports: crypto " Jianmin Wang
2021-04-05 16:14   ` Greg KH
2021-04-08 19:11     ` Re: [PATCH] backports: crypto user - make NETLINK_CRYPTO work Jianmin Wang
2021-04-09  6:36       ` Greg KH
2021-04-09 13:14         ` Jianmin Wang

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.