Re: [RFC] kdbus: use LSM hooks to restrict ability to send file descriptors

[Paul Moore <pmoore@redhat.com>]

On Friday, September 18, 2015 03:27:03 PM Paul Osmialowski wrote:
> The goal of this patch is to reproduce on kdbus the same behavior
> that is expressed by Unix Domain Sockets when it comes to restricting
> ability to pass opened file descriptors.
> 
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/message.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)

Hi Paul,

I've been reworking my original kdbus LSM/SELinux hooks in order to simplify 
things and make them a bit more consistent with the binder and other IPC-esque 
hooks, I'm hoping to post a RFC for them soon.

A few comments below ... 

> diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
> index ae565cd..b083431 100644
> --- a/ipc/kdbus/message.c
> +++ b/ipc/kdbus/message.c
> @@ -24,6 +24,7 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/security.h>
>  #include <net/sock.h>
> 
>  #include "bus.h"
> @@ -150,13 +151,19 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct
> kdbus_pool_slice *slice, for (i = 0; i < gaps->n_fds; ++i) {
>  			int fd;
> 
> -			fd = get_unused_fd_flags(O_CLOEXEC);
> -			if (fd < 0)
> +			if (gaps->fd_files[i] &&
> +				    security_file_receive(gaps->fd_files[i])) {
>  				incomplete_fds = true;
> +				fds[n_fds++] = -1;
> +			} else {
> +				fd = get_unused_fd_flags(O_CLOEXEC);
> +				if (fd < 0)
> +					incomplete_fds = true;

My patch is a little different (no fd_files[i] validity check, diff if 
structure, etc.) but the basic idea is the same.

> -			WARN_ON(!gaps->fd_files[i]);
> +				WARN_ON(!gaps->fd_files[i]);

You probably want to move this before the LSM hook.

> -			fds[n_fds++] = fd < 0 ? -1 : fd;
> +				fds[n_fds++] = fd < 0 ? -1 : fd;
> +			}
>  		}
> 
>  		/*
> @@ -178,6 +185,13 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct
> kdbus_pool_slice *slice, for (i = 0; i < gaps->n_memfds; ++i) {
>  		int memfd;
> 
> +		if (gaps->memfd_files[i] &&
> +			    security_file_receive(gaps->memfd_files[i])) {
> +			incomplete_fds = true;
> +			fds[n_fds++] = -1;
> +			continue;
> +		}

Similar to above, including the WARN_ON() movement.

>  		memfd = get_unused_fd_flags(O_CLOEXEC);
>  		if (memfd < 0) {
>  			incomplete_fds = true;

-- 
paul moore
security @ redhat