Skip to contentSkip to conversation

pkg-shadow support subordinate ids with user namespaces

PATCH In review Eric W. Biederman 11 participants 59 messages Jan 22, 2013 – Oct 27, 2013 34 messages not in the archive Library code

lore
Share & tools
lore
#0 lore raw

cover pkg-shadow support subordinate ids with user namespaces

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids
---

 etc/login.defs                     |    8 +
 lib/Makefile.am                    |    2 +
 lib/commonio.c                     |   30 ++
 lib/commonio.h                     |    1 +
 lib/getdef.c                       |    6 +
 lib/prototypes.h                   |    9 +
 lib/subordinateio.c                |  512 ++++++++++++++++++++++++++++++++++++
 lib/subordinateio.h                |   38 +++
 libmisc/Makefile.am                |    4 +
 libmisc/find_new_sub_gids.c        |   87 ++++++
 libmisc/find_new_sub_uids.c        |   87 ++++++
 libmisc/idmapping.c                |  126 +++++++++
 libmisc/idmapping.h                |   44 +++
 libmisc/user_busy.c                |   18 +-
 man/Makefile.am                    |   10 +
 man/login.defs.5.xml               |    8 +
 man/login.defs.d/SUB_GID_COUNT.xml |   46 ++++
 man/login.defs.d/SUB_UID_COUNT.xml |   46 ++++
 man/newgidmap.1.xml                |  157 +++++++++++
 man/newuidmap.1.xml                |  154 +++++++++++
 man/subgid.5.xml                   |  120 +++++++++
 man/subuid.5.xml                   |  120 +++++++++
 man/usermod.8.xml                  |   80 ++++++
 src/Makefile.am                    |    5 +-
 src/newgidmap.c                    |  183 +++++++++++++
 src/newuidmap.c                    |  183 +++++++++++++
 src/newusers.c                     |  124 +++++++++
 src/useradd.c                      |  141 ++++++++++-
 src/userdel.c                      |  115 ++++++++
 src/usermod.c                      |  255 ++++++++++++++++++-
 30 files changed, 2708 insertions(+), 11 deletions(-)

A message that is not in the archive (sent off-list or not yet received): 87d2wxshu0.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#2 lore raw

PATCH 1/11 Documentation for /etc/subuid and /etc/subgid

---
 man/Makefile.am  |    4 ++
 man/subgid.5.xml |  120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/subuid.5.xml |  120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 244 insertions(+), 0 deletions(-)
 create mode 100644 man/subgid.5.xml
 create mode 100644 man/subuid.5.xml
man/Makefile.am +4 -0
index b39043e..be7f5e2 100644
@@ -43,6 +43,8 @@ man_MANS = \
 	man5/shadow.5 \
 	man1/su.1 \
 	man5/suauth.5 \
+	man5/subgid.5 \
+	man5/subuid.5 \
 	man8/useradd.8 \
 	man8/userdel.8 \
 	man8/usermod.8 \
@@ -94,6 +96,8 @@ man_XMANS = \
 	sg.1.xml \
 	su.1.xml \
 	suauth.5.xml \
+	subgid.5.xml \
+	subuid.5.xml \
 	useradd.8.xml \
 	userdel.8.xml \
 	usermod.8.xml \
man/subgid.5.xml new file +120 -0

120 lines not on this page · raw message

man/subuid.5.xml new file +120 -0

120 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#3 lore raw

PATCH 2/11 login.defs.5: Document the new variables in login.defs

In reply to a message not in the archive
---
 man/Makefile.am                    |    2 +
 man/login.defs.5.xml               |    8 ++++++
 man/login.defs.d/SUB_GID_COUNT.xml |   46 ++++++++++++++++++++++++++++++++++++
 man/login.defs.d/SUB_UID_COUNT.xml |   46 ++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 0 deletions(-)
 create mode 100644 man/login.defs.d/SUB_GID_COUNT.xml
 create mode 100644 man/login.defs.d/SUB_UID_COUNT.xml
man/Makefile.am +2 -0
index be7f5e2..fc617e9 100644
@@ -163,6 +163,8 @@ login_defs_v = \
 	USERDEL_CMD.xml \
 	USERGROUPS_ENAB.xml \
 	USE_TCB.xml \
+	SUB_GID_COUNT.xml \
+	SUB_UID_COUNT.xml \
 	SYS_GID_MAX.xml \
 	SYS_UID_MAX.xml
 
man/login.defs.5.xml +8 -0
index 459b866..d5279ca 100644
@@ -78,6 +78,8 @@
 <!ENTITY SULOG_FILE            SYSTEM "login.defs.d/SULOG_FILE.xml">
 <!ENTITY SU_NAME               SYSTEM "login.defs.d/SU_NAME.xml">
 <!ENTITY SU_WHEEL_ONLY         SYSTEM "login.defs.d/SU_WHEEL_ONLY.xml">
+<!ENTITY SUB_GID_COUNT         SYSTEM "login.defs.d/SUB_GID_COUNT.xml">
+<!ENTITY SUB_UID_COUNT         SYSTEM "login.defs.d/SUB_UID_COUNT.xml">
 <!ENTITY SYS_GID_MAX           SYSTEM "login.defs.d/SYS_GID_MAX.xml">
 <!ENTITY SYSLOG_SG_ENAB        SYSTEM "login.defs.d/SYSLOG_SG_ENAB.xml">
 <!ENTITY SYSLOG_SU_ENAB        SYSTEM "login.defs.d/SYSLOG_SU_ENAB.xml">
@@ -216,6 +218,8 @@
       &SULOG_FILE;
       &SU_NAME;
       &SU_WHEEL_ONLY;
+      &SUB_GID_COUNT; <!-- documents also SUB_GID_MIN SUB_GID_MAX -->
+      &SUB_UID_COUNT; <!-- documents also SUB_UID_MIN SUB_UID_MAX -->
       &SYS_GID_MAX; <!-- documents also SYS_GID_MIN -->
       &SYS_UID_MAX; <!-- documents also SYS_UID_MIN -->
       &SYSLOG_SG_ENAB;
@@ -393,6 +397,8 @@
 	    PASS_MAX_DAYS PASS_MIN_DAYS PASS_WARN_AGE
 	    <phrase condition="sha_crypt">SHA_CRYPT_MAX_ROUNDS
 	    SHA_CRYPT_MIN_ROUNDS</phrase>
+	    SUB_GID_COUNT SUB_GID_MAX SUB_GID_MIN
+	    SUB_UID_COUNT SUB_UID_MAX SUB_UID_MIN
 	    SYS_GID_MAX SYS_GID_MIN SYS_UID_MAX SYS_UID_MIN UID_MAX UID_MIN
 	    UMASK
 	  </para>
@@ -470,6 +476,8 @@
 	    GID_MAX GID_MIN
 	    MAIL_DIR MAX_MEMBERS_PER_GROUP
 	    PASS_MAX_DAYS PASS_MIN_DAYS PASS_WARN_AGE
+	    SUB_GID_COUNT SUB_GID_MAX SUB_GID_MIN
+	    SUB_UID_COUNT SUB_UID_MAX SUB_UID_MIN
 	    SYS_GID_MAX SYS_GID_MIN SYS_UID_MAX SYS_UID_MIN UID_MAX UID_MIN
 	    UMASK
 	    <phrase condition="tcb">TCB_AUTH_GROUP TCB_SYMLINK USE_TCB</phrase>
man/login.defs.d/SUB_GID_COUNT.xml new file +46 -0

46 lines not on this page · raw message

man/login.defs.d/SUB_UID_COUNT.xml new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#4 lore raw

PATCH 3/11 Implement commonio_append.

In reply to a message not in the archive

To support files that do not have a simple unique key implement commonio_append to allow new entries to be added.

---
 lib/commonio.c |   30 ++++++++++++++++++++++++++++++
 lib/commonio.h |    1 +
 2 files changed, 31 insertions(+), 0 deletions(-)
lib/commonio.c +30 -0
index 6b6ce7b..7a76975 100644
@@ -1113,6 +1113,36 @@ int commonio_update (struct commonio_db *db, const void *eptr)
 	return 1;
 }
 
+int commonio_append (struct commonio_db *db, const void *eptr)
+{
+	struct commonio_entry *p;
+	void *nentry;
+
+	if (!db->isopen || db->readonly) {
+		errno = EINVAL;
+		return 0;
+	}
+	nentry = db->ops->dup (eptr);
+	if (NULL == nentry) {
+		errno = ENOMEM;
+		return 0;
+	}
+	/* new entry */
+	p = (struct commonio_entry *) malloc (sizeof *p);
+	if (NULL == p) {
+		db->ops->free (nentry);
+		errno = ENOMEM;
+		return 0;
+	}
+
+	p->eptr = nentry;
+	p->line = NULL;
+	p->changed = true;
+	add_one_entry (db, p);
+
+	db->changed = true;
+	return 1;
+}
 
 void commonio_del_entry (struct commonio_db *db, const struct commonio_entry *p)
 {
lib/commonio.h +1 -0
index a1117a8..1866b89 100644
@@ -146,6 +146,7 @@ extern int commonio_lock_nowait (struct commonio_db *, bool log);
 extern int commonio_open (struct commonio_db *, int);
 extern /*@observer@*/ /*@null@*/const void *commonio_locate (struct commonio_db *, const char *);
 extern int commonio_update (struct commonio_db *, const void *);
+extern int commonio_append (struct commonio_db *, const void *);
 extern int commonio_remove (struct commonio_db *, const char *);
 extern int commonio_rewind (struct commonio_db *);
 extern /*@observer@*/ /*@null@*/const void *commonio_next (struct commonio_db *);
Signature · 1 line
1.7.5.4
#5 lore raw

PATCH 3/11 Implement commonio_append.

In reply to a message not in the archive

To support files that do not have a simple unique key implement commonio_append to allow new entries to be added.

---
 lib/commonio.c |   30 ++++++++++++++++++++++++++++++
 lib/commonio.h |    1 +
 2 files changed, 31 insertions(+), 0 deletions(-)
lib/commonio.c +30 -0
index 6b6ce7b..7a76975 100644
@@ -1113,6 +1113,36 @@ int commonio_update (struct commonio_db *db, const void *eptr)
 	return 1;
 }
 
