bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf 0/4] Expose network namespace cookies to user space
@ 2021-02-10 12:04 Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option Lorenz Bauer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-02-10 12:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: kernel-team, Lorenz Bauer, bpf, linux-alpha, linux-api,
	linux-arch, linux-fsdevel, linux-kernel, linux-kselftest,
	linux-mips, linux-parisc, netdev, sparclinux

We're working on a user space control plane for the BPF sk_lookup
hook [1]. The hook attaches to a network namespace and allows
control over which socket receives a new connection / packet.

Roughly, applications can give a socket to our user space component
to participate in custom bind semantics. This creates an edge case
where  an application can provide us with a socket that lives in
a different network namespace than our BPF sk_lookup program.
We'd like to return an error in this case.

Additionally, we have some user space state that is tied to the
network namespace. We currently use the inode of the nsfs entry
in a directory name, but this is suffers from inode reuse.

I'm proposing to fix both of these issues by adding a new
SO_NETNS_COOKIE socket option as well as a NS_GET_COOKIE ioctl.
Using these we get a stable, unique identifier for a network
namespace and check whether a socket belongs to the "correct"
namespace.

NS_GET_COOKIE could be renamed to NS_GET_NET_COOKIE. I kept the
name generic because it seems like other namespace types could
benefit from a cookie as well.

I'm trying to land this via the bpf tree since this is where the
netns cookie originated, please let me know if this isn't
appropriate.

1: https://www.kernel.org/doc/html/latest/bpf/prog_sk_lookup.html

Cc: bpf@vger.kernel.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: sparclinux@vger.kernel.org

Lorenz Bauer (4):
  net: add SO_NETNS_COOKIE socket option
  nsfs: add an ioctl to discover the network namespace cookie
  tools/testing: add test for NS_GET_COOKIE
  tools/testing: add a selftest for SO_NETNS_COOKIE

 arch/alpha/include/uapi/asm/socket.h          |  2 +
 arch/mips/include/uapi/asm/socket.h           |  2 +
 arch/parisc/include/uapi/asm/socket.h         |  2 +
 arch/sparc/include/uapi/asm/socket.h          |  2 +
 fs/nsfs.c                                     |  9 +++
 include/linux/sock_diag.h                     | 20 ++++++
 include/net/net_namespace.h                   | 11 ++++
 include/uapi/asm-generic/socket.h             |  2 +
 include/uapi/linux/nsfs.h                     |  2 +
 net/core/filter.c                             |  9 ++-
 net/core/sock.c                               |  7 +++
 tools/testing/selftests/net/.gitignore        |  1 +
 tools/testing/selftests/net/Makefile          |  2 +-
 tools/testing/selftests/net/so_netns_cookie.c | 61 +++++++++++++++++++
 tools/testing/selftests/nsfs/.gitignore       |  1 +
 tools/testing/selftests/nsfs/Makefile         |  2 +-
 tools/testing/selftests/nsfs/netns.c          | 57 +++++++++++++++++
 17 files changed, 185 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/net/so_netns_cookie.c
 create mode 100644 tools/testing/selftests/nsfs/netns.c

-- 
2.27.0


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

