2009-11-30 21:53:50

by Paul Nuzzi

[permalink] [raw]
Subject: [PATCH] Dynamic port labeling V2

Second version of the dynamic port labeling patch. Changed the name of
the selinuxfs interface to portcon and changed the interface to only
allow five arguments instead of the variable four or five.

Added a mechanism to add/delete/update port labels with an interface in
the selinuxfs filesystem. This will give administrators the ability to
update port labels faster than reloading the entire policy with
semanage. The administrator will also need less privilege since they
don't have to be authorized to reload the full policy.

A listing of all port labels will be output if the file /selinux/portcon
is read. Labels could be added or deleted with the following commands

echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22" > /selinux/portcon
echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22" > /selinux/portcon

Labels can be atomically changed with the add command.


Signed-off-by: Paul Nuzzi <[email protected]>

---
security/selinux/hooks.c | 1
security/selinux/include/classmap.h | 2
security/selinux/include/security.h | 9 ++
security/selinux/selinuxfs.c | 96 ++++++++++++++++++++++
security/selinux/ss/services.c | 154 ++++++++++++++++++++++++++++++++++++
5 files changed, 260 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a29d661..5aceb74 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1224,7 +1224,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
struct inode_security_struct *isec = inode->i_security;
u32 sid;
struct dentry *dentry;
-#define INITCONTEXTLEN 255
char *context = NULL;
unsigned len = 0;
int rc = 0;
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 8b32e95..925edb6 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -16,7 +16,7 @@ struct security_class_mapping secclass_map[] = {
{ "compute_av", "compute_create", "compute_member",
"check_context", "load_policy", "compute_relabel",
"compute_user", "setenforce", "setbool", "setsecparam",
- "setcheckreqprot", NULL } },
+ "setcheckreqprot", "portcon", NULL } },
{ "process",
{ "fork", "transition", "sigchld", "sigkill",
"sigstop", "signull", "signal", "ptrace", "getsched", "setsched",
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 2553266..25a5054 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -169,6 +169,15 @@ int security_fs_use(const char *fstype, unsigned int *behavior,
int security_genfs_sid(const char *fstype, char *name, u16 sclass,
u32 *sid);

+int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
+ char *scontext);
+
+int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
+ char *scontext);
+
+int security_ocon_port_read(char **buf);
+#define INITCONTEXTLEN 255
+
#ifdef CONFIG_NETLABEL
int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
u32 *sid);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index fab36fd..a21be6c 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -110,6 +110,7 @@ enum sel_inos {
SEL_COMPAT_NET, /* whether to use old compat network packet controls */
SEL_REJECT_UNKNOWN, /* export unknown reject handling to userspace */
SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
+ SEL_OCON_PORT, /* add OCON_PORT to the list */
SEL_INO_NEXT, /* The next inode number to use */
};

@@ -253,6 +254,100 @@ static const struct file_operations sel_disable_ops = {
.write = sel_write_disable,
};

+static ssize_t sel_write_ocon_port(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ ssize_t length;
+ unsigned int low = 0, high = 0, protocol = 0;
+ char *page, *scontext, *command;
+ u32 sid;
+#define OCONCOMMANDLEN 4
+#define OCON_ADD_COMMAND "add"
+#define OCON_DEL_COMMAND "del"
+
+ length = task_has_security(current, SECURITY__PORTCON);
+ if (length)
+ return length;
+
+ if (count >= PAGE_SIZE)
+ return -ENOMEM;
+ if (*ppos != 0) {
+ /* No partial writes. */
+ return -EINVAL;
+ }
+ page = (char *)get_zeroed_page(GFP_KERNEL);
+ if (!page)
+ goto nomem;
+ scontext = kzalloc(INITCONTEXTLEN, GFP_KERNEL);
+ if (!scontext)
+ goto nomem1;
+ command = kzalloc(OCONCOMMANDLEN, GFP_KERNEL);
+ if (!command)
+ goto nomem2;
+
+ length = -EFAULT;
+ if (copy_from_user(page, buf, count))
+ goto out;
+
+ length = sscanf(page, "%3s %255s %u %u %u", command, scontext,
+ &protocol, &low, &high);
+ if (length < 5) {
+ length = -EINVAL;
+ goto out;
+ }
+ length = security_context_to_sid(scontext, strlen(scontext), &sid);
+ if (length < 0) {
+ length = -EINVAL;
+ goto out;
+ }
+
+ if (strncmp(OCON_ADD_COMMAND, command, strlen(OCON_ADD_COMMAND)) == 0)
+ length = security_ocon_port_add(protocol, low, high, sid,
+ scontext);
+ else if (strncmp(OCON_DEL_COMMAND, command, strlen(OCON_DEL_COMMAND))
+ == 0)
+ length = security_ocon_port_del(protocol, low, high, sid,
+ scontext);
+ else {
+ length = -EINVAL;
+ goto out;
+ }
+ if (length < 0)
+ goto out;
+ length = count;
+out:
+ kfree(command);
+ kfree(scontext);
+ free_page((unsigned long) page);
+ return length;
+
+nomem2:
+ kfree(scontext);
+nomem1:
+ free_page((unsigned long) page);
+nomem:
+ return -ENOMEM;
+}
+
+static ssize_t sel_read_ocon_port(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ ssize_t length;
+ char *buf_tmp = NULL;
+
+ length = task_has_security(current, SECURITY__PORTCON);
+ if (length)
+ return length;
+
+ length = security_ocon_port_read(&buf_tmp);
+ return simple_read_from_buffer(buf, count, ppos, buf_tmp, length);
+}
+
+static const struct file_operations sel_ocon_port_ops = {
+ .write = sel_write_ocon_port,
+ .read = sel_read_ocon_port,
+};
+
static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -1596,6 +1691,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
+ [SEL_OCON_PORT] = {"portcon", &sel_ocon_port_ops, S_IRUSR|S_IWUSR},
/* last one */ {""}
};
ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f270e37..3b920e3 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1827,6 +1827,160 @@ err:

}

