Skip to contentSkip to conversation

Socket C/R additional features

PATCH In review Dan Smith 3 participants 7 messages Aug 18, 2009 – Aug 20, 2009

Subsystems (3)
lore
Share & tools
lore

Dan Smith

author
#0 lore raw

This is an updated version of my previous patch set, with the fourth patch removed. This is rebased on current v17-dev and includes Serge's recommended changes to may_setgid().

author
#1 lore raw

PATCH 1/3 Set socket flags on restore using sock_setsockopt() where possible (v2)

Fail on the TIMESTAMPING_* flags for the moment, with a TODO in place to handle them later.

Also remove other explicit flag checks because they're no longer copied blindly into the socket object, so existing checks will be sufficient.

Changes in v2:
- Avoid removing the sock->sk_socket check before sync'ing the socket.flags
- Rename sock_rst_flags() to sock_restore_flags()
- Rebase on top of Oren's cleanup patch

---
 net/checkpoint.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 102 insertions(+), 11 deletions(-)
net/checkpoint.c +102 -11
index c64483e..fdbf8e7 100644
@@ -178,10 +178,6 @@ static int sock_cptrst_verify(struct ckpt_hdr_socket *h)
 	if (!ckpt_validate_errno(h->sock.err))
 		return -EINVAL;
 
-	/* None of our supported types use this flag */
-	if (h->sock.flags & SOCK_DESTROY)
-		return -EINVAL;
-
 	return 0;
 }
 
@@ -238,11 +234,97 @@ static int sock_cptrst_bufopts(int op, struct sock *sk,
 	return 0;
 }
 