* [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option
  2021-02-10 12:04 [PATCH bpf 0/4] Expose network namespace cookies to user space Lorenz Bauer
@ 2021-02-10 12:04 ` Lorenz Bauer
  2021-02-10 14:51   ` Eric Dumazet
  2021-02-10 12:04 ` [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie Lorenz Bauer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Lorenz Bauer @ 2021-02-10 12:04 UTC (permalink / raw)
  To: Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
	David S. Miller, Arnd Bergmann, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Jakub Kicinski
  Cc: kernel-team, Lorenz Bauer, linux-api, linux-alpha, linux-kernel,
	linux-mips, linux-parisc, sparclinux, linux-arch, netdev, bpf

We need to distinguish which network namespace a socket belongs to.
BPF has the useful bpf_get_netns_cookie helper for this, but accessing
it from user space isn't possible. Add a read-only socket option that
returns the netns cookie, similar to SO_COOKIE. If network namespaces
are disabled, SO_NETNS_COOKIE returns the cookie of init_net.

The BPF helpers change slightly: instead of returning 0 when network
namespaces are disabled we return the init_net cookie as for the
socket option.

Cc: linux-api@vger.kernel.org
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 arch/alpha/include/uapi/asm/socket.h  |  2 ++
 arch/mips/include/uapi/asm/socket.h   |  2 ++
 arch/parisc/include/uapi/asm/socket.h |  2 ++
 arch/sparc/include/uapi/asm/socket.h  |  2 ++
 include/linux/sock_diag.h             | 20 ++++++++++++++++++++
 include/uapi/asm-generic/socket.h     |  2 ++
 net/core/filter.c                     |  9 ++++-----
 net/core/sock.c                       |  7 +++++++
 8 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 57420356ce4c..6b3daba60987 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -127,6 +127,8 @@
 #define SO_PREFER_BUSY_POLL	69
 #define SO_BUSY_POLL_BUDGET	70
 
+#define SO_NETNS_COOKIE		71
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 2d949969313b..cdf404a831b2 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -138,6 +138,8 @@
 #define SO_PREFER_BUSY_POLL	69
 #define SO_BUSY_POLL_BUDGET	70
 
+#define SO_NETNS_COOKIE		71
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index f60904329bbc..5b5351cdcb33 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -119,6 +119,8 @@
 #define SO_PREFER_BUSY_POLL	0x4043
 #define SO_BUSY_POLL_BUDGET	0x4044
 
+#define SO_NETNS_COOKIE		0x4045
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 848a22fbac20..ff79db753dce 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -120,6 +120,8 @@
 #define SO_PREFER_BUSY_POLL	 0x0048
 #define SO_BUSY_POLL_BUDGET	 0x0049
 
+#define SO_NETNS_COOKIE		 0x004a
+
 #if !defined(__KERNEL__)
 
 
diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index 0b9ecd8cf979..6e88436097b1 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -38,6 +38,26 @@ static inline u64 sock_gen_cookie(struct sock *sk)
 	return cookie;
 }
 
+static inline u64 __sock_gen_netns_cookie(struct sock *sk)
+{
+#ifdef CONFIG_NET_NS
+	return __net_gen_cookie(sk->sk_net.net);
+#else
+	return __net_gen_cookie(&init_net);
+#endif
+}
+
+static inline u64 sock_gen_netns_cookie(struct sock *sk)
+{
+	u64 cookie;
+
+	preempt_disable();
+	cookie = __sock_gen_netns_cookie(sk);
+	preempt_enable();
+
+	return cookie;
+}
+
 int sock_diag_check_cookie(struct sock *sk, const __u32 *cookie);
 void sock_diag_save_cookie(struct sock *sk, __u32 *cookie);
 
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 4dcd13d097a9..d588c244ec2f 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -122,6 +122,8 @@
 #define SO_PREFER_BUSY_POLL	69
 #define SO_BUSY_POLL_BUDGET	70
 
+#define SO_NETNS_COOKIE		71
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/net/core/filter.c b/net/core/filter.c
index e15d4741719a..51f47b6913f1 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4645,11 +4645,10 @@ static const struct bpf_func_proto bpf_get_socket_cookie_sock_ops_proto = {
 
 static u64 __bpf_get_netns_cookie(struct sock *sk)
 {
-#ifdef CONFIG_NET_NS
-	return __net_gen_cookie(sk ? sk->sk_net.net : &init_net);
-#else
-	return 0;
-#endif
+	if (sk)
+		return __sock_gen_netns_cookie(sk);
+
+	return __net_gen_cookie(&init_net);
 }
 
 BPF_CALL_1(bpf_get_netns_cookie_sock, struct sock *, ctx)
diff --git a/net/core/sock.c b/net/core/sock.c
index bbcd4b97eddd..2db201c210ca 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1606,6 +1606,13 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_bound_dev_if;
 		break;
 
+	case SO_NETNS_COOKIE:
+		lv = sizeof(u64);
+		if (len < lv)
+			return -EINVAL;
+		v.val64 = sock_gen_netns_cookie(sk);
+		break;
+
 	default:
 		/* We implement the SO_SNDLOWAT etc to not be settable
 		 * (1003.1g 7).
-- 
2.27.0


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

* [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie
  2021-02-10 12:04 [PATCH bpf 0/4] Expose network namespace cookies to user space Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option Lorenz Bauer
@ 2021-02-10 12:04 ` Lorenz Bauer
  2021-03-01 10:04   ` Christian Brauner
  2021-02-10 12:04 ` [PATCH bpf 3/4] tools/testing: add test for NS_GET_COOKIE Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 4/4] tools/testing: add a selftest for SO_NETNS_COOKIE Lorenz Bauer
  3 siblings, 1 reply; 9+ messages in thread
