[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130807143151.GA4560@cachalot>
Date: Wed, 7 Aug 2013 18:31:51 +0400
From: Vasily Kulikov <segoon@...nwall.com>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc: twaugh@...hat.com, amwang@...hat.com,
linux-security-module@...r.kernel.org,
kernel-hardening@...ts.openwall.com
Subject: Re: [RFC] KPortReserve : kernel version of portreserve utility
(CC'ed kernel-hardening)
Hi Tetsuo,
See my comments inlined,
On Sat, Aug 03, 2013 at 17:15 +0900, Tetsuo Handa wrote:
> Hello.
>
> There is a blog post regarding how to reliably reserve local port numbers at
> http://cyberelk.net/tim/2012/02/15/portreserve-systemd-solution/ . Recently I
> heard a trouble in a RHEL5 system where portreserve utility is not available.
>
> I wrote this trivial LSM module as a really race-proof solution. But I'm
> expecting that this functionality becomes available in a way that all users can
> use regardless of their skill to use SELinux/SMACK/TOMOYO/AppArmor.
>
> (Question 1) Should this functionality implemented as LSM module?
> (Question 2) If yes, should this functionality implemented as a part of Yama?
>
> Regards.
> --------------------
> >From cbc76e3955e01dc6e590af860830b888ce7cbd0b Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> Date: Sat, 3 Aug 2013 16:58:05 +0900
> Subject: [PATCH] KPortReserve : kernel version of portreserve utility
>
> This module reserves local port like /proc/sys/net/ipv4/ip_local_reserved_ports
> does, but this module is designed for stopping bind() requests with non-zero
> local port numbers from unwanted programs.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> ---
> security/Kconfig | 6 +
> security/Makefile | 2 +
> security/kportreserve/Kconfig | 35 ++++
> security/kportreserve/Makefile | 1 +
> security/kportreserve/kpr.c | 443 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 487 insertions(+), 0 deletions(-)
> create mode 100644 security/kportreserve/Kconfig
> create mode 100644 security/kportreserve/Makefile
> create mode 100644 security/kportreserve/kpr.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index e9c6ac7..f4058ff 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -122,6 +122,7 @@ source security/smack/Kconfig
> source security/tomoyo/Kconfig
> source security/apparmor/Kconfig
> source security/yama/Kconfig
> +source security/kportreserve/Kconfig
>
> source security/integrity/Kconfig
>
> @@ -132,6 +133,7 @@ choice
> default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
> + default DEFAULT_SECURITY_KPR if SECURITY_KPR
> default DEFAULT_SECURITY_DAC
>
> help
> @@ -153,6 +155,9 @@ choice
> config DEFAULT_SECURITY_YAMA
> bool "Yama" if SECURITY_YAMA=y
>
> + config DEFAULT_SECURITY_KPR
> + bool "KPortReserve" if SECURITY_KPR=y
> +
> config DEFAULT_SECURITY_DAC
> bool "Unix Discretionary Access Controls"
>
> @@ -165,6 +170,7 @@ config DEFAULT_SECURITY
> default "tomoyo" if DEFAULT_SECURITY_TOMOYO
> default "apparmor" if DEFAULT_SECURITY_APPARMOR
> default "yama" if DEFAULT_SECURITY_YAMA
> + default "kpr" if DEFAULT_SECURITY_KPR
> default "" if DEFAULT_SECURITY_DAC
>
> endmenu
> diff --git a/security/Makefile b/security/Makefile
> index c26c81e..87f95cc 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK) += smack
> subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
> subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> subdir-$(CONFIG_SECURITY_YAMA) += yama
> +subdir-$(CONFIG_SECURITY_KPR) += kportreserve
>
> # always enable default capabilities
> obj-y += commoncap.o
> @@ -23,6 +24,7 @@ obj-$(CONFIG_AUDIT) += lsm_audit.o
> obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
> obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
> obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
> +obj-$(CONFIG_SECURITY_KPR) += kportreserve/built-in.o
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> # Object integrity file lists
> diff --git a/security/kportreserve/Kconfig b/security/kportreserve/Kconfig
> new file mode 100644
> index 0000000..73ad5bc
> --- /dev/null
> +++ b/security/kportreserve/Kconfig
> @@ -0,0 +1,35 @@
> +config SECURITY_KPR
> + bool "KPortReserve support"
> + depends on SECURITY
> + depends on PROC_FS
> + select SECURITY_NETWORK
> + default n
> + help
> + This selects local port reserving module which is similar to
> + /proc/sys/net/ipv4/ip_local_reserved_ports . But this module is
> + designed for stopping bind() requests with non-zero local port
> + numbers from unwanted programs using white list reservations.
> +
> + If you are unsure how to answer this question, answer N.
> +
> + Usage:
> +
> + Use "add $port $program" format to add reservation.
> + The $port is a single port number between 0 and 65535.
> + The $program is the content of /proc/self/exe in TOMOYO's pathname
> + representation rule (i.e. consists with only ASCII printable
> + characters, and seen from the current thread's namespace's root (e.g.
> + /var/chroot/bin/bash for /bin/bash running inside /var/chroot/
> + chrooted environment)). The <kernel> means kernel threads).
> + For example,
> +
> + # echo "add 10000 /bin/bash" > /proc/reserved_local_port
> + # echo "add 20000 <kernel>" > /proc/reserved_local_port
> +
> + allows bind() on port 10000 to /bin/bash and allows bind() on port
> + 20000 to kernel threads.
Wait, it restrict the port to a *program*, not a user/group/security
domain? It means *any* user may run this program and obtain the port.
Is it intentional behaviour? I guess it would be MUCH more useful to
allow some port to this specific user, NOT a program. For most daemons
we have separate user accounts (SELinux contexts in some distros,
whatever), so it makes sense to limit the port to a UID/GID (or LSM's
security context).
> +
> + Use "del $port $program" format to remove reservation.
> +
> + Note that only port numbers which have at least one reservation are
> + checked by this module.
> diff --git a/security/kportreserve/Makefile b/security/kportreserve/Makefile
> new file mode 100644
> index 0000000..6342521
> --- /dev/null
> +++ b/security/kportreserve/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SECURITY_KPR) := kpr.o
> diff --git a/security/kportreserve/kpr.c b/security/kportreserve/kpr.c
> new file mode 100644
> index 0000000..7b19d32
> --- /dev/null
> +++ b/security/kportreserve/kpr.c
> @@ -0,0 +1,443 @@
> +/*
> + * kpr.c - kernel version of portreserve.
> + */
> +#include <linux/security.h>
> +#include <linux/proc_fs.h>
> +#include <linux/vmalloc.h>
> +#include <linux/net.h>
> +#include <linux/inet.h>
> +#include <linux/uaccess.h>
> +#include <net/sock.h>
> +#include <net/ip.h>
> +#include <net/ipv6.h>
> +
> +/* Max length of a line. */
> +#define MAX_LINE_LEN 16384
> +
> +/* Port numbers with at least one whitelist element exists. */
> +static unsigned long reserved_port_map[65536 / BITS_PER_LONG];
> +
> +/* Whitelist element. */
> +struct reserved_port_entry {
> + struct list_head list;
> + const char *exe;
> + u16 port;
> +};
> +/* List of whitelist elements. */
> +static LIST_HEAD(reserved_port_list);
> +
> +/**
> + * kpr_encode - Encode binary string to ascii string.
> + *
> + * @str: String in binary format.
> + *
> + * Returns pointer to @str in ascii format on success, NULL otherwise.
> + *
> + * This function uses kzalloc(), so caller must kfree() if this function
> + * didn't return NULL.
> + */
> +static char *kpr_encode(const char *str)
> +{
> + int i;
> + int len = 0;
> + const char *p = str;
> + char *cp;
> + char *cp0;
> + const int str_len = strlen(str);
> + for (i = 0; i < str_len; i++) {
> + const unsigned char c = p[i];
> + if (c == '\\')
> + len += 2;
> + else if (c > ' ' && c < 127)
> + len++;
> + else
> + len += 4;
> + }
> + len++;
> + cp = kzalloc(len, GFP_KERNEL);
> + if (!cp)
> + return NULL;
> + cp0 = cp;
> + p = str;
> + for (i = 0; i < str_len; i++) {
> + const unsigned char c = p[i];
> + if (c == '\\') {
> + *cp++ = '\\';
> + *cp++ = '\\';
> + } else if (c > ' ' && c < 127) {
> + *cp++ = c;
> + } else {
> + *cp++ = '\\';
> + *cp++ = (c >> 6) + '0';
> + *cp++ = ((c >> 3) & 7) + '0';
> + *cp++ = (c & 7) + '0';
> + }
> + }
> + return cp0;
> +}
> +
> +/**
> + * kpr_realpath - Returns realpath(3) of the given pathname but ignores chroot'ed root.
> + *
> + * @path: Pointer to "struct path".
> + *
> + * Returns the realpath of the given @path on success, NULL otherwise.
> + *
> + * This function uses kzalloc(), so caller must kfree() if this function
> + * didn't return NULL.
> + */
> +static char *kpr_realpath(struct path *path)
> +{
> + char *buf = NULL;
> + char *name = NULL;
> + unsigned int buf_len = PAGE_SIZE / 2;
> + while (1) {
> + char *pos;
> + buf_len <<= 1;
> + kfree(buf);
> + buf = kmalloc(buf_len, GFP_KERNEL);
> + if (!buf)
> + break;
> + pos = d_absolute_path(path, buf, buf_len);
> + if (IS_ERR(pos))
> + continue;
d_absolute_path() may fail in case of not only ENOMEM, but also EINVAL.
In this case you'll OOM the kernel with too big kmalloc(). buf_len
should be limited with some (not too big) number.
> + name = kpr_encode(pos);
> + break;
> + }
> + kfree(buf);
> + return name;
> +}
> +
> +/**
> + * kpr_get_exe - Get kpr_realpath() of current process.
Probably it should be called kpr_realpath_current() or
current_kpr_realpath() then?
> + *
> + * Returns the kpr_realpath() of current process on success, NULL otherwise.
> + *
> + * This function uses kzalloc(), so the caller must kfree()
> + * if this function didn't return NULL.
> + */
> +static const char *kpr_get_exe(void)
> +{
> + struct mm_struct *mm = current->mm;
> + const char *cp = NULL;
> + if (current->flags & PF_KTHREAD)
> + return kstrdup("<kernel>", GFP_KERNEL);
> + if (mm) {
> + down_read(&mm->mmap_sem);
> + if (mm->exe_file)
> + cp = kpr_realpath(&mm->exe_file->f_path);
> + up_read(&mm->mmap_sem);
> + }
> + return cp;
> +}
> +
> +/**
> + * kpr_socket_bind - Check permission for bind().
> + *
> + * @sock: Pointer to "struct socket".
> + * @addr: Pointer to "struct sockaddr".
> + * @addr_len: Size of @addr.
> + *
> + * Returns 0 on success, negative value otherwise.
> + */
> +static int kpr_socket_bind(struct socket *sock, struct sockaddr *addr,
> + int addr_len)
> +{
> + const char *exe;
> + u16 port;
> + switch (sock->sk->sk_family) {
> + case PF_INET:
> + case PF_INET6:
> + break;
> + default:
> + return 0;
> + }
> + switch (sock->type) {
> + case SOCK_STREAM:
> + case SOCK_DGRAM:
> + break;
> + default:
> + return 0;
> + }
> + switch (addr->sa_family) {
> + case AF_INET:
> + if (addr_len < sizeof(struct sockaddr_in))
> + return 0;
> + port = ((struct sockaddr_in *) addr)->sin_port;
> + break;
> + case AF_INET6:
> + if (addr_len < SIN6_LEN_RFC2133)
> + return 0;
> + port = ((struct sockaddr_in6 *) addr)->sin6_port;
> + break;
> + default:
> + return 0;
> + }
> + port = ntohs(port);
> + if (!test_bit(port, reserved_port_map))
> + return 0;
> + exe = kpr_get_exe();
> + if (!exe) {
> + pr_warn("Unable to read /proc/self/exe . Rejecting bind(%u) request.\n",
> + port);
> + return -ENOMEM;
> + } else {
> + struct reserved_port_entry *ptr;
> + int ret = 0;
> + rcu_read_lock();
> + list_for_each_entry_rcu(ptr, &reserved_port_list, list) {
> + if (port != ptr->port)
> + continue;
> + if (strcmp(exe, ptr->exe)) {
> + ret = -EADDRINUSE;
> + continue;
> + }
> + ret = 0;
> + break;
> + }
> + rcu_read_unlock();
> + kfree(exe);
> + return ret;
> + }
> +}
> +
> +/**
> + * kpr_read - read() for /proc/reserved_local_port interface.
> + *
> + * @file: Pointer to "struct file".
> + * @buf: Pointer to buffer.
> + * @count: Size of @buf.
> + * @ppos: Offset of @file.
> + *
> + * Returns bytes read on success, negative value otherwise.
> + */
> +static ssize_t kpr_read(struct file *file, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + ssize_t copied = 0;
> + int error = 0;
> + int record = 0;
> + loff_t offset = 0;
> + char *data = vmalloc(MAX_LINE_LEN);
> + if (!data)
> + return -ENOMEM;
> + while (1) {
> + struct reserved_port_entry *ptr;
> + int i = 0;
> + data[0] = '\0';
> + rcu_read_lock();
> + list_for_each_entry_rcu(ptr, &reserved_port_list, list) {
> + if (i++ < record)
> + continue;
> + snprintf(data, MAX_LINE_LEN - 1, "%u %s\n", ptr->port,
> + ptr->exe);
> + break;
> + }
> + rcu_read_unlock();
> + if (!data[0])
> + break;
> + for (i = 0; data[i]; i++) {
> + if (offset++ < *ppos)
> + continue;
> + if (put_user(data[i], buf)) {
> + error = -EFAULT;
> + break;
> + }
> + buf++;
> + copied++;
> + (*ppos)++;
> + }
> + record++;
> + }
> + vfree(data);
> + return copied ? copied : error;
> +}
> +
> +/**
> + * kpr_normalize_line - Format string.
> + *
> + * @buffer: The line to normalize.
> + *
> + * Returns nothing.
> + *
> + * Leading and trailing whitespaces are removed.
> + * Multiple whitespaces are packed into single space.
> + */
> +static void kpr_normalize_line(unsigned char *buffer)
> +{
> + unsigned char *sp = buffer;
> + unsigned char *dp = buffer;
> + bool first = true;
> + while (*sp && (*sp <= ' ' || *sp >= 127))
> + sp++;
> + while (*sp) {
> + if (!first)
> + *dp++ = ' ';
> + first = false;
> + while (*sp > ' ' && *sp < 127)
> + *dp++ = *sp++;
> + while (*sp && (*sp <= ' ' || *sp >= 127))
> + sp++;
> + }
> + *dp = '\0';
> +}
> +
> +/**
> + * kpr_find_entry - Find an existing entry.
> + *
> + * @port: Port number.
> + * @exe: Pathname. NULL for any.
> + *
> + * Returns pointer to existing entry if found, NULL otherwise.
> + */
> +static struct reserved_port_entry *kpr_find_entry(const u16 port,
> + const char *exe)
> +{
> + struct reserved_port_entry *ptr;
> + bool found = false;
> + rcu_read_lock();
> + list_for_each_entry_rcu(ptr, &reserved_port_list, list) {
> + if (port != ptr->port)
> + continue;
> + if (exe && strcmp(exe, ptr->exe))
> + continue;
> + found = true;
> + break;
> + }
> + rcu_read_unlock();
> + return found ? ptr : NULL;
> +}
> +
> +/**
> + * kpr_update_entry - Update the list of whitelist elements.
> + *
> + * @data: Line of data to parse.
> + *
> + * Returns 0 on success, negative value otherwise.
> + *
> + * Caller holds a mutex to protect from concurrent updates.
> + */
> +static int kpr_update_entry(const char *data)
> +{
> + struct reserved_port_entry *ptr;
> + unsigned int port;
> + if (sscanf(data, "add %u", &port) == 1 && port < 65536) {
> + const char *cp = strchr(data + 4, ' ');
> + if (!cp++ || strchr(cp, ' '))
> + return -EINVAL;
> + if (kpr_find_entry(port, cp))
> + return 0;
> + ptr = kzalloc(sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return -ENOMEM;
> + ptr->port = (u16) port;
> + ptr->exe = kstrdup(cp, GFP_KERNEL);
> + if (!ptr->exe) {
> + kfree(ptr);
> + return -ENOMEM;
> + }
> + list_add_tail_rcu(&ptr->list, &reserved_port_list);
> + set_bit(ptr->port, reserved_port_map);
> + } else if (sscanf(data, "del %u", &port) == 1 && port < 65536) {
> + const char *cp = strchr(data + 4, ' ');
> + if (!cp++ || strchr(cp, ' '))
> + return -EINVAL;
> + ptr = kpr_find_entry(port, cp);
> + if (!ptr)
> + return 0;
> + list_del_rcu(&ptr->list);
> + synchronize_rcu();
> + kfree(ptr->exe);
> + kfree(ptr);
> + if (!kpr_find_entry(port, NULL))
> + clear_bit(ptr->port, reserved_port_map);
> + } else {
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +/**
> + * kpr_write - write() for /proc/reserved_local_port interface.
> + *
> + * @file: Pointer to "struct file".
> + * @buf: Domainname to transit to.
> + * @count: Size of @buf.
> + * @ppos: Unused.
> + *
> + * Returns bytes parsed on success, negative value otherwise.
> + */
> +static ssize_t kpr_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + char *data;
> + ssize_t copied = 0;
> + int error;
> + if (!count)
> + return 0;
> + if (count > MAX_LINE_LEN - 1)
> + count = MAX_LINE_LEN - 1;
> + data = vmalloc(count + 1);
> + if (!data)
> + return -ENOMEM;
> + if (copy_from_user(data, buf, count)) {
> + error = -EFAULT;
> + goto out;
> + }
> + data[count] = '\0';
> + while (1) {
> + static DEFINE_MUTEX(lock);
> + char *cp = strchr(data, '\n');
> + int len;
> + if (!cp) {
> + error = -EINVAL;
> + break;
> + }
> + *cp = '\0';
> + len = strlen(data) + 1;
> + kpr_normalize_line(data);
> + if (mutex_lock_interruptible(&lock)) {
> + error = -EINTR;
> + break;
> + }
> + error = kpr_update_entry(data);
> + mutex_unlock(&lock);
> + if (error < 0)
> + break;
> + copied += len;
> + memmove(data, data + len, strlen(data + len) + 1);
> + }
> +out:
> + vfree(data);
> + return copied ? copied : error;
> +}
> +
> +/* List of hooks. */
> +static struct security_operations kpr_ops = {
> + .name = "kpr",
> + .socket_bind = kpr_socket_bind,
> +};
> +
> +/* Operations for /proc/reserved_local_port interface. */
> +static const struct file_operations kpr_operations = {
> + .write = kpr_write,
> + .read = kpr_read,
> +};
> +
> +/**
> + * kpr_init - Initialize this module.
> + *
> + * Returns 0 on success, negative value otherwise.
> + */
> +static int __init kpr_init(void)
> +{
> + if (!security_module_enable(&kpr_ops))
> + return 0;
> + if (!proc_create("reserved_local_port", 0644, NULL, &kpr_operations) ||
> + register_security(&kpr_ops))
> + panic("Failure registering kportreserve");
> + pr_info("KPortReserve initialized\n");
> + return 0;
> +}
> +
> +late_initcall(kpr_init);
> --
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
--
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