+int commonio_append (struct commonio_db *db, const void *eptr)
+{
+	struct commonio_entry *p;
+	void *nentry;
+
+	if (!db->isopen || db->readonly) {
+		errno = EINVAL;
+		return 0;
+	}
+	nentry = db->ops->dup (eptr);
+	if (NULL == nentry) {
+		errno = ENOMEM;
+		return 0;
+	}
+	/* new entry */
+	p = (struct commonio_entry *) malloc (sizeof *p);
+	if (NULL == p) {
+		db->ops->free (nentry);
+		errno = ENOMEM;
+		return 0;
+	}
+
+	p->eptr = nentry;
+	p->line = NULL;
+	p->changed = true;
+	add_one_entry (db, p);
+
+	db->changed = true;
+	return 1;
+}
 
 void commonio_del_entry (struct commonio_db *db, const struct commonio_entry *p)
 {
lib/commonio.h +1 -0
index a1117a8..1866b89 100644
@@ -146,6 +146,7 @@ extern int commonio_lock_nowait (struct commonio_db *, bool log);
 extern int commonio_open (struct commonio_db *, int);
 extern /*@observer@*/ /*@null@*/const void *commonio_locate (struct commonio_db *, const char *);
 extern int commonio_update (struct commonio_db *, const void *);
+extern int commonio_append (struct commonio_db *, const void *);
 extern int commonio_remove (struct commonio_db *, const char *);
 extern int commonio_rewind (struct commonio_db *);
 extern /*@observer@*/ /*@null@*/const void *commonio_next (struct commonio_db *);
Signature · 1 line
1.7.5.4
#6 lore raw

PATCH 4/11 Add backend support for suboridnate uids and gids

In reply to a message not in the archive

These files list the set of subordinate uids and gids that users are allowed
to use. The expect use case is with the user namespace but other uses are
allowed.

---
 etc/login.defs      |    8 +
 lib/Makefile.am     |    2 +
 lib/getdef.c        |    6 +
 lib/subordinateio.c |  512 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/subordinateio.h |   38 ++++
 5 files changed, 566 insertions(+), 0 deletions(-)
 create mode 100644 lib/subordinateio.c
 create mode 100644 lib/subordinateio.h
etc/login.defs +8 -0
index 5b6fab3..01a0089 100644
@@ -226,6 +226,10 @@ UID_MAX			60000
 # System accounts
 SYS_UID_MIN		  101
 SYS_UID_MAX		  999
+# Extra per user uids
+SUB_UID_MIN		   100000
+SUB_UID_MAX		600100000
+SUB_UID_COUNT		    10000
 
 #
 # Min/max values for automatic gid selection in groupadd
@@ -235,6 +239,10 @@ GID_MAX			60000
 # System accounts
 SYS_GID_MIN		  101
 SYS_GID_MAX		  999
+# Extra per user group ids
+SUB_GID_MIN		   100000
+SUB_GID_MAX		600100000
+SUB_GID_COUNT		    10000
 
 #
 # Max number of login retries if password is bad
lib/Makefile.am +2 -0
index c448dd3..6db86cd 100644
@@ -39,6 +39,8 @@ libshadow_la_SOURCES = \
 	pwio.c \
 	pwio.h \
 	pwmem.c \
+	subordinateio.h \
+	subordinateio.c \
 	selinux.c \
 	semanage.c \
 	sgetgrent.c \
lib/getdef.c +6 -0
index ac08163..f872bb3 100644
@@ -81,6 +81,12 @@ static struct itemdef def_table[] = {
 	{"SHA_CRYPT_MAX_ROUNDS", NULL},
 	{"SHA_CRYPT_MIN_ROUNDS", NULL},
 #endif
+	{"SUB_GID_COUNT", NULL},
+	{"SUB_GID_MAX", NULL},
+	{"SUB_GID_MIN", NULL},
+	{"SUB_UID_COUNT", NULL},
+	{"SUB_UID_MAX", NULL},
+	{"SUB_UID_MIN", NULL},
 	{"SULOG_FILE", NULL},
 	{"SU_NAME", NULL},
 	{"SYS_GID_MAX", NULL},
lib/subordinateio.c new file +512 -0

512 lines not on this page · raw message

lib/subordinateio.h new file +38 -0
new file mode 100644
index 0000000..f27b3c0
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2012- Eric W. Biederman
+ */
+
+#ifndef _SUBORDINATEIO_H
+#define _SUBORDINATEIO_H
+
+#include <sys/types.h>
+
+extern int sub_uid_close(void);
+extern bool is_sub_uid_range_free(uid_t start, unsigned long count);
+extern bool have_sub_uids(const char *owner, uid_t start, unsigned long count);
+extern bool sub_uid_file_present (void);
+extern bool sub_uid_assigned(const char *owner);
+extern int sub_uid_lock (void);
+extern int sub_uid_setdbname (const char *filename);
+extern /*@observer@*/const char *sub_uid_dbname (void);
+extern int sub_uid_open (int mode);
+extern int sub_uid_unlock (void);
+extern int sub_uid_add (const char *owner, uid_t start, unsigned long count);
+extern int sub_uid_remove (const char *owner, uid_t start, unsigned long count);
+extern uid_t sub_uid_find_free_range(uid_t min, uid_t max, unsigned long count);
+
+extern int sub_gid_close(void);
+extern bool is_sub_gid_range_free(gid_t start, unsigned long count);
+extern bool have_sub_gids(const char *owner, gid_t start, unsigned long count);
+extern bool sub_gid_file_present (void);
+extern bool sub_gid_assigned(const char *owner);
+extern int sub_gid_lock (void);
+extern int sub_gid_setdbname (const char *filename);
+extern /*@observer@*/const char *sub_gid_dbname (void);
+extern int sub_gid_open (int mode);
+extern int sub_gid_unlock (void);
+extern int sub_gid_add (const char *owner, gid_t start, unsigned long count);
+extern int sub_gid_remove (const char *owner, gid_t start, unsigned long count);
+extern uid_t sub_gid_find_free_range(gid_t min, gid_t max, unsigned long count);
+
+#endif
Signature · 1 line
1.7.5.4

A message that is not in the archive (sent off-list or not yet received): 87liblr344.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#8 lore raw

Re: [PATCH 04/11] Add backend support for suboridnate uids and gids

Quoted 38 lines · Eric W. Biederman
lib/subordinateio.c new file +38 -0
+static void *subordinate_parse (const char *line)
+{
+	static struct subordinate_range range;
+	char rangebuf[1024];
+	int i;
+	char *cp;
+	char *fields[NFIELDS];
+
+	/*
+	 * Copy the string to a temporary buffer so the substrings can
+	 * be modified to be NULL terminated.
+	 */
+	if (strlen (line) >= sizeof rangebuf)
+		return NULL;	/* fail if too long */
+	strcpy (rangebuf, line);
+
+	/*
+	 * Save a pointer to the start of each colon separated
+	 * field.  The fields are converted into NUL terminated strings.
+	 */
+
+	for (cp = rangebuf, i = 0; (i < NFIELDS) && (NULL != cp); i++) {
+		fields[i] = cp;
+		while (('\0' != *cp) && (':' != *cp)) {
+			cp++;
+		}
+
+		if ('\0' != *cp) {
+			*cp = '\0';
+			cp++;
+		} else {
+			cp = NULL;
+		}
+	}
+
+	/*
+	 * There must be exactly NFIELDS colon separated fields or
+	 * the entry is invalid.  Also, fields must be non-blank.

lib/subordinateio.c line 95

Quoted 10 lines
lib/subordinateio.c new file +10 -0
+	 */
+	if (i != NFIELDS || *fields[0] == '\0' || *fields[1] == '\0' || *fields[2] == '\0')
+		return NULL;
+	range.owner = fields[0];
+	if (getulong (fields[1], &range.start) == 0)
+		return NULL;
+	if (getulong (fields[2], &range.count) == 0)
+		return NULL;
+
+	return &range;

This function worries me, because you're returning local contents (both range itself, and range.owner).

-serge

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw

Re: [PATCH 04/11] Add backend support for suboridnate uids and gids

Quoted 44 lines · Serge E. Hallyn
Quoted 43 lines · Eric W. Biederman
lib/subordinateio.c new file +43 -0
+static void *subordinate_parse (const char *line)
+{
+	static struct subordinate_range range;
+	char rangebuf[1024];
+	int i;
+	char *cp;
+	char *fields[NFIELDS];
+
+	/*
+	 * Copy the string to a temporary buffer so the substrings can
+	 * be modified to be NULL terminated.
+	 */
+	if (strlen (line) >= sizeof rangebuf)
+		return NULL;	/* fail if too long */
+	strcpy (rangebuf, line);
+
+	/*
+	 * Save a pointer to the start of each colon separated
+	 * field.  The fields are converted into NUL terminated strings.
+	 */
+
+	for (cp = rangebuf, i = 0; (i < NFIELDS) && (NULL != cp); i++) {
+		fields[i] = cp;
+		while (('\0' != *cp) && (':' != *cp)) {
+			cp++;
+		}
+
+		if ('\0' != *cp) {
+			*cp = '\0';
+			cp++;
+		} else {
+			cp = NULL;
+		}
+	}
+
+	/*
+	 * There must be exactly NFIELDS colon separated fields or
+	 * the entry is invalid.  Also, fields must be non-blank.
+	 */
+	if (i != NFIELDS || *fields[0] == '\0' || *fields[1] == '\0' || *fields[2] == '\0')
+		return NULL;
+	range.owner = fields[0];
+	if (getulong (fields[1], &range.start) == 0)
Quoted 8 lines

lib/subordinateio.c line 95

Quoted 5 lines
lib/subordinateio.c new file +5 -0
+		return NULL;
+	if (getulong (fields[2], &range.count) == 0)
+		return NULL;
+
+	return &range;

This function worries me, because you're returning local contents (both range itself, and range.owner).

In practice that function is fine, and is mostly in the style of the other functions that implement parse. As parse is expected to return a reference to a statically allocated buffer.

Furthermore the only use in commonio.c is to do:

	if (NULL != parse(...))
        	dup(...)

But it does look like when I made the user name a string I did deviate from the norm and have failed to copy the string to a statically allocated buffer.

In practice it doesn't matter but it looks worth fixing for the sake of long term maintenance.

Eric

A message that is not in the archive (sent off-list or not yet received): 87a9ryyzh2.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#12 lore raw

Re: [PATCH 04/11] Add backend support for suboridnate uids and gids

Quoted 2 lines · Eric W. Biederman

In practice it doesn't matter but it looks worth fixing for the sake of long term maintenance.

I figured the users must all be ok, and in practice it would work out, but the fact remains subsequent function calls assigning to local stack can overwrite what you've returned. <shrug>

The rest of what I've looked over looked ok, thanks much!

-serge

#13 lore raw

Re: [PATCH 04/11] Add backend support for suboridnate uids and gids

Quoted 7 lines · Serge Hallyn
Quoted 2 lines · Eric W. Biederman

In practice it doesn't matter but it looks worth fixing for the sake of long term maintenance.

I figured the users must all be ok, and in practice it would work out, but the fact remains subsequent function calls assigning to local stack can overwrite what you've returned. <shrug>

Yes. Once I make rangebuf static then subordinate_parse is ok (at least interface wise). But the bug is real and I will get out an incremental patch shortly.

The subordinate_parse really isn't ok as I claimed a minute ago I had overlooked I copied everything into the local variable rangebuf before processing. In practice it works of course but that is luck.

Quoted 1 line

The rest of what I've looked over looked ok, thanks much!

Welcome.

Do you think you can start getting this into Ubuntu?

Eric

#14 lore raw

PATCH 1/1 subordinateio: Fix subordinate_parse to have an internal static buffer

In reply to Serge Hallyn

subordinate_parse is supposed to return a static structure that represents one line in /etc/subuid or /etc/subgid. I goofed and failed to make the variable rangebuf that holds the username of in the returned structure static.

Add this missing static specification.

---
 lib/subordinateio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
lib/subordinateio.c +1 -1
index 851cfa0..4cca992 100644
@@ -48,7 +48,7 @@ static void subordinate_free (/*@out@*/ /*@only@*/void *ent)
 static void *subordinate_parse (const char *line)
 {
 	static struct subordinate_range range;
-	char rangebuf[1024];
+	static char rangebuf[1024];
 	int i;
 	char *cp;
 	char *fields[NFIELDS];
Signature · 1 line
1.7.5.4
#15 lore raw

PATCH 5/11 Implement find_new_sub_uids find_new_sub_gids

In reply to a message not in the archive

Functions for finding new subordinate uid and gids ranges for use with useradd.

---
 lib/prototypes.h            |    9 ++++
 libmisc/Makefile.am         |    2 +
 libmisc/find_new_sub_gids.c |   87 +++++++++++++++++++++++++++++++++++++++++++
 libmisc/find_new_sub_uids.c |   87 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 185 insertions(+), 0 deletions(-)
 create mode 100644 libmisc/find_new_sub_gids.c
 create mode 100644 libmisc/find_new_sub_uids.c
lib/prototypes.h +9 -0
index 00d9e0b..487fb07 100644
@@ -149,6 +149,15 @@ extern int find_new_uid (bool sys_user,
                          uid_t *uid,
                          /*@null@*/uid_t const *preferred_uid);
 
+/* find_new_sub_gids.c */
+extern int find_new_sub_gids (const char *owner,
+			      gid_t *range_start, unsigned long *range_count);
+
+/* find_new_sub_uids.c */
+extern int find_new_sub_uids (const char *owner,
+			      uid_t *range_start, unsigned long *range_count);
+
+
 /* get_gid.c */
 extern int get_gid (const char *gidstr, gid_t *gid);
 
libmisc/Makefile.am +2 -0
index 9da2ec3..5de9193 100644
@@ -25,6 +25,8 @@ libmisc_a_SOURCES = \
 	failure.h \
 	find_new_gid.c \
 	find_new_uid.c \
+	find_new_sub_gids.c \
+	find_new_sub_uids.c \
 	getdate.h \
 	getdate.y \
 	getgr_nam_gid.c \
libmisc/find_new_sub_gids.c new file +87 -0

87 lines not on this page · raw message

libmisc/find_new_sub_uids.c new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#16 lore raw

PATCH 6/11 userdel: Add support for removing subordinate user and group ids.

In reply to a message not in the archive
---
 src/userdel.c |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 115 insertions(+), 0 deletions(-)
src/userdel.c +115 -0
index 4439f62..0c9b83d 100644
@@ -65,6 +65,7 @@
 #endif				/* WITH_TCB */
 /*@-exitarg@*/
 #include "exitcodes.h"
+#include "subordinateio.h"
 
 /*
  * exit status values
@@ -75,6 +76,8 @@
 #define E_GRP_UPDATE	10	/* can't update group file */
 #define E_HOMEDIR	12	/* can't remove home directory */
 #define E_SE_UPDATE	14	/* can't update SELinux user mapping */
+#define E_SUB_UID_UPDATE 16	/* can't update the subordinate uid file */
+#define E_SUB_GID_UPDATE 18	/* can't update the subordinate gid file */
 
 /*
  * Global variables
@@ -96,9 +99,13 @@ static bool is_shadow_pwd;
 static bool is_shadow_grp;
 static bool sgr_locked = false;
 #endif				/* SHADOWGRP */
+static bool is_sub_uid;
+static bool is_sub_gid;
 static bool pw_locked  = false;
 static bool gr_locked   = false;
 static bool spw_locked  = false;
+static bool sub_uid_locked = false;
+static bool sub_gid_locked = false;
 
 /* local function prototypes */
 static void usage (int status);
@@ -437,6 +444,34 @@ static void close_files (void)
 		sgr_locked = false;
 	}
 #endif				/* SHADOWGRP */
+
+	if (is_sub_uid) {
+		if (sub_uid_close () == 0) {
+			fprintf (stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_uid_dbname ()));
+			fail_exit (E_SUB_UID_UPDATE);
+		}
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+			/* continue */
+		}
+		sub_uid_locked = false;
+	}
+
+	if (is_sub_gid) {
+		if (sub_gid_close () == 0) {
+			fprintf (stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_gid_dbname ()));
+			fail_exit (E_SUB_GID_UPDATE);
+		}
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+			/* continue */
+		}
+		sub_gid_locked = false;
+	}
 }
 
 /*
@@ -474,6 +509,20 @@ static void fail_exit (int code)
 		}
 	}
 #endif				/* SHADOWGRP */
+	if (sub_uid_locked) {
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+			/* continue */
+		}
+	}
+	if (sub_gid_locked) {
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+			/* continue */
+		}
+	}
 
 #ifdef WITH_AUDIT
 	audit_logger (AUDIT_DEL_USER, Prog,
@@ -595,6 +644,58 @@ static void open_files (void)
 		}
 	}
 #endif				/* SHADOWGRP */
+	if (is_sub_uid) {
+		if (sub_uid_lock () == 0) {
+			fprintf (stderr,
+				_("%s: cannot lock %s; try again later.\n"),
+				Prog, sub_uid_dbname ());
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_DEL_USER, Prog,
+				"locking subordinate user file",
+				user_name, (unsigned int) user_id,
+				SHADOW_AUDIT_FAILURE);
+#endif				/* WITH_AUDIT */
+			fail_exit (E_SUB_UID_UPDATE);
+		}
+		sub_uid_locked = true;
+		if (sub_uid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+				_("%s: cannot open %s\n"), Prog, sub_uid_dbname ());
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_DEL_USER, Prog,
+				"opening subordinate user file",
+				user_name, (unsigned int) user_id,
+				SHADOW_AUDIT_FAILURE);
+#endif				/* WITH_AUDIT */
+			fail_exit (E_SUB_UID_UPDATE);
+		}
+	}
+	if (is_sub_gid) {
+		if (sub_gid_lock () == 0) {
+			fprintf (stderr,
+				_("%s: cannot lock %s; try again later.\n"),
+				Prog, sub_gid_dbname ());
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_DEL_USER, Prog,
+				"locking subordinate group file",
+				user_name, (unsigned int) user_id,
+				SHADOW_AUDIT_FAILURE);
+#endif				/* WITH_AUDIT */
+			fail_exit (E_SUB_GID_UPDATE);
+		}
+		sub_gid_locked = true;
+		if (sub_gid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+				_("%s: cannot open %s\n"), Prog, sub_gid_dbname ());
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_DEL_USER, Prog,
+				"opening subordinate group file",
+				user_name, (unsigned int) user_id,
+				SHADOW_AUDIT_FAILURE);
+#endif				/* WITH_AUDIT */
+			fail_exit (E_SUB_GID_UPDATE);
+		}
+	}
 }
 
 /*
@@ -619,6 +720,18 @@ static void update_user (void)
 		         Prog, user_name, spw_dbname ());
 		fail_exit (E_PW_UPDATE);
 	}
+	if (is_sub_uid && sub_uid_remove(user_name, 0, ULONG_MAX) == 0) {
+		fprintf (stderr,
+			_("%s: cannot remove entry %lu from %s\n"),
+			Prog, (unsigned long)user_id, sub_uid_dbname ());
+		fail_exit (E_SUB_UID_UPDATE);
+	}
+	if (is_sub_gid && sub_gid_remove(user_name, 0, ULONG_MAX) == 0) {
+		fprintf (stderr,
+			_("%s: cannot remove entry %lu from %s\n"),
+			Prog, (unsigned long)user_id, sub_gid_dbname ());
+		fail_exit (E_SUB_GID_UPDATE);
+	}
 #ifdef WITH_AUDIT
 	audit_logger (AUDIT_DEL_USER, Prog,
 	              "deleting user entries",
@@ -966,6 +1079,8 @@ int main (int argc, char **argv)
 #ifdef SHADOWGRP
 	is_shadow_grp = sgr_file_present ();
 #endif				/* SHADOWGRP */
+	is_sub_uid = sub_uid_file_present ();
+	is_sub_gid = sub_gid_file_present ();
 
 	/*
 	 * Start with a quick check to see if the user exists.
Signature · 1 line
1.7.5.4
#17 lore raw

PATCH 7/11 useradd: Add support for subordinate user identifiers

In reply to a message not in the archive
---
 src/useradd.c |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 140 insertions(+), 1 deletions(-)
src/useradd.c +140 -1
index bfddfc6..87ba98d 100644
@@ -65,6 +65,7 @@
 #include "sgroupio.h"
 #endif
 #include "shadowio.h"
+#include "subordinateio.h"
 #ifdef WITH_TCB
 #include "tcbfuncs.h"
 #endif
@@ -121,12 +122,20 @@ static bool is_shadow_pwd;
 static bool is_shadow_grp;
 static bool sgr_locked = false;
 #endif
+static bool is_sub_uid = false;
+static bool is_sub_gid = false;
 static bool pw_locked = false;
 static bool gr_locked = false;
 static bool spw_locked = false;
+static bool sub_uid_locked = false;
+static bool sub_gid_locked = false;
 static char **user_groups;	/* NULL-terminated list */
 static long sys_ngroups;
 static bool do_grp_update = false;	/* group files need to be updated */
+static uid_t sub_uid_start;	/* New subordinate uid range */
+static unsigned long sub_uid_count;
+static gid_t sub_gid_start;	/* New subordinate gid range */
+static unsigned long sub_gid_count;
 
 static bool
     bflg = false,		/* new default root of home directory */
@@ -168,6 +177,8 @@ static bool home_added = false;
 #define E_GRP_UPDATE	10	/* can't update group file */
 #define E_HOMEDIR	12	/* can't create home directory */
 #define E_SE_UPDATE	14	/* can't update SELinux user mapping */
+#define E_SUB_UID_UPDATE 16	/* can't update the subordinate uid file */
+#define E_SUB_GID_UPDATE 18	/* can't update the subordinate gid file */
 
 #define DGROUP			"GROUP="
 #define DHOME			"HOME="
@@ -268,6 +279,32 @@ static void fail_exit (int code)
 		}
 	}
 #endif
+	if (sub_uid_locked) {
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_ADD_USER, Prog,
+			              "unlocking subodinate user file",
+			              user_name, AUDIT_NO_ID,
+			              SHADOW_AUDIT_FAILURE);
+#endif
+			/* continue */
+		}
+	}
+	if (sub_gid_locked) {
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_ADD_USER, Prog,
+			              "unlocking subodinate group file",
+			              user_name, AUDIT_NO_ID,
+			              SHADOW_AUDIT_FAILURE);
+#endif
+			/* continue */
+		}
+	}
 
 #ifdef WITH_AUDIT
 	audit_logger (AUDIT_ADD_USER, Prog,
@@ -1378,6 +1415,18 @@ static void close_files (void)
 		}
 #endif
 	}