From: Lorenz Bauer @ 2021-02-10 12:04 UTC (permalink / raw)
  To: Alexander Viro, David S. Miller, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: kernel-team, Lorenz Bauer, linux-api, linux-fsdevel,
	linux-kernel, netdev, bpf

Network namespaces have a globally unique non-zero identifier aka a
cookie, in line with socket cookies. Add an ioctl to retrieve the
cookie from user space without going via BPF.

Cc: linux-api@vger.kernel.org
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 fs/nsfs.c                   |  9 +++++++++
 include/net/net_namespace.h | 11 +++++++++++
 include/uapi/linux/nsfs.h   |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 800c1d0eb0d0..d7865e39c049 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -11,6 +11,7 @@
 #include <linux/user_namespace.h>
 #include <linux/nsfs.h>
 #include <linux/uaccess.h>
+#include <net/net_namespace.h>
 
 #include "internal.h"
 
@@ -191,6 +192,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 	struct user_namespace *user_ns;
 	struct ns_common *ns = get_proc_ns(file_inode(filp));
 	uid_t __user *argp;
+	struct net *net_ns;
+	u64 cookie;
 	uid_t uid;
 
 	switch (ioctl) {
@@ -209,6 +212,12 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 		argp = (uid_t __user *) arg;
 		uid = from_kuid_munged(current_user_ns(), user_ns->owner);
 		return put_user(uid, argp);
+	case NS_GET_COOKIE:
+		if (ns->ops->type != CLONE_NEWNET)
+			return -EINVAL;
+		net_ns = container_of(ns, struct net, ns);
+		cookie = net_gen_cookie(net_ns);
+		return put_user(cookie, (u64 __user *)arg);
 	default:
 		return -ENOTTY;
 	}
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 29567875f428..bbd22dfa9345 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -226,6 +226,17 @@ struct net *get_net_ns_by_fd(int fd);
 
 u64 __net_gen_cookie(struct net *net);
 
+static inline u64 net_gen_cookie(struct net *net)
+{
+	u64 cookie;
+
+	preempt_disable();
+	cookie = __net_gen_cookie(net);
+	preempt_enable();
+
+	return cookie;
+}
+
 #ifdef CONFIG_SYSCTL
 void ipx_register_sysctl(void);
 void ipx_unregister_sysctl(void);
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index a0c8552b64ee..86611c2cf908 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -15,5 +15,7 @@
 #define NS_GET_NSTYPE		_IO(NSIO, 0x3)
 /* Get owner UID (in the caller's user namespace) for a user namespace */
 #define NS_GET_OWNER_UID	_IO(NSIO, 0x4)