+int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
+ char *scontext)
+{
+ int rc = 0;
+ struct ocontext *c;
+ struct context ctx;
+ struct ocontext *add = kzalloc(sizeof(struct ocontext), GFP_KERNEL);
+
+ if (!add)
+ return -ENOMEM;
+ else if (low > high) {
+ kfree(add);
+ return -EINVAL;
+ }
+
+ read_lock(&policy_rwlock);
+ rc = string_to_context_struct(&policydb, &sidtab, scontext,
+ strlen(scontext), &ctx, sid);
+ if (rc) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ add->u.port.protocol = protocol;
+ add->u.port.low_port = low;
+ add->u.port.high_port = high;
+ add->sid[0] = sid;
+ add->context[0] = ctx;
+ for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
+ if (add->u.port.protocol == c->u.port.protocol &&
+ add->u.port.low_port == c->u.port.low_port &&
+ add->u.port.high_port == c->u.port.high_port) {
+ printk(KERN_DEBUG "Duplicate netport %d - %d.. "
+ "changing permissions\n",
+ add->u.port.low_port, add->u.port.high_port);
+ c->sid[0] = add->sid[0];
+ context_destroy(&c->context[0]);
+ c->context[0] = add->context[0];
+ context_destroy(&add->context[0]);
+ kfree(add);
+ avc_ss_reset(0);
+ goto out;
+ }
+ }
+
+ add->next = policydb.ocontexts[OCON_PORT];
+ policydb.ocontexts[OCON_PORT] = add;
+ avc_ss_reset(0);
+out:
+ if (rc) {
+ context_destroy(&add->context[0]);
+ kfree(add);
+ }
+ read_unlock(&policy_rwlock);
+ return rc;
+}
+
+int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
+ char *scontext)
+{
+ int rc = 0;
+ struct ocontext *c, *before_c;
+ struct context ctx;
+
+ if (low > high)
+ return -EINVAL;
+
+ read_lock(&policy_rwlock);
+ rc = string_to_context_struct(&policydb, &sidtab, scontext,
+ strlen(scontext), &ctx, sid);
+ if (rc) {
+ rc = -EINVAL;
+ goto out;
+ }
+ for (before_c = NULL, c = policydb.ocontexts[OCON_PORT]; c;
+ before_c = c, c = c->next) {
+ if (c->u.port.protocol == protocol &&
+ c->u.port.low_port == low &&
+ c->u.port.high_port == high &&
+ c->context[0].type == ctx.type &&
+ c->context[0].role == ctx.role &&
+ c->context[0].user == ctx.user) {
+ if (before_c == NULL)
+ policydb.ocontexts[OCON_PORT] = c->next;
+ else
+ before_c->next = c->next;
+ context_destroy(&c->context[0]);
+ kfree(c);
+ avc_ss_reset(0);
+ break;
+ }
+ }
+ if (c == NULL) {
+ printk(KERN_DEBUG "Netport not found %d - %d\n", low, high);
+ rc = -ENXIO;
+ }
+out:
+ context_destroy(&ctx);
+ read_unlock(&policy_rwlock);
+ return rc;
+}
+
+int security_ocon_port_read(char **buf)
+{
+ unsigned int buf_size = 0, old_size = 0, len;
+ struct ocontext *c;
+ char *scontext;
+ char *buf_tmp = NULL;
+ int rc = 0;
+ int line_size = INITCONTEXTLEN + (sizeof(unsigned int) * 3) +
+ (sizeof(char) * 5);
+ char addline[line_size];
+
+ /* protocol low-high scontext */
+ read_lock(&policy_rwlock);
+ if (policydb.ocontexts[OCON_PORT] == NULL)
+ goto out;
+
+ for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
+ rc = context_struct_to_string(&(c->context[0]), &scontext,
+ &len);
+ if (rc < 0) {
+ kfree(buf_tmp);
+ rc = -EINVAL;
+ goto out;
+ }
+ if (len > INITCONTEXTLEN) {
+ kfree(buf_tmp);
+ kfree(scontext);
+ rc = -EOVERFLOW;
+ goto out;
+ }
+ snprintf(addline, line_size, "%u %u-%u %s\n",
+ c->u.port.protocol, c->u.port.low_port,
+ c->u.port.high_port, scontext);
+ kfree(scontext);
+ buf_size += strlen(addline);
+ buf_tmp = krealloc(buf_tmp, buf_size, GFP_KERNEL);
+ if (!buf_tmp) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ for (rc = old_size; rc < buf_size; rc++)
+ buf_tmp[rc] = '\0';
+ strncat(buf_tmp, addline, buf_size);
+ old_size = buf_size;
+ }
+
+ *buf = buf_tmp;
+out:
+ read_unlock(&policy_rwlock);
+ return rc;
+}
+
/**
* security_port_sid - Obtain the SID for a port.
* @protocol: protocol number


2009-12-01 04:52:33

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Paul Nuzzi wrote:
> Second version of the dynamic port labeling patch.

So I've looked through both versions of this patch and I can't
help but think that you'd get better mileage out of a file system
interface than this SELinux specific implementation. If you had
something like

/port/22

with default owner root and mode rw-------

/port/3306

with default owner root and mode rw-rw-rw-

you could address a bunch of the complaints about port ownership that
you hear every day. Further, if the port filesystem supported xattrs
you could tie in SELinux as easily as you are doing it below and get
Smack for an extra $1.98, not to mention saving every other LSM the
grief of defining Yet Another way to define port accesses.

It bothers me that there is a perfectly reasonable way to provide the
specific behavior you're looking for (SELinux label on a port) that
generalizes so cleanly and that it's not being proposed.


> Changed the name of
> the selinuxfs interface to portcon and changed the interface to only
> allow five arguments instead of the variable four or five.
>
> Added a mechanism to add/delete/update port labels with an interface in
> the selinuxfs filesystem. This will give administrators the ability to
> update port labels faster than reloading the entire policy with
> semanage. The administrator will also need less privilege since they
> don't have to be authorized to reload the full policy.
>
> A listing of all port labels will be output if the file /selinux/portcon
> is read. Labels could be added or deleted with the following commands
>
> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22" > /selinux/portcon
> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22" > /selinux/portcon
>
> Labels can be atomically changed with the add command.
>
>
> Signed-off-by: Paul Nuzzi <[email protected]>
>
> ---
> security/selinux/hooks.c | 1
> security/selinux/include/classmap.h | 2
> security/selinux/include/security.h | 9 ++
> security/selinux/selinuxfs.c | 96 ++++++++++++++++++++++
> security/selinux/ss/services.c | 154 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 260 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a29d661..5aceb74 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1224,7 +1224,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> struct inode_security_struct *isec = inode->i_security;
> u32 sid;
> struct dentry *dentry;
> -#define INITCONTEXTLEN 255
> char *context = NULL;
> unsigned len = 0;
> int rc = 0;
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 8b32e95..925edb6 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -16,7 +16,7 @@ struct security_class_mapping secclass_map[] = {
> { "compute_av", "compute_create", "compute_member",
> "check_context", "load_policy", "compute_relabel",
> "compute_user", "setenforce", "setbool", "setsecparam",
> - "setcheckreqprot", NULL } },
> + "setcheckreqprot", "portcon", NULL } },
> { "process",
> { "fork", "transition", "sigchld", "sigkill",
> "sigstop", "signull", "signal", "ptrace", "getsched", "setsched",
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 2553266..25a5054 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -169,6 +169,15 @@ int security_fs_use(const char *fstype, unsigned int *behavior,
> int security_genfs_sid(const char *fstype, char *name, u16 sclass,
> u32 *sid);
>
> +int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
> + char *scontext);
> +
> +int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
> + char *scontext);
> +
> +int security_ocon_port_read(char **buf);
> +#define INITCONTEXTLEN 255
> +
> #ifdef CONFIG_NETLABEL
> int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
> u32 *sid);
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index fab36fd..a21be6c 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -110,6 +110,7 @@ enum sel_inos {
> SEL_COMPAT_NET, /* whether to use old compat network packet controls */
> SEL_REJECT_UNKNOWN, /* export unknown reject handling to userspace */
> SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
> + SEL_OCON_PORT, /* add OCON_PORT to the list */
> SEL_INO_NEXT, /* The next inode number to use */
> };
>
> @@ -253,6 +254,100 @@ static const struct file_operations sel_disable_ops = {
> .write = sel_write_disable,
> };
>
> +static ssize_t sel_write_ocon_port(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + ssize_t length;
> + unsigned int low = 0, high = 0, protocol = 0;
> + char *page, *scontext, *command;
> + u32 sid;
> +#define OCONCOMMANDLEN 4
> +#define OCON_ADD_COMMAND "add"
> +#define OCON_DEL_COMMAND "del"
> +
> + length = task_has_security(current, SECURITY__PORTCON);
> + if (length)
> + return length;
> +
> + if (count >= PAGE_SIZE)
> + return -ENOMEM;
> + if (*ppos != 0) {
> + /* No partial writes. */
> + return -EINVAL;
> + }
> + page = (char *)get_zeroed_page(GFP_KERNEL);
> + if (!page)
> + goto nomem;
> + scontext = kzalloc(INITCONTEXTLEN, GFP_KERNEL);
> + if (!scontext)
> + goto nomem1;
> + command = kzalloc(OCONCOMMANDLEN, GFP_KERNEL);
> + if (!command)
> + goto nomem2;
> +
> + length = -EFAULT;
> + if (copy_from_user(page, buf, count))
> + goto out;
> +
> + length = sscanf(page, "%3s %255s %u %u %u", command, scontext,
> + &protocol, &low, &high);
> + if (length < 5) {
> + length = -EINVAL;
> + goto out;
> + }
> + length = security_context_to_sid(scontext, strlen(scontext), &sid);
> + if (length < 0) {
> + length = -EINVAL;
> + goto out;
> + }
> +
> + if (strncmp(OCON_ADD_COMMAND, command, strlen(OCON_ADD_COMMAND)) == 0)
> + length = security_ocon_port_add(protocol, low, high, sid,
> + scontext);
> + else if (strncmp(OCON_DEL_COMMAND, command, strlen(OCON_DEL_COMMAND))
> + == 0)
> + length = security_ocon_port_del(protocol, low, high, sid,
> + scontext);
> + else {
> + length = -EINVAL;
> + goto out;
> + }
> + if (length < 0)
> + goto out;
> + length = count;
> +out:
> + kfree(command);
> + kfree(scontext);
> + free_page((unsigned long) page);
> + return length;
> +
> +nomem2:
> + kfree(scontext);
> +nomem1:
> + free_page((unsigned long) page);
> +nomem:
> + return -ENOMEM;
> +}
> +
> +static ssize_t sel_read_ocon_port(struct file *filp, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + ssize_t length;
> + char *buf_tmp = NULL;
> +
> + length = task_has_security(current, SECURITY__PORTCON);
> + if (length)
> + return length;
> +
> + length = security_ocon_port_read(&buf_tmp);
> + return simple_read_from_buffer(buf, count, ppos, buf_tmp, length);
> +}
> +
> +static const struct file_operations sel_ocon_port_ops = {
> + .write = sel_write_ocon_port,
> + .read = sel_read_ocon_port,
> +};
> +
> static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
> size_t count, loff_t *ppos)
> {
> @@ -1596,6 +1691,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
> [SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> [SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
> [SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
> + [SEL_OCON_PORT] = {"portcon", &sel_ocon_port_ops, S_IRUSR|S_IWUSR},
> /* last one */ {""}
> };
> ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index f270e37..3b920e3 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1827,6 +1827,160 @@ err:
>
> }
>
> +int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
> + char *scontext)
> +{
> + int rc = 0;
> + struct ocontext *c;
> + struct context ctx;
> + struct ocontext *add = kzalloc(sizeof(struct ocontext), GFP_KERNEL);
> +
> + if (!add)
> + return -ENOMEM;
> + else if (low > high) {
> + kfree(add);
> + return -EINVAL;
> + }
> +
> + read_lock(&policy_rwlock);
> + rc = string_to_context_struct(&policydb, &sidtab, scontext,
> + strlen(scontext), &ctx, sid);
> + if (rc) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + add->u.port.protocol = protocol;
> + add->u.port.low_port = low;
> + add->u.port.high_port = high;
> + add->sid[0] = sid;
> + add->context[0] = ctx;
> + for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
> + if (add->u.port.protocol == c->u.port.protocol &&
> + add->u.port.low_port == c->u.port.low_port &&
> + add->u.port.high_port == c->u.port.high_port) {
> + printk(KERN_DEBUG "Duplicate netport %d - %d.. "
> + "changing permissions\n",
> + add->u.port.low_port, add->u.port.high_port);
> + c->sid[0] = add->sid[0];
> + context_destroy(&c->context[0]);
> + c->context[0] = add->context[0];
> + context_destroy(&add->context[0]);
> + kfree(add);
> + avc_ss_reset(0);
> + goto out;
> + }
> + }
> +
> + add->next = policydb.ocontexts[OCON_PORT];
> + policydb.ocontexts[OCON_PORT] = add;
> + avc_ss_reset(0);
> +out:
> + if (rc) {
> + context_destroy(&add->context[0]);
> + kfree(add);
> + }
> + read_unlock(&policy_rwlock);
> + return rc;
> +}
> +
> +int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
> + char *scontext)
> +{
> + int rc = 0;
> + struct ocontext *c, *before_c;
> + struct context ctx;
> +
> + if (low > high)
> + return -EINVAL;
> +
> + read_lock(&policy_rwlock);
> + rc = string_to_context_struct(&policydb, &sidtab, scontext,
> + strlen(scontext), &ctx, sid);
> + if (rc) {
> + rc = -EINVAL;
> + goto out;
> + }
> + for (before_c = NULL, c = policydb.ocontexts[OCON_PORT]; c;
> + before_c = c, c = c->next) {
> + if (c->u.port.protocol == protocol &&
> + c->u.port.low_port == low &&
> + c->u.port.high_port == high &&
> + c->context[0].type == ctx.type &&
> + c->context[0].role == ctx.role &&
> + c->context[0].user == ctx.user) {
> + if (before_c == NULL)
> + policydb.ocontexts[OCON_PORT] = c->next;
> + else
> + before_c->next = c->next;
> + context_destroy(&c->context[0]);
> + kfree(c);
> + avc_ss_reset(0);
> + break;
> + }
> + }
> + if (c == NULL) {
> + printk(KERN_DEBUG "Netport not found %d - %d\n", low, high);
> + rc = -ENXIO;
> + }
> +out:
> + context_destroy(&ctx);
> + read_unlock(&policy_rwlock);
> + return rc;
> +}
> +
> +int security_ocon_port_read(char **buf)
> +{
> + unsigned int buf_size = 0, old_size = 0, len;
> + struct ocontext *c;
> + char *scontext;
> + char *buf_tmp = NULL;
> + int rc = 0;
> + int line_size = INITCONTEXTLEN + (sizeof(unsigned int) * 3) +
> + (sizeof(char) * 5);
> + char addline[line_size];
> +
> + /* protocol low-high scontext */
> + read_lock(&policy_rwlock);
> + if (policydb.ocontexts[OCON_PORT] == NULL)
> + goto out;
> +
> + for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
> + rc = context_struct_to_string(&(c->context[0]), &scontext,
> + &len);
> + if (rc < 0) {
> + kfree(buf_tmp);
> + rc = -EINVAL;
> + goto out;
> + }
> + if (len > INITCONTEXTLEN) {
> + kfree(buf_tmp);
> + kfree(scontext);
> + rc = -EOVERFLOW;
> + goto out;
> + }
> + snprintf(addline, line_size, "%u %u-%u %s\n",
> + c->u.port.protocol, c->u.port.low_port,
> + c->u.port.high_port, scontext);
> + kfree(scontext);
> + buf_size += strlen(addline);
> + buf_tmp = krealloc(buf_tmp, buf_size, GFP_KERNEL);
> + if (!buf_tmp) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + for (rc = old_size; rc < buf_size; rc++)
> + buf_tmp[rc] = '\0';
> + strncat(buf_tmp, addline, buf_size);
> + old_size = buf_size;
> + }
> +
> + *buf = buf_tmp;
> +out:
> + read_unlock(&policy_rwlock);
> + return rc;
> +}
> +
> /**
> * security_port_sid - Obtain the SID for a port.
> * @protocol: protocol number
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to [email protected] with
> the words "unsubscribe selinux" without quotes as the message.
>
>
>

2009-12-01 15:28:56

by David P. Quigley

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

I have several questions about this.

1) Where is it located?
2) Is your proposal to implement it as a new fs with a name something
like portfs?
3) How does it get populated initially? Do you have a file for each port
right off the bat? Do you only have files for ports with policy or whose
permissions differ from the default?
4) How do I assign a label to the port? You have an issue here that
these files are objects themselves. You can't just label the file with
what you want the port labeled because now you can't mediate access to
these file objects outside of the label on the port itself.

On Mon, 2009-11-30 at 20:52 -0800, Casey Schaufler wrote:
> Paul Nuzzi wrote:
> > Second version of the dynamic port labeling patch.
>
> So I've looked through both versions of this patch and I can't
> help but think that you'd get better mileage out of a file system
> interface than this SELinux specific implementation. If you had
> something like
>
> /port/22
>
> with default owner root and mode rw-------
>
> /port/3306
>
> with default owner root and mode rw-rw-rw-
>
> you could address a bunch of the complaints about port ownership that
> you hear every day. Further, if the port filesystem supported xattrs
> you could tie in SELinux as easily as you are doing it below and get
> Smack for an extra $1.98, not to mention saving every other LSM the
> grief of defining Yet Another way to define port accesses.
>
> It bothers me that there is a perfectly reasonable way to provide the
> specific behavior you're looking for (SELinux label on a port) that
> generalizes so cleanly and that it's not being proposed.
>
>
> > Changed the name of
> > the selinuxfs interface to portcon and changed the interface to only
> > allow five arguments instead of the variable four or five.
> >
> > Added a mechanism to add/delete/update port labels with an interface in
> > the selinuxfs filesystem. This will give administrators the ability to
> > update port labels faster than reloading the entire policy with
> > semanage. The administrator will also need less privilege since they
> > don't have to be authorized to reload the full policy.
> >
> > A listing of all port labels will be output if the file /selinux/portcon
> > is read. Labels could be added or deleted with the following commands
> >
> > echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22" > /selinux/portcon
> > echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22" > /selinux/portcon
> >
> > Labels can be atomically changed with the add command.
> >
> >
> > Signed-off-by: Paul Nuzzi <[email protected]>
> >
> > ---
> > security/selinux/hooks.c | 1
> > security/selinux/include/classmap.h | 2
> > security/selinux/include/security.h | 9 ++
> > security/selinux/selinuxfs.c | 96 ++++++++++++++++++++++
> > security/selinux/ss/services.c | 154 ++++++++++++++++++++++++++++++++++++
> > 5 files changed, 260 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index a29d661..5aceb74 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1224,7 +1224,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> > struct inode_security_struct *isec = inode->i_security;
> > u32 sid;
> > struct dentry *dentry;
> > -#define INITCONTEXTLEN 255
> > char *context = NULL;
> > unsigned len = 0;
> > int rc = 0;
> > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> > index 8b32e95..925edb6 100644
> > --- a/security/selinux/include/classmap.h
> > +++ b/security/selinux/include/classmap.h
> > @@ -16,7 +16,7 @@ struct security_class_mapping secclass_map[] = {
> > { "compute_av", "compute_create", "compute_member",
> > "check_context", "load_policy", "compute_relabel",
> > "compute_user", "setenforce", "setbool", "setsecparam",
> > - "setcheckreqprot", NULL } },
> > + "setcheckreqprot", "portcon", NULL } },
> > { "process",
> > { "fork", "transition", "sigchld", "sigkill",
> > "sigstop", "signull", "signal", "ptrace", "getsched", "setsched",
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index 2553266..25a5054 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -169,6 +169,15 @@ int security_fs_use(const char *fstype, unsigned int *behavior,
> > int security_genfs_sid(const char *fstype, char *name, u16 sclass,
> > u32 *sid);
> >
> > +int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
> > + char *scontext);
> > +
> > +int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
> > + char *scontext);
> > +
> > +int security_ocon_port_read(char **buf);
> > +#define INITCONTEXTLEN 255
> > +
> > #ifdef CONFIG_NETLABEL
> > int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr,
> > u32 *sid);
> > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> > index fab36fd..a21be6c 100644
> > --- a/security/selinux/selinuxfs.c
> > +++ b/security/selinux/selinuxfs.c
> > @@ -110,6 +110,7 @@ enum sel_inos {
> > SEL_COMPAT_NET, /* whether to use old compat network packet controls */
> > SEL_REJECT_UNKNOWN, /* export unknown reject handling to userspace */
> > SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
> > + SEL_OCON_PORT, /* add OCON_PORT to the list */
> > SEL_INO_NEXT, /* The next inode number to use */
> > };
> >
> > @@ -253,6 +254,100 @@ static const struct file_operations sel_disable_ops = {
> > .write = sel_write_disable,
> > };
> >
> > +static ssize_t sel_write_ocon_port(struct file *file, const char __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + ssize_t length;
> > + unsigned int low = 0, high = 0, protocol = 0;
> > + char *page, *scontext, *command;
> > + u32 sid;
> > +#define OCONCOMMANDLEN 4
> > +#define OCON_ADD_COMMAND "add"
> > +#define OCON_DEL_COMMAND "del"
> > +
> > + length = task_has_security(current, SECURITY__PORTCON);
> > + if (length)
> > + return length;
> > +
> > + if (count >= PAGE_SIZE)
> > + return -ENOMEM;
> > + if (*ppos != 0) {
> > + /* No partial writes. */
> > + return -EINVAL;
> > + }
> > + page = (char *)get_zeroed_page(GFP_KERNEL);
> > + if (!page)
> > + goto nomem;
> > + scontext = kzalloc(INITCONTEXTLEN, GFP_KERNEL);
> > + if (!scontext)
> > + goto nomem1;
> > + command = kzalloc(OCONCOMMANDLEN, GFP_KERNEL);
> > + if (!command)
> > + goto nomem2;
> > +
> > + length = -EFAULT;
> > + if (copy_from_user(page, buf, count))
> > + goto out;
> > +
> > + length = sscanf(page, "%3s %255s %u %u %u", command, scontext,
> > + &protocol, &low, &high);
> > + if (length < 5) {
> > + length = -EINVAL;
> > + goto out;
> > + }
> > + length = security_context_to_sid(scontext, strlen(scontext), &sid);
> > + if (length < 0) {
> > + length = -EINVAL;
> > + goto out;
> > + }
> > +
> > + if (strncmp(OCON_ADD_COMMAND, command, strlen(OCON_ADD_COMMAND)) == 0)
> > + length = security_ocon_port_add(protocol, low, high, sid,
> > + scontext);
> > + else if (strncmp(OCON_DEL_COMMAND, command, strlen(OCON_DEL_COMMAND))
> > + == 0)
> > + length = security_ocon_port_del(protocol, low, high, sid,
> > + scontext);
> > + else {
> > + length = -EINVAL;
> > + goto out;
> > + }
> > + if (length < 0)
> > + goto out;
> > + length = count;
> > +out:
> > + kfree(command);
> > + kfree(scontext);
> > + free_page((unsigned long) page);
> > + return length;
> > +
> > +nomem2:
> > + kfree(scontext);
> > +nomem1:
> > + free_page((unsigned long) page);
> > +nomem:
> > + return -ENOMEM;
> > +}
> > +
> > +static ssize_t sel_read_ocon_port(struct file *filp, char __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + ssize_t length;
> > + char *buf_tmp = NULL;
> > +
> > + length = task_has_security(current, SECURITY__PORTCON);
> > + if (length)
> > + return length;
> > +
> > + length = security_ocon_port_read(&buf_tmp);
> > + return simple_read_from_buffer(buf, count, ppos, buf_tmp, length);
> > +}
> > +
> > +static const struct file_operations sel_ocon_port_ops = {
> > + .write = sel_write_ocon_port,
> > + .read = sel_read_ocon_port,
> > +};
> > +
> > static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
> > size_t count, loff_t *ppos)
> > {
> > @@ -1596,6 +1691,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
> > [SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> > [SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
> > [SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
> > + [SEL_OCON_PORT] = {"portcon", &sel_ocon_port_ops, S_IRUSR|S_IWUSR},
> > /* last one */ {""}
> > };
> > ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index f270e37..3b920e3 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -1827,6 +1827,160 @@ err:
> >
> > }
> >
> > +int security_ocon_port_add(u16 protocol, u16 low, u16 high, u32 sid,
> > + char *scontext)
> > +{
> > + int rc = 0;
> > + struct ocontext *c;
> > + struct context ctx;
> > + struct ocontext *add = kzalloc(sizeof(struct ocontext), GFP_KERNEL);
> > +
> > + if (!add)
> > + return -ENOMEM;
> > + else if (low > high) {
> > + kfree(add);
> > + return -EINVAL;
> > + }
> > +
> > + read_lock(&policy_rwlock);
> > + rc = string_to_context_struct(&policydb, &sidtab, scontext,
> > + strlen(scontext), &ctx, sid);
> > + if (rc) {
> > + rc = -EINVAL;
> > + goto out;
> > + }
> > +
> > + add->u.port.protocol = protocol;
> > + add->u.port.low_port = low;
> > + add->u.port.high_port = high;
> > + add->sid[0] = sid;
> > + add->context[0] = ctx;
> > + for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
> > + if (add->u.port.protocol == c->u.port.protocol &&
> > + add->u.port.low_port == c->u.port.low_port &&
> > + add->u.port.high_port == c->u.port.high_port) {
> > + printk(KERN_DEBUG "Duplicate netport %d - %d.. "
> > + "changing permissions\n",
> > + add->u.port.low_port, add->u.port.high_port);
> > + c->sid[0] = add->sid[0];
> > + context_destroy(&c->context[0]);
> > + c->context[0] = add->context[0];
> > + context_destroy(&add->context[0]);
> > + kfree(add);
> > + avc_ss_reset(0);
> > + goto out;
> > + }
> > + }
> > +
> > + add->next = policydb.ocontexts[OCON_PORT];
> > + policydb.ocontexts[OCON_PORT] = add;
> > + avc_ss_reset(0);
> > +out:
> > + if (rc) {
> > + context_destroy(&add->context[0]);
> > + kfree(add);
> > + }
> > + read_unlock(&policy_rwlock);
> > + return rc;
> > +}
> > +
> > +int security_ocon_port_del(u16 protocol, u16 low, u16 high, u32 sid,
> > + char *scontext)
> > +{
> > + int rc = 0;
> > + struct ocontext *c, *before_c;
> > + struct context ctx;
> > +
> > + if (low > high)
> > + return -EINVAL;
> > +
> > + read_lock(&policy_rwlock);
> > + rc = string_to_context_struct(&policydb, &sidtab, scontext,
> > + strlen(scontext), &ctx, sid);
> > + if (rc) {
> > + rc = -EINVAL;
> > + goto out;
> > + }
> > + for (before_c = NULL, c = policydb.ocontexts[OCON_PORT]; c;
> > + before_c = c, c = c->next) {
> > + if (c->u.port.protocol == protocol &&
> > + c->u.port.low_port == low &&
> > + c->u.port.high_port == high &&
> > + c->context[0].type == ctx.type &&
> > + c->context[0].role == ctx.role &&
> > + c->context[0].user == ctx.user) {
> > + if (before_c == NULL)
> > + policydb.ocontexts[OCON_PORT] = c->next;
> > + else
> > + before_c->next = c->next;
> > + context_destroy(&c->context[0]);
> > + kfree(c);
> > + avc_ss_reset(0);
> > + break;
> > + }
> > + }
> > + if (c == NULL) {
> > + printk(KERN_DEBUG "Netport not found %d - %d\n", low, high);
> > + rc = -ENXIO;
> > + }
> > +out:
> > + context_destroy(&ctx);
> > + read_unlock(&policy_rwlock);
> > + return rc;
> > +}
> > +
> > +int security_ocon_port_read(char **buf)
> > +{
> > + unsigned int buf_size = 0, old_size = 0, len;
> > + struct ocontext *c;
> > + char *scontext;
> > + char *buf_tmp = NULL;
> > + int rc = 0;
> > + int line_size = INITCONTEXTLEN + (sizeof(unsigned int) * 3) +
> > + (sizeof(char) * 5);
> > + char addline[line_size];
> > +
> > + /* protocol low-high scontext */
> > + read_lock(&policy_rwlock);
> > + if (policydb.ocontexts[OCON_PORT] == NULL)
> > + goto out;
> > +
> > + for (c = policydb.ocontexts[OCON_PORT]; c; c = c->next) {
> > + rc = context_struct_to_string(&(c->context[0]), &scontext,
> > + &len);
> > + if (rc < 0) {
> > + kfree(buf_tmp);
> > + rc = -EINVAL;
> > + goto out;
> > + }
> > + if (len > INITCONTEXTLEN) {
> > + kfree(buf_tmp);
> > + kfree(scontext);
> > + rc = -EOVERFLOW;
> > + goto out;
> > + }
> > + snprintf(addline, line_size, "%u %u-%u %s\n",
> > + c->u.port.protocol, c->u.port.low_port,
> > + c->u.port.high_port, scontext);
> > + kfree(scontext);
> > + buf_size += strlen(addline);
> > + buf_tmp = krealloc(buf_tmp, buf_size, GFP_KERNEL);
> > + if (!buf_tmp) {
> > + rc = -ENOMEM;
> > + goto out;
> > + }
> > + for (rc = old_size; rc < buf_size; rc++)
> > + buf_tmp[rc] = '\0';
> > + strncat(buf_tmp, addline, buf_size);
> > + old_size = buf_size;
> > + }
> > +
> > + *buf = buf_tmp;
> > +out:
> > + read_unlock(&policy_rwlock);
> > + return rc;
> > +}
> > +
> > /**
> > * security_port_sid - Obtain the SID for a port.
> > * @protocol: protocol number
> >
> >
> >
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to [email protected] with
> > the words "unsubscribe selinux" without quotes as the message.
> >
> >
> >
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to [email protected] with
> the words "unsubscribe selinux" without quotes as the message.

2009-12-01 15:39:43

by Paul Nuzzi

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Dave brought up some good questions. A pseudo file-system based on ports
seems like a good idea but I think there is going be scaling issues.
SELinux also supports multiple labels for ports. Port 22 can be labeled
as ssh_port and if the label is removed, reverts to reserved_port. Do
you know of a way xattrs can handle this?


On Tue, 2009-12-01 at 10:06 -0500, David P. Quigley wrote:
> I have several questions about this.
>
> 1) Where is it located?
> 2) Is your proposal to implement it as a new fs with a name something
> like portfs?
> 3) How does it get populated initially? Do you have a file for each port
> right off the bat? Do you only have files for ports with policy or whose
> permissions differ from the default?
> 4) How do I assign a label to the port? You have an issue here that
> these files are objects themselves. You can't just label the file with
> what you want the port labeled because now you can't mediate access to
> these file objects outside of the label on the port itself.
>
> On Mon, 2009-11-30 at 20:52 -0800, Casey Schaufler wrote:
> > Paul Nuzzi wrote:
> > > Second version of the dynamic port labeling patch.
> >
> > So I've looked through both versions of this patch and I can't
> > help but think that you'd get better mileage out of a file system
> > interface than this SELinux specific implementation. If you had
> > something like
> >
> > /port/22
> >
> > with default owner root and mode rw-------
> >
> > /port/3306
> >
> > with default owner root and mode rw-rw-rw-
> >
> > you could address a bunch of the complaints about port ownership that
> > you hear every day. Further, if the port filesystem supported xattrs
> > you could tie in SELinux as easily as you are doing it below and get
> > Smack for an extra $1.98, not to mention saving every other LSM the
> > grief of defining Yet Another way to define port accesses.
> >
> > It bothers me that there is a perfectly reasonable way to provide the
> > specific behavior you're looking for (SELinux label on a port) that
> > generalizes so cleanly and that it's not being proposed.
> >
> >
> > > Changed the name of
> > > the selinuxfs interface to portcon and changed the interface to only
> > > allow five arguments instead of the variable four or five.
> > >
> > > Added a mechanism to add/delete/update port labels with an interface in
> > > the selinuxfs filesystem. This will give administrators the ability to
> > > update port labels faster than reloading the entire policy with
> > > semanage. The administrator will also need less privilege since they
> > > don't have to be authorized to reload the full policy.
> > >
> > > A listing of all port labels will be output if the file /selinux/portcon
> > > is read. Labels could be added or deleted with the following commands
> > >
> > > echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22" > /selinux/portcon
> > > echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22" > /selinux/portcon
> > >
> > > Labels can be atomically changed with the add command.
> > >
> > >
> > > Signed-off-by: Paul Nuzzi <[email protected]>
> > >

2009-12-02 02:38:40

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Paul Nuzzi wrote:
> Dave brought up some good questions.

He often does. He's like that.

> A pseudo file-system based on ports
> seems like a good idea but I think there is going be scaling issues.
>

I'm curious what those scaling issues might be, given the success
of /proc which deals with many processes.

> SELinux also supports multiple labels for ports.

Is there a fixed set? There's no reason you couldn't do what Smack
does with sockets, which is multiple well known attribute names.
That will work even if there isn't a fixed set, come to think of
it.

> Port 22 can be labeled
> as ssh_port and if the label is removed, reverts to reserved_port. Do
> you know of a way xattrs can handle this?
>

setxattr SELINIX_PORTXATTR_UNLABELED_CONTEXT reserved_port
setxattr SELINIX_PORTXATTR_CONTEXT ssh_port

if you delete the SELINIX_PORTXATTR_CONTEXT attribute it gets
reset to SELINIX_PORTXATTR_UNLABELED_CONTEXT. Simple.

> On Tue, 2009-12-01 at 10:06 -0500, David P. Quigley wrote:
>
>> I have several questions about this.
>>
>> 1) Where is it located?
>>

Don't know, don't much care. /sys or /sysfs or /security or /dev.

>> 2) Is your proposal to implement it as a new fs with a name something
>> like portfs?
>>

Yes. Alternatively it could be an extension to an existing special
purpose filesystem, but I don't have a favorite candidate.

>> 3) How does it get populated initially? Do you have a file for each port
>> right off the bat? Do you only have files for ports with policy or whose
>> permissions differ from the default?
>>

I like the notion of populating it all at once, but I expect
that there are those who'd holler for doing in on demand.

>> 4) How do I assign a label to the port? You have an issue here that
>> these files are objects themselves. You can't just label the file with
>> what you want the port labeled because now you can't mediate access to
>> these file objects outside of the label on the port itself.
>>

On some systems you could in fact put the label of the bound process
on the object and be very happy, but I understand that SELinux is
not an example of such a system.

Put as many xattrs on the object as you like and let the LSM decide
which matters to what purpose. setxattr to put labels explicitly on
the port, and the LSM gets to decide what xattr to put on the port
when it is bound or closed. Of course, there will need to be a way
to associate the portfs entry with the port (I don't know, maybe the
port number gets you the inode number as /proc maps pid to inode?)
Yes, that implies LSM specific code in portfs, or maybe portfs
specific code in the LSM. Smack already treats attribute setting
specially on socket fd's, and it's not so terrible.

>> On Mon, 2009-11-30 at 20:52 -0800, Casey Schaufler wrote:
>>
>>> Paul Nuzzi wrote:
>>>
>>>> Second version of the dynamic port labeling patch.
>>>>
>>> So I've looked through both versions of this patch and I can't
>>> help but think that you'd get better mileage out of a file system
>>> interface than this SELinux specific implementation. If you had
>>> something like
>>>
>>> /port/22
>>>
>>> with default owner root and mode rw-------
>>>
>>> /port/3306
>>>
>>> with default owner root and mode rw-rw-rw-
>>>
>>> you could address a bunch of the complaints about port ownership that
>>> you hear every day. Further, if the port filesystem supported xattrs
>>> you could tie in SELinux as easily as you are doing it below and get
>>> Smack for an extra $1.98, not to mention saving every other LSM the
>>> grief of defining Yet Another way to define port accesses.
>>>
>>> It bothers me that there is a perfectly reasonable way to provide the
>>> specific behavior you're looking for (SELinux label on a port) that
>>> generalizes so cleanly and that it's not being proposed.
>>>
>>>
>>>
>>>> Changed the name of
>>>> the selinuxfs interface to portcon and changed the interface to only
>>>> allow five arguments instead of the variable four or five.
>>>>
>>>> Added a mechanism to add/delete/update port labels with an interface in
>>>> the selinuxfs filesystem. This will give administrators the ability to
>>>> update port labels faster than reloading the entire policy with
>>>> semanage. The administrator will also need less privilege since they
>>>> don't have to be authorized to reload the full policy.
>>>>
>>>> A listing of all port labels will be output if the file /selinux/portcon
>>>> is read. Labels could be added or deleted with the following commands
>>>>
>>>> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22" > /selinux/portcon
>>>> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22" > /selinux/portcon
>>>>
>>>> Labels can be atomically changed with the add command.
>>>>
>>>>
>>>> Signed-off-by: Paul Nuzzi <[email protected]>
>>>>
>>>>
>
>
>
>

2009-12-03 19:39:29

by Joshua Brindle

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Paul Nuzzi wrote:
> Second version of the dynamic port labeling patch. Changed the name of
> the selinuxfs interface to portcon and changed the interface to only
> allow five arguments instead of the variable four or five.
>
> Added a mechanism to add/delete/update port labels with an interface in
> the selinuxfs filesystem. This will give administrators the ability to
> update port labels faster than reloading the entire policy with
> semanage. The administrator will also need less privilege since they
> don't have to be authorized to reload the full policy.
>
> A listing of all port labels will be output if the file /selinux/portcon
> is read. Labels could be added or deleted with the following commands
>
> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22"> /selinux/portcon
> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22"> /selinux/portcon
>

Aside from the conversation Dave and Casey are having I still think this
isn't quite right. First, while you can atomically change a single port
label with the add command above you can't atomically change multiple
entries, which I think is completely necessary (you don't want to have
strange labeling states when changing a set of ports to a new label.

Also, if you are dealing with ranges you need to essentially pop off all
the specific ports, change the range and push all the specific ports
back on. With the current interface I don't see how that is possible at
all.

Also, while having a text parser in the kernel makes it easier to use
with echo I think it is alot of code in the kernel for no good reason.
There is no reason not to make a userspace tool that converts the
textual representation into a serialized struct and feed it to the
kernel. We typically tell users not to mess around in /selinux anyway,
since we have a libselinux interface to do that.

We also need to be able to get that information back out somehow, and we
need to be able to keep the on-disk policy consistent with the changes
we are making at runtime. setsebool -P does this, but it rebuilds the
policy, which you are trying to avoid. How do you make these portcon
changes persist across reboots? I don't imagine very many scenarios
where you only want to temporarily change portcons.

It seems like you'd need to manage an on-disk file of all the ports and
load them right after loading the policy (which is still racy but the
default port sid should prevent any traffic on the ports.

2009-12-04 00:21:13

by Russell Coker

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

On Fri, 4 Dec 2009, Joshua Brindle <[email protected]> wrote:
> Aside from the conversation Dave and Casey are having I still think this
> isn't quite right. First, while you can atomically change a single port
> label with the add command above you can't atomically change multiple
> entries, which I think is completely necessary (you don't want to have
> strange labeling states when changing a set of ports to a new label.

Why is it necessary to change multiple ports at the same time?

We support atomic changes of multiple booleans at the same time due to
possible interactions between them. But I don't think that we have any such
issues with port contexts.

> Also, while having a text parser in the kernel makes it easier to use
> with echo I think it is alot of code in the kernel for no good reason.
> There is no reason not to make a userspace tool that converts the
> textual representation into a serialized struct and feed it to the
> kernel. We typically tell users not to mess around in /selinux anyway,
> since we have a libselinux interface to do that.

It does seem likely that significant code complexity can be avoided by not
having a plain text interface in this case.

--
[email protected]
http://etbe.coker.com.au/ My Main Blog
http://doc.coker.com.au/ My Documents Blog

2009-12-04 14:42:43

by Paul Nuzzi

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

On Thu, 2009-12-03 at 14:31 -0500, Joshua Brindle wrote:
> Paul Nuzzi wrote:
> > Second version of the dynamic port labeling patch. Changed the name of
> > the selinuxfs interface to portcon and changed the interface to only
> > allow five arguments instead of the variable four or five.
> >
> > Added a mechanism to add/delete/update port labels with an interface in
> > the selinuxfs filesystem. This will give administrators the ability to
> > update port labels faster than reloading the entire policy with
> > semanage. The administrator will also need less privilege since they
> > don't have to be authorized to reload the full policy.
> >
> > A listing of all port labels will be output if the file /selinux/portcon
> > is read. Labels could be added or deleted with the following commands
> >
> > echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22"> /selinux/portcon
> > echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22"> /selinux/portcon
> >
>
> Aside from the conversation Dave and Casey are having I still think this
> isn't quite right. First, while you can atomically change a single port
> label with the add command above you can't atomically change multiple
> entries, which I think is completely necessary (you don't want to have
> strange labeling states when changing a set of ports to a new label.

Can you think of a situation where we would need to atomically change
multiple entries? I envisioned a sys admin stopping their application
or server, relabeling the ports and then restarting them. Maybe there
is a specific case you know about that I've overlooked?

> Also, if you are dealing with ranges you need to essentially pop off all
> the specific ports, change the range and push all the specific ports
> back on. With the current interface I don't see how that is possible at
> all.

If you want to change the label on a range you can do it with the atomic
add operation. The only time you would need to pop all the ports and
push them back is resizing a range.

> Also, while having a text parser in the kernel makes it easier to use
> with echo I think it is alot of code in the kernel for no good reason.
> There is no reason not to make a userspace tool that converts the
> textual representation into a serialized struct and feed it to the
> kernel. We typically tell users not to mess around in /selinux anyway,
> since we have a libselinux interface to do that.
>
> We also need to be able to get that information back out somehow, and we
> need to be able to keep the on-disk policy consistent with the changes
> we are making at runtime. setsebool -P does this, but it rebuilds the
> policy, which you are trying to avoid. How do you make these portcon
> changes persist across reboots? I don't imagine very many scenarios
> where you only want to temporarily change portcons.
>
> It seems like you'd need to manage an on-disk file of all the ports and
> load them right after loading the policy (which is still racy but the
> default port sid should prevent any traffic on the ports.

There is no question that a userspace tool like setsebool will have to
be written to save the modified policy. I used a text parsing interface
to stay consistent with the current selinuxfs interfaces where you can
echo numbers into files to modify functionality. Would adding a
structure ingesting write interface break consistency?

>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to [email protected] with
> the words "unsubscribe selinux" without quotes as the message.

2009-12-04 16:03:54

by Joshua Brindle

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Paul Nuzzi wrote:
> On Thu, 2009-12-03 at 14:31 -0500, Joshua Brindle wrote:
>> Paul Nuzzi wrote:
>>> Second version of the dynamic port labeling patch. Changed the name of
>>> the selinuxfs interface to portcon and changed the interface to only
>>> allow five arguments instead of the variable four or five.
>>>
>>> Added a mechanism to add/delete/update port labels with an interface in
>>> the selinuxfs filesystem. This will give administrators the ability to
>>> update port labels faster than reloading the entire policy with
>>> semanage. The administrator will also need less privilege since they
>>> don't have to be authorized to reload the full policy.
>>>
>>> A listing of all port labels will be output if the file /selinux/portcon
>>> is read. Labels could be added or deleted with the following commands
>>>
>>> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22"> /selinux/portcon
>>> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22"> /selinux/portcon
>>>
>> Aside from the conversation Dave and Casey are having I still think this
>> isn't quite right. First, while you can atomically change a single port
>> label with the add command above you can't atomically change multiple
>> entries, which I think is completely necessary (you don't want to have
>> strange labeling states when changing a set of ports to a new label.
>
> Can you think of a situation where we would need to atomically change
> multiple entries? I envisioned a sys admin stopping their application
> or server, relabeling the ports and then restarting them. Maybe there
> is a specific case you know about that I've overlooked?
>

Maybe not, and it isn't like labeling filesystems is atomic so maybe I'm
out in left field here.

>> Also, if you are dealing with ranges you need to essentially pop off all
>> the specific ports, change the range and push all the specific ports
>> back on. With the current interface I don't see how that is possible at
>> all.
>
> If you want to change the label on a range you can do it with the atomic
> add operation. The only time you would need to pop all the ports and
> push them back is resizing a range.
>

I actually was thinking about resizing a range. You just put the system
in a strange state by having to remove alot of labels and then readd
them. It seems most reliable to add the entire set of portcons every
time (using some file on disk and a text parser to parse them in to an
ocontext like struct then feeding it in), that way the ordering is
always exactly like it is in the file and there is a persistent file on
disk that can be loaded at policy load time.

Users could literally do a setportcon -e to pop up an editor with all
the portcons and reorder, modify, re-range, etc and it would all take
effect at once.

I don't know though, I may be overthinking this. Right now isn't ideal,
portcons have to be stored in a libsemanage "database" and they get
added to the policydb at policy build time. You could generate out a
portcon file that sits next to the policy.24 and gets fed in at load
time but making modifications to that and keeping them in sync would be
a pain, unless libsemanage made the change and regenerated the file (it
would be cheaper than rebuilding the entire policy) but that would also
mean you'd have to modify libsemanage which I typically don't wish on
anyone.


I guess one issue I'm having is that in SELinux there are about 3 ways
to do any 1 think in the typical case (sometimes there are 5 or more).
And this adds a completely orthogonal and not integrated with the rest
way of changing port labels, which isn't ideal. We already have trouble
telling people how to choose one over the other in many cases.

>> Also, while having a text parser in the kernel makes it easier to use
>> with echo I think it is alot of code in the kernel for no good reason.
>> There is no reason not to make a userspace tool that converts the
>> textual representation into a serialized struct and feed it to the
>> kernel. We typically tell users not to mess around in /selinux anyway,
>> since we have a libselinux interface to do that.
>>
>> We also need to be able to get that information back out somehow, and we
>> need to be able to keep the on-disk policy consistent with the changes
>> we are making at runtime. setsebool -P does this, but it rebuilds the
>> policy, which you are trying to avoid. How do you make these portcon
>> changes persist across reboots? I don't imagine very many scenarios
>> where you only want to temporarily change portcons.
>>
>> It seems like you'd need to manage an on-disk file of all the ports and
>> load them right after loading the policy (which is still racy but the
>> default port sid should prevent any traffic on the ports.
>
> There is no question that a userspace tool like setsebool will have to
> be written to save the modified policy. I used a text parsing interface
> to stay consistent with the current selinuxfs interfaces where you can
> echo numbers into files to modify functionality. Would adding a
> structure ingesting write interface break consistency?
>

Sort of. In some cases you can echo numbers in like booleans and enforce
and others you can't like load, all the compute functions, etc. We don't
have to do text parsing yet and that adds a bunch of stuff to the kernel
that isn't completely necessary. I'll defer to the kernel guys on this
one though, since it isn't my area of expertise.

2009-12-07 17:32:34

by Paul Nuzzi

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

On Fri, 2009-12-04 at 11:03 -0500, Joshua Brindle wrote:
> Paul Nuzzi wrote:
> > On Thu, 2009-12-03 at 14:31 -0500, Joshua Brindle wrote:
> >> Paul Nuzzi wrote:
> >>> Second version of the dynamic port labeling patch. Changed the name of
> >>> the selinuxfs interface to portcon and changed the interface to only
> >>> allow five arguments instead of the variable four or five.
> >>>
> >>> Added a mechanism to add/delete/update port labels with an interface in
> >>> the selinuxfs filesystem. This will give administrators the ability to
> >>> update port labels faster than reloading the entire policy with
> >>> semanage. The administrator will also need less privilege since they
> >>> don't have to be authorized to reload the full policy.
> >>>
> >>> A listing of all port labels will be output if the file /selinux/portcon
> >>> is read. Labels could be added or deleted with the following commands
> >>>
> >>> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22"> /selinux/portcon
> >>> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22"> /selinux/portcon
> >>>
> >> Aside from the conversation Dave and Casey are having I still think this
> >> isn't quite right. First, while you can atomically change a single port
> >> label with the add command above you can't atomically change multiple
> >> entries, which I think is completely necessary (you don't want to have
> >> strange labeling states when changing a set of ports to a new label.
> >
> > Can you think of a situation where we would need to atomically change
> > multiple entries? I envisioned a sys admin stopping their application
> > or server, relabeling the ports and then restarting them. Maybe there
> > is a specific case you know about that I've overlooked?
> >
>
> Maybe not, and it isn't like labeling filesystems is atomic so maybe I'm
> out in left field here.
>
> >> Also, if you are dealing with ranges you need to essentially pop off all
> >> the specific ports, change the range and push all the specific ports
> >> back on. With the current interface I don't see how that is possible at
> >> all.
> >
> > If you want to change the label on a range you can do it with the atomic
> > add operation. The only time you would need to pop all the ports and
> > push them back is resizing a range.
> >
>
> I actually was thinking about resizing a range. You just put the system
> in a strange state by having to remove alot of labels and then readd
> them. It seems most reliable to add the entire set of portcons every
> time (using some file on disk and a text parser to parse them in to an
> ocontext like struct then feeding it in), that way the ordering is
> always exactly like it is in the file and there is a persistent file on
> disk that can be loaded at policy load time.
>
> Users could literally do a setportcon -e to pop up an editor with all
> the portcons and reorder, modify, re-range, etc and it would all take
> effect at once.
>
> I don't know though, I may be overthinking this. Right now isn't ideal,
> portcons have to be stored in a libsemanage "database" and they get
> added to the policydb at policy build time. You could generate out a
> portcon file that sits next to the policy.24 and gets fed in at load
> time but making modifications to that and keeping them in sync would be
> a pain, unless libsemanage made the change and regenerated the file (it
> would be cheaper than rebuilding the entire policy) but that would also
> mean you'd have to modify libsemanage which I typically don't wish on
> anyone.
>

Sounds like your idea would present some a few problems. To implement
this functionality user-space would have to take out a policy lock in
the kernel for an indefinite amount of time. That brings up some issues
on scalability. Typically atomic operations are small and fast. This
also sounds like a toolchain issue. The proposed functionality can be
done in the front end of semanage or setportcon.

> I guess one issue I'm having is that in SELinux there are about 3 ways
> to do any 1 think in the typical case (sometimes there are 5 or more).
> And this adds a completely orthogonal and not integrated with the rest
> way of changing port labels, which isn't ideal. We already have trouble
> telling people how to choose one over the other in many cases.
>
> >> Also, while having a text parser in the kernel makes it easier to use
> >> with echo I think it is alot of code in the kernel for no good reason.
> >> There is no reason not to make a userspace tool that converts the
> >> textual representation into a serialized struct and feed it to the
> >> kernel. We typically tell users not to mess around in /selinux anyway,
> >> since we have a libselinux interface to do that.
> >>
> >> We also need to be able to get that information back out somehow, and we
> >> need to be able to keep the on-disk policy consistent with the changes
> >> we are making at runtime. setsebool -P does this, but it rebuilds the
> >> policy, which you are trying to avoid. How do you make these portcon
> >> changes persist across reboots? I don't imagine very many scenarios
> >> where you only want to temporarily change portcons.
> >>
> >> It seems like you'd need to manage an on-disk file of all the ports and
> >> load them right after loading the policy (which is still racy but the
> >> default port sid should prevent any traffic on the ports.
> >
> > There is no question that a userspace tool like setsebool will have to
> > be written to save the modified policy. I used a text parsing interface
> > to stay consistent with the current selinuxfs interfaces where you can
> > echo numbers into files to modify functionality. Would adding a
> > structure ingesting write interface break consistency?
> >
>
> Sort of. In some cases you can echo numbers in like booleans and enforce
> and others you can't like load, all the compute functions, etc. We don't
> have to do text parsing yet and that adds a bunch of stuff to the kernel
> that isn't completely necessary. I'll defer to the kernel guys on this
> one though, since it isn't my area of expertise.
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to [email protected] with
> the words "unsubscribe selinux" without quotes as the message.

2009-12-18 05:33:46

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

On Fri, Dec 4, 2009 at 11:03, Joshua Brindle <[email protected]> wrote:
> Paul Nuzzi wrote:
>> On Thu, 2009-12-03 at 14:31 -0500, Joshua Brindle wrote:
>>> Paul Nuzzi wrote:
>>>> Added a mechanism to add/delete/update port labels with an interface in
>>>> the selinuxfs filesystem.  This will give administrators the ability to
>>>> update port labels faster than reloading the entire policy with
>>>> semanage.  The administrator will also need less privilege since they
>>>> don't have to be authorized to reload the full policy.
>>>>
>>>> A listing of all port labels will be output if the file /selinux/portcon
>>>> is read.  Labels could be added or deleted with the following commands
>>>>
>>>> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22"> /selinux/portcon
>>>> echo -n "add system_u:object_r:telnetd_port_t:s0 6 22 22"> /selinux/portcon
>>>>
>>> Aside from the conversation Dave and Casey are having I still think this
>>> isn't quite right. First, while you can atomically change a single port
>>> label with the add command above you can't atomically change multiple
>>> entries, which I think is completely necessary (you don't want to have
>>> strange labeling states when changing a set of ports to a new label.
>>
>> Can you think of a situation where we would need to atomically change
>> multiple entries?  I envisioned a sys admin stopping their application
>> or server, relabeling the ports and then restarting them.  Maybe there
>> is a specific case you know about that I've overlooked?
>>
>
> Maybe not, and it isn't like labeling filesystems is atomic so maybe I'm out
> in left field here.
>
> [...]
>
> I actually was thinking about resizing a range. You just put the system in a
> strange state by having to remove alot of labels and then readd them. It
> seems most reliable to add the entire set of portcons every time (using some
> file on disk and a text parser to parse them in to an ocontext like struct
> then feeding it in), that way the ordering is always exactly like it is in
> the file and there is a persistent file on disk that can be loaded at policy
> load time.
>
> Users could literally do a setportcon -e to pop up an editor with all the
> portcons and reorder, modify, re-range, etc and it would all take effect at
> once.

Hrm, wow.... this seems like a whole lot of re-engineering of work
that has already been done and works well, specifically iptables.

The iptables code already allows almost any combination of access
rules, with conditionals, nested filters, ranges, etc... and it has a
design that allows atomic replacement of the entire firewall ruleset.

Furthermore, using tools like Shorewall (http://shorewall.net), I can
customize exactly what my filtering and forwarding rules are using
simple human-readable configuration files. Shorewall even generates
an iptables restore file so I either get the old set of firewall rules
or the new set, nothing halfway in-between.

I know that SELinux labeling is already partially supported through
the security table... can't we figure out some way to extend that to
this port-based labeling as well? Another thing that would be nice is
to be able to apply some amount of discrete access control to firewall
rule updates, so that a "network admin" could adjust most firewall
rules, while the "security admin" would be responsible for packet and
network interface labeling.

I have to say... looking at IPsec+iptables solutions for robust and
secure packet labeling to IP port-based security labels seems a big
step backwards to me.

Putting my various admin hats on, what I would *LOVE* to have from a
security standpoint would be something along the lines of
setroubleshoot, but designed to handle audits from many computer
systems and to allow communication between people with different
security roles. Here's an example use-case:

So say a web developer is preparing to roll out a big application
install, and needs to add a new HTTP listen port (say, 8083). She
tells apache to listen on that port and gets a "permission denied",
along with a notification of some kind that her action has triggered a
security warning. As a result, she goes ahead and fills out a
"network security" change request form, and then puts the ticket
number into the SETroubleshoot interface tagged to the security
warning.

A few minutes later, the network security guy sits down and opens up
his audit window and sees the warning that his friendly web developer
triggered. He checks the trouble ticket and updates the HTTP
type-enforcement rules to allow that port as well, then sends off a
notice to the web developer that everything is OK.

The web developer then works with his users to beta-test the web
application and discovers that his software is unable to talk to one
of its database backends on another server because that database has
"sensitive financial" data and requires labelled IPsec connections to
access it. Those audit messages (even from the same server) then go
to a separate "data security" person, who must himself add the new
labelling rules to allow that software access to some financial data.

I could almost build this system today with a bunch of custom
userspace firewall management and audit-log filtering software... but
I have no real way of actually restricting my "data security" person
from fiddling with the HTTP port labelling, nor can I prevent my basic
network security guy from changing my fancy labelled IPsec rules.

Cheers,
Kyle Moffett

2009-12-18 15:38:37

by Joshua Brindle

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

Paul Nuzzi wrote:
> On Fri, 2009-12-04 at 11:03 -0500, Joshua Brindle wrote:
>> effect at once.
>>
>> I don't know though, I may be overthinking this. Right now isn't ideal,
>> portcons have to be stored in a libsemanage "database" and they get
>> added to the policydb at policy build time. You could generate out a
>> portcon file that sits next to the policy.24 and gets fed in at load
>> time but making modifications to that and keeping them in sync would be
>> a pain, unless libsemanage made the change and regenerated the file (it
>> would be cheaper than rebuilding the entire policy) but that would also
>> mean you'd have to modify libsemanage which I typically don't wish on
>> anyone.
>>
>
> Sounds like your idea would present some a few problems. To implement
> this functionality user-space would have to take out a policy lock in
> the kernel for an indefinite amount of time. That brings up some issues
> on scalability. Typically atomic operations are small and fast. This
> also sounds like a toolchain issue. The proposed functionality can be
> done in the front end of semanage or setportcon.
>

Thanks to Kyle for digging this thread back up.

I understand there are issues (though I disagree with your assertion
above wrt policy lock, you'd just need to fill in an ocontext and take a
lock to switch the pointer IIUC). I don't like this "well, its a
toolchain issue". Sure it is, and the toolchain issue should be solved
before the kernel code goes forward.

Adding something to the kernel and then figuring out how to use it in
userspace later is a recipe for disaster, at best it accidentally works
but at worst the userspace interface is broken and cumbersome or the
kernel part has to be completely rewritten to work the way userspace
needs it to.

Not that you need my ACK but I'd need to see the expected userspace
interface and how it works with the kernel interface to support this.

The expected userspace interface from my perspective would be semanage
since we already have multiple ways of setting portcons and adding
another hurts usability.

2009-12-18 18:46:51

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] Dynamic port labeling V2

On Friday 18 December 2009 12:33:24 am Kyle Moffett wrote:
> On Fri, Dec 4, 2009 at 11:03, Joshua Brindle <[email protected]> wrote:
> > Paul Nuzzi wrote:
> >> On Thu, 2009-12-03 at 14:31 -0500, Joshua Brindle wrote:
> >>> Paul Nuzzi wrote:
> >>>> Added a mechanism to add/delete/update port labels with an interface
> >>>> in the selinuxfs filesystem. This will give administrators the
> >>>> ability to update port labels faster than reloading the entire policy
> >>>> with semanage. The administrator will also need less privilege since
> >>>> they don't have to be authorized to reload the full policy.
> >>>>
> >>>> A listing of all port labels will be output if the file
> >>>> /selinux/portcon is read. Labels could be added or deleted with the
> >>>> following commands
> >>>>
> >>>> echo -n "del system_u:object_r:ssh_port_t:s0 6 22 22">
> >>>> /selinux/portcon echo -n "add system_u:object_r:telnetd_port_t:s0 6 22
> >>>> 22"> /selinux/portcon
> >>>
> >>> Aside from the conversation Dave and Casey are having I still think
> >>> this isn't quite right. First, while you can atomically change a single
> >>> port label with the add command above you can't atomically change
> >>> multiple entries, which I think is completely necessary (you don't want
> >>> to have strange labeling states when changing a set of ports to a new
> >>> label.
> >>
> >> Can you think of a situation where we would need to atomically change
> >> multiple entries? I envisioned a sys admin stopping their application
> >> or server, relabeling the ports and then restarting them. Maybe there
> >> is a specific case you know about that I've overlooked?
> >
> > Maybe not, and it isn't like labeling filesystems is atomic so maybe I'm
> > out in left field here.
> >
> > [...]
> >
> > I actually was thinking about resizing a range. You just put the system
> > in a strange state by having to remove alot of labels and then readd
> > them. It seems most reliable to add the entire set of portcons every time
> > (using some file on disk and a text parser to parse them in to an
> > ocontext like struct then feeding it in), that way the ordering is always
> > exactly like it is in the file and there is a persistent file on disk
> > that can be loaded at policy load time.
> >
> > Users could literally do a setportcon -e to pop up an editor with all the
> > portcons and reorder, modify, re-range, etc and it would all take effect
> > at once.
>
> Hrm, wow.... this seems like a whole lot of re-engineering of work
> that has already been done and works well, specifically iptables.
>
> The iptables code already allows almost any combination of access
> rules, with conditionals, nested filters, ranges, etc... and it has a
> design that allows atomic replacement of the entire firewall ruleset.
>
> Furthermore, using tools like Shorewall (http://shorewall.net), I can
> customize exactly what my filtering and forwarding rules are using
> simple human-readable configuration files. Shorewall even generates
> an iptables restore file so I either get the old set of firewall rules
> or the new set, nothing halfway in-between.
>
> I know that SELinux labeling is already partially supported through
> the security table... can't we figure out some way to extend that to
> this port-based labeling as well?

The SELinux packet labeling that is done via iptables, Secmark, is different
from the SELinux port labeling that is done via policy/semanage. The key here
is that iptables is used by SELinux to label packets, not ports. This
approach seems to be consistent with everything I know about iptables, but if
I'm missing something please let me know.

> Another thing that would be nice is to be able to apply some amount of
> discrete access control to firewall rule updates, so that a "network admin"
> could adjust most firewall rules, while the "security admin" would be
> responsible for packet and network interface labeling.

This is something that has been discussed in the past, although perhaps in
person and not on any mailing lists, but no one has done any serious work on
it as far as I know. As usual, we're always open to patches :)

> Putting my various admin hats on, what I would *LOVE* to have from a
> security standpoint would be something along the lines of
> setroubleshoot, but designed to handle audits from many computer
> systems and to allow communication between people with different
> security roles. Here's an example use-case:
>
> So say a web developer is preparing to roll out a big application
> install, and needs to add a new HTTP listen port (say, 8083). She
> tells apache to listen on that port and gets a "permission denied",
> along with a notification of some kind that her action has triggered a
> security warning. As a result, she goes ahead and fills out a
> "network security" change request form, and then puts the ticket
> number into the SETroubleshoot interface tagged to the security
> warning.

I'm not really a SETroubleshoot or userspace guru, but I believe at least this
part should be possible today ... although the change management
infrastructure is left as an exercise for the reader :)

> A few minutes later, the network security guy sits down and opens up
> his audit window and sees the warning that his friendly web developer
> triggered. He checks the trouble ticket and updates the HTTP
> type-enforcement rules to allow that port as well, then sends off a
> notice to the web developer that everything is OK.

I'm pretty sure some of the audit dispatcher/plugin stuff allows for sending
security events to a central repository for reporting, analysis and other
nefarious purposes.

> The web developer then works with his users to beta-test the web
> application and discovers that his software is unable to talk to one
> of its database backends on another server because that database has
> "sensitive financial" data and requires labelled IPsec connections to
> access it. Those audit messages (even from the same server) then go
> to a separate "data security" person, who must himself add the new
> labelling rules to allow that software access to some financial data.

Sort of a "wash, rinse, repeat" of the previous issues.

> I could almost build this system today with a bunch of custom
> userspace firewall management and audit-log filtering software... but
> I have no real way of actually restricting my "data security" person
> from fiddling with the HTTP port labelling, nor can I prevent my basic
> network security guy from changing my fancy labelled IPsec rules.

I'll let the policy guys address this last bit ...

--
paul moore
linux @ hp