+	if (is_sub_uid  && (sub_uid_close () == 0)) {
+		fprintf (stderr,
+		         _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname ());
+		SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_uid_dbname ()));
+		fail_exit (E_SUB_UID_UPDATE);
+	}
+	if (is_sub_gid  && (sub_gid_close () == 0)) {
+		fprintf (stderr,
+		         _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname ());
+		SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_gid_dbname ()));
+		fail_exit (E_SUB_GID_UPDATE);
+	}
 	if (is_shadow_pwd) {
 		if (spw_unlock () == 0) {
 			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname ());
@@ -1432,6 +1481,34 @@ static void close_files (void)
 		sgr_locked = false;
 	}
 #endif
+	if (is_sub_uid) {
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_ADD_USER, Prog,
+				"unlocking subordinate user file",
+				user_name, AUDIT_NO_ID,
+				SHADOW_AUDIT_FAILURE);
+#endif
+			/* continue */
+		}
+		sub_uid_locked = false;
+	}
+	if (is_sub_gid) {
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+#ifdef WITH_AUDIT
+			audit_logger (AUDIT_ADD_USER, Prog,
+				"unlocking subordinate group file",
+				user_name, AUDIT_NO_ID,
+				SHADOW_AUDIT_FAILURE);
+#endif
+			/* continue */
+		}
+		sub_gid_locked = false;
+	}
 }
 
 /*
@@ -1486,6 +1563,36 @@ static void open_files (void)
 		}
 	}
 #endif
+	if (is_sub_uid) {
+		if (sub_uid_lock () == 0) {
+			fprintf (stderr,
+			         _("%s: cannot lock %s; try again later.\n"),
+			         Prog, sub_uid_dbname ());
+			fail_exit (E_SUB_UID_UPDATE);
+		}
+		sub_uid_locked = true;
+		if (sub_uid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+			         _("%s: cannot open %s\n"),
+			         Prog, sub_uid_dbname ());
+			fail_exit (E_SUB_UID_UPDATE);
+		}
+	}
+	if (is_sub_gid) {
+		if (sub_gid_lock () == 0) {
+			fprintf (stderr,
+			         _("%s: cannot lock %s; try again later.\n"),
+			         Prog, sub_gid_dbname ());
+			fail_exit (E_SUB_GID_UPDATE);
+		}
+		sub_gid_locked = true;
+		if (sub_gid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+			         _("%s: cannot open %s\n"),
+			         Prog, sub_gid_dbname ());
+			fail_exit (E_SUB_GID_UPDATE);
+		}
+	}
 }
 
 static void open_shadow (void)
@@ -1732,13 +1839,27 @@ static void usr_update (void)
 #endif
 		fail_exit (E_PW_UPDATE);
 	}
+	if (is_sub_uid &&
+	    (sub_uid_add(user_name, sub_uid_start, sub_uid_count) == 0)) {
+		fprintf (stderr,
+		         _("%s: failed to prepare the new %s entry\n"),
+		         Prog, sub_uid_dbname ());
+		fail_exit (E_SUB_UID_UPDATE);
+	}
+	if (is_sub_gid &&
+	    (sub_gid_add(user_name, sub_gid_start, sub_gid_count) == 0)) {
+		fprintf (stderr,
+		         _("%s: failed to prepare the new %s entry\n"),
+		         Prog, sub_uid_dbname ());
+		fail_exit (E_SUB_GID_UPDATE);
+	}
+
 #ifdef WITH_AUDIT
 	audit_logger (AUDIT_ADD_USER, Prog,
 	              "adding user",
 	              user_name, (unsigned int) user_id,
 	              SHADOW_AUDIT_SUCCESS);
 #endif
-
 	/*
 	 * Do any group file updates for this user.
 	 */
@@ -1884,6 +2005,8 @@ int main (int argc, char **argv)
 #ifdef SHADOWGRP
 	is_shadow_grp = sgr_file_present ();
 #endif
+	is_sub_uid = sub_uid_file_present ();
+	is_sub_gid = sub_gid_file_present ();
 
 	get_defaults ();
 
@@ -2034,6 +2157,22 @@ int main (int argc, char **argv)
 		grp_add ();
 	}
 
+	if (is_sub_uid) {
+		if (find_new_sub_uids(user_name, &sub_uid_start, &sub_uid_count) < 0) {
+			fprintf (stderr,
+				_("%s: can't find subordinate user range\n"),
+				Prog);
+			fail_exit(E_SUB_UID_UPDATE);
+		}
+	}
+	if (is_sub_gid) {
+		if (find_new_sub_gids(user_name, &sub_gid_start, &sub_gid_count) < 0) {
+			fprintf (stderr,
+				_("%s: can't find subordinate group range\n"),
+				Prog);
+			fail_exit(E_SUB_GID_UPDATE);
+		}
+	}
 	usr_update ();
 
 	if (mflg) {
Signature · 1 line
1.7.5.4
#18 lore raw

PATCH 8/11 Add support for detecting busy subordinate user ids

In reply to a message not in the archive
---
 libmisc/user_busy.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
libmisc/user_busy.c +13 -5
index 168f9d5..04cfc31 100644
@@ -38,11 +38,13 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <dirent.h>
+#include <fcntl.h>
 #include "defines.h"
 #include "prototypes.h"
+#include "subordinateio.h"
 
 #ifdef __linux__
-static int check_status (const char *sname, uid_t uid);
+static int check_status (const char *name, const char *sname, uid_t uid);
 static int user_busy_processes (const char *name, uid_t uid);
 #else				/* !__linux__ */
 static int user_busy_utmp (const char *name);
@@ -102,7 +104,7 @@ static int user_busy_utmp (const char *name)
 #endif				/* !__linux__ */
 
 #ifdef __linux__
-static int check_status (const char *sname, uid_t uid)
+static int check_status (const char *name, const char *sname, uid_t uid)
 {
 	/* 40: /proc/xxxxxxxxxx/task/xxxxxxxxxx/status + \0 */
 	char status[40];
@@ -125,7 +127,10 @@ static int check_status (const char *sname, uid_t uid)
 			            &ruid, &euid, &suid) == 3) {
 				if (   (ruid == (unsigned long) uid)
 				    || (euid == (unsigned long) uid)
-				    || (suid == (unsigned long) uid)) {
+				    || (suid == (unsigned long) uid)
+				    || have_sub_uids(name, ruid, 1)
+				    || have_sub_uids(name, euid, 1)
+				    || have_sub_uids(name, suid, 1)) {
 					(void) fclose (sfile);
 					return 1;
 				}
@@ -153,6 +158,8 @@ static int user_busy_processes (const char *name, uid_t uid)
 	struct stat sbroot;
 	struct stat sbroot_process;
 
+	sub_uid_open (O_RDONLY);
+
 	proc = opendir ("/proc");
 	if (proc == NULL) {
 		perror ("opendir /proc");
@@ -196,7 +203,7 @@ static int user_busy_processes (const char *name, uid_t uid)
 			continue;
 		}
 
-		if (check_status (tmp_d_name, uid) != 0) {
+		if (check_status (name, tmp_d_name, uid) != 0) {
 			(void) closedir (proc);
 			fprintf (stderr,
 			         _("%s: user %s is currently used by process %d\n"),
@@ -216,7 +223,7 @@ static int user_busy_processes (const char *name, uid_t uid)
 				if (tid == pid) {
 					continue;
 				}
-				if (check_status (task_path+6, uid) != 0) {
+				if (check_status (name, task_path+6, uid) != 0) {
 					(void) closedir (proc);
 					fprintf (stderr,
 					         _("%s: user %s is currently used by process %d\n"),
@@ -231,6 +238,7 @@ static int user_busy_processes (const char *name, uid_t uid)
 	}
 
 	(void) closedir (proc);
+	sub_uid_close();
 	return 0;
 }
 #endif				/* __linux__ */
Signature · 1 line
1.7.5.4
#19 lore raw

PATCH 9/11 usermod: Add support for subordinate uids and gids.

In reply to a message not in the archive
---
 man/usermod.8.xml |   80 +++++++++++++++++
 src/usermod.c     |  255 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 332 insertions(+), 3 deletions(-)
man/usermod.8.xml +80 -0
index 322d181..0a23416 100644
@@ -391,6 +391,86 @@
       </varlistentry>
       <varlistentry>
 	<term>
+	  <option>-v</option>, <option>--add-sub-uids</option>
+	  <replaceable>FIRST</replaceable>-<replaceable>LAST</replaceable>
+	</term>
+	<listitem>
+	  <para>
+	    Add a range of subordinate uids to the users account. 
+	  </para>
+	  <para>
+	    This option may be specified multiple times to add multiple ranges to a users account.
+	  </para>
+	  <para>
+	     No checks will be performed with regard to
+	     <option>SUB_UID_MIN</option>, <option>SUB_UID_MAX</option>, or
+	     <option>SUB_UID_COUNT</option> from /etc/login.defs.
+	  </para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term>
+	  <option>-V</option>, <option>--del-sub-uids</option>
+	  <replaceable>FIRST</replaceable>-<replaceable>LAST</replaceable>
+	</term>
+	<listitem>
+	  <para>
+	    Remove a range of subordinate uids from the users account.
+	  </para>
+	  <para>
+	    This option may be specified multiple times to remove multiple ranges to a users account.
+	    When both <option>--del-sub-uids</option> and <option>--add-sub-uids</option> are specified
+	    remove of all subordinate uid ranges happens before any subordinate uid ranges are added.
+	  </para>
+	  <para>
+	     No checks will be performed with regard to
+	     <option>SUB_UID_MIN</option>, <option>SUB_UID_MAX</option>, or
+	     <option>SUB_UID_COUNT</option> from /etc/login.defs.
+	  </para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term>
+	  <option>-w</option>, <option>--add-sub-gids</option>
+	  <replaceable>FIRST</replaceable>-<replaceable>LAST</replaceable>
+	</term>
+	<listitem>
+	  <para>
+	    Add a range of subordinate gids to the users account.
+	  </para>
+	  <para>
+	    This option may be specified multiple times to add multiple ranges to a users account.
+	  </para>
+	  <para>
+	     No checks will be performed with regard to
+	     <option>SUB_GID_MIN</option>, <option>SUB_GID_MAX</option>, or
+	     <option>SUB_GID_COUNT</option> from /etc/login.defs.
+	  </para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term>
+	  <option>-W</option>, <option>--del-sub-gids</option>
+	  <replaceable>FIRST</replaceable>-<replaceable>LAST</replaceable>
+	</term>
+	<listitem>
+	  <para>
+	    Remove a range of subordinate gids from the users account.
+	  </para>
+	  <para>
+	    This option may be specified multiple times to remove multiple ranges to a users account.
+	    When both <option>--del-sub-gids</option> and <option>--add-sub-gids</option> are specified
+	    remove of all subordinate gid ranges happens before any subordinate gid ranges are added.
+	  </para>
+	  <para>
+	     No checks will be performed with regard to
+	     <option>SUB_GID_MIN</option>, <option>SUB_GID_MAX</option>, or
+	     <option>SUB_GID_COUNT</option> from /etc/login.defs.
+	  </para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term>
 	  <option>-Z</option>, <option>--selinux-user</option>
 	  <replaceable>SEUSER</replaceable>
 	</term>
src/usermod.c +252 -3

369 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#20 lore raw

PATCH 10/11 newusers: Add support for assiging subordinate uids and gids.

In reply to a message not in the archive
---
 src/newusers.c |  124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 124 insertions(+), 0 deletions(-)
src/newusers.c +124 -0
index c25f40e..e5e832c 100644
@@ -65,6 +65,7 @@
 #include "pwio.h"
 #include "sgroupio.h"
 #include "shadowio.h"
+#include "subordinateio.h"
 #include "chkname.h"
 
 /*
@@ -82,6 +83,8 @@ static long sha_rounds = 5000;
 #endif				/* USE_SHA_CRYPT */
 #endif				/* !USE_PAM */
 
+static bool is_sub_uid = false;
+static bool is_sub_gid = false;
 static bool is_shadow;
 #ifdef SHADOWGRP
 static bool is_shadow_grp;
@@ -90,6 +93,8 @@ static bool sgr_locked = false;
 static bool pw_locked = false;
 static bool gr_locked = false;
 static bool spw_locked = false;
+static bool sub_uid_locked = false;
+static bool sub_gid_locked = false;
 
 /* local function prototypes */
 static void usage (int status);
@@ -178,6 +183,20 @@ static void fail_exit (int code)
 		}
 	}
 #endif
+	if (sub_uid_locked) {
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+			/* continue */
+		}
+	}
+	if (sub_gid_locked) {
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+			/* continue */
+		}
+	}
 
 	exit (code);
 }
@@ -732,6 +751,24 @@ static void open_files (void)
 		sgr_locked = true;
 	}
 #endif
+	if (is_sub_uid) {
+		if (sub_uid_lock () == 0) {
+			fprintf (stderr,
+			         _("%s: cannot lock %s; try again later.\n"),
+			         Prog, sub_uid_dbname ());
+			fail_exit (EXIT_FAILURE);
+		}
+		sub_uid_locked = true;
+	}
+	if (is_sub_gid) {
+		if (sub_gid_lock () == 0) {
+			fprintf (stderr,
+			         _("%s: cannot lock %s; try again later.\n"),
+			         Prog, sub_gid_dbname ());
+			fail_exit (EXIT_FAILURE);
+		}
+		sub_gid_locked = true;
+	}
 
 	if (pw_open (O_RDWR) == 0) {
 		fprintf (stderr, _("%s: cannot open %s\n"), Prog, pw_dbname ());
@@ -751,6 +788,22 @@ static void open_files (void)
 		fail_exit (EXIT_FAILURE);
 	}
 #endif
+	if (is_sub_uid) {
+		if (sub_uid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+			         _("%s: cannot open %s\n"),
+			         Prog, sub_uid_dbname ());
+			fail_exit (EXIT_FAILURE);
+		}
+	}
+	if (is_sub_gid) {
+		if (sub_gid_open (O_RDWR) == 0) {
+			fprintf (stderr,
+			         _("%s: cannot open %s\n"),
+			         Prog, sub_gid_dbname ());
+			fail_exit (EXIT_FAILURE);
+		}
+	}
 }
 
 /*
@@ -795,6 +848,19 @@ static void close_files (void)
 		SYSLOG ((LOG_ERR, "failure while writing changes to %s", gr_dbname ()));
 		fail_exit (EXIT_FAILURE);
 	}
+	if (is_sub_uid  && (sub_uid_close () == 0)) {
+		fprintf (stderr,
+		         _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname ());
+		SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_uid_dbname ()));
+		fail_exit (EXIT_FAILURE);
+	}
+	if (is_sub_gid  && (sub_gid_close () == 0)) {
+		fprintf (stderr,
+		         _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname ());
+		SYSLOG ((LOG_ERR, "failure while writing changes to %s", sub_gid_dbname ()));
+		fail_exit (EXIT_FAILURE);
+	}
+
 	if (gr_unlock () == 0) {
 		fprintf (stderr,
 		         _("%s: failed to unlock %s\n"),
@@ -823,6 +889,22 @@ static void close_files (void)
 		sgr_locked = false;
 	}
 #endif
+	if (is_sub_uid) {
+		if (sub_uid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_uid_dbname ()));
+			/* continue */
+		}
+		sub_uid_locked = false;
+	}
+	if (is_sub_gid) {
+		if (sub_gid_unlock () == 0) {
+			fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ());
+			SYSLOG ((LOG_ERR, "failed to unlock %s", sub_gid_dbname ()));
+			/* continue */
+		}
+		sub_gid_locked = false;
+	}
 }
 
 int main (int argc, char **argv)
@@ -864,6 +946,8 @@ int main (int argc, char **argv)
 #ifdef SHADOWGRP
 	is_shadow_grp = sgr_file_present ();
 #endif
+	is_sub_uid = sub_uid_file_present ();
+	is_sub_gid = sub_gid_file_present ();
 
 	open_files ();
 
@@ -1044,6 +1128,46 @@ int main (int argc, char **argv)
 			errors++;
 			continue;
 		}