+/* Returns a unique non-zero identifier for a network namespace */
+#define NS_GET_COOKIE		_IO(NSIO, 0x5)
 
 #endif /* __LINUX_NSFS_H */
-- 
2.27.0


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

* [PATCH bpf 3/4] tools/testing: add test for NS_GET_COOKIE
  2021-02-10 12:04 [PATCH bpf 0/4] Expose network namespace cookies to user space Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie Lorenz Bauer
@ 2021-02-10 12:04 ` Lorenz Bauer
  2021-02-10 12:04 ` [PATCH bpf 4/4] tools/testing: add a selftest for SO_NETNS_COOKIE Lorenz Bauer
  3 siblings, 0 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-02-10 12:04 UTC (permalink / raw)
  To: Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: kernel-team, Lorenz Bauer, linux-kernel, linux-kselftest, netdev, bpf

Check that NS_GET_COOKIE returns a non-zero value, and that distinct
network namespaces have different cookies.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/testing/selftests/nsfs/.gitignore |  1 +
 tools/testing/selftests/nsfs/Makefile   |  2 +-
 tools/testing/selftests/nsfs/netns.c    | 57 +++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/nsfs/netns.c

diff --git a/tools/testing/selftests/nsfs/.gitignore b/tools/testing/selftests/nsfs/.gitignore
index ed79ebdf286e..ca31b216215b 100644
--- a/tools/testing/selftests/nsfs/.gitignore
+++ b/tools/testing/selftests/nsfs/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 owner
 pidns
+netns
diff --git a/tools/testing/selftests/nsfs/Makefile b/tools/testing/selftests/nsfs/Makefile
index dd9bd50b7b93..93793cdb5a7c 100644
--- a/tools/testing/selftests/nsfs/Makefile
+++ b/tools/testing/selftests/nsfs/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-TEST_GEN_PROGS := owner pidns
+TEST_GEN_PROGS := owner pidns netns
 
 CFLAGS := -Wall -Werror
 
