* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-01 15:49 [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms Eric Paris
@ 2007-08-02 14:39 ` Stephen Smalley
2007-08-02 15:18 ` Joshua Brindle
2007-08-09 19:19 ` Stephen Smalley
2007-08-21 19:40 ` Stephen Smalley
2 siblings, 1 reply; 15+ messages in thread
From: Stephen Smalley @ 2007-08-02 14:39 UTC (permalink / raw)
To: Eric Paris; +Cc: selinux, jmorris, Joshua Brindle, Karl MacMillan, Paul Moore
On Wed, 2007-08-01 at 11:49 -0400, Eric Paris wrote:
> Allow policy to select, in much the same way as it selects MLS support,
> how the kernel should handle access decisions which contain either
> unknown classes or unknown permissions in unknown classes. The three
> choices are (from a kernel perspective)
>
> 0 - Deny unknown security access. (default)
> 2 - reject loading policy if it does not contain all definitions
> 4 - allow unknown security access
If we leave the default as the current behavior (i.e. deny access if
checking an unknown class or permission), then what exactly have we
achieved with this patch? The next time we introduce a new permission
check in the kernel, anyone who installs that kernel on an existing
distribution release will still encounter breakage. It will only start
providing benefit for newer distribution releases with policies built to
have the non-default setting.
Other points to ponder:
- Would this have helped with the recent netlabel breakage? I don't
think so, as that was introducing a further check for an existing
class/perm in the unlabeled case, not a new class/perm.
- Does this help with letting us fix the ioctl hook? Problem there is
again that we are more likely to start mapping ioctls to read/write or
getattr/setattr instead of the generic "ioctl" fallback rather than
introducing new perms.
- We still can't easily add new perms to common definitions (displaces
class-specific ones).
- Does this help us with secmark at all, e.g. can we eliminate
compat_net? Do we want to eliminate compat_net yet? Fedora still seems
to have compat_net=1.
- We still can't purge obsolete classes/perms or reorganize the existing
ones safely with this change.
So...is it worth it by itself?
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>
> Biggest change between this patch and the last on list is that I changed
> handle_unknown to not be needlessly bit shifted. I also dropped a check
> in context_struct_computeav which was redundant.
>
> security/selinux/ss/policydb.c | 8 +++++
> security/selinux/ss/policydb.h | 10 ++++++
> security/selinux/ss/services.c | 65 ++++++++++++++++++++++++++++++++++-----
> 3 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f05f97a..588dcfd 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -677,6 +677,8 @@ void policydb_destroy(struct policydb *p)
> }
> kfree(p->type_attr_map);
>
> + kfree(p->undefined_perms);
> +
> return;
> }
>
> @@ -1530,6 +1532,12 @@ int policydb_read(struct policydb *p, void *fp)
> goto bad;
> }
> }
> + p->handle_unknown = le32_to_cpu(buf[1]) & POLICYDB_CONFIG_UNKNOWN_MASK;
> +
> + if (p->handle_unknown > ALLOW_UNKNOWN) {
> + printk(KERN_ERR "selinux: invalid options for handle_unknown\n");
> + goto bad;
> + }
>
> info = policydb_lookup_compat(p->policyvers);
> if (!info) {
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 8319d5f..f84e856 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -242,6 +242,9 @@ struct policydb {
> struct ebitmap *type_attr_map;
>
> unsigned int policyvers;
> +
> + int handle_unknown;
> + u32 *undefined_perms;
> };
>
> extern void policydb_destroy(struct policydb *p);
> @@ -253,6 +256,13 @@ extern int policydb_read(struct policydb *p, void *fp);
>
> #define POLICYDB_CONFIG_MLS 1
>
> +/* the config flags related to unknown classes/perms are bits 2 and 3 */
> +#define DENY_UNKNOWN 0x00000000
> +#define REJECT_UNKNOWN 0x00000002
> +#define ALLOW_UNKNOWN 0x00000004
> +
> +#define POLICYDB_CONFIG_UNKNOWN_MASK (DENY_UNKNOWN | REJECT_UNKNOWN | ALLOW_UNKNOWN)
> +
> #define OBJECT_R "object_r"
> #define OBJECT_R_VAL 1
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 0ae032f..d3f3a43 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -292,6 +292,7 @@ static int context_struct_compute_av(struct context *scontext,
> struct class_datum *tclass_datum;
> struct ebitmap *sattr, *tattr;
> struct ebitmap_node *snode, *tnode;
> + const struct selinux_class_perm *kdefs = &selinux_class_perm;
> unsigned int i, j;
>
> /*
> @@ -305,13 +306,6 @@ static int context_struct_compute_av(struct context *scontext,
> tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
> tclass = SECCLASS_NETLINK_SOCKET;
>
> - if (!tclass || tclass > policydb.p_classes.nprim) {
> - printk(KERN_ERR "security_compute_av: unrecognized class %d\n",
> - tclass);
> - return -EINVAL;
> - }
> - tclass_datum = policydb.class_val_to_struct[tclass - 1];
> -
> /*
> * Initialize the access vectors to the default values.
> */
> @@ -322,6 +316,36 @@ static int context_struct_compute_av(struct context *scontext,
> avd->seqno = latest_granting;
>
> /*
> + * Check for all the invalid cases.
> + * - tclass 0
> + * - tclass > policy and > kernel
> + * - tclass > policy but is a userspace class
> + * - tclass > policy but we do not allow unknowns
> + */
> + if (unlikely(!tclass))
> + goto inval_class;
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + if (tclass > kdefs->cts_len ||
> + !kdefs->class_to_string[tclass - 1] ||
> + policydb.handle_unknown != ALLOW_UNKNOWN)
> + goto inval_class;
> +
> + /*
> + * Kernel class and we ALLOW_UNKNOWN so pad the allow decision
> + * the pad will be all 1 for unknown classes.
> + */
> + if (tclass <= kdefs->cts_len && (policydb.handle_unknown == ALLOW_UNKNOWN))
> + avd->allowed = policydb.undefined_perms[tclass - 1];
> +
> + /*
> + * Not in policy. Since decision is completed (all 1 or all 0) return.
> + */
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + return 0;
> +
> + tclass_datum = policydb.class_val_to_struct[tclass - 1];
> +
> + /*
> * If a specific type enforcement rule was defined for
> * this permission check, then use it.
> */
> @@ -387,6 +411,10 @@ static int context_struct_compute_av(struct context *scontext,
> }
>
> return 0;
> +
> +inval_class:
> + printk(KERN_ERR "security_compute_av: unrecognized class %d\n", tclass);
> + return -EINVAL;
> }
>
> static int security_validtrans_handle_fail(struct context *ocontext,
> @@ -1054,6 +1082,13 @@ static int validate_classes(struct policydb *p)
> const char *def_class, *def_perm, *pol_class;
> struct symtab *perms;
>
> + if (p->handle_unknown == ALLOW_UNKNOWN) {
> + u32 num_classes = kdefs->cts_len;
> + p->undefined_perms = kcalloc(num_classes, sizeof(u32), GFP_KERNEL);
> + if (!p->undefined_perms)
> + return -ENOMEM;
> + }
> +
> for (i = 1; i < kdefs->cts_len; i++) {
> def_class = kdefs->class_to_string[i];
> if (!def_class)
> @@ -1062,6 +1097,10 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: class %s not defined in policy\n",
> def_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[i-1] = ~0U;
> + if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> pol_class = p->p_class_val_to_name[i-1];
> @@ -1087,12 +1126,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= perm_val;
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
> @@ -1130,12 +1173,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= (1 << j);
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
>
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 14:39 ` Stephen Smalley
@ 2007-08-02 15:18 ` Joshua Brindle
2007-08-02 15:33 ` Stephen Smalley
0 siblings, 1 reply; 15+ messages in thread
From: Joshua Brindle @ 2007-08-02 15:18 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Eric Paris, selinux, jmorris, Karl MacMillan, Paul Moore
Stephen Smalley wrote:
> On Wed, 2007-08-01 at 11:49 -0400, Eric Paris wrote:
>
>> Allow policy to select, in much the same way as it selects MLS support,
>> how the kernel should handle access decisions which contain either
>> unknown classes or unknown permissions in unknown classes. The three
>> choices are (from a kernel perspective)
>>
>> 0 - Deny unknown security access. (default)
>> 2 - reject loading policy if it does not contain all definitions
>> 4 - allow unknown security access
>>
>
> If we leave the default as the current behavior (i.e. deny access if
> checking an unknown class or permission), then what exactly have we
> achieved with this patch? The next time we introduce a new permission
> check in the kernel, anyone who installs that kernel on an existing
> distribution release will still encounter breakage. It will only start
> providing benefit for newer distribution releases with policies built to
> have the non-default setting.
>
>
I don't think we should change the default behavior of the policy, this
could have some very unintended consequences to people building policies
for security oriented systems that may not be aware of the build
infrastructure changes necessary to keep the old behavior.
> Other points to ponder:
> - Would this have helped with the recent netlabel breakage? I don't
> think so, as that was introducing a further check for an existing
> class/perm in the unlabeled case, not a new class/perm.
> - Does this help with letting us fix the ioctl hook? Problem there is
> again that we are more likely to start mapping ioctls to read/write or
> getattr/setattr instead of the generic "ioctl" fallback rather than
> introducing new perms.
>
How many policies give ioctl without giving read/write or
getattr/setattr? Is this really an issue?
> - We still can't easily add new perms to common definitions (displaces
> class-specific ones).
>
I'm not sure we are going to be able to handle this one until we extend
object class/perm discovery to the kernel object managers, its a
completely different problem from what this is attempting to solve.
> - Does this help us with secmark at all, e.g. can we eliminate
> compat_net? Do we want to eliminate compat_net yet? Fedora still seems
> to have compat_net=1.
> - We still can't purge obsolete classes/perms or reorganize the existing
> ones safely with this change.
>
Ditto.
> So...is it worth it by itself?
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 15:18 ` Joshua Brindle
@ 2007-08-02 15:33 ` Stephen Smalley
2007-08-02 17:06 ` Joshua Brindle
2007-08-02 17:25 ` Eric Paris
0 siblings, 2 replies; 15+ messages in thread
From: Stephen Smalley @ 2007-08-02 15:33 UTC (permalink / raw)
To: Joshua Brindle; +Cc: Eric Paris, selinux, jmorris, Karl MacMillan, Paul Moore
On Thu, 2007-08-02 at 11:18 -0400, Joshua Brindle wrote:
> Stephen Smalley wrote:
> > On Wed, 2007-08-01 at 11:49 -0400, Eric Paris wrote:
> >
> >> Allow policy to select, in much the same way as it selects MLS support,
> >> how the kernel should handle access decisions which contain either
> >> unknown classes or unknown permissions in unknown classes. The three
> >> choices are (from a kernel perspective)
> >>
> >> 0 - Deny unknown security access. (default)
> >> 2 - reject loading policy if it does not contain all definitions
> >> 4 - allow unknown security access
> >>
> >
> > If we leave the default as the current behavior (i.e. deny access if
> > checking an unknown class or permission), then what exactly have we
> > achieved with this patch? The next time we introduce a new permission
> > check in the kernel, anyone who installs that kernel on an existing
> > distribution release will still encounter breakage. It will only start
> > providing benefit for newer distribution releases with policies built to
> > have the non-default setting.
> >
> >
> I don't think we should change the default behavior of the policy, this
> could have some very unintended consequences to people building policies
> for security oriented systems that may not be aware of the build
> infrastructure changes necessary to keep the old behavior.
One would generally expect such people to be using policies that define
all the classes and perms they care about, right? Even more so in the
-mls world, since you need them defined in order to define MLS
constraints on them.
That is certainly a more specialized and security-aware community than
the typical user, so it is fair to impose stronger requirements on them
to configure their systems correctly.
Versus a typical kernel developer or user who just wants to run the
latest kernel on his existing distro, and not have everything break the
next time we add a new permission check.
If we require new policy to be used in order to gain the benefit of the
compatibility behavior, then we aren't actually providing compatibility.
If we could require policy upgrades in the first place when using a
newer kernel, then we wouldn't have the problem that motivated this
patch.
So either this patch cannot achieve its ends (regardless of default) or
the default is wrong.
> > Other points to ponder:
> > - Would this have helped with the recent netlabel breakage? I don't
> > think so, as that was introducing a further check for an existing
> > class/perm in the unlabeled case, not a new class/perm.
> > - Does this help with letting us fix the ioctl hook? Problem there is
> > again that we are more likely to start mapping ioctls to read/write or
> > getattr/setattr instead of the generic "ioctl" fallback rather than
> > introducing new perms.
> >
> How many policies give ioctl without giving read/write or
> getattr/setattr? Is this really an issue?
Today we typically allow ioctl whenever we allow read or write. But
remapping ioctl to read/write or getattr/setattr based on _IOC_DIR() may
yield cases where we were only giving read perms (read + ioctl) but now
we need read + write or read + setattr. I don't if it will be an issue
in practice, but it is technically a compatibility issue if it can ever
cause a denial that would have been allowed previously.
> > - We still can't easily add new perms to common definitions (displaces
> > class-specific ones).
> >
> I'm not sure we are going to be able to handle this one until we extend
> object class/perm discovery to the kernel object managers, its a
> completely different problem from what this is attempting to solve.
Yep, just wondering if it would make more sense to tackle that one
directly, at which point this one becomes moot, right? Because if the
object manager discovers that the class/perm isn't defined by the
policy, it can disable the calling of the check altogether (which would
also be more efficient than having the security server handle it).
> > - Does this help us with secmark at all, e.g. can we eliminate
> > compat_net? Do we want to eliminate compat_net yet? Fedora still seems
> > to have compat_net=1.
> > - We still can't purge obsolete classes/perms or reorganize the existing
> > ones safely with this change.
> >
> Ditto.
>
> > So...is it worth it by itself?
> >
You didn't answer the question ;)
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 15:33 ` Stephen Smalley
@ 2007-08-02 17:06 ` Joshua Brindle
2007-08-02 17:25 ` Eric Paris
1 sibling, 0 replies; 15+ messages in thread
From: Joshua Brindle @ 2007-08-02 17:06 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Eric Paris, selinux, jmorris, Karl MacMillan, Paul Moore
Stephen Smalley wrote:
> On Thu, 2007-08-02 at 11:18 -0400, Joshua Brindle wrote:
>
>
>>> So...is it worth it by itself?
>>>
>>>
>
> You didn't answer the question ;)
>
>
Hrm..
I thought it was at least useful sometimes but actually objclass/perm
discoverability that lets the object managers short circuit decisions
when object classes aren't defined seems like it has alot more benefits.
Really the object manager knows how to deal with a lack of policy
knowledge about its object classes better than the security server does.
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 15:33 ` Stephen Smalley
2007-08-02 17:06 ` Joshua Brindle
@ 2007-08-02 17:25 ` Eric Paris
2007-08-02 18:43 ` Paul Moore
2007-08-02 19:58 ` Stephen Smalley
1 sibling, 2 replies; 15+ messages in thread
From: Eric Paris @ 2007-08-02 17:25 UTC (permalink / raw)
To: Stephen Smalley
Cc: Joshua Brindle, selinux, jmorris, Karl MacMillan, Paul Moore
On Thu, 2007-08-02 at 11:33 -0400, Stephen Smalley wrote:
> On Thu, 2007-08-02 at 11:18 -0400, Joshua Brindle wrote:
> > Stephen Smalley wrote:
> One would generally expect such people to be using policies that define
> all the classes and perms they care about, right? Even more so in the
> -mls world, since you need them defined in order to define MLS
> constraints on them.
>
> That is certainly a more specialized and security-aware community than
> the typical user, so it is fair to impose stronger requirements on them
> to configure their systems correctly.
>
> Versus a typical kernel developer or user who just wants to run the
> latest kernel on his existing distro, and not have everything break the
> next time we add a new permission check.
>
> If we require new policy to be used in order to gain the benefit of the
> compatibility behavior, then we aren't actually providing compatibility.
> If we could require policy upgrades in the first place when using a
> newer kernel, then we wouldn't have the problem that motivated this
> patch.
>
> So either this patch cannot achieve its ends (regardless of default) or
> the default is wrong.
I will ask the question, is there any concern of people with strict
security goals grabbing some new kernel with this new feature and
shooting themselves in the foot? Are there adequate safeguards at such
sites that they would not make this mistake? I'm considering it for
RHEL5, will users of RHEL5 get themselves in trouble if we default to
allow? Would all such interesting users either use the RHEL5 MLS policy
which would I assume default to reject_unknown? Would they use custom
policy and most likely default to something they don't want unknowingly?
I also see a big different between "update your policy once and I'll
stop breaking your X/DCCP/whatever until policy fixes it" and "every
time Eric does some work you need a new policy"
>
> > > Other points to ponder:
> > > - Would this have helped with the recent netlabel breakage? I don't
> > > think so, as that was introducing a further check for an existing
> > > class/perm in the unlabeled case, not a new class/perm.
Maybe instead a versioning scheme? Every security check comes with a
'version.' Every time we add/change a security check we add one to the
'version' for that check. If the policy handed to the kernel has a
'version' less than the value we get from the security hook call coded
in kernel we can just allow it? Maybe that could be a solution to the
issue I am addressing now along with things like this netlabel change?
It certainly would have to touch a whole lot of stuff and might be a
pain to keep up with. Lets say I move a security hook around in a
function, I need to decide if the version need to be bumped on that hook
or not. Not impossible, but not pretty. Also probably means a policy
version update, which in turn breaks the discussion which we have
ongoing above....
> > > - Does this help with letting us fix the ioctl hook? Problem there is
> > > again that we are more likely to start mapping ioctls to read/write or
> > > getattr/setattr instead of the generic "ioctl" fallback rather than
> > > introducing new perms.
> > >
> > How many policies give ioctl without giving read/write or
> > getattr/setattr? Is this really an issue?
>
> Today we typically allow ioctl whenever we allow read or write. But
> remapping ioctl to read/write or getattr/setattr based on _IOC_DIR() may
> yield cases where we were only giving read perms (read + ioctl) but now
> we need read + write or read + setattr. I don't if it will be an issue
> in practice, but it is technically a compatibility issue if it can ever
> cause a denial that would have been allowed previously.
versioning like I talk about isn't pretty, but it could solve the
problem...
>
> > > - We still can't easily add new perms to common definitions (displaces
> > > class-specific ones).
next policy bump move (hey when I add my 'version' right?)
class-specific perms starting at the last bit and working forward while
make common perms start at the front and move back? Guess that's just a
hack though and makes validating and stuff a whole lot harder since we
couldn't go from 0 to nperms.
> > >
> > I'm not sure we are going to be able to handle this one until we extend
> > object class/perm discovery to the kernel object managers, its a
> > completely different problem from what this is attempting to solve.
>
> Yep, just wondering if it would make more sense to tackle that one
> directly, at which point this one becomes moot, right? Because if the
> object manager discovers that the class/perm isn't defined by the
> policy, it can disable the calling of the check altogether (which would
> also be more efficient than having the security server handle it).
/me is confuzzled who in the kernel do you want to discover that
something isn't defined and to just 'not call' ? How does this
something know not to do the new netlabel check?
>
> > > - Does this help us with secmark at all, e.g. can we eliminate
> > > compat_net? Do we want to eliminate compat_net yet? Fedora still seems
> > > to have compat_net=1.
I want compat_net off. 99.9% of people pay a perf penalty on networking
and don't use any of it. The only network hooks policy cares about are
bind/connect which don't care.
No, as it stand it does not help with secmark, and I don't think it
should. Look at it this way, if you change the label on a file the
kernel shouldn't realize policy doesn't know you changed the label and
handle it special. If you add iptables rules and change the label on a
network object you need to update policy. We are going to need some
sort of working together somewhere to get secmark rules used by default,
but I don't think this is the place.
> > > - We still can't purge obsolete classes/perms or reorganize the existing
> > > ones safely with this change.
nope, although when do start allowing this we are going to end up with
some sort of kernel/userspace upgrade required at the same time, or at
least I assume so.....
> > >
> > Ditto.
> >
> > > So...is it worth it by itself?
obviously I thought so, but my opinion doesn't count for much on this
one, does it?
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 17:25 ` Eric Paris
@ 2007-08-02 18:43 ` Paul Moore
2007-08-02 19:44 ` Joshua Brindle
2007-08-02 19:58 ` Stephen Smalley
1 sibling, 1 reply; 15+ messages in thread
From: Paul Moore @ 2007-08-02 18:43 UTC (permalink / raw)
To: Eric Paris
Cc: Stephen Smalley, Joshua Brindle, selinux, jmorris, Karl MacMillan
On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
> Maybe instead a versioning scheme? Every security check comes with a
> 'version.' Every time we add/change a security check we add one to the
> 'version' for that check. If the policy handed to the kernel has a
> 'version' less than the value we get from the security hook call coded
> in kernel we can just allow it? Maybe that could be a solution to the
> issue I am addressing now along with things like this netlabel change?
> It certainly would have to touch a whole lot of stuff and might be a
> pain to keep up with. Lets say I move a security hook around in a
> function, I need to decide if the version need to be bumped on that hook
> or not. Not impossible, but not pretty. Also probably means a policy
> version update, which in turn breaks the discussion which we have
> ongoing above....
While I'm not an expert on the policy aspects of this discussion and how the
kernel and userspace objects managers interact I think at some point we are
going to need something like what Eric proposes above. With the requirement
that new kernels must not regress when using old policy we are going to run
into issues as we move forward and add new permission checks. While I don't
think we've seen it yet, it is reasonable to think that this problem will
expand into userspace as we develop more and more userspace object managers
such a X, CUPS, Postgres, etc.
I know the changes aren't likely to be pretty, I'm kinda cringing at the
thought actually, but I think it's a necessary evil for the long term
survivability of SELinux. At least that's what I took away from Eric and
Stephen's comments on this thread and I tend to agree.
--
paul moore
linux security @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 18:43 ` Paul Moore
@ 2007-08-02 19:44 ` Joshua Brindle
2007-08-02 20:06 ` Stephen Smalley
2007-08-02 20:25 ` Eamon Walsh
0 siblings, 2 replies; 15+ messages in thread
From: Joshua Brindle @ 2007-08-02 19:44 UTC (permalink / raw)
To: Paul Moore; +Cc: Eric Paris, Stephen Smalley, selinux, jmorris, Karl MacMillan
Paul Moore wrote:
> On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
>
>> Maybe instead a versioning scheme? Every security check comes with a
>> 'version.' Every time we add/change a security check we add one to the
>> 'version' for that check. If the policy handed to the kernel has a
>> 'version' less than the value we get from the security hook call coded
>> in kernel we can just allow it? Maybe that could be a solution to the
>> issue I am addressing now along with things like this netlabel change?
>> It certainly would have to touch a whole lot of stuff and might be a
>> pain to keep up with. Lets say I move a security hook around in a
>> function, I need to decide if the version need to be bumped on that hook
>> or not. Not impossible, but not pretty. Also probably means a policy
>> version update, which in turn breaks the discussion which we have
>> ongoing above....
>>
>
> While I'm not an expert on the policy aspects of this discussion and how the
> kernel and userspace objects managers interact I think at some point we are
> going to need something like what Eric proposes above. With the requirement
> that new kernels must not regress when using old policy we are going to run
> into issues as we move forward and add new permission checks. While I don't
> think we've seen it yet, it is reasonable to think that this problem will
> expand into userspace as we develop more and more userspace object managers
> such a X, CUPS, Postgres, etc.
>
> I know the changes aren't likely to be pretty, I'm kinda cringing at the
> thought actually, but I think it's a necessary evil for the long term
> survivability of SELinux. At least that's what I took away from Eric and
> Stephen's comments on this thread and I tend to agree.
>
I'd have to give a pretty hard NAK (for what it would be worth) to the
versioning system, it simply doesn't scale; in the past we've used
policy versions to decide how some permission checks work (see fine
grained netlink discussions) and it was fairly non-ideal. I have to go
back to the 'the object manager should decide how to deal with unknown
permissions' argument. The object manager knows how its objects are
used, and how the security system affects those objects, it is in the
best position to make those decisions, the security server can only make
a rough estimation on what the object manager and user want to do, which
may or may not be accurate.
Here is an example, granted kernexec uses the reboot capability but
suppose we wanted to make it a separate permission, I'm fairly certain
that we'd want to always deny it if the policy didn't know about it,
whereas SE-X is completely unusable without policy and still has OS
level policy determining who can talk to the X daemon itself, SE-X would
have to make a determination of whether to run at all or run with course
grained permissions, chances are most people would want the latter. This
means we'd actually have environments where some object managers deny by
default and others allow (or even short circuit access attempts
altogether). This is ideal from a scalability and dynamic system
perspective and seems by far to be the best solution.
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 19:44 ` Joshua Brindle
@ 2007-08-02 20:06 ` Stephen Smalley
2007-08-02 21:05 ` Joshua Brindle
2007-08-02 20:25 ` Eamon Walsh
1 sibling, 1 reply; 15+ messages in thread
From: Stephen Smalley @ 2007-08-02 20:06 UTC (permalink / raw)
To: Joshua Brindle; +Cc: Paul Moore, Eric Paris, selinux, jmorris, Karl MacMillan
On Thu, 2007-08-02 at 15:44 -0400, Joshua Brindle wrote:
> Paul Moore wrote:
> > On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
> >
> >> Maybe instead a versioning scheme? Every security check comes with a
> >> 'version.' Every time we add/change a security check we add one to the
> >> 'version' for that check. If the policy handed to the kernel has a
> >> 'version' less than the value we get from the security hook call coded
> >> in kernel we can just allow it? Maybe that could be a solution to the
> >> issue I am addressing now along with things like this netlabel change?
> >> It certainly would have to touch a whole lot of stuff and might be a
> >> pain to keep up with. Lets say I move a security hook around in a
> >> function, I need to decide if the version need to be bumped on that hook
> >> or not. Not impossible, but not pretty. Also probably means a policy
> >> version update, which in turn breaks the discussion which we have
> >> ongoing above....
> >>
> >
> > While I'm not an expert on the policy aspects of this discussion and how the
> > kernel and userspace objects managers interact I think at some point we are
> > going to need something like what Eric proposes above. With the requirement
> > that new kernels must not regress when using old policy we are going to run
> > into issues as we move forward and add new permission checks. While I don't
> > think we've seen it yet, it is reasonable to think that this problem will
> > expand into userspace as we develop more and more userspace object managers
> > such a X, CUPS, Postgres, etc.
> >
> > I know the changes aren't likely to be pretty, I'm kinda cringing at the
> > thought actually, but I think it's a necessary evil for the long term
> > survivability of SELinux. At least that's what I took away from Eric and
> > Stephen's comments on this thread and I tend to agree.
> >
>
> I'd have to give a pretty hard NAK (for what it would be worth) to the
> versioning system, it simply doesn't scale; in the past we've used
> policy versions to decide how some permission checks work (see fine
> grained netlink discussions) and it was fairly non-ideal.
Well, that was "policy format version" vs. a separate version that
identifies the set of security checks known to / expected by the policy.
Or, as I said in my other email, a bitmap of check indices, where each
check (not permission, but individual check) has its own unique index
assigned when it is first introduced and never recycled.
> I have to go
> back to the 'the object manager should decide how to deal with unknown
> permissions' argument. The object manager knows how its objects are
> used, and how the security system affects those objects, it is in the
> best position to make those decisions, the security server can only make
> a rough estimation on what the object manager and user want to do, which
> may or may not be accurate.
It's actually a policy issue - what set of checks are required in order
to meet a given policy.
> Here is an example, granted kernexec uses the reboot capability but
> suppose we wanted to make it a separate permission, I'm fairly certain
> that we'd want to always deny it if the policy didn't know about it,
Actually, I'm sure Fedora would want to allow it by default if it wasn't
yet defined.
> whereas SE-X is completely unusable without policy and still has OS
> level policy determining who can talk to the X daemon itself, SE-X would
> have to make a determination of whether to run at all or run with course
> grained permissions, chances are most people would want the latter.
That's a policy issue ;)
> This
> means we'd actually have environments where some object managers deny by
> default and others allow (or even short circuit access attempts
> altogether). This is ideal from a scalability and dynamic system
> perspective and seems by far to be the best solution.
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 20:06 ` Stephen Smalley
@ 2007-08-02 21:05 ` Joshua Brindle
0 siblings, 0 replies; 15+ messages in thread
From: Joshua Brindle @ 2007-08-02 21:05 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Paul Moore, Eric Paris, selinux, jmorris, Karl MacMillan
Stephen Smalley wrote:
> On Thu, 2007-08-02 at 15:44 -0400, Joshua Brindle wrote:
>
>> Paul Moore wrote:
>>
>>> On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
>>>
>>>
>>>> Maybe instead a versioning scheme? Every security check comes with a
>>>> 'version.' Every time we add/change a security check we add one to the
>>>> 'version' for that check. If the policy handed to the kernel has a
>>>> 'version' less than the value we get from the security hook call coded
>>>> in kernel we can just allow it? Maybe that could be a solution to the
>>>> issue I am addressing now along with things like this netlabel change?
>>>> It certainly would have to touch a whole lot of stuff and might be a
>>>> pain to keep up with. Lets say I move a security hook around in a
>>>> function, I need to decide if the version need to be bumped on that hook
>>>> or not. Not impossible, but not pretty. Also probably means a policy
>>>> version update, which in turn breaks the discussion which we have
>>>> ongoing above....
>>>>
>>>>
>>> While I'm not an expert on the policy aspects of this discussion and how the
>>> kernel and userspace objects managers interact I think at some point we are
>>> going to need something like what Eric proposes above. With the requirement
>>> that new kernels must not regress when using old policy we are going to run
>>> into issues as we move forward and add new permission checks. While I don't
>>> think we've seen it yet, it is reasonable to think that this problem will
>>> expand into userspace as we develop more and more userspace object managers
>>> such a X, CUPS, Postgres, etc.
>>>
>>> I know the changes aren't likely to be pretty, I'm kinda cringing at the
>>> thought actually, but I think it's a necessary evil for the long term
>>> survivability of SELinux. At least that's what I took away from Eric and
>>> Stephen's comments on this thread and I tend to agree.
>>>
>>>
>> I'd have to give a pretty hard NAK (for what it would be worth) to the
>> versioning system, it simply doesn't scale; in the past we've used
>> policy versions to decide how some permission checks work (see fine
>> grained netlink discussions) and it was fairly non-ideal.
>>
>
> Well, that was "policy format version" vs. a separate version that
> identifies the set of security checks known to / expected by the policy.
> Or, as I said in my other email, a bitmap of check indices, where each
> check (not permission, but individual check) has its own unique index
> assigned when it is first introduced and never recycled.
>
>
I was just giving an example on my the policy versioning idea doesn't
scale, I know it would be something separate from policy format version
but that doesn't necessarily make it better.
>> I have to go
>> back to the 'the object manager should decide how to deal with unknown
>> permissions' argument. The object manager knows how its objects are
>> used, and how the security system affects those objects, it is in the
>> best position to make those decisions, the security server can only make
>> a rough estimation on what the object manager and user want to do, which
>> may or may not be accurate.
>>
>
> It's actually a policy issue - what set of checks are required in order
> to meet a given policy.
>
>
Yes, I was agreeing with that before but when it comes down to it the
object manager is always responsible for enforcing said policy, and it
can always make the decision not to. If the object manager decides a
lack of security policy means that something should be denied wouldn't
it know better than a security server with no concept of the object classes?
>> Here is an example, granted kernexec uses the reboot capability but
>> suppose we wanted to make it a separate permission, I'm fairly certain
>> that we'd want to always deny it if the policy didn't know about it,
>>
>
> Actually, I'm sure Fedora would want to allow it by default if it wasn't
> yet defined.
>
>
I'd certainly hope not but that probably isn't the issue.
>> whereas SE-X is completely unusable without policy and still has OS
>> level policy determining who can talk to the X daemon itself, SE-X would
>> have to make a determination of whether to run at all or run with course
>> grained permissions, chances are most people would want the latter.
>>
>
> That's a policy issue ;)
>
>
Its also an object manager issue, as explained above.
>> This
>> means we'd actually have environments where some object managers deny by
>> default and others allow (or even short circuit access attempts
>> altogether). This is ideal from a scalability and dynamic system
>> perspective and seems by far to be the best solution.
>>
>
>
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 19:44 ` Joshua Brindle
2007-08-02 20:06 ` Stephen Smalley
@ 2007-08-02 20:25 ` Eamon Walsh
2007-08-02 21:53 ` Daniel J Walsh
1 sibling, 1 reply; 15+ messages in thread
From: Eamon Walsh @ 2007-08-02 20:25 UTC (permalink / raw)
To: Joshua Brindle
Cc: Paul Moore, Eric Paris, Stephen Smalley, selinux, jmorris,
Karl MacMillan
Joshua Brindle wrote:
> Paul Moore wrote:
>> On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
>>
>>> Maybe instead a versioning scheme? Every security check comes with a
>>> 'version.' Every time we add/change a security check we add one to the
>>> 'version' for that check. If the policy handed to the kernel has a
>>> 'version' less than the value we get from the security hook call coded
>>> in kernel we can just allow it? Maybe that could be a solution to the
>>> issue I am addressing now along with things like this netlabel change?
>>> It certainly would have to touch a whole lot of stuff and might be a
>>> pain to keep up with. Lets say I move a security hook around in a
>>> function, I need to decide if the version need to be bumped on that hook
>>> or not. Not impossible, but not pretty. Also probably means a policy
>>> version update, which in turn breaks the discussion which we have
>>> ongoing above....
>>>
>> While I'm not an expert on the policy aspects of this discussion and how the
>> kernel and userspace objects managers interact I think at some point we are
>> going to need something like what Eric proposes above. With the requirement
>> that new kernels must not regress when using old policy we are going to run
>> into issues as we move forward and add new permission checks. While I don't
>> think we've seen it yet, it is reasonable to think that this problem will
>> expand into userspace as we develop more and more userspace object managers
>> such a X, CUPS, Postgres, etc.
>>
>> I know the changes aren't likely to be pretty, I'm kinda cringing at the
>> thought actually, but I think it's a necessary evil for the long term
>> survivability of SELinux. At least that's what I took away from Eric and
>> Stephen's comments on this thread and I tend to agree.
>>
>
> I'd have to give a pretty hard NAK (for what it would be worth) to the
> versioning system, it simply doesn't scale; in the past we've used
> policy versions to decide how some permission checks work (see fine
> grained netlink discussions) and it was fairly non-ideal. I have to go
> back to the 'the object manager should decide how to deal with unknown
> permissions' argument. The object manager knows how its objects are
> used, and how the security system affects those objects, it is in the
> best position to make those decisions, the security server can only make
> a rough estimation on what the object manager and user want to do, which
> may or may not be accurate.
I would suggest exporting the allow/deny bit to userspace in the same
manner as the enforcing mode. The userspace AVC could then honor the
behavior unless the object manager overrides it (I've been meaning to
allow the object manager to override the enforcing mode so you could for
example have a permissive X server on an otherwise enforcing system).
The selinux_set_mapping() call could be modified to return details on
which specific classes and permissions were unknown. Perhaps this could
be done through a callback which would also be called on policy reload,
given that class/perm values could appear or disappear at these times.
>
> Here is an example, granted kernexec uses the reboot capability but
> suppose we wanted to make it a separate permission, I'm fairly certain
> that we'd want to always deny it if the policy didn't know about it,
> whereas SE-X is completely unusable without policy and still has OS
> level policy determining who can talk to the X daemon itself, SE-X would
> have to make a determination of whether to run at all or run with course
> grained permissions, chances are most people would want the latter. This
> means we'd actually have environments where some object managers deny by
> default and others allow (or even short circuit access attempts
> altogether). This is ideal from a scalability and dynamic system
> perspective and seems by far to be the best solution.
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>
--
Eamon Walsh <ewalsh@tycho.nsa.gov>
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 20:25 ` Eamon Walsh
@ 2007-08-02 21:53 ` Daniel J Walsh
0 siblings, 0 replies; 15+ messages in thread
From: Daniel J Walsh @ 2007-08-02 21:53 UTC (permalink / raw)
To: Eamon Walsh
Cc: Joshua Brindle, Paul Moore, Eric Paris, Stephen Smalley, selinux,
jmorris, Karl MacMillan
Eamon Walsh wrote:
> Joshua Brindle wrote:
>> Paul Moore wrote:
>>> On Thursday, August 2 2007 1:25:12 pm Eric Paris wrote:
>>>
>>>> Maybe instead a versioning scheme? Every security check comes with a
>>>> 'version.' Every time we add/change a security check we add one to
>>>> the
>>>> 'version' for that check. If the policy handed to the kernel has a
>>>> 'version' less than the value we get from the security hook call coded
>>>> in kernel we can just allow it? Maybe that could be a solution to
>>>> the
>>>> issue I am addressing now along with things like this netlabel change?
>>>> It certainly would have to touch a whole lot of stuff and might be a
>>>> pain to keep up with. Lets say I move a security hook around in a
>>>> function, I need to decide if the version need to be bumped on that
>>>> hook
>>>> or not. Not impossible, but not pretty. Also probably means a policy
>>>> version update, which in turn breaks the discussion which we have
>>>> ongoing above....
>>>>
>>> While I'm not an expert on the policy aspects of this discussion and
>>> how the kernel and userspace objects managers interact I think at
>>> some point we are going to need something like what Eric proposes
>>> above. With the requirement that new kernels must not regress when
>>> using old policy we are going to run into issues as we move forward
>>> and add new permission checks. While I don't think we've seen it
>>> yet, it is reasonable to think that this problem will expand into
>>> userspace as we develop more and more userspace object managers such
>>> a X, CUPS, Postgres, etc.
>>>
>>> I know the changes aren't likely to be pretty, I'm kinda cringing at
>>> the thought actually, but I think it's a necessary evil for the long
>>> term survivability of SELinux. At least that's what I took away
>>> from Eric and Stephen's comments on this thread and I tend to agree.
>>>
>>
>> I'd have to give a pretty hard NAK (for what it would be worth) to
>> the versioning system, it simply doesn't scale; in the past we've
>> used policy versions to decide how some permission checks work (see
>> fine grained netlink discussions) and it was fairly non-ideal. I have
>> to go back to the 'the object manager should decide how to deal with
>> unknown permissions' argument. The object manager knows how its
>> objects are used, and how the security system affects those objects,
>> it is in the best position to make those decisions, the security
>> server can only make a rough estimation on what the object manager
>> and user want to do, which may or may not be accurate.
>
> I would suggest exporting the allow/deny bit to userspace in the same
> manner as the enforcing mode. The userspace AVC could then honor the
> behavior unless the object manager overrides it (I've been meaning to
> allow the object manager to override the enforcing mode so you could
> for example have a permissive X server on an otherwise enforcing system).
If we could get this per domain, this would be a huge step forward.
Customers looking to ship policy worry about putting out a policy that
could break apps, before it got enough testing. They want to put out
apolicy for an application and run it for a few weeks on many different
realworld environments and gather the avc's to make the poicy foolproof.
>
> The selinux_set_mapping() call could be modified to return details on
> which specific classes and permissions were unknown. Perhaps this
> could be done through a callback which would also be called on policy
> reload, given that class/perm values could appear or disappear at
> these times.
>
>>
>> Here is an example, granted kernexec uses the reboot capability but
>> suppose we wanted to make it a separate permission, I'm fairly
>> certain that we'd want to always deny it if the policy didn't know
>> about it, whereas SE-X is completely unusable without policy and
>> still has OS level policy determining who can talk to the X daemon
>> itself, SE-X would have to make a determination of whether to run at
>> all or run with course grained permissions, chances are most people
>> would want the latter. This means we'd actually have environments
>> where some object managers deny by default and others allow (or even
>> short circuit access attempts altogether). This is ideal from a
>> scalability and dynamic system perspective and seems by far to be the
>> best solution.
>>
>>
>> --
>> This message was distributed to subscribers of the selinux mailing list.
>> If you no longer wish to subscribe, send mail to
>> majordomo@tycho.nsa.gov 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 majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-02 17:25 ` Eric Paris
2007-08-02 18:43 ` Paul Moore
@ 2007-08-02 19:58 ` Stephen Smalley
1 sibling, 0 replies; 15+ messages in thread
From: Stephen Smalley @ 2007-08-02 19:58 UTC (permalink / raw)
To: Eric Paris; +Cc: Joshua Brindle, selinux, jmorris, Karl MacMillan, Paul Moore
On Thu, 2007-08-02 at 13:25 -0400, Eric Paris wrote:
> On Thu, 2007-08-02 at 11:33 -0400, Stephen Smalley wrote:
> > On Thu, 2007-08-02 at 11:18 -0400, Joshua Brindle wrote:
> > > Stephen Smalley wrote:
>
> > One would generally expect such people to be using policies that define
> > all the classes and perms they care about, right? Even more so in the
> > -mls world, since you need them defined in order to define MLS
> > constraints on them.
> >
> > That is certainly a more specialized and security-aware community than
> > the typical user, so it is fair to impose stronger requirements on them
> > to configure their systems correctly.
> >
> > Versus a typical kernel developer or user who just wants to run the
> > latest kernel on his existing distro, and not have everything break the
> > next time we add a new permission check.
> >
> > If we require new policy to be used in order to gain the benefit of the
> > compatibility behavior, then we aren't actually providing compatibility.
> > If we could require policy upgrades in the first place when using a
> > newer kernel, then we wouldn't have the problem that motivated this
> > patch.
> >
> > So either this patch cannot achieve its ends (regardless of default) or
> > the default is wrong.
>
> I will ask the question, is there any concern of people with strict
> security goals grabbing some new kernel with this new feature and
> shooting themselves in the foot? Are there adequate safeguards at such
> sites that they would not make this mistake? I'm considering it for
> RHEL5, will users of RHEL5 get themselves in trouble if we default to
> allow? Would all such interesting users either use the RHEL5 MLS policy
> which would I assume default to reject_unknown? Would they use custom
> policy and most likely default to something they don't want unknowingly?
How does this feature benefit a RHEL (or other enterprise distro) user,
whose kernel and policy are vendor-supplied (else warranty and
certification invalidated) and should be consistent already in their set
of permissions?
The point is for a new permission check in mainline to not break Fedora
(or other community distro).
Just updating to a new kernel introduces a risk that you've got an
uncontrolled operation or code path altogether, as security hooks seem
often to be an afterthought. Or if it is an already hooked
operation/code path and you've added a new check to it as a refinement,
then all you "lose" is that refinement, not any security properties you
had in the prior kernel.
Don't get me wrong, I prefer the secure default - I just wondered
whether it makes the patch fail to meet the original goal.
> I also see a big different between "update your policy once and I'll
> stop breaking your X/DCCP/whatever until policy fixes it" and "every
> time Eric does some work you need a new policy"
Fair enough - it offers potential benefit once a policy with
always_allow set gets sufficiently widely deployed. But if akpm is
regression testing on a Fedora release that is no longer getting updates
in a VM, policy update won't ever hit it and the new kernel will still
break it.
> >
> > > > Other points to ponder:
> > > > - Would this have helped with the recent netlabel breakage? I don't
> > > > think so, as that was introducing a further check for an existing
> > > > class/perm in the unlabeled case, not a new class/perm.
>
> Maybe instead a versioning scheme? Every security check comes with a
> 'version.' Every time we add/change a security check we add one to the
> 'version' for that check. If the policy handed to the kernel has a
> 'version' less than the value we get from the security hook call coded
> in kernel we can just allow it? Maybe that could be a solution to the
> issue I am addressing now along with things like this netlabel change?
> It certainly would have to touch a whole lot of stuff and might be a
> pain to keep up with. Lets say I move a security hook around in a
> function, I need to decide if the version need to be bumped on that hook
> or not. Not impossible, but not pretty. Also probably means a policy
> version update, which in turn breaks the discussion which we have
> ongoing above....
Possibly, and would be more useful for things like secmark where we are
not just adding permission checks but instead replacing one set of
legacy checks with a newer check. Likewise for the desired ioctl
checking. Or alternatively, a per-check index with a bitmap in policy
to select which ones are required by the policy.
> > > > - We still can't easily add new perms to common definitions (displaces
> > > > class-specific ones).
>
> next policy bump move (hey when I add my 'version' right?)
> class-specific perms starting at the last bit and working forward while
> make common perms start at the front and move back? Guess that's just a
> hack though and makes validating and stuff a whole lot harder since we
> couldn't go from 0 to nperms.
Object class and permission discovery solves it more cleanly. We have
it now for userspace, just need it in kernel.
> > > >
> > > I'm not sure we are going to be able to handle this one until we extend
> > > object class/perm discovery to the kernel object managers, its a
> > > completely different problem from what this is attempting to solve.
> >
> > Yep, just wondering if it would make more sense to tackle that one
> > directly, at which point this one becomes moot, right? Because if the
> > object manager discovers that the class/perm isn't defined by the
> > policy, it can disable the calling of the check altogether (which would
> > also be more efficient than having the security server handle it).
>
> /me is confuzzled who in the kernel do you want to discover that
> something isn't defined and to just 'not call' ? How does this
> something know not to do the new netlabel check?
The SELinux module and hook functions. It would do something equivalent
to the mapping and discovery code in libselinux, except that upon
discovering that a class or permission isn't defined in the policy, we
want to mark the check as disabled rather than treat it as an error.
> >
> > > > - Does this help us with secmark at all, e.g. can we eliminate
> > > > compat_net? Do we want to eliminate compat_net yet? Fedora still seems
> > > > to have compat_net=1.
>
> I want compat_net off. 99.9% of people pay a perf penalty on networking
> and don't use any of it. The only network hooks policy cares about are
> bind/connect which don't care.
>
> No, as it stand it does not help with secmark, and I don't think it
> should. Look at it this way, if you change the label on a file the
> kernel shouldn't realize policy doesn't know you changed the label and
> handle it special. If you add iptables rules and change the label on a
> network object you need to update policy. We are going to need some
> sort of working together somewhere to get secmark rules used by default,
> but I don't think this is the place.
By "help with secmark", I mean eliminate the need for compat_net=1
altogether. It makes it hard to know what the system is enforcing
(can't tell from policy, have to go look at /selinux/compat_net)
> > > >
> > > Ditto.
> > >
> > > > So...is it worth it by itself?
>
> obviously I thought so, but my opinion doesn't count for much on this
> one, does it?
And it may yet be. But better to find out now before it gets merged...
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-01 15:49 [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms Eric Paris
2007-08-02 14:39 ` Stephen Smalley
@ 2007-08-09 19:19 ` Stephen Smalley
2007-08-21 19:40 ` Stephen Smalley
2 siblings, 0 replies; 15+ messages in thread
From: Stephen Smalley @ 2007-08-09 19:19 UTC (permalink / raw)
To: Eric Paris; +Cc: selinux, jmorris
On Wed, 2007-08-01 at 11:49 -0400, Eric Paris wrote:
> Allow policy to select, in much the same way as it selects MLS support,
> how the kernel should handle access decisions which contain either
> unknown classes or unknown permissions in unknown classes. The three
> choices are (from a kernel perspective)
>
> 0 - Deny unknown security access. (default)
> 2 - reject loading policy if it does not contain all definitions
> 4 - allow unknown security access
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Anyone else want to speak up for or against this patch?
> ---
>
> Biggest change between this patch and the last on list is that I changed
> handle_unknown to not be needlessly bit shifted. I also dropped a check
> in context_struct_computeav which was redundant.
>
> security/selinux/ss/policydb.c | 8 +++++
> security/selinux/ss/policydb.h | 10 ++++++
> security/selinux/ss/services.c | 65 ++++++++++++++++++++++++++++++++++-----
> 3 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f05f97a..588dcfd 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -677,6 +677,8 @@ void policydb_destroy(struct policydb *p)
> }
> kfree(p->type_attr_map);
>
> + kfree(p->undefined_perms);
> +
> return;
> }
>
> @@ -1530,6 +1532,12 @@ int policydb_read(struct policydb *p, void *fp)
> goto bad;
> }
> }
> + p->handle_unknown = le32_to_cpu(buf[1]) & POLICYDB_CONFIG_UNKNOWN_MASK;
> +
> + if (p->handle_unknown > ALLOW_UNKNOWN) {
> + printk(KERN_ERR "selinux: invalid options for handle_unknown\n");
> + goto bad;
> + }
>
> info = policydb_lookup_compat(p->policyvers);
> if (!info) {
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 8319d5f..f84e856 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -242,6 +242,9 @@ struct policydb {
> struct ebitmap *type_attr_map;
>
> unsigned int policyvers;
> +
> + int handle_unknown;
> + u32 *undefined_perms;
> };
>
> extern void policydb_destroy(struct policydb *p);
> @@ -253,6 +256,13 @@ extern int policydb_read(struct policydb *p, void *fp);
>
> #define POLICYDB_CONFIG_MLS 1
>
> +/* the config flags related to unknown classes/perms are bits 2 and 3 */
> +#define DENY_UNKNOWN 0x00000000
> +#define REJECT_UNKNOWN 0x00000002
> +#define ALLOW_UNKNOWN 0x00000004
> +
> +#define POLICYDB_CONFIG_UNKNOWN_MASK (DENY_UNKNOWN | REJECT_UNKNOWN | ALLOW_UNKNOWN)
> +
> #define OBJECT_R "object_r"
> #define OBJECT_R_VAL 1
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 0ae032f..d3f3a43 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -292,6 +292,7 @@ static int context_struct_compute_av(struct context *scontext,
> struct class_datum *tclass_datum;
> struct ebitmap *sattr, *tattr;
> struct ebitmap_node *snode, *tnode;
> + const struct selinux_class_perm *kdefs = &selinux_class_perm;
> unsigned int i, j;
>
> /*
> @@ -305,13 +306,6 @@ static int context_struct_compute_av(struct context *scontext,
> tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
> tclass = SECCLASS_NETLINK_SOCKET;
>
> - if (!tclass || tclass > policydb.p_classes.nprim) {
> - printk(KERN_ERR "security_compute_av: unrecognized class %d\n",
> - tclass);
> - return -EINVAL;
> - }
> - tclass_datum = policydb.class_val_to_struct[tclass - 1];
> -
> /*
> * Initialize the access vectors to the default values.
> */
> @@ -322,6 +316,36 @@ static int context_struct_compute_av(struct context *scontext,
> avd->seqno = latest_granting;
>
> /*
> + * Check for all the invalid cases.
> + * - tclass 0
> + * - tclass > policy and > kernel
> + * - tclass > policy but is a userspace class
> + * - tclass > policy but we do not allow unknowns
> + */
> + if (unlikely(!tclass))
> + goto inval_class;
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + if (tclass > kdefs->cts_len ||
> + !kdefs->class_to_string[tclass - 1] ||
> + policydb.handle_unknown != ALLOW_UNKNOWN)
> + goto inval_class;
> +
> + /*
> + * Kernel class and we ALLOW_UNKNOWN so pad the allow decision
> + * the pad will be all 1 for unknown classes.
> + */
> + if (tclass <= kdefs->cts_len && (policydb.handle_unknown == ALLOW_UNKNOWN))
> + avd->allowed = policydb.undefined_perms[tclass - 1];
> +
> + /*
> + * Not in policy. Since decision is completed (all 1 or all 0) return.
> + */
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + return 0;
> +
> + tclass_datum = policydb.class_val_to_struct[tclass - 1];
> +
> + /*
> * If a specific type enforcement rule was defined for
> * this permission check, then use it.
> */
> @@ -387,6 +411,10 @@ static int context_struct_compute_av(struct context *scontext,
> }
>
> return 0;
> +
> +inval_class:
> + printk(KERN_ERR "security_compute_av: unrecognized class %d\n", tclass);
> + return -EINVAL;
> }
>
> static int security_validtrans_handle_fail(struct context *ocontext,
> @@ -1054,6 +1082,13 @@ static int validate_classes(struct policydb *p)
> const char *def_class, *def_perm, *pol_class;
> struct symtab *perms;
>
> + if (p->handle_unknown == ALLOW_UNKNOWN) {
> + u32 num_classes = kdefs->cts_len;
> + p->undefined_perms = kcalloc(num_classes, sizeof(u32), GFP_KERNEL);
> + if (!p->undefined_perms)
> + return -ENOMEM;
> + }
> +
> for (i = 1; i < kdefs->cts_len; i++) {
> def_class = kdefs->class_to_string[i];
> if (!def_class)
> @@ -1062,6 +1097,10 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: class %s not defined in policy\n",
> def_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[i-1] = ~0U;
> + if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> pol_class = p->p_class_val_to_name[i-1];
> @@ -1087,12 +1126,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= perm_val;
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
> @@ -1130,12 +1173,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= (1 << j);
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms
2007-08-01 15:49 [PATCH] kernel: selinux: policy selectable handling of unknown classes and perms Eric Paris
2007-08-02 14:39 ` Stephen Smalley
2007-08-09 19:19 ` Stephen Smalley
@ 2007-08-21 19:40 ` Stephen Smalley
2 siblings, 0 replies; 15+ messages in thread
From: Stephen Smalley @ 2007-08-21 19:40 UTC (permalink / raw)
To: Eric Paris; +Cc: selinux, jmorris
On Wed, 2007-08-01 at 11:49 -0400, Eric Paris wrote:
> Allow policy to select, in much the same way as it selects MLS support,
> how the kernel should handle access decisions which contain either
> unknown classes or unknown permissions in unknown classes. The three
> choices are (from a kernel perspective)
>
> 0 - Deny unknown security access. (default)
> 2 - reject loading policy if it does not contain all definitions
> 4 - allow unknown security access
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
It would be nice to have SELinux emit a KERN_INFO message about which
behavior was selected when policy is loaded, so that you can easily
tell.
> ---
>
> Biggest change between this patch and the last on list is that I changed
> handle_unknown to not be needlessly bit shifted. I also dropped a check
> in context_struct_computeav which was redundant.
>
> security/selinux/ss/policydb.c | 8 +++++
> security/selinux/ss/policydb.h | 10 ++++++
> security/selinux/ss/services.c | 65 ++++++++++++++++++++++++++++++++++-----
> 3 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f05f97a..588dcfd 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -677,6 +677,8 @@ void policydb_destroy(struct policydb *p)
> }
> kfree(p->type_attr_map);
>
> + kfree(p->undefined_perms);
> +
> return;
> }
>
> @@ -1530,6 +1532,12 @@ int policydb_read(struct policydb *p, void *fp)
> goto bad;
> }
> }
> + p->handle_unknown = le32_to_cpu(buf[1]) & POLICYDB_CONFIG_UNKNOWN_MASK;
> +
> + if (p->handle_unknown > ALLOW_UNKNOWN) {
> + printk(KERN_ERR "selinux: invalid options for handle_unknown\n");
> + goto bad;
> + }
>
> info = policydb_lookup_compat(p->policyvers);
> if (!info) {
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 8319d5f..f84e856 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -242,6 +242,9 @@ struct policydb {
> struct ebitmap *type_attr_map;
>
> unsigned int policyvers;
> +
> + int handle_unknown;
> + u32 *undefined_perms;
> };
>
> extern void policydb_destroy(struct policydb *p);
> @@ -253,6 +256,13 @@ extern int policydb_read(struct policydb *p, void *fp);
>
> #define POLICYDB_CONFIG_MLS 1
>
> +/* the config flags related to unknown classes/perms are bits 2 and 3 */
> +#define DENY_UNKNOWN 0x00000000
> +#define REJECT_UNKNOWN 0x00000002
> +#define ALLOW_UNKNOWN 0x00000004
> +
> +#define POLICYDB_CONFIG_UNKNOWN_MASK (DENY_UNKNOWN | REJECT_UNKNOWN | ALLOW_UNKNOWN)
> +
> #define OBJECT_R "object_r"
> #define OBJECT_R_VAL 1
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 0ae032f..d3f3a43 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -292,6 +292,7 @@ static int context_struct_compute_av(struct context *scontext,
> struct class_datum *tclass_datum;
> struct ebitmap *sattr, *tattr;
> struct ebitmap_node *snode, *tnode;
> + const struct selinux_class_perm *kdefs = &selinux_class_perm;
> unsigned int i, j;
>
> /*
> @@ -305,13 +306,6 @@ static int context_struct_compute_av(struct context *scontext,
> tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
> tclass = SECCLASS_NETLINK_SOCKET;
>
> - if (!tclass || tclass > policydb.p_classes.nprim) {
> - printk(KERN_ERR "security_compute_av: unrecognized class %d\n",
> - tclass);
> - return -EINVAL;
> - }
> - tclass_datum = policydb.class_val_to_struct[tclass - 1];
> -
> /*
> * Initialize the access vectors to the default values.
> */
> @@ -322,6 +316,36 @@ static int context_struct_compute_av(struct context *scontext,
> avd->seqno = latest_granting;
>
> /*
> + * Check for all the invalid cases.
> + * - tclass 0
> + * - tclass > policy and > kernel
> + * - tclass > policy but is a userspace class
> + * - tclass > policy but we do not allow unknowns
> + */
> + if (unlikely(!tclass))
> + goto inval_class;
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + if (tclass > kdefs->cts_len ||
> + !kdefs->class_to_string[tclass - 1] ||
> + policydb.handle_unknown != ALLOW_UNKNOWN)
> + goto inval_class;
> +
> + /*
> + * Kernel class and we ALLOW_UNKNOWN so pad the allow decision
> + * the pad will be all 1 for unknown classes.
> + */
> + if (tclass <= kdefs->cts_len && (policydb.handle_unknown == ALLOW_UNKNOWN))
> + avd->allowed = policydb.undefined_perms[tclass - 1];
> +
> + /*
> + * Not in policy. Since decision is completed (all 1 or all 0) return.
> + */
> + if (unlikely(tclass > policydb.p_classes.nprim))
> + return 0;
> +
> + tclass_datum = policydb.class_val_to_struct[tclass - 1];
> +
> + /*
> * If a specific type enforcement rule was defined for
> * this permission check, then use it.
> */
> @@ -387,6 +411,10 @@ static int context_struct_compute_av(struct context *scontext,
> }
>
> return 0;
> +
> +inval_class:
> + printk(KERN_ERR "security_compute_av: unrecognized class %d\n", tclass);
> + return -EINVAL;
> }
>
> static int security_validtrans_handle_fail(struct context *ocontext,
> @@ -1054,6 +1082,13 @@ static int validate_classes(struct policydb *p)
> const char *def_class, *def_perm, *pol_class;
> struct symtab *perms;
>
> + if (p->handle_unknown == ALLOW_UNKNOWN) {
> + u32 num_classes = kdefs->cts_len;
> + p->undefined_perms = kcalloc(num_classes, sizeof(u32), GFP_KERNEL);
> + if (!p->undefined_perms)
> + return -ENOMEM;
> + }
> +
> for (i = 1; i < kdefs->cts_len; i++) {
> def_class = kdefs->class_to_string[i];
> if (!def_class)
> @@ -1062,6 +1097,10 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: class %s not defined in policy\n",
> def_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[i-1] = ~0U;
> + if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> pol_class = p->p_class_val_to_name[i-1];
> @@ -1087,12 +1126,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= perm_val;
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
> @@ -1130,12 +1173,16 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
> "security: permission %s in class %s not defined in policy\n",
> def_perm, pol_class);
> + if (p->handle_unknown == ALLOW_UNKNOWN)
> + p->undefined_perms[class_val-1] |= (1 << j);
> + else if (p->handle_unknown == REJECT_UNKNOWN)
> + return -EINVAL;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> printk(KERN_ERR
> - "security: permission %s in class %s not found in policy\n",
> + "security: permission %s in class %s not found in policy, bad policy\n",
> def_perm, pol_class);
> return -EINVAL;
> }
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 15+ messages in thread