linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] nsfs: Add an ioctl() to return the namespace type
       [not found] <fdce894d-8385-b4b4-da3c-6282a7e4ecba@gmail.com>
@ 2016-12-19 14:38 ` Michael Kerrisk (man-pages)
  2016-12-19 14:38 ` [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns Michael Kerrisk (man-pages)
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-12-19 14:38 UTC (permalink / raw)
  To: Eric W. Biederman, Serge E. Hallyn
  Cc: mtk.manpages, linux-api, linux-kernel, linux-fsdevel,
	Andrey Vagin, James Bottomley, W. Trevor King, Alexander Viro,
	Jonathan Corbet

Linux 4.9 added two ioctl() operations that can be used to discover:

* the parental relationships for hierarchical namespaces (user and PID)
  [NS_GET_PARENT]
* the user namespaces that owns a specified non-user-namespace
  [NS_GET_USERNS]

For no good reason that I can glean, NS_GET_USERNS was made synonymous
with NS_GET_PARENT for user namespaces. It might have been better if
NS_GET_USERNS had returned an error if the supplied file descriptor
referred to a user namespace, since it suggests that the caller may be
confused. More particularly, if it had generated an error, then I wouldn't
need the new ioctl() operation proposed here. (On the other hand, what
I propose here may be more generally useful.)

I would like to write code that can answer the question: "what
capabilities does process X have in namespace Y"? (where Y is defined
by a file descriptor referring to one of the /proc/PID/ns/xxxx
files). The rules that determine the answer to this question are
described in the capabilities(7) manual page and involve working out
the chain of relationships between the user namespace of process X and
the namespace Y.

Namespace Y might be a user namespace (in which case my code would
just use Y) or a non-user namespace (in which case my code needs to
use NS_GET_USERNS to get the user namespace associated with Y). The
problem is that there is no way to tell the difference by looking at
the file descriptor (and if I try to use NS_GET_USERNS on a Y that is
a user namespace, I get the parent user namespace of Y, which is not
what I want).

This patch therefore adds a new ioctl(), NS_GET_NSTYPE, which, given
a file descriptor that refers to a user namespace, returns the
namespace type (one of the CLONE_NEW* constants).

Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
---
 fs/nsfs.c                 | 2 ++
 include/uapi/linux/nsfs.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 8c9fb29..5d53476 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -172,6 +172,8 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 		if (!ns->ops->get_parent)
 			return -EINVAL;
 		return open_related_ns(ns, ns->ops->get_parent);
+	case NS_GET_NSTYPE:
+		return ns->ops->type;
 	default:
 		return -ENOTTY;
 	}
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index 3af6172..2b48df1 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -9,5 +9,8 @@
 #define NS_GET_USERNS	_IO(NSIO, 0x1)
 /* Returns a file descriptor that refers to a parent namespace */
 #define NS_GET_PARENT	_IO(NSIO, 0x2)
+/* Returns the type of namespace (CLONE_NEW* value) referred to by
+   file descriptor */
+#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
 
 #endif /* __LINUX_NSFS_H */
-- 
2.5.5

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