diff --git a/tools/testing/selftests/nsfs/netns.c b/tools/testing/selftests/nsfs/netns.c
new file mode 100644
index 000000000000..8ab862667b45
--- /dev/null
+++ b/tools/testing/selftests/nsfs/netns.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+
+#define NSIO    0xb7
+#define NS_GET_COOKIE   _IO(NSIO, 0x5)
+
+#define pr_err(fmt, ...) \
+		({ \
+			fprintf(stderr, "%s:%d:" fmt ": %m\n", \
+				__func__, __LINE__, ##__VA_ARGS__); \
+			1; \
+		})
+
+int main(int argc, char *argvp[])
+{
+	uint64_t cookie1, cookie2;
+	char path[128];
+	int ns;
+
+	snprintf(path, sizeof(path), "/proc/%d/ns/net", getpid());
+	ns = open(path, O_RDONLY);
+	if (ns < 0)
+		return pr_err("Unable to open %s", path);
+
+	if (ioctl(ns, NS_GET_COOKIE, &cookie1))
+		return pr_err("Unable to get first namespace cookie");
+
+	if (!cookie1)
+		return pr_err("NS_GET_COOKIE returned zero first cookie");
+
+	close(ns);
+	if (unshare(CLONE_NEWNET))
+		return pr_err("unshare");
+
+	ns = open(path, O_RDONLY);
+	if (ns < 0)
+		return pr_err("Unable to open %s", path);
+
+	if (ioctl(ns, NS_GET_COOKIE, &cookie2))
+		return pr_err("Unable to get second namespace cookie");
+
+	if (!cookie2)
+		return pr_err("NS_GET_COOKIE returned zero second cookie");
+
+	if (cookie1 == cookie2)
+		return pr_err("NS_GET_COOKIE returned identical cookies for distinct ns");
+
+	close(ns);
+	return 0;
+}
-- 
2.27.0


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

* [PATCH bpf 4/4] tools/testing: add a selftest for SO_NETNS_COOKIE
  2021-02-10 12:04 [PATCH bpf 0/4] Expose network namespace cookies to user space Lorenz Bauer
                   ` (2 preceding siblings ...)
  2021-02-10 12:04 ` [PATCH bpf 3/4] tools/testing: add test for NS_GET_COOKIE Lorenz Bauer
@ 2021-02-10 12:04 ` Lorenz Bauer
  3 siblings, 0 replies; 9+ messages in thread
From: Lorenz Bauer @ 2021-02-10 12:04 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Shuah Khan, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko
  Cc: kernel-team, Lorenz Bauer, linux-kernel, netdev, linux-kselftest, bpf

Make sure that SO_NETNS_COOKIE returns a non-zero value, and
that sockets from different namespaces have a distinct cookie
value.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/testing/selftests/net/.gitignore        |  1 +
 tools/testing/selftests/net/Makefile          |  2 +-
 tools/testing/selftests/net/so_netns_cookie.c | 61 +++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/so_netns_cookie.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 61ae899cfc17..19deb9cdf72f 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -30,3 +30,4 @@ hwtstamp_config
 rxtimestamp
 timestamping
 txtimestamp
+so_netns_cookie
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index fa5fa425d148..a0f45a71a8f1 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -27,7 +27,7 @@ TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
 TEST_GEN_FILES += tcp_mmap tcp_inq psock_snd txring_overwrite
 TEST_GEN_FILES += udpgso udpgso_bench_tx udpgso_bench_rx ip_defrag
-TEST_GEN_FILES += so_txtime ipv6_flowlabel ipv6_flowlabel_mgr
+TEST_GEN_FILES += so_txtime ipv6_flowlabel ipv6_flowlabel_mgr so_netns_cookie
 TEST_GEN_FILES += tcp_fastopen_backup_key
 TEST_GEN_FILES += fin_ack_lat
 TEST_GEN_FILES += reuseaddr_ports_exhausted
diff --git a/tools/testing/selftests/net/so_netns_cookie.c b/tools/testing/selftests/net/so_netns_cookie.c
new file mode 100644
index 000000000000..b39e87e967cd
--- /dev/null
+++ b/tools/testing/selftests/net/so_netns_cookie.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+#ifndef SO_NETNS_COOKIE
+#define SO_NETNS_COOKIE 71
+#endif
+
+#define pr_err(fmt, ...) \
+	({ \
+		fprintf(stderr, "%s:%d:" fmt ": %m\n", \
+			__func__, __LINE__, ##__VA_ARGS__); \
+		1; \
+	})
+
+int main(int argc, char *argvp[])
+{
+	uint64_t cookie1, cookie2;
+	socklen_t vallen;
+	int sock1, sock2;
+
+	sock1 = socket(AF_INET, SOCK_STREAM, 0);
+	if (sock1 < 0)
+		return pr_err("Unable to create TCP socket");
+
+	vallen = sizeof(cookie1);
+	if (getsockopt(sock1, SOL_SOCKET, SO_NETNS_COOKIE, &cookie1, &vallen) != 0)
+		return pr_err("getsockopt(SOL_SOCKET, SO_NETNS_COOKIE)");
+
+	if (!cookie1)
+		return pr_err("SO_NETNS_COOKIE returned zero cookie");
+
+	if (unshare(CLONE_NEWNET))
+		return pr_err("unshare");
+
+	sock2 = socket(AF_INET, SOCK_STREAM, 0);
+	if (sock2 < 0)
+		return pr_err("Unable to create TCP socket");
+
+	vallen = sizeof(cookie2);
+	if (getsockopt(sock2, SOL_SOCKET, SO_NETNS_COOKIE, &cookie2, &vallen) != 0)
+		return pr_err("getsockopt(SOL_SOCKET, SO_NETNS_COOKIE)");
+
+	if (!cookie2)
+		return pr_err("SO_NETNS_COOKIE returned zero cookie");
+
+	if (cookie1 == cookie2)
+		return pr_err("SO_NETNS_COOKIE returned identical cookies for distinct ns");
+
+	close(sock1);
+	close(sock2);
+	return 0;
+}
-- 
2.27.0


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

* Re: [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option
  2021-02-10 12:04 ` [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option Lorenz Bauer
@ 2021-02-10 14:51   ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2021-02-10 14:51 UTC (permalink / raw)
  To: Lorenz Bauer, Richard Henderson, Ivan Kokshaysky, Matt Turner,
	Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
	David S. Miller, Arnd Bergmann, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Jakub Kicinski
  Cc: kernel-team, linux-api, linux-alpha, linux-kernel, linux-mips,
	linux-parisc, sparclinux, linux-arch, netdev, bpf



On 2/10/21 1:04 PM, Lorenz Bauer wrote:
> We need to distinguish which network namespace a socket belongs to.
> BPF has the useful bpf_get_netns_cookie helper for this, but accessing
> it from user space isn't possible. Add a read-only socket option that
> returns the netns cookie, similar to SO_COOKIE. If network namespaces
> are disabled, SO_NETNS_COOKIE returns the cookie of init_net.
> 
> The BPF helpers change slightly: instead of returning 0 when network
> namespaces are disabled we return the init_net cookie as for the
> socket option.
> 
> Cc: linux-api@vger.kernel.org
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---

...

>  
> +static inline u64 __sock_gen_netns_cookie(struct sock *sk)
> +{
> +#ifdef CONFIG_NET_NS
> +	return __net_gen_cookie(sk->sk_net.net);
> +#else
> +	return __net_gen_cookie(&init_net);
> +#endif
> +}
> +
> +static inline u64 sock_gen_netns_cookie(struct sock *sk)
> +{
> +	u64 cookie;
> +
> +	preempt_disable();
> +	cookie = __sock_gen_netns_cookie(sk);
> +	preempt_enable();
> +
> +	return cookie;
> +}
> +
> 

I suggest we make net->net_cookie a mere u64 initialized in setup_net(),
instead of having to preempt_disable() around reading it.

(Here and in your patch 2/4)

Your patches would be much simpler.

Cleanup patch :

https://patchwork.kernel.org/project/netdevbpf/patch/20210210144144.24284-1-eric.dumazet@gmail.com/



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

* Re: [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie
  2021-02-10 12:04 ` [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie Lorenz Bauer
@ 2021-03-01 10:04   ` Christian Brauner
  2021-03-02  9:47     ` Lorenz Bauer
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Brauner @ 2021-03-01 10:04 UTC (permalink / raw)
  To: Lorenz Bauer
  Cc: Alexander Viro, David S. Miller, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	kernel-team, linux-api, linux-fsdevel, linux-kernel, netdev, bpf

On Wed, Feb 10, 2021 at 12:04:23PM +0000, Lorenz Bauer wrote:
> Network namespaces have a globally unique non-zero identifier aka a
> cookie, in line with socket cookies. Add an ioctl to retrieve the
> cookie from user space without going via BPF.
> 
> Cc: linux-api@vger.kernel.org
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
>  fs/nsfs.c                   |  9 +++++++++
>  include/net/net_namespace.h | 11 +++++++++++
>  include/uapi/linux/nsfs.h   |  2 ++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/fs/nsfs.c b/fs/nsfs.c
> index 800c1d0eb0d0..d7865e39c049 100644
> --- a/fs/nsfs.c
> +++ b/fs/nsfs.c
> @@ -11,6 +11,7 @@
>  #include <linux/user_namespace.h>
>  #include <linux/nsfs.h>
>  #include <linux/uaccess.h>
> +#include <net/net_namespace.h>
>  
>  #include "internal.h"
>  
> @@ -191,6 +192,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  	struct user_namespace *user_ns;
>  	struct ns_common *ns = get_proc_ns(file_inode(filp));
>  	uid_t __user *argp;
> +	struct net *net_ns;
> +	u64 cookie;
>  	uid_t uid;
>  
>  	switch (ioctl) {
> @@ -209,6 +212,12 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  		argp = (uid_t __user *) arg;
>  		uid = from_kuid_munged(current_user_ns(), user_ns->owner);
>  		return put_user(uid, argp);
> +	case NS_GET_COOKIE:
> +		if (ns->ops->type != CLONE_NEWNET)
> +			return -EINVAL;
> +		net_ns = container_of(ns, struct net, ns);
> +		cookie = net_gen_cookie(net_ns);
> +		return put_user(cookie, (u64 __user *)arg);

Hey Lorenz,

Just to make sure: is it intentional that any user can retrieve the
cookie associated with any network namespace, i.e. you don't require any
form of permission checking in the owning user namespace of the network
namespace?

Christian

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

* Re: [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie
  2021-03-01 10:04   ` Christian Brauner
@ 2021-03-02  9:47     ` Lorenz Bauer
  2021-03-02 10:14       ` Christian Brauner
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenz Bauer @ 2021-03-02  9:47 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Alexander Viro, David S. Miller, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	kernel-team, Linux API, linux-fsdevel, LKML, Networking, bpf

On Mon, 1 Mar 2021 at 10:04, Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> Hey Lorenz,
>
> Just to make sure: is it intentional that any user can retrieve the
> cookie associated with any network namespace, i.e. you don't require any
> form of permission checking in the owning user namespace of the network
> namespace?
>
> Christian

Hi Christian,

I've decided to drop the patch set for now, but that was my intention, yes. Is
there a downside I'm not aware of?

Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

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

* Re: [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie
  2021-03-02  9:47     ` Lorenz Bauer
@ 2021-03-02 10:14       ` Christian Brauner
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Brauner @ 2021-03-02 10:14 UTC (permalink / raw)
  To: Lorenz Bauer
  Cc: Alexander Viro, David S. Miller, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	kernel-team, Linux API, linux-fsdevel, LKML, Networking, bpf

On Tue, Mar 02, 2021 at 09:47:10AM +0000, Lorenz Bauer wrote:
> On Mon, 1 Mar 2021 at 10:04, Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> >
> > Hey Lorenz,
> >
> > Just to make sure: is it intentional that any user can retrieve the
> > cookie associated with any network namespace, i.e. you don't require any
> > form of permission checking in the owning user namespace of the network
> > namespace?
> >
> > Christian
> 
> Hi Christian,
> 
> I've decided to drop the patch set for now, but that was my intention, yes. Is
> there a downside I'm not aware of?

It depends on whether this cookie is in any way security or at least
information sensitive. For example, would leaking it between
unprivileged containers with different user+network namespace pairs
allow one container to gain access to information about the other
container that it shouldn't.

Christian

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

end of thread, other threads:[~2021-03-02 10:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 12:04 [PATCH bpf 0/4] Expose network namespace cookies to user space Lorenz Bauer
2021-02-10 12:04 ` [PATCH bpf 1/4] net: add SO_NETNS_COOKIE socket option Lorenz Bauer
2021-02-10 14:51   ` Eric Dumazet
2021-02-10 12:04 ` [PATCH bpf 2/4] nsfs: add an ioctl to discover the network namespace cookie Lorenz Bauer
2021-03-01 10:04   ` Christian Brauner
2021-03-02  9:47     ` Lorenz Bauer
2021-03-02 10:14       ` Christian Brauner
2021-02-10 12:04 ` [PATCH bpf 3/4] tools/testing: add test for NS_GET_COOKIE Lorenz Bauer
2021-02-10 12:04 ` [PATCH bpf 4/4] tools/testing: add a selftest for SO_NETNS_COOKIE Lorenz Bauer

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).