[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 1/2] SELinux Context Label based audit filtering



On Thu, 2006-02-02 at 15:17 -0500, Stephen Smalley wrote:
> On Thu, 2006-02-02 at 13:41 -0600, Dustin Kirkland wrote:
> > Kernel audit component
> > 
> > This patch is against David Woodhouse's last update of his audit-2.6 git
> > tree, plus a patch submitted by Amy Griffis on 2006-01-17 that adds
> > support for strings in audit rules.  This patch can be found here:
> > https://www.redhat.com/archives/linux-audit/2006-January/msg00064.html
> 
> Patch was encoded by your mailer.

That it was.  Apparently Evolution automatically encodes messages when
signed with a GPG key.  Which means a difficult choice between making
the patch readable and establishing authenticity ;)

> > Note that this code actually only provides enough functionality to
> > filter on _task_ labels.  I'm looking for input or acknowledgment from
> > the SELinux guys (cc'd) on the validity of the approach herein.
> > Additionally, I'm open to suggestions on how I might similarly collect
> > object and user labels for the same filtering mechanism (if required).
> > I hope to easily extend this patch to handle those as well, though I
> > wanted to put this much forth immediately to incorporate suggestions.
> 
> Object security contexts are already being harvested along the way, e.g.
> audit_inode() -> audit_inode_context(), so you already have them
> available at the point of filter checking.  Other (less expensive
> option) for both the object contexts and the task context would be to
> instead only harvest the SIDs (via new interfaces) and to provide
> interfaces for getting specific fields and compare them rather than
> having to allocate memory and generate full contexts each time, as we've
> discussed before on the list.  That does require changes to the SELinux
> module and new interfaces from it, of course.

Let the efficiency games begin...

I'll gladly pursue the less expensive option, though I will require some
guidance from you in implementing these new SELinux exported API's.

In this case, I guess I would like to have SELinux export something like
the following hypothetical function, which takes as input a sid and the
field position, and SELinux returns a char pointer to the requested
string:
char *security_get_field_from_sid(u32 sid, u32 field);

As it seems similar in functionality, should it live somewhere near
security_sid_to_context() in security/selinux/ss/services.c?  That
function expects a preallocated string...  Should it be possible to
return a const char* from the hypothetical security_get_field_from_sid()
to simplify the caller's mem management?  Where's the const string
located in SELinux located that could be so sliced up?  I'm really
looking for some pointers here.

Thus, the audit code would first need to call something like:
u32 security_get_sid(???);

What arguments would be required to such a function?  Could it be
general purpose enough for inode/ipc/etc objects, as well as tasks?

> > Comments appreciated...
> 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -140,6 +140,11 @@
> >  #define AUDIT_PERS	10
> >  #define AUDIT_ARCH	11
> >  #define AUDIT_MSGTYPE	12
> > +#define AUDIT_SE_USER	13	/* security label user */
> > +#define AUDIT_SE_ROLE	14	/* security label role */
> > +#define AUDIT_SE_TYPE	15	/* security label type */
> > +#define AUDIT_SE_CAT	16	/* security label category */
> > +#define AUDIT_SE_SENS	17	/* security label sensitivity */
> 
> There can be two levels in the MLS field (a low and a high), so you have
> potentially two sensitivities and two category sets, plus you may want
> to match on a particular category in a category set, not the entire
> thing.  Also, the above doesn't seem very extensible to cope with
> potential future extension of the SELinux security context.

Well, the audit rule filter structure needs some unique way to identify
each field, which we're doing with a unique integer per field.  Looking
at the lines preceding these new #define's, you'll see the rest of the
fields that we're able to filter upon.

I can easily throw in:
AUDIT_SE_CAT_L
AUDIT_SE_CAT_H
AUDIT_SE_SENS_L
AUDIT_SE_SENS_H

And add those to the switch statement.  That can continue on basically
indefinitely.  I don't know how much growth you expect in the context
label, but the audit system would have to stay in sync with your
changes.

But if I read you correctly, you'd like to see an entirely different
approach.

I suppose we could set the type of the field to a single AUDIT_SE_LABEL,
and elsewhere in the audit_rule_data structure store the offset (the
element number) of the SELinux label to match.  Unfortunately, I'm not
seeing a clean place to drop that offset integer into the structure.
Perhaps in audit_rule_data->fieldflags[i], but I don't really think
that's what that structure member was intended for.

Other suggestions?

> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -157,15 +157,23 @@ struct audit_context {
> >  #endif
> >  };
> >  
> > +static char *audit_get_task_label(void);
> >  
> >  /* Compare a task_struct with an audit_rule.  Return 1 on match, 0
> >   * otherwise. */
> >  static int audit_filter_rules(struct task_struct *tsk,
> >  			      struct audit_krule *rule,
> >  			      struct audit_context *ctx,
> > -			      enum audit_state *state)
> > +			      enum audit_state *state,
> > +			      char *label)
> >  {
> >  	int i, j;
> > +	char *user, *role, *type, *cat, *sens;
> > +	user = strsep(&label, ":");
> > +	role = strsep(&label, ":");
> > +	type = strsep(&label, ":");
> > +	cat  = strsep(&label, ":");
> > +	sens = strsep(&label, ":");
> 
> Audit code should not be directly parsing SELinux contexts.
> You want the SELinux module to perform the splitting, and preferably to
> even just give you the field directly as a const char * from SID so that
> you never have to allocate and generate the entire context string.

Ok.  I think the comments/questions I posted above should address
this...


:-Dustin

Attachment: signature.asc
Description: This is a digitally signed message part


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]