+
+		/*
+		 * Add subordinate uids if the user does not have them.
+		 */
+		if (is_sub_uid && !sub_uid_assigned(fields[0])) {
+			uid_t sub_uid_start = 0;
+			unsigned long sub_uid_count = 0;
+			if (find_new_sub_uids(fields[0], &sub_uid_start, &sub_uid_count) == 0) {
+				if (sub_uid_add(fields[0], sub_uid_start, sub_uid_count) == 0) {
+					fprintf (stderr,
+						_("%s: failed to prepare new %s entry\n"),
+						Prog, sub_uid_dbname ());
+				}
+			} else {
+				fprintf (stderr,
+					_("%s: can't find subordinate user range\n"),
+					Prog);
+				errors++;
+			}
+		}
+	
+		/*
+		 * Add subordinate gids if the user does not have them.
+		 */
+		if (is_sub_gid && !sub_gid_assigned(fields[0])) {
+			gid_t sub_gid_start = 0;
+			unsigned long sub_gid_count = 0;
+			if (find_new_sub_gids(fields[0], &sub_gid_start, &sub_gid_count) == 0) {
+				if (sub_gid_add(fields[0], sub_gid_start, sub_gid_count) == 0) {
+					fprintf (stderr,
+						_("%s: failed to prepare new %s entry\n"),
+						Prog, sub_uid_dbname ());
+				}
+			} else {
+				fprintf (stderr,
+					_("%s: can't find subordinate group range\n"),
+					Prog);
+				errors++;
+			}
+		}
 	}
 
 	/*
Signature · 1 line
1.7.5.4
#21 lore raw

PATCH 11/11 newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

In reply to a message not in the archive
---
 libmisc/Makefile.am |    2 +
 libmisc/idmapping.c |  126 +++++++++++++++++++++++++++++++++++
 libmisc/idmapping.h |   44 ++++++++++++
 man/Makefile.am     |    4 +
 man/newgidmap.1.xml |  157 +++++++++++++++++++++++++++++++++++++++++++
 man/newuidmap.1.xml |  154 +++++++++++++++++++++++++++++++++++++++++++
 src/Makefile.am     |    5 +-
 src/newgidmap.c     |  183 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/newuidmap.c     |  183 +++++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 856 insertions(+), 2 deletions(-)
 create mode 100644 libmisc/idmapping.c
 create mode 100644 libmisc/idmapping.h
 create mode 100644 man/newgidmap.1.xml
 create mode 100644 man/newuidmap.1.xml
 create mode 100644 src/newgidmap.c
 create mode 100644 src/newuidmap.c
libmisc/Makefile.am +2 -0
index 5de9193..76f3c05 100644
@@ -32,6 +32,8 @@ libmisc_a_SOURCES = \
 	getgr_nam_gid.c \
 	getrange.c \
 	hushed.c \
+	idmapping.h \
+	idmapping.c \
 	isexpired.c \
 	limits.c \
 	list.c log.c \
libmisc/idmapping.c new file +126 -0

126 lines not on this page · raw message

libmisc/idmapping.h new file +44 -0

44 lines not on this page · raw message

man/Makefile.am +4 -0
index fc617e9..b50ca41 100644
@@ -30,7 +30,9 @@ man_MANS = \
 	man1/login.1 \
 	man5/login.defs.5 \
 	man8/logoutd.8 \
+	man1/newgidmap.1 \
 	man1/newgrp.1 \
+	man1/newuidmap.1 \
 	man8/newusers.8 \
 	man8/nologin.8 \
 	man1/passwd.1 \
@@ -83,7 +85,9 @@ man_XMANS = \
 	login.access.5.xml \
 	login.defs.5.xml \
 	logoutd.8.xml \
+	newgidmap.1.xml \
 	newgrp.1.xml \
+	newuidmap.1.xml \
 	newusers.8.xml \
 	nologin.8.xml \
 	passwd.1.xml \
man/newgidmap.1.xml new file +157 -0

157 lines not on this page · raw message

man/newuidmap.1.xml new file +154 -0

154 lines not on this page · raw message

src/Makefile.am +3 -2
index 88cae99..1390cae 100644
@@ -24,7 +24,8 @@ INCLUDES = \
 
 bin_PROGRAMS   = groups login su
 sbin_PROGRAMS  = nologin
-ubin_PROGRAMS  = faillog lastlog chage chfn chsh expiry gpasswd newgrp passwd
+ubin_PROGRAMS  = faillog lastlog chage chfn chsh expiry gpasswd newgrp passwd \
+	newgidmap newuidmap
 usbin_PROGRAMS = \
 	chgpasswd \
 	chpasswd \
@@ -49,7 +50,7 @@ usbin_PROGRAMS = \
 noinst_PROGRAMS = id sulogin
 
 suidbins       = su
-suidubins      = chage chfn chsh expiry gpasswd newgrp passwd
+suidubins      = chage chfn chsh expiry gpasswd newgrp passwd newuidmap newgidmap
 if ACCT_TOOLS_SETUID
 	suidubins += chage chgpasswd chpasswd groupadd groupdel groupmod newusers useradd userdel usermod
 endif
src/newgidmap.c new file +183 -0

183 lines not on this page · raw message

src/newuidmap.c new file +183 -0

183 lines not on this page · raw message

Signature · 1 line
1.7.5.4

A message that is not in the archive (sent off-list or not yet received): 87ehhdpoag.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#23 lore raw

Re: [PATCH 11/11] newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Note you need the following bit on top of your patch to make newuidmap and newgidmap link against libselinux. Otherwise selinux-enabled builds (default in ubuntu) fail.

src/Makefile.am +2 -0
Index: shadow-4.1.5.1/src/Makefile.am
===================================================================
@@ -80,6 +80,8 @@
 endif
 
 chage_LDADD    = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX)
+newuidmap_LDADD    = $(LDADD) $(LIBSELINUX)
+newgidmap_LDADD    = $(LDADD) $(LIBSELINUX)
 chfn_LDADD     = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD)
 chgpasswd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBSELINUX) $(LIBCRYPT)
 chsh_LDADD     = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD)

A message that is not in the archive (sent off-list or not yet received): [email protected]

#25 lore raw

Re: [PATCH 11/11] newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Another patch needed, to handle the shortopts in usermod, and add a missing break when handling wlfg (leading to Wflg being handled as well).

src/usermod.c +3 -2
Index: shadow-userns/src/usermod.c
===================================================================
@@ -993,9 +993,9 @@
 		};
 		while ((c = getopt_long (argc, argv,
 #ifdef WITH_SELINUX
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:UZ:",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:UZ:v:w:V:W:",
 #else				/* !WITH_SELINUX */
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:U",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:w:V:W:",
 #endif				/* !WITH_SELINUX */
 			                 long_options, NULL)) != -1) {
 			switch (c) {
@@ -1141,6 +1141,7 @@
 					exit(E_BAD_ARG);
 				}
 				wflg = true;
+                break;
 			case 'W':
 				if (prepend_range (optarg, &del_sub_gids) == 0) {
 					fprintf (stderr,
#26 lore raw

Re: [PATCH 11/11] newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

In reply to a message not in the archive
Quoted 3 lines · Serge E. Hallyn

Note you need the following bit on top of your patch to make newuidmap and newgidmap link against libselinux. Otherwise selinux-enabled builds (default in ubuntu) fail.

Interesting. I can't imagine what brings in libselinux. But I am in favor of whatever works.

Eric

Quoted 13 lines

Index: shadow-4.1.5.1/src/Makefile.am
===================================================================

src/Makefile.am +2 -0
@@ -80,6 +80,8 @@
 endif
 
 chage_LDADD    = $(LDADD) $(LIBPAM_SUID) $(LIBAUDIT) $(LIBSELINUX)
+newuidmap_LDADD    = $(LDADD) $(LIBSELINUX)
+newgidmap_LDADD    = $(LDADD) $(LIBSELINUX)
 chfn_LDADD     = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD)
 chgpasswd_LDADD = $(LDADD) $(LIBPAM_SUID) $(LIBSELINUX) $(LIBCRYPT)
 chsh_LDADD     = $(LDADD) $(LIBPAM) $(LIBSELINUX) $(LIBCRYPT_NOPAM) $(LIBSKEY) $(LIBMD)

A message that is not in the archive (sent off-list or not yet received): 87sj5ai8us.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#28 lore raw

Re: [PATCH 11/11] newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 8 lines · Eric W. Biederman
Quoted 3 lines · Serge E. Hallyn

Note you need the following bit on top of your patch to make newuidmap and newgidmap link against libselinux. Otherwise selinux-enabled builds (default in ubuntu) fail.

Interesting. I can't imagine what brings in libselinux. But I am in favor of whatever works.

FWIW, it came in through libshadow (presumably shadowio.c which includes commonio, which calls set_selinux_file_context). Oh, I also see newuidmap.c #including subordinateio which includes commonio.

libtool: link: gcc -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall -Wl,-Bsymbolic-functions -Wl,-z -Wl,relro -o newgidmap newgidmap.o ../libmisc/libmisc.a ../lib/.libs/libshadow.a ../lib/.libs/libshadow.a(selinux.o): In function `set_selinux_file_context':
/build/buildd/shadow-4.1.5.1/lib/selinux.c:64: undefined reference to `matchpathcon'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:70: undefined reference to `setfscreatecon'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:75: undefined reference to `freecon'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:58: undefined reference to `is_selinux_enabled'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:65: undefined reference to `security_getenforce'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:71: undefined reference to `security_getenforce'
../lib/.libs/libshadow.a(selinux.o): In function `reset_selinux_file_context':
/build/buildd/shadow-4.1.5.1/lib/selinux.c:94: undefined reference to `setfscreatecon'
/build/buildd/shadow-4.1.5.1/lib/selinux.c:90: undefined reference to `is_selinux_enabled'
collect2: error: ld returned 1 exit status

#29 lore raw

Re: [PATCH 05/11] Implement find_new_sub_uids find_new_sub_gids

In reply to a message not in the archive

My apologies for the lateness of my reply; I have a few small thoughts about these patches.

Quoted 8 lines · Eric W. Biederman

Functions for finding new subordinate uid and gids ranges for use with useradd.

[...]

(quoted diff) +3 -0
+int find_new_sub_gids (const char *owner,
+		       gid_t *range_start, unsigned long *range_count)
+{

Review

Quoted 10 lines
(quoted diff) +10 -0
+	unsigned long min, max;
+	unsigned long count;
+	gid_t start;
+
+	assert (range_start != NULL);
+	assert (range_count != NULL);
+
+	min = getdef_ulong ("SUB_GID_MIN", 100000UL);
+	max = getdef_ulong ("SUB_GID_MAX", 600100000UL);
+	count = getdef_ulong ("SUB_GID_COUNT", 10000);

I'd like to see a quick

	if (min >= max || count >= max || (min + count) >= max) {
		/* fail loudly */
	}

right here. The sanity of the rest of the function depends upon these invariants.

Quoted 47 lines
(quoted diff) +28 -0
+
+	/* Is there a preferred range that works? */
+	if ((*range_count != 0) &&
+	    (*range_start >= min) &&
+	    (((*range_start) + (*range_count) - 1) <= max) &&
+	    is_sub_gid_range_free(*range_start, *range_count)) {
+		return 0;
+	}
+
+	if (max < (min + count)) {
+		(void) fprintf (stderr,
+				_("%s: Invalid configuration: SUB_GID_MIN (%lu), SUB_GID_MAX (%lu)\n"),
+			Prog, min, max);
+		return -1;
+	}
+	start = sub_gid_find_free_range(min, max, count);
+	if (start == (gid_t)-1) {
+		fprintf (stderr,
+		         _("%s: Can't get unique secondary GID range\n"),
+		         Prog);
+		SYSLOG ((LOG_WARN, "no more available secondary GIDs on the system"));
+		return -1;
+	}
+	*range_start = start;
+	*range_count = count;
+	return 0;
+}
+
libmisc/find_new_sub_uids.c new file +11 -0
new file mode 100644
index 0000000..f1720f9
 
[...]
 
+/*
+ * find_new_sub_uids - Find a new unused range of UIDs.
+ *
+ * If successful, find_new_sub_uids provides a range of unused
+ * user IDs in the [SUB_UID_MIN:SUB_UID_MAX] range.
+ * 
+ * Return 0 on success, -1 if no unused UIDs are available.
+ */
+int find_new_sub_uids (const char *owner,
+		       uid_t *range_start, unsigned long *range_count)
+{

libmisc/find_new_sub_uids.c

Quoted 10 lines
libmisc/find_new_sub_uids.c new file +10 -0
+	unsigned long min, max;
+	unsigned long count;
+	uid_t start;
+
+	assert (range_start != NULL);
+	assert (range_count != NULL);
+
+	min = getdef_ulong ("SUB_UID_MIN", 100000UL);
+	max = getdef_ulong ("SUB_UID_MAX", 600100000UL);
+	count = getdef_ulong ("SUB_UID_COUNT", 10000);

Same here, of course.

Quoted 26 lines
libmisc/find_new_sub_uids.c new file +26 -0
+	/* Is there a preferred range that works? */
+	if ((*range_count != 0) &&
+	    (*range_start >= min) &&
+	    (((*range_start) + (*range_count) - 1) <= max) &&
+	    is_sub_uid_range_free(*range_start, *range_count)) {
+		return 0;
+	}
+
+	if (max < (min + count)) {
+		(void) fprintf (stderr,
+				_("%s: Invalid configuration: SUB_UID_MIN (%lu), SUB_UID_MAX (%lu)\n"),
+			Prog, min, max);
+		return -1;
+	}
+	start = sub_uid_find_free_range(min, max, count);
+	if (start == (uid_t)-1) {
+		fprintf (stderr,
+		         _("%s: Can't get unique secondary UID range\n"),
+		         Prog);
+		SYSLOG ((LOG_WARN, "no more available secondary UIDs on the system"));
+		return -1;
+	}
+	*range_start = start;
+	*range_count = count;
+	return 0;
+}

libmisc/find_new_sub_uids.c

Quoted 13 lines · Eric W. Biederman
libmisc/find_new_sub_uids.c new file +13 -0
+struct map_range *get_map_ranges(int ranges, int argc, char **argv)
+{
+	struct map_range *mappings, *mapping;
+	int idx, argidx;
+
+	if ((ranges * 3) > argc) {
+		fprintf(stderr, "ranges: %u argc: %d\n",
+			ranges, argc);
+		fprintf(stderr,
+			_( "%s: Not enough arguments to form %u mappings\n"),
+			Prog, ranges);
+		return NULL;
+	}

If 'ranges' is large enough, 3*ranges can wrap around to be less than 'argc', pass this test, and continue to the for loop below, which could scribble all over memory until running into an unmapped page.

The current use appears safe, and I don't see how this could be exploited even if it weren't currently safe, but I'd really like to see this test re-written in a way that does not allow wraparound.

Quoted 9 lines
libmisc/find_new_sub_uids.c new file +9 -0
+	mappings = calloc(ranges, sizeof(*mappings));
+	if (!mappings) {
+		fprintf(stderr, _( "%s: Memory allocation failure\n"),
+			Prog);
+		exit(EXIT_FAILURE);
+	}
+
+	/* Gather up the ranges from the command line */
+	mapping = mappings;

libmisc/find_new_sub_uids.c

Quoted 10 lines
libmisc/find_new_sub_uids.c new file +10 -0
+	for (idx = 0; idx < ranges; idx++, argidx += 3, mapping++) {
+		if (!getulong(argv[argidx + 0], &mapping->upper))
+			return NULL;
+		if (!getulong(argv[argidx + 1], &mapping->lower))
+			return NULL;
+		if (!getulong(argv[argidx + 2], &mapping->count))
+			return NULL;
+	}
+	return mappings;
+}

Thanks,

Seth

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

#30 lore raw

Re: [PATCH 11/11] newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

In reply to a message not in the archive

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]):

Hi,

src/newuidmap.c line 57

Quoted 9 lines
src/newuidmap.c new file +9 -0
+static bool verify_range(struct passwd *pw, struct map_range *range)
+{
+	/* An empty range is invalid */
+	if (range->count == 0)
+		return false;
+
+	/* Test /etc/subuid */
+	if (have_sub_uids(pw->pw_name, range->lower, range->count))
+		return true;

I think the have_sub_uids() test should be skipped if we started out as root. Is there a reason not to do that?

-serge

A message that is not in the archive (sent off-list or not yet received): [email protected]

#32 lore raw

Re: [PATCH 11/11] newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

Quoted 4 lines · Serge E. Hallyn

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]):

Hi,

Quoted 12 lines

src/newuidmap.c line 57

Quoted 9 lines
src/newuidmap.c new file +9 -0
+static bool verify_range(struct passwd *pw, struct map_range *range)
+{
+	/* An empty range is invalid */
+	if (range->count == 0)
+		return false;
+
+	/* Test /etc/subuid */
+	if (have_sub_uids(pw->pw_name, range->lower, range->count))
+		return true;

I think the have_sub_uids() test should be skipped if we started out as root. Is there a reason not to do that?

The only reason I can see for root to use this binary is if it a flavor of root that has dropped some capbilities. Is there a reason for root to use newuidmap and newgid map at all?

Otherwise I think it makes sense to enforce whatever the system choosen policy is on root, because root is opting in.

Eric

A message that is not in the archive (sent off-list or not yet received): 87zjpw278b.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#34 lore raw

Re: [PATCH 11/11] newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

Quoted 19 lines · Eric W. Biederman
Quoted 4 lines · Serge E. Hallyn

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]):

Hi,

Quoted 12 lines

src/newuidmap.c line 57

Quoted 9 lines
src/newuidmap.c new file +9 -0
+static bool verify_range(struct passwd *pw, struct map_range *range)
+{
+	/* An empty range is invalid */
+	if (range->count == 0)
+		return false;
+
+	/* Test /etc/subuid */
+	if (have_sub_uids(pw->pw_name, range->lower, range->count))
+		return true;

I think the have_sub_uids() test should be skipped if we started out as root. Is there a reason not to do that?

Quoted 3 lines

The only reason I can see for root to use this binary is if it a flavor of root that has dropped some capbilities. Is there a reason for root to use newuidmap and newgid map at all?

Of course. It keeps things simpler for creating mapped containers. Otherwise we have to special-case the "i am root" vs "i am not root" case when untarring a rootfs for a container.

Quoted 8 lines

Otherwise I think it makes sense to enforce whatever the system choosen policy is on root, because root is opting in.

Eric
_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

#35 lore raw

Re: [PATCH 11/11] newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

Quoted 20 lines · Serge Hallyn
Quoted 19 lines · Eric W. Biederman
Quoted 16 lines · Serge E. Hallyn

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]):

Hi,

Review

Quoted 9 lines
(quoted diff) +9 -0
+static bool verify_range(struct passwd *pw, struct map_range *range)
+{
+	/* An empty range is invalid */
+	if (range->count == 0)
+		return false;
+
+	/* Test /etc/subuid */
+	if (have_sub_uids(pw->pw_name, range->lower, range->count))
+		return true;

I think the have_sub_uids() test should be skipped if we started out as root. Is there a reason not to do that?

Quoted 7 lines
Quoted 3 lines

The only reason I can see for root to use this binary is if it a flavor of root that has dropped some capbilities. Is there a reason for root to use newuidmap and newgid map at all?

Of course. It keeps things simpler for creating mapped containers. Otherwise we have to special-case the "i am root" vs "i am not root" case when untarring a rootfs for a container.

I guess my practical question is don't we want to reserve a range of subuid's for root owned containers as well. Just so that we know someone is using those uids and we don't get them allocated to another purpose?

Or am I missing something here?

Eric

A message that is not in the archive (sent off-list or not yet received): 87iowjya4j.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#37 lore raw

Re: [PATCH 11/11] newuidmap, newgidmap: New suid helpers for using subordinate uids and gids

Quoted 30 lines · Eric W. Biederman
Quoted 20 lines · Serge Hallyn
Quoted 19 lines · Eric W. Biederman
Quoted 16 lines · Serge E. Hallyn

Quoting Eric W. Biederman (ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]):

Hi,

Review

Quoted 9 lines
(quoted diff) +9 -0
+static bool verify_range(struct passwd *pw, struct map_range *range)
+{
+	/* An empty range is invalid */
+	if (range->count == 0)
+		return false;
+
+	/* Test /etc/subuid */
+	if (have_sub_uids(pw->pw_name, range->lower, range->count))
+		return true;

I think the have_sub_uids() test should be skipped if we started out as root. Is there a reason not to do that?

Quoted 7 lines
Quoted 3 lines

The only reason I can see for root to use this binary is if it a flavor of root that has dropped some capbilities. Is there a reason for root to use newuidmap and newgid map at all?

Of course. It keeps things simpler for creating mapped containers. Otherwise we have to special-case the "i am root" vs "i am not root" case when untarring a rootfs for a container.

Quoted 6 lines

I guess my practical question is don't we want to reserve a range of subuid's for root owned containers as well. Just so that we know someone is using those uids and we don't get them allocated to another purpose?

Or am I missing something here?

That's a not unreasonable argument, but two counters: 1. "someone" using the uids used by root would mean root assigned those uids to someone. That's a misconfiguration, in other words this is keeping root from shooting himself in the foot. It's not in the kernel though, so maybe it's ok. 2. I like to think that any time something fails as my user, if it fails due to permission reasons, then if I run it as root it should work. That's useful for debugging, and just seems coherent with how most things work.

-serge

#38 lore raw
In reply to a message not in the archive

I am waaaay behind on email.

Quoted 4 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

Could you give an example of this? (If this takes off I'll probably want to add support to toybox, but from the man pages I don't understand what it's for.)

Quoted 3 lines

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

They must use a contiguous range with count, not "landley:4000-4999,6103,7002-7005"?

Quoted 8 lines

useradd, and newusers are modifed to add users to those files. userdel is modeifed to remove users from those files. usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

I'm not finding uid_map and gid_map in Documentation/filesystems/proc.txt, is this a pending patch?

Rob

#39 lore raw
Quoted 11 lines · Rob Landley

I am waaaay behind on email.

Quoted 4 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

Could you give an example of this? (If this takes off I'll probably want to add support to toybox, but from the man pages I don't understand what it's for.)

The interesting programs for me are useradd, userdel, which I don't think toybox plays with and newuidmap and newgidmap.

The primary application is running unprivileged containers with user namespaces. A user namespace allows setuid and setgid in between uids and gids that are mapped for that user namespace.

/proc/<pid>/uid_map gives the mapping from uids in the user namespace to uids on the host system. The format is multiple lines of the form:
<userns_uid> <system_uid> <count>

/proc/<pid>/gid_map has the same format excpet for gids.

/etc/subuid and /etc/subgid list the extra system uids that we will allow a user to use in the context of user namespaces.

The idea is that unshare or clone will be used to create a new process and then wait while another process of the same user outside of the user namespace will call newuidmap and newgidmap to write to set the mappings.

I have appened a trivial command line example of using newuidmap and newgidmap and unshare.

Without privileged helper programs people are limited to using their own uid as the underlying uid in a user-namespace which seriously limits what kind of systems you can run in a container.

Quoted 6 lines
Quoted 3 lines

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

They must use a contiguous range with count, not
"landley:4000-4999,6103,7002-7005"?

No. Although I can see the argument for that format. Using multiple ranges per line is harder to parse, harder to ensure that you properly handle arbitarily long lines, and harder to tell if the last number in the range is included or not. And I have been burnt badly by implementations having arbitrarly short line limits in /etc/group.

So to encode your example it would be:
landley:4000:1000
landley:6103:1
landley:7002:4

As I allow multiple lines for the same user.

In practice I only expect there will need to be a single line per user in most instances as a single range should be enough for most uses.

Also a contiguous range is important for sanity so a sysadmin can look at a set of uids and easily recognize they all belong to fred, and even more important when working with /proc/<pid>/uid_map as the kernel currently only supports 5 uid mapping ranges.

Quoted 5 lines
Quoted 4 lines

useradd, and newusers are modifed to add users to those files. userdel is modeifed to remove users from those files. usermod is modified to give manual control of what goes in those files.

Quoted 7 lines
Quoted 4 lines

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

I'm not finding uid_map and gid_map in Documentation/filesystems/proc.txt, is this a pending patch?

Interesting. I hadn't even registered on the existence of Documentation/filesystems/proc.txt.

I have sent out man page patches to document or at least start documenting uid_map and gid_map. But having just done a git update it looks like Michael hasn't pushed those changes out in the man-pages git tree yet. :( He may be waiting until 3.8 goes final.

Eric

----
#!/bin/bash
set -x
set -e

export IFIFO=/tmp/shadow-test-$$-in
export OFIFO=/tmp/shadow-test-$$-out
rm -f $IFIFO $OFIFO
mkfifo $IFIFO
mkfifo $OFIFO
unshare --user /bin/bash <<'EOF' &
echo waiting-for-uid-and-gid-maps > $OFIFO read LINE < $IFIFO
$SHELL -l -i < /dev/tty > /dev/tty 2> /dev/tty EOF
child=$!
read LINE < $OFIFO
uid=$(id --user)
gid=$(id --group)
newuidmap $child 0 $uid 1 1 100000 9999 65534 109999 1 newgidmap $child 0 $gid 1 1 100000 9999 65534 109999 1 echo uid-and-gid-maps > $IFIFO
wait $child
rm -f $IFIFO $OFIFO

#40 lore raw
In reply to a message not in the archive

Hi Eric,

Quoted 14 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

Quoted 8 lines

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Why patch shadow tools? Why not implement the feature as a PAM module? All other capabilities granting things are implemented as PAM modules:
pam_group, pam_namespace, pam_cap. I don't see why it cannot be fully modularized, a common admin doesn't need multiple uid/gid user_ns for non-root users at all, why patch basic tools?

Thanks,

Signature · 2 lines
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
#41 lore raw
Quoted 22 lines · Vasily Kulikov

Hi Eric,

Quoted 19 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

Quoted 6 lines
Quoted 3 lines

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Why patch shadow tools? Why not implement the feature as a PAM module?

I need hooks into useradd and userdel to managed the subordinate user ids and group ids when users are added and removed from the system. PAM doesn't appear to have any hooks like that at all.

Furthermore shadow-utils is where other uids and gids are allocated and it makes sense to keep the allocation functions together so if it makes sense they can talk to each other

Quoted 2 lines

All other capabilities granting things are implemented as PAM modules:
pam_group, pam_namespace, pam_cap.

Except when you want to program the mapping is not at login time. Programming the mapping needs to happen when a user namespace is set up. The admin of the user namespace knows which uids and gids will be used in that user namespace. The system is configured to know which uids and gids that user can map to.

Quoted 3 lines

I don't see why it cannot be fully
modularized, a common admin doesn't need multiple uid/gid user_ns for non-root users at all, why patch basic tools?

The common admin doesn't _yet_ need multiple uid/gid user_ns for non-root users.

As of linux 3.8 you don't need any special privilege to create a user namespace. Once you have a user namespace you can create other namespaces. This is good for building containers for light weigh virtualization and testing and this is good for sandboxing subprocess. The chrome-browser sandbox uses a setuid application to set a up namespaces for this purpose.

A couple of extra uids should make the sandboxing uses much more complete. Not to mention the advantages of being able to run another distro in a lighter more portable form than hardware virtualization.

Given the habit of the distributions of enabling everything possible I believe it will become common for distributions to allocate multiple uids and gids to users by default.

Eric

A message that is not in the archive (sent off-list or not yet received): 87vcafyy0k.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#43 lore raw
Quoted 8 lines · Eric W. Biederman
Quoted 2 lines · Vasily Kulikov

Why patch shadow tools? Why not implement the feature as a PAM module?

I need hooks into useradd and userdel to managed the subordinate user ids and group ids when users are added and removed from the system. PAM doesn't appear to have any hooks like that at all.

Quoted 8 lines

Furthermore shadow-utils is where other uids and gids are allocated and it makes sense to keep the allocation functions together so if it makes sense they can talk to each other

Quoted 2 lines

All other capabilities granting things are implemented as PAM modules:
pam_group, pam_namespace, pam_cap.

Except when you want to program the mapping is not at login time.

[...]

Understood. So, a user needs to:

1) be able to reserve [ug]id ranges (more specifically, root allocated the range). These ranges should not be allocated by useradd, etc. afterwards.

2) be able to write to uid_map/gid_map files anytime with reserved values of current user.

In this case patching shadow utils looks appropriate, yes.

Thanks,

Signature · 2 lines
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
#44 lore raw
In reply to a message not in the archive
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#46 lore raw
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

However the last I heard Serge was working on getting these changes into Ubuntu.

So the intention is to get this code merged but I don't know what more needs to be done at this point.

Eric

A message that is not in the archive (sent off-list or not yet received): 87zjyw489z.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#48 lore raw
Quoted 50 lines · Eric W. Biederman
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

Quoted 6 lines

However the last I heard Serge was working on getting these changes into Ubuntu.

So the intention is to get this code merged but I don't know what more needs to be done at this point.

I understand, this was more a question for the package maintainers. It would be interesting for us to have those changes more widely available than just @Ubuntu

A message that is not in the archive (sent off-list or not yet received): [email protected]

#50 lore raw
Quoted 54 lines · Glauber Costa
Quoted 53 lines · Eric W. Biederman
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

However the last I heard Serge was working on getting these changes into Ubuntu.

Quoted 6 lines
Quoted 3 lines

So the intention is to get this code merged but I don't know what more needs to be done at this point.

I understand, this was more a question for the package maintainers. It would be interesting for us to have those changes more widely available than just @Ubuntu

Well, I would aim to get it into Debian, from where it should make it into all its downstreams eventually... But I know that's not what you mean :)

Note that the core of this really isn't a big deal, and you can easily implement your own distro-independent wrappers. Just provide an easy tool for admins to assign subuids to users, and a small setuid-root binary to allow users, subject to those constraints, to write to /proc/$$/uid_maps.

Shadow integration will be nice, but for your use case you should be able to by-step it until shadow integration is complete.

-serge

A message that is not in the archive (sent off-list or not yet received): 20130225143451.GE4387@sergelap

#52 lore raw
Quoted 66 lines · Serge Hallyn
Quoted 54 lines · Glauber Costa
Quoted 53 lines · Eric W. Biederman
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

However the last I heard Serge was working on getting these changes into Ubuntu.

Quoted 6 lines
Quoted 3 lines

So the intention is to get this code merged but I don't know what more needs to be done at this point.

I understand, this was more a question for the package maintainers. It would be interesting for us to have those changes more widely available than just @Ubuntu

Well, I would aim to get it into Debian, from where it should make it into all its downstreams eventually... But I know that's not what you mean :)

Quoted 9 lines

Note that the core of this really isn't a big deal, and you can easily implement your own distro-independent wrappers. Just provide an easy tool for admins to assign subuids to users, and a small setuid-root binary to allow users, subject to those constraints, to write to /proc/$$/uid_maps.

Shadow integration will be nice, but for your use case you should be able to by-step it until shadow integration is complete.

Well, the main problem is that I don't talk on behalf of any distro. We distribute OpenVZ, and would like to create containers such that each container has its own user range - all that without having the containers users conflicting with users created by useradd's normal operation.

I am *hoping* that by selecting ranges high enough I will avoid conflicts at least in the beginning, but it is a bit of guesswork.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#54 lore raw
Quoted 77 lines · Glauber Costa
Quoted 66 lines · Serge Hallyn
Quoted 54 lines · Glauber Costa
Quoted 53 lines · Eric W. Biederman
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

However the last I heard Serge was working on getting these changes into Ubuntu.

Quoted 6 lines
Quoted 3 lines

So the intention is to get this code merged but I don't know what more needs to be done at this point.

I understand, this was more a question for the package maintainers. It would be interesting for us to have those changes more widely available than just @Ubuntu

Well, I would aim to get it into Debian, from where it should make it into all its downstreams eventually... But I know that's not what you mean :)

Quoted 9 lines

Note that the core of this really isn't a big deal, and you can easily implement your own distro-independent wrappers. Just provide an easy tool for admins to assign subuids to users, and a small setuid-root binary to allow users, subject to those constraints, to write to /proc/$$/uid_maps.

Shadow integration will be nice, but for your use case you should be able to by-step it until shadow integration is complete.

Quoted 8 lines

Well, the main problem is that I don't talk on behalf of any distro. We distribute OpenVZ, and would like to create containers such that each container has its own user range - all that without having the containers users conflicting with users created by useradd's normal operation.

I am *hoping* that by selecting ranges high enough I will avoid conflicts at least in the beginning, but it is a bit of guesswork.

Oh, it's not too hard to uidmapshift a container, so if not having conflicting ranges with what is auto-handed-out by distros, I'm not saying don't worry about it but it shouldn't be that big of a deal.

I don't recall whether Eric's patches included a system-wide 'start subuid ranges here' number. If not we should add oen, and you could perhaps grab uids 50000-350000, and ask anyone using openvz, once distros support subuids, to set 350001 as the lowest available subuid.

-serge

#55 lore raw
In reply to a message not in the archive
Quoted 8 lines · Glauber Costa

Well, the main problem is that I don't talk on behalf of any distro. We distribute OpenVZ, and would like to create containers such that each container has its own user range - all that without having the containers users conflicting with users created by useradd's normal operation.

I am *hoping* that by selecting ranges high enough I will avoid conflicts at least in the beginning, but it is a bit of guesswork.

Two suggestions.
1) Use /etc/subuid even if the disto doesn't yet. Where in your case you reserve the subordinate uids for root.

2) The default range for normal uids is 1000 - 60000.

    The default range for subordinate uids is 100000- 600100000.

That leaves most of the uids between 600100000 and 4294967296 unclaimed, while leaving enough that each user can have 10000 subordinate uids by default.

Eric

#56 lore raw
In reply to a message not in the archive
Quoted 46 lines · Eric W. Biederman
Quoted 38 lines · Glauber Costa
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 12 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids

Quoted 5 lines

Hi,

Is there any intention to merge this (or any later version thereof) ? I intend to start excluding uid ranges for containers usage in OpenVZ, and support for that in tooling would come in handy.

Quoted 6 lines

I don't know what the state of the main pkg-shadow package is. I have heard anything and the repository seems to have been dormant since the last release almost a year ago.

However the last I heard Serge was working on getting these changes into Ubuntu.

I need to get back to this hopefully later this week. However since the final userns patches won't be in the raring kernel, the merge request will become low priority. I expect it'll be easier to push in may (when the next devel release opens up) than now.

Quoted 11 lines

So the intention is to get this code merged but I don't know what more needs to be done at this point.

Eric

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

#57 lore raw
In reply to a message not in the archive

Hi Nekral,

This isn't a great urgency, but the final userns kernel support needed for this patchset to be useful should land in 3.9, so it isn't that far off. Do you have any objections to/feedback on this set? Would this be possible to get into Debian?

thanks,
-serge

#58 lore raw
In reply to a message not in the archive

Hi Eric, here are some minor fixups to your patch set. I think the NOTE section is left over from chsh, and doesn't apply here.

man/newgidmap.1.xml +0 -15
diff -ur shadow-4.1.5.1.orig/man/newgidmap.1.xml shadow-4.1.5.1/man/newgidmap.1.xml
@@ -98,21 +98,6 @@
     </variablelist>
   </refsect1>
 
-  <refsect1 id='note'>
-    <title>NOTE</title>
-    <para>
-      The only restriction placed on the login shell is that the command
-      name must be listed in <filename>/etc/shells</filename>, unless the
-      invoker is the superuser, and then any value may be added. An
-      account with a restricted login shell may not change her login shell.
-      For this reason, placing <filename>/bin/rsh</filename> in
-      <filename>/etc/shells</filename> is discouraged since accidentally
-      changing to a restricted shell would prevent the user from ever
-      changing her login shell back to its original value.
-    </para>
-  </refsect1>
-
-
   <refsect1 id='files'>
     <title>FILES</title>
     <variablelist>
man/newuidmap.1.xml +0 -15
diff -ur shadow-4.1.5.1.orig/man/newuidmap.1.xml shadow-4.1.5.1/man/newuidmap.1.xml
@@ -95,21 +95,6 @@
     </variablelist>
   </refsect1>
 
-  <refsect1 id='note'>
-    <title>NOTE</title>
-    <para>
-      The only restriction placed on the login shell is that the command
-      name must be listed in <filename>/etc/shells</filename>, unless the
-      invoker is the superuser, and then any value may be added. An
-      account with a restricted login shell may not change her login shell.
-      For this reason, placing <filename>/bin/rsh</filename> in
-      <filename>/etc/shells</filename> is discouraged since accidentally
-      changing to a restricted shell would prevent the user from ever
-      changing her login shell back to its original value.
-    </para>
-  </refsect1>
-
-
   <refsect1 id='files'>
     <title>FILES</title>
     <variablelist>
man/subgid.5.xml +6 -6
diff -ur shadow-4.1.5.1.orig/man/subgid.5.xml shadow-4.1.5.1/man/subgid.5.xml
@@ -48,7 +48,7 @@
     <title>DESCRIPTION</title>
     <para>
       Each line in <filename>/etc/subgid</filename> contains
-      a user id and a range of suboridinate user ids that user
+      a user name and a range of suboridinate group ids that user
       is allowed to use.
 
       This is specified with three fields delimited by colons
@@ -60,21 +60,21 @@
 	<para>login name</para>
       </listitem>
       <listitem>
-	<para>numerical subordinate user ID</para>
+	<para>numerical subordinate group ID</para>
       </listitem>
       <listitem>
-	<para>numerical subordinate user ID count</para>
+	<para>numerical subordinate group ID count</para>
       </listitem>
     </itemizedlist>
 
     <para>
-      This file specifies the group IDs to be that each user may use
-      with the <command>newgidmap</command> command that ordinary users can use to
+      This file specifies the group IDs that ordinary users may use
+      with the <command>newgidmap</command> command to
       configure gid mapping in a user namespace.
     </para>
 
     <para>
-      Multiple ranges may be specified per user ID.
+      Multiple ranges may be specified per user.
     </para>
 
   </refsect1>
man/subuid.5.xml +4 -4
diff -ur shadow-4.1.5.1.orig/man/subuid.5.xml shadow-4.1.5.1/man/subuid.5.xml
@@ -48,7 +48,7 @@
     <title>DESCRIPTION</title>
     <para>
       Each line in <filename>/etc/subuid</filename> contains
-      a user id and a range of suboridinate user ids that user
+      a user name and a range of suboridinate user ids that user
       is allowed to use.
 
       This is specified with three fields delimited by colons
@@ -68,13 +68,13 @@
     </itemizedlist>
 
     <para>
-      This file specifies the user IDs to be that each user may use
-      with the <command>newuidmap</command> command that ordinary users can use to
+      This file specifies the user IDs that ordinary users may use
+      with the <command>newuidmap</command> command to
       configure uid mapping in a user namespace.
     </para>
 
     <para>
-      Multiple ranges may be specified per user ID.
+      Multiple ranges may be specified per user.
     </para>
 
   </refsect1>
src/usermod.c +4 -4
diff -ur shadow-4.1.5.1.orig/src/usermod.c shadow-4.1.5.1/src/usermod.c
@@ -410,9 +410,9 @@
 	(void) fputs (_("  -u, --uid UID                 new UID for the user account\n"), usageout);
 	(void) fputs (_("  -U, --unlock                  unlock the user account\n"), usageout);
 	(void) fputs (_("  -v, --add-subuids FIRST-LAST  add range of subordinate uids\n"), usageout);
-	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remvoe range of subordinate uids\n"), usageout);
+	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remove range of subordinate uids\n"), usageout);
 	(void) fputs (_("  -w, --add-subgids FIRST-LAST  add range of subordinate gids\n"), usageout);
-	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remvoe range of subordinate gids\n"), usageout);
+	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remove range of subordinate gids\n"), usageout);
 #ifdef WITH_SELINUX
 	(void) fputs (_("  -Z, --selinux-user SEUSER     new SELinux user mapping for the user account\n"), usageout);
 #endif				/* WITH_SELINUX */
@@ -993,9 +993,9 @@
 		};
 		while ((c = getopt_long (argc, argv,
 #ifdef WITH_SELINUX
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:UZ:",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:Z:",
 #else				/* !WITH_SELINUX */
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:U",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:",
 #endif				/* !WITH_SELINUX */
 			                 long_options, NULL)) != -1) {
 			switch (c) {
#59 lore raw
In reply to a message not in the archive
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 13 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids
---

OK, now we're ready for this.

Eric, I have no skills to decide whether your patches can be included or not. My proposal is to go ahead and include them in the upcomign 4.2 release, that will be compiled and uploaded in Debian as soon as released, so that it gets extensive testing.

We now have an "upstream" git repository at

http://github.com/shadow-maint/shadow.git

Would you mind pushing your set of patches there?

That requires an account on github and include you in the project members (Serge Hallyn can do that).

I would prefer this over committing/pushing myself.

I really apologize for the too long delay working on this. We now need to revive shadow's development.

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20130728171451.GX5670-FvNwPcshoeM/MCprI7ZU+I/[email protected]

#61 lore raw
Quoted 53 lines · Christian PERRIER
Quoted 24 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those files.

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 13 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid
login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids
Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group ids.
useradd: Add support for subordinate user identifiers
Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate uids and gids
---

OK, now we're ready for this.

Eric, I have no skills to decide whether your patches can be included or not. My proposal is to go ahead and include them in the upcomign 4.2 release, that will be compiled and uploaded in Debian as soon as released, so that it gets extensive testing.

We now have an "upstream" git repository at

http://github.com/shadow-maint/shadow.git

Would you mind pushing your set of patches there?

Quoted 7 lines

That requires an account on github and include you in the project members (Serge Hallyn can do that).

I would prefer this over committing/pushing myself.

I really apologize for the too long delay working on this. We now need to revive shadow's development.

Understood.

At this point Serge has taken over stewardship of those patches and has a version with all of the known bug fixes applied that has been reviewed and included in Ubuntu. So I expect the most responsible way is to just pull the branch with those changes that is in Ubuntu.

Serge does that sound right?

Eric

A message that is not in the archive (sent off-list or not yet received): 87r4eilg6y.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#63 lore raw
Quoted 76 lines · ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected] wrote:
Quoted 10 lines · Christian PERRIER
Quoted 9 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

Quoted 5 lines
Quoted 5 lines

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those

files.

Quoted 10 lines
Quoted 10 lines

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 7 lines
Quoted 7 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group

ids.

Quoted 5 lines
Quoted 5 lines

useradd: Add support for subordinate user identifiers Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate

uids and gids

Quoted 14 lines
Quoted 1 line

---

OK, now we're ready for this.

Eric, I have no skills to decide whether your patches can be included or not. My proposal is to go ahead and include them in the upcomign 4.2 release, that will be compiled and uploaded in Debian as soon as released, so that it gets extensive testing.

We now have an "upstream" git repository at

http://github.com/shadow-maint/shadow.git

Quoted 8 lines

Would you mind pushing your set of patches there?

That requires an account on github and include you in the project members (Serge Hallyn can do that).

I would prefer this over committing/pushing myself.

I really apologize for the too long delay working on this. We now

need

Quoted 1 line

to revive shadow's development.

Understood.

At this point Serge has taken over stewardship of those patches and has a version with all of the known bug fixes applied that has been reviewed
and included in Ubuntu. So I expect the most responsible way is to just
pull the branch with those changes that is in Ubuntu.

Quoted 3 lines

Serge does that sound right?

Eric

Sorry think I just sent a private reply. To repeat, I can do this when I'm back at a kbd, maybe Friday, definately Monday.

Thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 11218395-363e-46cd-b7a1-4488079a4986-2ueSQiBKiTY7tOexoI0I+QC/[email protected]

#66 lore raw
Quoted 78 lines · Serge Hallyn
Quoted 76 lines · ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected] wrote:
Quoted 10 lines · Christian PERRIER
Quoted 9 lines · Eric W. Biederman

The kernel support for user namespaces allows ordinary users to use multiple uids and gids if they can get a trusted program to tell the kernel the set of subordinate uids and gids they are allowed to use.

This is my work to make that trusted program.
Two new files are added /etc/subuid /etc/subgid that specify ranges of uids and gids that users may uses.

Quoted 5 lines
Quoted 5 lines

useradd, and newusers are modifed to add users to those files.

userdel is modeifed to remove users from those files.

usermod is modified to give manual control of what goes in those

files.

Quoted 10 lines
Quoted 10 lines

newuidmap and newgidmap read the new files and update /proc/[pid]/uid_map and /proc/[pid]/gid_map respectively as requested by their command line parameters and as allowed by the /etc/subuid and /etc/subgid.

The following patches are against the current developent trunk of pkg-shadow svn rev 3745. With minor tweaking of man/Makefile.am these patches also apply to shadow 4.1.5.

Quoted 7 lines
Quoted 7 lines

Eric W. Biederman (11):
Documentation for /etc/subuid and /etc/subgid login.defs.5: Document the new variables in login.defs Implement commonio_append.
Add backend support for suboridnate uids and gids Implement find_new_sub_uids find_new_sub_gids
userdel: Add support for removing subordinate user and group

ids.

Quoted 5 lines
Quoted 5 lines

useradd: Add support for subordinate user identifiers Add support for detecting busy subordinate user ids
usermod: Add support for subordinate uids and gids.
newusers: Add support for assiging subordinate uids and gids. newuidmap,newgidmap: New suid helpers for using subordinate

uids and gids

Quoted 14 lines
Quoted 1 line

---

OK, now we're ready for this.

Eric, I have no skills to decide whether your patches can be included or not. My proposal is to go ahead and include them in the upcomign 4.2 release, that will be compiled and uploaded in Debian as soon as released, so that it gets extensive testing.

We now have an "upstream" git repository at

http://github.com/shadow-maint/shadow.git

Quoted 8 lines

Would you mind pushing your set of patches there?

That requires an account on github and include you in the project members (Serge Hallyn can do that).

I would prefer this over committing/pushing myself.

I really apologize for the too long delay working on this. We now

need

Quoted 1 line

to revive shadow's development.

Understood.

At this point Serge has taken over stewardship of those patches and has a version with all of the known bug fixes applied that has been reviewed
and included in Ubuntu. So I expect the most responsible way is to just
pull the branch with those changes that is in Ubuntu.

Quoted 5 lines
Quoted 3 lines

Serge does that sound right?

Eric

Sorry think I just sent a private reply. To repeat, I can do this when I'm back at a kbd, maybe Friday, definately Monday.

I rebased and pushed the patchset yesterday.

thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): [email protected]

#68 lore raw

Hi,

Quoted 2 lines · On Tue, Aug 06, 2013 at 02:54:03PM +0000…

I rebased and pushed the patchset yesterday.

Thanks (this removes me the burden of finding how to merge a branch).

I started to review and read about user namespaces (thanks to Michael!) o
Here are some questions/remarks:

1] It would be nice to be able to disable the new tools / manpages /
options on some systems (i.e. they only make sense on Linux, with recent Kernel). It would be OK for me if it is enabled by default. Do you agree?

2] I think we can assume that UID/GIDs are (at least) 32 bits, right?
(or at least when the feature is not disabled - see point 1)

3] For PAM versions of shadow, I think it would be nice to authenticate
the caller of newuidmap / newgidmap. Do you agree?

4] What happens when newuidmap / newgidmap are used within a new namespace?
Is there a risk to do something wrong by recursively creating shells in cascaded namespace and using these tools? (I don't think so) Is there a way to enforce that it would only be used in the top-most namespace? (Maybe it's not needed to forbid it)
It would be worth to have at least a paragraph about it in the manpages. (i.e. IDs in /etc/sub*id are according to the caller's namespace)

5] It would be worth documenting on which processes newuidmap / newgidmap
can act.
With newuidmap, an user can set the UID mapping for any process with restrictions on the process and on the requested ranges. We could also mention that the kernel enforce also restriction (5 lines at most; can only be written once)

5.1] The restriction on the caller or processes seems to be the followings:

    if ((getuid() != pw->pw_uid) ||   // not needed (already enforced
                                      // by get_my_pwent()) - but does not
                                      // harm.
        (getgid() != pw->pw_gid) ||   // I don't understand this
                                      // restriction. If I execute a
                                      // shell with another GID (e.g. using
                                      // newgrp) why should it be denied?
        (pw->pw_uid != st.st_uid) ||
        (pw->pw_gid != st.st_gid)) {  // Is it also needed? What would be
                                      // the problem

6] Documenting the APIs would be useful. Especially the ones in
lib/subordinateio.c (after 5 minutes, I get lost with the is_range_free, range_exists, find_range, have_range, ...)

7] I'm not sure about the handling of limits of ranges.
* First, I'm not sure what is intended. (e.g. is SUB_GID_MAX included in the range)
* In libmisc/find_new_sub_uids.c: Is it intended to forbid min == max (I'm not claiming this is very useful, but if SUB_GID_MIN=SUB_GID_MAX and SUB_GID_COUNT is set to 1, this should be valid to set a single mapping for one UID)
* In find_free_range(), min==max is also forbidden.
* In find_free_range(), the check (high > low) forbids to reuse a hole of just 1 UID starting at min
...

8] Is there a need to check somewhere that that UID_MAX < SUB_UID_MIN in
login.defs? Or is it valid to have overlaps?

9] Add manpage NOTE that newuidmap / newgidmap can be used only once on a
given process?

10] useradd adds sub*id when the /etc/sub*id files are present, right?
Is it also correct/intended for system users?
I think there should be options to enable or disable the feature. (I would have chosen the first solution, but I can also understand that we consider that the presence of the files is a request to use the feature by default)

11] Ranges will be defined for new users. Do you think there would be a
need for another tool to initialize a system for this feature? (similar to pwconv)

12] There could be additions in pwck:
* check that users in /etc/sub*id exist
* check overlaps?
* check overlaps with UIDs?
* ...

I do not think that they are blocking points. (i.e. there could be a release without a solution)

Best Regards,

Signature · 1 line
Nekral

A message that is not in the archive (sent off-list or not yet received): 20130806225332.GA14109-pDMkYksm/[email protected]

#70 lore raw
Quoted 8 lines · Nicolas François

Hi,

Quoted 2 lines · On Tue, Aug 06, 2013 at 02:54:03PM +0000…

I rebased and pushed the patchset yesterday.

Thanks (this removes me the burden of finding how to merge a branch).

Quoted 8 lines

I started to review and read about user namespaces (thanks to Michael!) o
Here are some questions/remarks:

1] It would be nice to be able to disable the new tools / manpages /
options on some systems (i.e. they only make sense on Linux, with recent Kernel). It would be OK for me if it is enabled by default. Do you agree?

Makes sense.

Quoted 2 lines

2] I think we can assume that UID/GIDs are (at least) 32 bits, right?
(or at least when the feature is not disabled - see point 1)

Hm, what about systems compiled with CONFIG_UID16?

Quoted 2 lines

3] For PAM versions of shadow, I think it would be nice to authenticate
the caller of newuidmap / newgidmap. Do you agree?

I'm not sure what you mean. Do you mean that pam_something.so could otherwise call newuidmap/newgidmap before being authenticated?

Quoted 5 lines

4] What happens when newuidmap / newgidmap are used within a new namespace?
Is there a risk to do something wrong by recursively creating shells in cascaded namespace and using these tools? (I don't think so) Is there a way to enforce that it would only be used in the top-most namespace? (Maybe it's not needed to forbid it)

The kernel user namespace implementation is designed to support nesting.

Actually I'm not 100% sure, but I think that setuid-root is currently only honored in the initial user namespace. If that were changed, then calling newuidmap from a child user namespace would only give the caller privilege over his child user namespace. Therefore he could only delegate userids from his own (non-init) namespace to his own descendent user namespaces.

calling newuidmap on a pid in the initial user namespace results in -EPERM (as otherwise an unprivileged user could remap privileged tasks to his permitted subuids :)

Quoted 3 lines

It would be worth to have at least a paragraph about it in the manpages. (i.e. IDs in /etc/sub*id are according to the caller's namespace)

Agreed.

Quoted 6 lines

5] It would be worth documenting on which processes newuidmap / newgidmap
can act.
With newuidmap, an user can set the UID mapping for any process with restrictions on the process and on the requested ranges. We could also mention that the kernel enforce also restriction (5 lines at most; can only be written once)

Note that there are under-development very detailed manpages for namespaces(7) and user_namespaces(7). I agree the newuidmap manpage should document what it can act, a one or two line addition to DESCRIPTION. Then it should simply refer to the other manpages.

Quoted 8 lines

5.1] The restriction on the caller or processes seems to be the followings:

    if ((getuid() != pw->pw_uid) ||   // not needed (already enforced
                                      // by get_my_pwent()) - but does not
                                      // harm.
        (getgid() != pw->pw_gid) ||   // I don't understand this
                                      // restriction. If I execute a
                                      // shell with another GID (e.g. using
                                      // newgrp) why should it be denied?

I'm not sure. I don't know whether Eric was following conventions elsewhere in the code, or had specific attacks in mind.

Quoted 3 lines
        (pw->pw_uid != st.st_uid) ||
        (pw->pw_gid != st.st_gid)) {  // Is it also needed? What would be
                                      // the problem

IIUC uid 1000, authorized to use subuids 100000-110000, could otherwise use newuidmap to remap a namespace owned by uid 1001.

Quoted 3 lines

6] Documenting the APIs would be useful. Especially the ones in
lib/subordinateio.c (after 5 minutes, I get lost with the is_range_free, range_exists, find_range, have_range, ...)

In-line (above each function) or elsewhere?

Quoted 7 lines

7] I'm not sure about the handling of limits of ranges.
* First, I'm not sure what is intended. (e.g. is SUB_GID_MAX included in the range)
* In libmisc/find_new_sub_uids.c: Is it intended to forbid min == max (I'm not claiming this is very useful, but if SUB_GID_MIN=SUB_GID_MAX and SUB_GID_COUNT is set to 1, this should be valid to set a single mapping for one UID)

Agreed that sounds like it should be allowed.

Quoted 7 lines

* In find_free_range(), min==max is also forbidden.
* In find_free_range(), the check (high > low) forbids to reuse a hole of just 1 UID starting at min
...

8] Is there a need to check somewhere that that UID_MAX < SUB_UID_MIN in
login.defs? Or is it valid to have overlaps?

That sounds like a dangerous condition to me, and I had assumed it was already checked somewhere.

Quoted 2 lines

9] Add manpage NOTE that newuidmap / newgidmap can be used only once on a
given process?

Agreed.

Quoted 6 lines

10] useradd adds sub*id when the /etc/sub*id files are present, right?
Is it also correct/intended for system users?
I think there should be options to enable or disable the feature. (I would have chosen the first solution, but I can also understand that we consider that the presence of the files is a request to use the feature by default)

Not sure on both of those. Actually given the (artificial) definition of a system user in useradd(8), it sounds to me like subuids could be useful for system users on a system where tasks get started in a userns by boot scripts using a system userid.

Quoted 3 lines

11] Ranges will be defined for new users. Do you think there would be a
need for another tool to initialize a system for this feature? (similar to pwconv)

Do you mean to add ranges for existing users?

Quoted 3 lines

12] There could be additions in pwck:
* check that users in /etc/sub*id exist
* check overlaps?

Overlaps may actually be a feature in some cases, as a way to allow users to share data.

Quoted 5 lines

* check overlaps with UIDs?
* ...

I do not think that they are blocking points. (i.e. there could be a release without a solution)

I'll wait for comments or ("no time for a comment") on any of these from Eric, then look at addressing those that I can.

thanks,
-serge

#71 lore raw
Quoted 23 lines · Serge Hallyn
Quoted 8 lines · Nicolas François

Hi,

Quoted 2 lines · On Tue, Aug 06, 2013 at 02:54:03PM +0000, [email protected] wrote:

I rebased and pushed the patchset yesterday.

Thanks (this removes me the burden of finding how to merge a branch).

Quoted 8 lines

I started to review and read about user namespaces (thanks to Michael!) o
Here are some questions/remarks:

1] It would be nice to be able to disable the new tools / manpages /
options on some systems (i.e. they only make sense on Linux, with recent Kernel). It would be OK for me if it is enabled by default. Do you agree?

Makes sense.

Quoted 2 lines

2] I think we can assume that UID/GIDs are (at least) 32 bits, right?
(or at least when the feature is not disabled - see point 1)

Quoted 7 lines

Hm, what about systems compiled with CONFIG_UID16?

Quoted 2 lines

3] For PAM versions of shadow, I think it would be nice to authenticate
the caller of newuidmap / newgidmap. Do you agree?

I'm not sure what you mean. Do you mean that pam_something.so could otherwise call newuidmap/newgidmap before being authenticated?

It sounded to me like he was suggesting newuidmap/newgidmap should make an appropriate pam call. I am not familiar with that part of pam so I don't know what would be an appropriate pam call.

The closest analog I can think of is newgrp, and to my knowledge newgrp does not cal into pam.

Quoted 15 lines
Quoted 5 lines

4] What happens when newuidmap / newgidmap are used within a new namespace?
Is there a risk to do something wrong by recursively creating shells in cascaded namespace and using these tools? (I don't think so) Is there a way to enforce that it would only be used in the top-most namespace? (Maybe it's not needed to forbid it)

The kernel user namespace implementation is designed to support nesting.

Actually I'm not 100% sure, but I think that setuid-root is currently only honored in the initial user namespace. If that were changed, then calling newuidmap from a child user namespace would only give the caller privilege over his child user namespace. Therefore he could only delegate userids from his own (non-init) namespace to his own descendent user namespaces.

Quoted 3 lines

calling newuidmap on a pid in the initial user namespace results in -EPERM (as otherwise an unprivileged user could remap privileged tasks to his permitted subuids :)

There are slightly funny things with setuid and setcap.

The short version however is that once in a user namespace you will never have more permissions than your the root user in your user namespace.

So from the outside of the user namespace you can't do anything bad even with nesting. Aka there is no defined means to escapte privileges from inside a user namespace to anything privileged outside a user namespace.

From inside the user namespace you can only follow /etc/subuid or /etc/subgid (when setting up a nested user namespace). And if that is a concern the root user of a user namespace can just create a new mount namespace and mount over the top of them.

So I don't see anything to concern about.

Quoted 13 lines
Quoted 3 lines

It would be worth to have at least a paragraph about it in the manpages. (i.e. IDs in /etc/sub*id are according to the caller's namespace)

Agreed.

Quoted 6 lines

5] It would be worth documenting on which processes newuidmap / newgidmap
can act.
With newuidmap, an user can set the UID mapping for any process with restrictions on the process and on the requested ranges. We could also mention that the kernel enforce also restriction (5 lines at most; can only be written once)

Quoted 4 lines

Note that there are under-development very detailed manpages for namespaces(7) and user_namespaces(7). I agree the newuidmap manpage should document what it can act, a one or two line addition to DESCRIPTION. Then it should simply refer to the other manpages.

Yeah. I should follow up and see what has happend with all of that.

Quoted 11 lines
Quoted 8 lines

5.1] The restriction on the caller or processes seems to be the followings:

    if ((getuid() != pw->pw_uid) ||   // not needed (already enforced
                                      // by get_my_pwent()) - but does not
                                      // harm.
        (getgid() != pw->pw_gid) ||   // I don't understand this
                                      // restriction. If I execute a
                                      // shell with another GID (e.g. using
                                      // newgrp) why should it be denied?

I'm not sure. I don't know whether Eric was following conventions elsewhere in the code, or had specific attacks in mind.

I don't have the context handy, but I think that was general paranoia. On the lines of if something strange is going on don't allow it.

Quoted 21 lines
Quoted 3 lines
        (pw->pw_uid != st.st_uid) ||
        (pw->pw_gid != st.st_gid)) {  // Is it also needed? What would be
                                      // the problem

IIUC uid 1000, authorized to use subuids 100000-110000, could otherwise use newuidmap to remap a namespace owned by uid 1001.

Quoted 3 lines

6] Documenting the APIs would be useful. Especially the ones in
lib/subordinateio.c (after 5 minutes, I get lost with the is_range_free, range_exists, find_range, have_range, ...)

In-line (above each function) or elsewhere?

Quoted 7 lines

7] I'm not sure about the handling of limits of ranges.
* First, I'm not sure what is intended. (e.g. is SUB_GID_MAX included in the range)
* In libmisc/find_new_sub_uids.c: Is it intended to forbid min == max (I'm not claiming this is very useful, but if SUB_GID_MIN=SUB_GID_MAX and SUB_GID_COUNT is set to 1, this should be valid to set a single mapping for one UID)

Quoted 6 lines

Agreed that sounds like it should be allowed.

Quoted 4 lines

* In find_free_range(), min==max is also forbidden.
* In find_free_range(), the check (high > low) forbids to reuse a hole of just 1 UID starting at min
...

Scratching my head. When ranges are bounded by two numbers it gets weird. All I remember for certain is I deliberately choose the file format of uid, length so that it would be hard to make off by one mistakes in the files.

Quoted 5 lines
Quoted 2 lines

8] Is there a need to check somewhere that that UID_MAX < SUB_UID_MIN in
login.defs? Or is it valid to have overlaps?

That sounds like a dangerous condition to me, and I had assumed it was already checked somewhere.

These were fairly arbitrary defines, with no sanity checks and I didn't add any. I just had sensible defaults. As that tends to be the unix way. You can have rope to shoot yourself if you really want it.

I really have no opinion on the need for sanity checks one way or another.

Quoted 12 lines
Quoted 2 lines

9] Add manpage NOTE that newuidmap / newgidmap can be used only once on a
given process?

Agreed.

Quoted 6 lines

10] useradd adds sub*id when the /etc/sub*id files are present, right?
Is it also correct/intended for system users?
I think there should be options to enable or disable the feature. (I would have chosen the first solution, but I can also understand that we consider that the presence of the files is a request to use the feature by default)

Quoted 4 lines

Not sure on both of those. Actually given the (artificial) definition of a system user in useradd(8), it sounds to me like subuids could be useful for system users on a system where tasks get started in a userns by boot scripts using a system userid.

I believe the question was. If /etc/subuid does not exist and we add a new user do we want to add an entry in /etc/subuid. Similarly for /etc/subgid.

I followed convention in shadow and figured that if /etc/subuid and /etc/subgid are not present we don't want to add subuids and subgids.

I don't expect we want to add subuids and subgids for system users by default but as Serge says there may be instances where we want to add them manually.

Quoted 5 lines
Quoted 3 lines

11] Ranges will be defined for new users. Do you think there would be a
need for another tool to initialize a system for this feature? (similar to pwconv)

Do you mean to add ranges for existing users?

I believe it was either useradd or usermod that I modified so that I could add subuids and subgids for an existing user manually.

As for upgrades I don't know. Serge with your distro hat on do you see a use for automatically giving user on a system subids and subgids when they upgrade versions of shadow?

Quoted 6 lines
Quoted 3 lines

12] There could be additions in pwck:
* check that users in /etc/sub*id exist
* check overlaps?

Overlaps may actually be a feature in some cases, as a way to allow users to share data.

I know I deliberately allowed (but would never create by default) multiple users having access to the same range of subordinate uids.

Shared read-only system images may benefit from such a practice.

In a similar vein there may be uses of being able to log in to a system with what is ordinarily a subordinate uid.

The distinction is a distinction of use not a distinction made in the kernel.

That said having overlaps is usually a mistake and a warning in the manual process may be called for in case someone simply didn't see something.

Quoted 8 lines
Quoted 5 lines

* check overlaps with UIDs?
* ...

I do not think that they are blocking points. (i.e. there could be a release without a solution)

I'll wait for comments or ("no time for a comment") on any of these from Eric, then look at addressing those that I can.

I hope that helps put some context to things. I am totally not looking at the userspace issues around newuidmap and newgidmap right now, and I appreciate others being able to look at it and make things better.

Eric

_______________________________________________
Containers mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 87eha5peb8.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#73 lore raw

Hello,

Quoted 17 lines · On Wed, Aug 07, 2013 at 11:04:59AM -0700…
Quoted 16 lines · Serge Hallyn
Quoted 14 lines · Nicolas François
Quoted 2 lines · On Tue, Aug 06, 2013 at 02:54:03PM +0000…

I rebased and pushed the patchset yesterday.

Thanks (this removes me the burden of finding how to merge a branch).

I started to review and read about user namespaces (thanks to Michael!) o
Here are some questions/remarks:

1] It would be nice to be able to disable the new tools / manpages /
options on some systems (i.e. they only make sense on Linux, with recent Kernel). It would be OK for me if it is enabled by default. Do you agree?

Quoted 6 lines
Quoted 6 lines

Makes sense.

Quoted 2 lines

2] I think we can assume that UID/GIDs are (at least) 32 bits, right?
(or at least when the feature is not disabled - see point 1)

Hm, what about systems compiled with CONFIG_UID16?

At config time, we can check that uid_t and gid_t allow for 32 bit IDs.

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

3] For PAM versions of shadow, I think it would be nice to authenticate
the caller of newuidmap / newgidmap. Do you agree?

I'm not sure what you mean. Do you mean that pam_something.so could otherwise call newuidmap/newgidmap before being authenticated?

Quoted 6 lines

It sounded to me like he was suggesting newuidmap/newgidmap should make an appropriate pam call. I am not familiar with that part of pam so I don't know what would be an appropriate pam call.

The closest analog I can think of is newgrp, and to my knowledge newgrp does not cal into pam.

I was thinking about something like in chsh. But the analogy with newgrp is better. So lets forget about it.

Quoted 29 lines
Quoted 15 lines
Quoted 5 lines

4] What happens when newuidmap / newgidmap are used within a new namespace?
Is there a risk to do something wrong by recursively creating shells in cascaded namespace and using these tools? (I don't think so) Is there a way to enforce that it would only be used in the top-most namespace? (Maybe it's not needed to forbid it)

The kernel user namespace implementation is designed to support nesting.

Actually I'm not 100% sure, but I think that setuid-root is currently only honored in the initial user namespace. If that were changed, then calling newuidmap from a child user namespace would only give the caller privilege over his child user namespace. Therefore he could only delegate userids from his own (non-init) namespace to his own descendent user namespaces.

Quoted 3 lines

calling newuidmap on a pid in the initial user namespace results in -EPERM (as otherwise an unprivileged user could remap privileged tasks to his permitted subuids :)

There are slightly funny things with setuid and setcap.

The short version however is that once in a user namespace you will never have more permissions than your the root user in your user namespace.

So from the outside of the user namespace you can't do anything bad even with nesting. Aka there is no defined means to escapte privileges from inside a user namespace to anything privileged outside a user namespace.

Quoted 6 lines

From inside the user namespace you can only follow /etc/subuid or /etc/subgid (when setting up a nested user namespace). And if that is a concern the root user of a user namespace can just create a new mount namespace and mount over the top of them.

So I don't see anything to concern about.

OK.

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

It would be worth to have at least a paragraph about it in the manpages. (i.e. IDs in /etc/sub*id are according to the caller's namespace)

Agreed.

Lets ignore it currently. It will be users/admins' responsibility to setup the right ranges usable in the caller environment.

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

5] It would be worth documenting on which processes newuidmap / newgidmap
can act.
With newuidmap, an user can set the UID mapping for any process with restrictions on the process and on the requested ranges. We could also mention that the kernel enforce also restriction (5 lines at most; can only be written once)

Quoted 6 lines
Quoted 4 lines

Note that there are under-development very detailed manpages for namespaces(7) and user_namespaces(7). I agree the newuidmap manpage should document what it can act, a one or two line addition to DESCRIPTION. Then it should simply refer to the other manpages.

Yeah. I should follow up and see what has happend with all of that.

I will add references to these manpages.

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

5.1] The restriction on the caller or processes seems to be the followings:

    if ((getuid() != pw->pw_uid) ||   // not needed (already enforced
                                      // by get_my_pwent()) - but does not
                                      // harm.
        (getgid() != pw->pw_gid) ||   // I don't understand this
                                      // restriction. If I execute a
                                      // shell with another GID (e.g. using
                                      // newgrp) why should it be denied?
Quoted 5 lines
Quoted 2 lines

I'm not sure. I don't know whether Eric was following conventions elsewhere in the code, or had specific attacks in mind.

I don't have the context handy, but I think that was general paranoia. On the lines of if something strange is going on don't allow it.

This test ("getgid() != pw->pw_gid") can be bypassed by executing newuidmap in another shell, with the right gid.
I propose to remove it.

Quoted 1 line
Quoted 1 line
Quoted 1 line
        (pw->pw_uid != st.st_uid) ||

That part is needed.
Is there also the need to check that the process cannot change its effective UID?

Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
        (pw->pw_gid != st.st_gid)) {  // Is it also needed? What would be
                                      // the problem

IIUC uid 1000, authorized to use subuids 100000-110000, could otherwise use newuidmap to remap a namespace owned by uid 1001.

OK for the test on UID, but I don't get the point for checking the GID. Note that we can also put this restriction and wait for a user request.

Also regarding this block, the permission of /proc/<pid> are checked. I've read that there is the concept of a namespace owner (uid/gid of /proc/<pid>/uid_map. Is it also something to be checked?

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

6] Documenting the APIs would be useful. Especially the ones in
lib/subordinateio.c (after 5 minutes, I get lost with the is_range_free, range_exists, find_range, have_range, ...)

In-line (above each function) or elsewhere?

A small paragraph above each function would be perfect.

Quoted 15 lines
Quoted 8 lines
Quoted 7 lines

7] I'm not sure about the handling of limits of ranges.
* First, I'm not sure what is intended. (e.g. is SUB_GID_MAX included in the range)
* In libmisc/find_new_sub_uids.c: Is it intended to forbid min == max (I'm not claiming this is very useful, but if SUB_GID_MIN=SUB_GID_MAX and SUB_GID_COUNT is set to 1, this should be valid to set a single mapping for one UID)

Quoted 6 lines

Agreed that sounds like it should be allowed.

Quoted 4 lines

* In find_free_range(), min==max is also forbidden.
* In find_free_range(), the check (high > low) forbids to reuse a hole of just 1 UID starting at min
...

Quoted 4 lines

Scratching my head. When ranges are bounded by two numbers it gets weird. All I remember for certain is I deliberately choose the file format of uid, length so that it would be hard to make off by one mistakes in the files.

I will create tests for the boundary conditions.

Quoted 11 lines
Quoted 5 lines
Quoted 2 lines

8] Is there a need to check somewhere that that UID_MAX < SUB_UID_MIN in
login.defs? Or is it valid to have overlaps?

That sounds like a dangerous condition to me, and I had assumed it was already checked somewhere.

These were fairly arbitrary defines, with no sanity checks and I didn't add any. I just had sensible defaults. As that tends to be the unix way. You can have rope to shoot yourself if you really want it.

I really have no opinion on the need for sanity checks one way or another.

I can add a warning in pwck (can't find of a better place (useradd?)).

Quoted 21 lines
Quoted 12 lines
Quoted 2 lines

9] Add manpage NOTE that newuidmap / newgidmap can be used only once on a
given process?

Agreed.

Quoted 6 lines

10] useradd adds sub*id when the /etc/sub*id files are present, right?
Is it also correct/intended for system users?
I think there should be options to enable or disable the feature. (I would have chosen the first solution, but I can also understand that we consider that the presence of the files is a request to use the feature by default)

Quoted 4 lines

Not sure on both of those. Actually given the (artificial) definition of a system user in useradd(8), it sounds to me like subuids could be useful for system users on a system where tasks get started in a userns by boot scripts using a system userid.

I believe the question was. If /etc/subuid does not exist and we add a new user do we want to add an entry in /etc/subuid. Similarly for /etc/subgid.

Quoted 6 lines

I followed convention in shadow and figured that if /etc/subuid and /etc/subgid are not present we don't want to add subuids and subgids.

I don't expect we want to add subuids and subgids for system users by default but as Serge says there may be instances where we want to add them manually.

OK. That would change useradd (disabling setting sub*id when rflg is set). We could add options to useradd to change this default, or just let the user use usermod.

Quoted 28 lines
Quoted 5 lines
Quoted 3 lines

11] Ranges will be defined for new users. Do you think there would be a
need for another tool to initialize a system for this feature? (similar to pwconv)

Do you mean to add ranges for existing users?

I believe it was either useradd or usermod that I modified so that I could add subuids and subgids for an existing user manually.

As for upgrades I don't know. Serge with your distro hat on do you see a use for automatically giving user on a system subids and subgids when they upgrade versions of shadow?

Quoted 6 lines
Quoted 3 lines

12] There could be additions in pwck:
* check that users in /etc/sub*id exist
* check overlaps?

Overlaps may actually be a feature in some cases, as a way to allow users to share data.

I know I deliberately allowed (but would never create by default) multiple users having access to the same range of subordinate uids.

Shared read-only system images may benefit from such a practice.

In a similar vein there may be uses of being able to log in to a system with what is ordinarily a subordinate uid.

Quoted 6 lines

The distinction is a distinction of use not a distinction made in the kernel.

That said having overlaps is usually a mistake and a warning in the manual process may be called for in case someone simply didn't see something.

OK.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

* check overlaps with UIDs?
* ...

I do not think that they are blocking points. (i.e. there could be a release without a solution)

Quoted 6 lines
Quoted 2 lines

I'll wait for comments or ("no time for a comment") on any of these from Eric, then look at addressing those that I can.

I hope that helps put some context to things. I am totally not looking at the userspace issues around newuidmap and newgidmap right now, and I appreciate others being able to look at it and make things better.

Thanks for your feedback.

Signature · 1 line
Nekral

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 20130303153726.GA14737-anj0Drq5vpzx6HRWoRZK3AC/[email protected]

#76 lore raw
Quoted 6 lines · Serge E. Hallyn

Hi Nekral,

This isn't a great urgency, but the final userns kernel support needed for this patchset to be useful should land in 3.9, so it isn't that far off. Do you have any objections to/feedback on this set? Would this be possible to get into Debian?

In Debian jessie, maybe, if someone takes shadow over....or if Nicolas finds some motivation and time to work again on shadow.

In Debian wheezy, definitely no....but this is of no use, as Debian wheezy will certainly not use a 3.9 kernel.

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 20130304055654.GE2629-FvNwPcshoeM/MCprI7ZU+I/[email protected]

#79 lore raw
Quoted 11 lines · Christian PERRIER
Quoted 6 lines · Serge E. Hallyn

Hi Nekral,

This isn't a great urgency, but the final userns kernel support needed for this patchset to be useful should land in 3.9, so it isn't that far off. Do you have any objections to/feedback on this set? Would this be possible to get into Debian?

In Debian jessie, maybe, if someone takes shadow over....or if Nicolas finds some motivation and time to work again on shadow.

Is help mainly needed in bug resolution? I'll try to help out there (sent the trivial patch to fix one of the bugs, will email - having discussed in irc - thoughts on another).

Quoted 3 lines

In Debian wheezy, definitely no....but this is of no use, as Debian wheezy will certainly not use a 3.9 kernel.

Right, I was only looking forward, jessie would be great.

thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#82 lore raw

Quoting Dwight Engen ([email protected]):
...

Quoted 17 lines

diff -ur shadow-4.1.5.1.orig/src/usermod.c shadow-4.1.5.1/src/usermod.c

src/usermod.c +2 -2
@@ -410,9 +410,9 @@
 	(void) fputs (_("  -u, --uid UID                 new UID for the user account\n"), usageout);
 	(void) fputs (_("  -U, --unlock                  unlock the user account\n"), usageout);
 	(void) fputs (_("  -v, --add-subuids FIRST-LAST  add range of subordinate uids\n"), usageout);
-	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remvoe range of subordinate uids\n"), usageout);
+	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remove range of subordinate uids\n"), usageout);
 	(void) fputs (_("  -w, --add-subgids FIRST-LAST  add range of subordinate gids\n"), usageout);
-	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remvoe range of subordinate gids\n"), usageout);
+	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remove range of subordinate gids\n"), usageout);
 #ifdef WITH_SELINUX
 	(void) fputs (_("  -Z, --selinux-user SEUSER     new SELinux user mapping for the user account\n"), usageout);
 #endif				/* WITH_SELINUX */
@@ -993,9 +993,9 @@
 		};

src/usermod.c line 1001

Quoted 10 lines
src/usermod.c +2 -2
 		while ((c = getopt_long (argc, argv,
 #ifdef WITH_SELINUX
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:UZ:",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:Z:",
 #else				/* !WITH_SELINUX */
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:U",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:",
 #endif				/* !WITH_SELINUX */
 			                 long_options, NULL)) != -1) {
 			switch (c) {

This hunk I've already emailed to Eric :) Note there is also a "break;" needed around line 1141.

Great to see more people testing this. Hopefully it will help to ease the mind of the maintainers in considering merging this.

thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): 20130307215457.GB9348-anj0Drq5vpzx6HRWoRZK3AC/[email protected]

#84 lore raw
Quoted 33 lines · Serge E. Hallyn

Quoting Dwight Engen ([email protected]):
...

Quoted 17 lines

diff -ur shadow-4.1.5.1.orig/src/usermod.c shadow-4.1.5.1/src/usermod.c

src/usermod.c +2 -2
@@ -410,9 +410,9 @@
 	(void) fputs (_("  -u, --uid UID                 new UID for the user account\n"), usageout);
 	(void) fputs (_("  -U, --unlock                  unlock the user account\n"), usageout);
 	(void) fputs (_("  -v, --add-subuids FIRST-LAST  add range of subordinate uids\n"), usageout);
-	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remvoe range of subordinate uids\n"), usageout);
+	(void) fputs (_("  -V, --del-subuids FIRST-LAST  remove range of subordinate uids\n"), usageout);
 	(void) fputs (_("  -w, --add-subgids FIRST-LAST  add range of subordinate gids\n"), usageout);
-	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remvoe range of subordinate gids\n"), usageout);
+	(void) fputs (_("  -W, --del-subgids FIRST-LAST  remove range of subordinate gids\n"), usageout);
 #ifdef WITH_SELINUX
 	(void) fputs (_("  -Z, --selinux-user SEUSER     new SELinux user mapping for the user account\n"), usageout);
 #endif				/* WITH_SELINUX */
@@ -993,9 +993,9 @@
 		};

src/usermod.c line 1001

Quoted 10 lines
src/usermod.c +2 -2
 		while ((c = getopt_long (argc, argv,
 #ifdef WITH_SELINUX
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:UZ:",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:Z:",
 #else				/* !WITH_SELINUX */
-			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:U",
+			                 "ac:d:e:f:g:G:hl:Lmop:R:s:u:Uv:V:w:W:",
 #endif				/* !WITH_SELINUX */
 			                 long_options, NULL)) != -1) {
 			switch (c) {

This hunk I've already emailed to Eric :) Note there is also a "break;" needed around line 1141.

Quoted 2 lines

Great to see more people testing this. Hopefully it will help to ease the mind of the maintainers in considering merging this.

Yes. It looks like we are going to have to setup a public repository somewhere to collect all of the changes and bug fixes. Not that I expect too many but clearly there are a few needed.

Sigh I was hoping shadow had an active mainter, and creating a public feature branch would not have been necessary.

Eric

A message that is not in the archive (sent off-list or not yet received): 876212rf9b.fsf-aS9lmoZGLiVWk0Htik3J/[email protected]

#86 lore raw
Quoted 3 lines · Eric W. Biederman

Yes. It looks like we are going to have to setup a public repository somewhere to collect all of the changes and bug fixes. Not that I expect too many but clearly there are a few needed.

That exists.

Shadow upstream (and Debian package) is maintained in an SVN repository hosted on alioth.debian.org.

I can make it as easy as possible to people to contribute by opening commit access there.

We can also, if that helps in attracting contributors, convert the repo to git (I will need help there because the repo has a quite long history and we want to keep it). I know that, nowadays, svn often looks a bit scary (and, TBH, feature-limited) to many FLOSS contributors.

In short, nothing is hidden nor closed. Indeed, I think that shadow might benefit from more input, given that my friend Nicolas has limited time.

The only constraint is indeed giving French cheese names to releases. This is something I won't compromise with...:-). We invented this tradition when we took shadow over, back in 2005 (doh!) and that has to remain the French Touch in the software..:-)

I can act as a facilitator for all this. I definitely *don't have the skills to decide what contributions are "good" or not. Shadow being a sensitive piece of software, we might need to be careful at the beginning and include fixes and new features with care.

This is indeed where git and its great branching features might help a lot.

May I welcome interested people to join the Pkg-shadow-devel-XbBxUvOt3X2LieD7tvxI8l/[email protected] mailing list so that we reduce the CC list of this thread?

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20130308065019.GI5885-FvNwPcshoeM/MCprI7ZU+I/[email protected]

#88 lore raw
Quoted 11 lines · Christian PERRIER
Quoted 3 lines · Eric W. Biederman

Yes. It looks like we are going to have to setup a public repository somewhere to collect all of the changes and bug fixes. Not that I expect too many but clearly there are a few needed.

That exists.

Shadow upstream (and Debian package) is maintained in an SVN repository hosted on alioth.debian.org.

Quoted 6 lines

I can make it as easy as possible to people to contribute by opening commit access there.

We can also, if that helps in attracting contributors, convert the repo to git (I will need help there because the repo has a quite long history and we want to keep it). I know that, nowadays, svn often

Hi,

I've not used svn at all, but would it help if I take a shot at this conversion (maintaining full history) putting the result into github for a first run?

-serge

A message that is not in the archive (sent off-list or not yet received): 20130602134823.GB8004-anj0Drq5vpzx6HRWoRZK3AC/[email protected]

#90 lore raw
Quoted 5 lines · Serge E. Hallyn

Hi,

I've not used svn at all, but would it help if I take a shot at this conversion (maintaining full history) putting the result into github for a first run?

Why not?

At first glance, I'd rather keep the "official" copy on alioth.debian.org though I would indeed be delighted (as Debian maintainer) if someone else steps in to take "upstream" development over so that I can stick with Debian packaging.

_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 20130602140436.GK9152-FvNwPcshoeM/MCprI7ZU+I/[email protected]

#92 lore raw
Quoted 4 lines · Christian PERRIER
Quoted 2 lines · Serge E. Hallyn

Hi,

Quoted 8 lines
Quoted 3 lines

I've not used svn at all, but would it help if I take a shot at this conversion (maintaining full history) putting the result into github for a first run?

Why not?

At first glance, I'd rather keep the "official" copy on alioth.debian.org

Right, I intend for it to end up back there, github would just be for a trial run.

I'll give this a shot in the next three days.

thanks,
-serge