* [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns
       [not found] <fdce894d-8385-b4b4-da3c-6282a7e4ecba@gmail.com>
  2016-12-19 14:38 ` [PATCH 1/2] nsfs: Add an ioctl() to return the namespace type Michael Kerrisk (man-pages)
@ 2016-12-19 14:38 ` Michael Kerrisk (man-pages)
  2016-12-21  3:13   ` Andrei Vagin
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-12-19 14:38 UTC (permalink / raw)
  To: Eric W. Biederman, Serge E. Hallyn
  Cc: mtk.manpages, linux-api, linux-kernel, linux-fsdevel,
	Andrey Vagin, James Bottomley, W. Trevor King, Alexander Viro,
	Jonathan Corbet

# Some open questions about this patch below.
#
One of the rules regarding capabilities is:

    A process that resides in the parent of the user namespace and
    whose effective user ID matches the owner of the namespace has
    all capabilities in the namespace.

Therefore, in order to write code that discovers whether process X has
capabilities in namespace Y, we need a way to find out who the creator
of a user namespace is. This patch adds an NS_GET_CREATOR_UID ioctl()
that returns the (munged) UID of the creator of the user namespace
referred to by the specified file descriptor.

If the supplied file descriptor does not refer to a user namespace,
the operation fails with the error EINVAL.

Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
---
 fs/nsfs.c                 | 6 ++++++
 include/uapi/linux/nsfs.h | 8 +++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

Open questions:

* Would it be preferabe to separate the logic for NS_GET_CREATOR_UID
  into a small helper function?
* Is this a correct use of container_of()? I did not immediately
  see another way to get to the user_namespace struct, but I
  may well have missed something.

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 5d53476..26f6d94 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -163,6 +163,7 @@ int open_related_ns(struct ns_common *ns,
 static long ns_ioctl(struct file *filp, unsigned int ioctl,
 			unsigned long arg)
 {
+	struct user_namespace *user_ns;
 	struct ns_common *ns = get_proc_ns(file_inode(filp));
 
 	switch (ioctl) {
@@ -174,6 +175,11 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
 		return open_related_ns(ns, ns->ops->get_parent);
 	case NS_GET_NSTYPE:
 		return ns->ops->type;
+	case NS_GET_CREATOR_UID:
+		if (ns->ops->type != CLONE_NEWUSER)
+			return -EINVAL;
+		user_ns = container_of(ns, struct user_namespace, ns);
+		return from_kuid_munged(current_user_ns(), user_ns->owner);
 	default:
 		return -ENOTTY;
 	}
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index 2b48df1..b3c6c78 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -6,11 +6,13 @@
 #define NSIO	0xb7
 
 /* Returns a file descriptor that refers to an owning user namespace */
-#define NS_GET_USERNS	_IO(NSIO, 0x1)
+#define NS_GET_USERNS		_IO(NSIO, 0x1)
 /* Returns a file descriptor that refers to a parent namespace */
-#define NS_GET_PARENT	_IO(NSIO, 0x2)
+#define NS_GET_PARENT		_IO(NSIO, 0x2)
 /* Returns the type of namespace (CLONE_NEW* value) referred to by
    file descriptor */
-#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
+#define NS_GET_NSTYPE		_IO(NSIO, 0x3)
+/* Get creator UID for a user namespace */
+#define NS_GET_CREATOR_UID	_IO(NSIO, 0x4)
 
 #endif /* __LINUX_NSFS_H */
-- 
2.5.5

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

* Re: [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns
  2016-12-19 14:38 ` [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns Michael Kerrisk (man-pages)
@ 2016-12-21  3:13   ` Andrei Vagin
  2016-12-22  7:17     ` Michael Kerrisk (man-pages)
  2016-12-22  7:23     ` Eric W. Biederman
  0 siblings, 2 replies; 5+ messages in thread
From: Andrei Vagin @ 2016-12-21  3:13 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Eric W. Biederman, Serge E. Hallyn, linux-api, linux-kernel,
	linux-fsdevel, Andrey Vagin, James Bottomley, W. Trevor King,
	Alexander Viro, Jonathan Corbet

On Mon, Dec 19, 2016 at 03:38:35PM +0100, Michael Kerrisk (man-pages) wrote:
> # Some open questions about this patch below.
> #
> One of the rules regarding capabilities is:
> 
>     A process that resides in the parent of the user namespace and
>     whose effective user ID matches the owner of the namespace has
>     all capabilities in the namespace.
> 
> Therefore, in order to write code that discovers whether process X has
> capabilities in namespace Y, we need a way to find out who the creator
> of a user namespace is. This patch adds an NS_GET_CREATOR_UID ioctl()
> that returns the (munged) UID of the creator of the user namespace
> referred to by the specified file descriptor.
> 
> If the supplied file descriptor does not refer to a user namespace,
> the operation fails with the error EINVAL.
> 
> Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
> ---
>  fs/nsfs.c                 | 6 ++++++
>  include/uapi/linux/nsfs.h | 8 +++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> Open questions:
> 
> * Would it be preferabe to separate the logic for NS_GET_CREATOR_UID
>   into a small helper function?
> * Is this a correct use of container_of()? I did not immediately
>   see another way to get to the user_namespace struct, but I
>   may well have missed something.
> 
> diff --git a/fs/nsfs.c b/fs/nsfs.c
> index 5d53476..26f6d94 100644
> --- a/fs/nsfs.c
> +++ b/fs/nsfs.c
> @@ -163,6 +163,7 @@ int open_related_ns(struct ns_common *ns,
>  static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  			unsigned long arg)
>  {
> +	struct user_namespace *user_ns;
>  	struct ns_common *ns = get_proc_ns(file_inode(filp));
>  
>  	switch (ioctl) {
> @@ -174,6 +175,11 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>  		return open_related_ns(ns, ns->ops->get_parent);
>  	case NS_GET_NSTYPE:
>  		return ns->ops->type;
> +	case NS_GET_CREATOR_UID:
> +		if (ns->ops->type != CLONE_NEWUSER)
> +			return -EINVAL;
> +		user_ns = container_of(ns, struct user_namespace, ns);
> +		return from_kuid_munged(current_user_ns(), user_ns->owner);

uid_t is "unsigned int", ioctl() returns long, so it may be hard to
distinguish user id-s from errors on x32.

off-topic: What is about user_ns->group? I can't find where it is used...

>  	default:
>  		return -ENOTTY;
>  	}
> diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
> index 2b48df1..b3c6c78 100644
> --- a/include/uapi/linux/nsfs.h
> +++ b/include/uapi/linux/nsfs.h
> @@ -6,11 +6,13 @@
>  #define NSIO	0xb7
>  
>  /* Returns a file descriptor that refers to an owning user namespace */
> -#define NS_GET_USERNS	_IO(NSIO, 0x1)
> +#define NS_GET_USERNS		_IO(NSIO, 0x1)
>  /* Returns a file descriptor that refers to a parent namespace */
> -#define NS_GET_PARENT	_IO(NSIO, 0x2)
> +#define NS_GET_PARENT		_IO(NSIO, 0x2)
>  /* Returns the type of namespace (CLONE_NEW* value) referred to by
>     file descriptor */
> -#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
> +#define NS_GET_NSTYPE		_IO(NSIO, 0x3)
> +/* Get creator UID for a user namespace */
> +#define NS_GET_CREATOR_UID	_IO(NSIO, 0x4)
>  
>  #endif /* __LINUX_NSFS_H */
> -- 
> 2.5.5
> 

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

* Re: [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns
  2016-12-21  3:13   ` Andrei Vagin
@ 2016-12-22  7:17     ` Michael Kerrisk (man-pages)
  2016-12-22  7:23     ` Eric W. Biederman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Kerrisk (man-pages) @ 2016-12-22  7:17 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: mtk.manpages, Eric W. Biederman, Serge E. Hallyn, linux-api,
	linux-kernel, linux-fsdevel, Andrey Vagin, James Bottomley,
	W. Trevor King, Alexander Viro, Jonathan Corbet

Hi Andrei,

On 12/21/2016 04:13 AM, Andrei Vagin wrote:
> On Mon, Dec 19, 2016 at 03:38:35PM +0100, Michael Kerrisk (man-pages) wrote:
>> # Some open questions about this patch below.
>> #
>> One of the rules regarding capabilities is:
>>
>>     A process that resides in the parent of the user namespace and
>>     whose effective user ID matches the owner of the namespace has
>>     all capabilities in the namespace.
>>
>> Therefore, in order to write code that discovers whether process X has
>> capabilities in namespace Y, we need a way to find out who the creator
>> of a user namespace is. This patch adds an NS_GET_CREATOR_UID ioctl()
>> that returns the (munged) UID of the creator of the user namespace
>> referred to by the specified file descriptor.
>>
>> If the supplied file descriptor does not refer to a user namespace,
>> the operation fails with the error EINVAL.
>>
>> Signed-off-by: Michael Kerrisk <mtk-manpages@gmail.com>
>> ---
>>  fs/nsfs.c                 | 6 ++++++
>>  include/uapi/linux/nsfs.h | 8 +++++---
>>  2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> Open questions:
>>
>> * Would it be preferabe to separate the logic for NS_GET_CREATOR_UID
>>   into a small helper function?
>> * Is this a correct use of container_of()? I did not immediately
>>   see another way to get to the user_namespace struct, but I
>>   may well have missed something.
>>
>> diff --git a/fs/nsfs.c b/fs/nsfs.c
>> index 5d53476..26f6d94 100644
>> --- a/fs/nsfs.c
>> +++ b/fs/nsfs.c
>> @@ -163,6 +163,7 @@ int open_related_ns(struct ns_common *ns,
>>  static long ns_ioctl(struct file *filp, unsigned int ioctl,
>>  			unsigned long arg)
>>  {
>> +	struct user_namespace *user_ns;
>>  	struct ns_common *ns = get_proc_ns(file_inode(filp));
>>  
>>  	switch (ioctl) {
>> @@ -174,6 +175,11 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>>  		return open_related_ns(ns, ns->ops->get_parent);
>>  	case NS_GET_NSTYPE:
>>  		return ns->ops->type;
>> +	case NS_GET_CREATOR_UID:
>> +		if (ns->ops->type != CLONE_NEWUSER)
>> +			return -EINVAL;
>> +		user_ns = container_of(ns, struct user_namespace, ns);
>> +		return from_kuid_munged(current_user_ns(), user_ns->owner);
> 
> uid_t is "unsigned int", ioctl() returns long, so it may be hard to
> distinguish user id-s from errors on x32.

Good point. So, we could instead return the UID via a buffer pointed to 
by the ioctl() arg. That would seem better, right?

> off-topic: What is about user_ns->group? I can't find where it is used...

I've no idea. Like you, I can't see any place where it's being used.

Cheers,

Michael


>>  	default:
>>  		return -ENOTTY;
>>  	}
>> diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
>> index 2b48df1..b3c6c78 100644
>> --- a/include/uapi/linux/nsfs.h
>> +++ b/include/uapi/linux/nsfs.h
>> @@ -6,11 +6,13 @@
>>  #define NSIO	0xb7
>>  
>>  /* Returns a file descriptor that refers to an owning user namespace */
>> -#define NS_GET_USERNS	_IO(NSIO, 0x1)
>> +#define NS_GET_USERNS		_IO(NSIO, 0x1)
>>  /* Returns a file descriptor that refers to a parent namespace */
>> -#define NS_GET_PARENT	_IO(NSIO, 0x2)
>> +#define NS_GET_PARENT		_IO(NSIO, 0x2)
>>  /* Returns the type of namespace (CLONE_NEW* value) referred to by
>>     file descriptor */
>> -#define NS_GET_NSTYPE	_IO(NSIO, 0x3)
>> +#define NS_GET_NSTYPE		_IO(NSIO, 0x3)
>> +/* Get creator UID for a user namespace */
>> +#define NS_GET_CREATOR_UID	_IO(NSIO, 0x4)
>>  
>>  #endif /* __LINUX_NSFS_H */
>> -- 
>> 2.5.5
>>
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

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

* Re: [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns
  2016-12-21  3:13   ` Andrei Vagin
  2016-12-22  7:17     ` Michael Kerrisk (man-pages)
@ 2016-12-22  7:23     ` Eric W. Biederman
  1 sibling, 0 replies; 5+ messages in thread
From: Eric W. Biederman @ 2016-12-22  7:23 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: Michael Kerrisk (man-pages),
	Serge E. Hallyn, linux-api, linux-kernel, linux-fsdevel,
	Andrey Vagin, James Bottomley, W. Trevor King, Alexander Viro,
	Jonathan Corbet

Andrei Vagin <avagin@virtuozzo.com> writes:

> On Mon, Dec 19, 2016 at 03:38:35PM +0100, Michael Kerrisk (man-pages) wrote:
>> @@ -174,6 +175,11 @@ static long ns_ioctl(struct file *filp, unsigned int ioctl,
>>  		return open_related_ns(ns, ns->ops->get_parent);
>>  	case NS_GET_NSTYPE:
>>  		return ns->ops->type;
>> +	case NS_GET_CREATOR_UID:
>> +		if (ns->ops->type != CLONE_NEWUSER)
>> +			return -EINVAL;
>> +		user_ns = container_of(ns, struct user_namespace, ns);
>> +		return from_kuid_munged(current_user_ns(), user_ns->owner);
>
> uid_t is "unsigned int", ioctl() returns long, so it may be hard to
> distinguish user id-s from errors on x32.

Very good point.

> off-topic: What is about user_ns->group? I can't find where it is
> used...

Over design. I put it in because I thought it might be useful.  It turns
out it never was used so we can clean things up and remove it.  The
group has never been exposed to userspace so no one will care.

Eric

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

end of thread, other threads:[~2016-12-22  7:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <fdce894d-8385-b4b4-da3c-6282a7e4ecba@gmail.com>
2016-12-19 14:38 ` [PATCH 1/2] nsfs: Add an ioctl() to return the namespace type Michael Kerrisk (man-pages)
2016-12-19 14:38 ` [PATCH 2/2] nsfs: Add an ioctl() to return creator UID of a userns Michael Kerrisk (man-pages)
2016-12-21  3:13   ` Andrei Vagin
2016-12-22  7:17     ` Michael Kerrisk (man-pages)
2016-12-22  7:23     ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).