+static int sock_restore_flags(struct socket *sock,
+                             struct ckpt_hdr_socket *h)
+{
+       int ret;
+       int v = 1;
+       unsigned long sk_flags = h->sock.flags;
+       unsigned long sock_flags = h->socket.flags;
+
+       if (test_and_clear_bit(SOCK_URGINLINE, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_OOBINLINE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_KEEPOPEN, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_BROADCAST, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_BROADCAST,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_RCVTSTAMP, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_RCVTSTAMPNS, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_TIMESTAMPNS,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_DBG, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_DEBUG,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_LOCALROUTE, &sk_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_DONTROUTE,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       if (test_and_clear_bit(SOCK_PASSCRED, &sock_flags)) {
+               ret = sock_setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
+                                     (char *)&v, sizeof(v));
+               if (ret)
+                       return ret;
+       }
+
+       /* TODO: Handle SOCK_TIMESTAMPING_* flags */
+       if (test_bit(SOCK_TIMESTAMPING_TX_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_TX_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RX_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RX_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_SOFTWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_RAW_HARDWARE, &sk_flags) ||
+           test_bit(SOCK_TIMESTAMPING_SYS_HARDWARE, &sk_flags)) {
+               ckpt_debug("SOF_TIMESTAMPING_* flags are not supported\n");
+               return -ENOSYS;
+       }
+
+       /* Anything that is still set in the flags that isn't part of
+        * our protocol's default set, indicates an error
+        */
+       if (sk_flags & ~sock->sk->sk_flags) {
+               ckpt_debug("Unhandled sock flags: %lx\n", sk_flags);
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
 static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 		       struct ckpt_hdr_socket *h, int op)
 {
 	if (sk->sk_socket) {
-		CKPT_COPY(op, h->socket.flags, sk->sk_socket->flags);
 		CKPT_COPY(op, h->socket.state, sk->sk_socket->state);
 	}
 
@@ -259,12 +341,6 @@ static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 	CKPT_COPY(op, h->sock.state, sk->sk_state);
 	CKPT_COPY(op, h->sock.backlog, sk->sk_max_ack_backlog);
 
-	/* TODO:
-	 * Break out setting each of the flags to use setsockopt() or
-	 * perform proper security check
-	 */
-	CKPT_COPY(op, h->sock.flags, sk->sk_flags);
-
 	if (sock_cptrst_bufopts(op, sk, h))
 		return -EINVAL;
 
@@ -298,6 +374,21 @@ static int sock_cptrst(struct ckpt_ctx *ctx, struct sock *sk,
 		return -EINVAL;
 	}
 
+	if (op == CKPT_CPT) {
+		h->sock.flags = sk->sk_flags;
+		h->socket.flags = sk->sk_socket->flags;
+	} else {
+		int ret;
+		mm_segment_t old_fs;
+
+		old_fs = get_fs();
+		set_fs(KERNEL_DS);
+		ret = sock_restore_flags(sk->sk_socket, h);
+		set_fs(old_fs);
+		if (ret)
+			return ret;
+	}
+
 	if ((h->socket.state == SS_CONNECTED) &&
 	    (h->sock.state != TCP_ESTABLISHED)) {
 		ckpt_debug("socket/sock in inconsistent state: %i/%i",
Signature · 1 line
1.6.2.5
author
#2 lore raw

PATCH 2/3 Expose may_setuid() in user.h and add may_setgid() (v2)

In reply to Dan Smith

Make these helpers available to others.

Changes in v2:
- Avoid checking the groupinfo in ctx->realcred against the current in may_setgid()

Cc Serge Hallyn <[email protected]>
---
 include/linux/user.h |    9 +++++++++
 kernel/user.c        |   13 ++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)
include/linux/user.h +9 -0
index 68daf84..c231e9c 100644
@@ -1 +1,10 @@
+#ifndef _LINUX_USER_H
+#define _LINUX_USER_H
+
 #include <asm/user.h>
+#include <linux/sched.h>
+
+extern int may_setuid(struct user_namespace *ns, uid_t uid);
+extern int may_setgid(gid_t gid);
+
+#endif
kernel/user.c +12 -1
index a535ed6..a78fde7 100644
@@ -604,7 +604,7 @@ int checkpoint_user(struct ckpt_ctx *ctx, void *ptr)
 	return do_checkpoint_user(ctx, (struct user_struct *) ptr);
 }
 
-static int may_setuid(struct user_namespace *ns, uid_t uid)
+int may_setuid(struct user_namespace *ns, uid_t uid)
 {
 	/*
 	 * this next check will one day become
@@ -631,6 +631,17 @@ static int may_setuid(struct user_namespace *ns, uid_t uid)
 	return 0;
 }
 
+int may_setgid(gid_t gid)
+{
+	if (capable(CAP_SETGID))
+		return 1;
+
+	if (in_egroup_p(gid))
+		return 1;
+
+	return 0;
+}
+
 static struct user_struct *do_restore_user(struct ckpt_ctx *ctx)
 {
 	struct user_struct *u;
Signature · 1 line
1.6.2.5
Acked-by on v1 2/3
#3 lore raw

Re: [PATCH 2/3] Expose may_setuid() in user.h and add may_setgid() (v2)

Quoted 7 lines · Dan Smith

Make these helpers available to others.

Changes in v2:
- Avoid checking the groupinfo in ctx->realcred against the current in may_setgid()

Cc Serge Hallyn <[email protected]>
Quoted 54 lines
---
 include/linux/user.h |    9 +++++++++
 kernel/user.c        |   13 ++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)
include/linux/user.h +9 -0
index 68daf84..c231e9c 100644
@@ -1 +1,10 @@
+#ifndef _LINUX_USER_H
+#define _LINUX_USER_H
+
 #include <asm/user.h>
+#include <linux/sched.h>
+
+extern int may_setuid(struct user_namespace *ns, uid_t uid);
+extern int may_setgid(gid_t gid);
+
+#endif
kernel/user.c +12 -1
index a535ed6..a78fde7 100644
@@ -604,7 +604,7 @@ int checkpoint_user(struct ckpt_ctx *ctx, void *ptr)
 	return do_checkpoint_user(ctx, (struct user_struct *) ptr);
 }
 
-static int may_setuid(struct user_namespace *ns, uid_t uid)
+int may_setuid(struct user_namespace *ns, uid_t uid)
 {
 	/*
 	 * this next check will one day become
@@ -631,6 +631,17 @@ static int may_setuid(struct user_namespace *ns, uid_t uid)
 	return 0;
 }
 
+int may_setgid(gid_t gid)
+{
+	if (capable(CAP_SETGID))
+		return 1;
+
+	if (in_egroup_p(gid))
+		return 1;
+
+	return 0;
+}
+
 static struct user_struct *do_restore_user(struct ckpt_ctx *ctx)
 {
 	struct user_struct *u;
Signature · 1 line
1.6.2.5
author
#4 lore raw

PATCH 3/3 Save and restore UNIX socket peer credentials (v2)

In reply to Dan Smith

This saves the uid/gid of the sk_peercred structure in the checkpoint stream. On restart, it uses may_setuid() and may_setgid() to determine if the uid/gid from the checkpoint stream may be used.

Changes in v2:
- Adjust for may_setgid() change

---
 include/linux/checkpoint_hdr.h |    2 ++
 net/unix/checkpoint.c          |   29 ++++++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
include/linux/checkpoint_hdr.h +2 -0
index 4d5c22a..78f1f27 100644
@@ -414,6 +414,8 @@ struct ckpt_hdr_socket_unix {
 	struct ckpt_hdr h;
 	__s32 this;
 	__s32 peer;
+	__u32 peercred_uid;
+	__u32 peercred_gid;
 	__u32 flags;
 	__u32 laddr_len;
 	__u32 raddr_len;
net/unix/checkpoint.c +16 -13
index 81252e3..366bc80 100644
@@ -3,6 +3,7 @@
 #include <linux/fs_struct.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
+#include <linux/user.h>
 #include <net/af_unix.h>
 #include <net/tcp_states.h>
 
@@ -94,6 +95,9 @@ int unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock)
 		goto out;
 	}
 
+	un->peercred_uid = sock->sk->sk_peercred.uid;
+	un->peercred_gid = sock->sk->sk_peercred.gid;
+
 	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) un);
 	if (ret < 0)
 		goto out;
@@ -217,19 +221,6 @@ static int unix_join(struct ckpt_ctx *ctx,
 	unix_sk(a)->peer = b;
 	unix_sk(b)->peer = a;
 
-	/* TODO:
-	 * Checkpoint the credentials, restore them here if the values match
-	 * the restored creds or we may_setuid()
-	 */
-
-	a->sk_peercred.pid = task_tgid_vnr(current);
-	a->sk_peercred.uid = ctx->realcred->uid;
-	a->sk_peercred.gid = ctx->realcred->gid;
-
-	b->sk_peercred.pid = a->sk_peercred.pid;
-	b->sk_peercred.uid = a->sk_peercred.uid;
-	b->sk_peercred.gid = a->sk_peercred.gid;
-
 	if (!UNIX_ADDR_EMPTY(un->raddr_len))
 		addr = unix_makeaddr(&un->raddr, un->raddr_len);
 	else if (!UNIX_ADDR_EMPTY(un->laddr_len))
@@ -295,6 +286,18 @@ static int unix_restore_connected(struct ckpt_ctx *ctx,
 		goto out;
 	}
 
+	this->sk_peercred.pid = task_tgid_vnr(current);
+
+	if (may_setuid(ctx->realcred->user->user_ns, un->peercred_uid) &&
+	    may_setgid(un->peercred_gid)) {
+		this->sk_peercred.uid = un->peercred_uid;
+		this->sk_peercred.gid = un->peercred_gid;
+	} else {
+		ckpt_debug("peercred %i:%i would require setuid",
+			   un->peercred_uid, un->peercred_gid);
+		return -1;
+	}
+
 	/* Prime the socket's buffer limit with the maximum.  These will be
 	 * overwritten with the values in the checkpoint stream in a later
 	 * phase.
Signature · 1 line
1.6.2.5
Acked-by on v1 3/3
#5 lore raw

Re: [PATCH 3/3] Save and restore UNIX socket peer credentials (v2)

Quoted 73 lines · Dan Smith

This saves the uid/gid of the sk_peercred structure in the checkpoint stream. On restart, it uses may_setuid() and may_setgid() to determine if the uid/gid from the checkpoint stream may be used.

Changes in v2:
- Adjust for may_setgid() change

---
 include/linux/checkpoint_hdr.h |    2 ++
 net/unix/checkpoint.c          |   29 ++++++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)
include/linux/checkpoint_hdr.h +2 -0
index 4d5c22a..78f1f27 100644
@@ -414,6 +414,8 @@ struct ckpt_hdr_socket_unix {
 	struct ckpt_hdr h;
 	__s32 this;
 	__s32 peer;
+	__u32 peercred_uid;
+	__u32 peercred_gid;
 	__u32 flags;
 	__u32 laddr_len;
 	__u32 raddr_len;
net/unix/checkpoint.c +5 -13
index 81252e3..366bc80 100644
@@ -3,6 +3,7 @@
 #include <linux/fs_struct.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
+#include <linux/user.h>
 #include <net/af_unix.h>
 #include <net/tcp_states.h>
 
@@ -94,6 +95,9 @@ int unix_checkpoint(struct ckpt_ctx *ctx, struct socket *sock)
 		goto out;
 	}
 
+	un->peercred_uid = sock->sk->sk_peercred.uid;
+	un->peercred_gid = sock->sk->sk_peercred.gid;
+
 	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) un);
 	if (ret < 0)
 		goto out;
@@ -217,19 +221,6 @@ static int unix_join(struct ckpt_ctx *ctx,
 	unix_sk(a)->peer = b;
 	unix_sk(b)->peer = a;
 
-	/* TODO:
-	 * Checkpoint the credentials, restore them here if the values match
-	 * the restored creds or we may_setuid()
-	 */
-
-	a->sk_peercred.pid = task_tgid_vnr(current);
-	a->sk_peercred.uid = ctx->realcred->uid;
-	a->sk_peercred.gid = ctx->realcred->gid;
-
-	b->sk_peercred.pid = a->sk_peercred.pid;
-	b->sk_peercred.uid = a->sk_peercred.uid;
-	b->sk_peercred.gid = a->sk_peercred.gid;
-
 	if (!UNIX_ADDR_EMPTY(un->raddr_len))
 		addr = unix_makeaddr(&un->raddr, un->raddr_len);
 	else if (!UNIX_ADDR_EMPTY(un->laddr_len))
@@ -295,6 +286,18 @@ static int unix_restore_connected(struct ckpt_ctx *ctx,
 		goto out;
 	}
 
+	this->sk_peercred.pid = task_tgid_vnr(current);

net/unix/checkpoint.c static int unix_restore_connected(struct ckpt_ctx *ctx, · line 299

Quoted 10 lines
net/unix/checkpoint.c +10 -0
+
+	if (may_setuid(ctx->realcred->user->user_ns, un->peercred_uid) &&
+	    may_setgid(un->peercred_gid)) {
+		this->sk_peercred.uid = un->peercred_uid;
+		this->sk_peercred.gid = un->peercred_gid;
+	} else {
+		ckpt_debug("peercred %i:%i would require setuid",
+			   un->peercred_uid, un->peercred_gid);
+		return -1;
+	}

Ok, except don't you need to do a goto out; here?

Other than that,

Quoted 11 lines
net/unix/checkpoint.c +1 -0
+
 	/* Prime the socket's buffer limit with the maximum.  These will be
 	 * overwritten with the values in the checkpoint stream in a later
 	 * phase.
Signature · 6 lines
1.6.2.5

_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers
maintainer Acked-by
#6 lore raw
In reply to Dan Smith

From: Dan Smith <[email protected]>
Date: Tue, 18 Aug 2009 12:57:12 -0700

Quoted 3 lines

This is an updated version of my previous patch set, with the fourth patch removed. This is rebased on current v17-dev and includes Serge's recommended changes to may_setgid().

I have no fundamental problems with these changes, you can attach my ACK to the networking bits if you want: