[apparmor] [patch 05/21] Add stub rules to indicate compilation support for given features.

[Steve Beattie]

On Tue, Mar 18, 2014 at 04:48:01PM -0700, John Johansen wrote:
> On 03/18/2014 04:21 PM, Steve Beattie wrote:
> > On Mon, Mar 17, 2014 at 04:29:15PM -0700, john.johansen at canonical.com wrote:
> >> Policy enforcement needs to be able to support older userspaces and
> >> compilers that don't know about new features. The absence of a feature
> >> in the policydb indicates that feature mediation is not present for
> >> it.
> >>
> >> We add stub rules, that provide a none 0 start state for features that
> >> are supported at compile time. This can be used by the kernel to
> >> indicate that it should enforce a given feature. This does not indicate
> >> the feature is allowed, in an abscence of other rules for the feature
> >> the feature will be denied.
> >>
> >> Note: this will break the minimize tests when run with kernels that
> >>       support mount or dbus rules. A patch to specify these features to
> >>       the parser is needed to fix this.
> >>
> >> Signed-off-by: John Johansen <john.johansen at canonical.com>
> > 
> > This seems a bit goofy to me, squishing these into dfa rule
> > structures. Is there a longer term plan to handle this more cleanly, or
> > am I misunderstanding how this patch works?
> > 
> Hrmmm, no I don't have longer term plans to change how this works, except
> to cleanup how/when these goofy rules are injected. Let me try to explain
> this better. So either we do it in the dfa OR we have an external table to
> consult.
> 
> Doing it in the dfa has the advantage of not needing an external table and
> its overhead, when the policydb can easily encode this info. Basically the
> policydb is layed out so that every rule class starts with a class identifier.
> 
> 
>   start state[class ptrace] --> ptrace start state 
> 
> 
> feeding the class id into the dfa gets you to what is the start state for
> that class. It may be shared with other classes it doesn't matter. During
> the unpack the kernel walks the first state to find the different states
> for the classes it supports.
> 
> If there aren't any rules for that class in the dfa, the walk results in a
> none matching start state for that class. This is what we actually are using
> to determine if policy expects enforcement of that class, not the perm on
> the start state.
> 
> If policy has rules for that class there will be a valid start state for
> the class. The case that doesn't work is if a profile doesn't have a rule
> for the class. But that doesn't mean that the profile doesn't expect the
> class to be enforced, policy may have been written expecting that class
> to be enforced but the profile doesn't have permission to use it.
> 
> For profiles that have rules of a given class this dummy rule isn't required.
> But for profiles that don't have rules a dummy rule is required to generate
> the start state. The permission on the rule is needed to keep dfa
> minimization from removing it.
> 
> We could go with an external table of supported classes, but it would
> require more work and require extra validation to get the same thing. And
> it would also have to cope with our extended class support which will
> allow us to support more classes than the base 255.
> 
> So I see it as more work for no gain.
> 
> What I do plan to do is improve the current rule object classes to remove
> the need for the current kludge of generating these here. It will get
> embedded in the classes and just happen correctly. I just never got that
> far. The other change I plan for is the permission that will tag these
> rules when we move to the newer extended permission set. It will be
> AA_CONTINUE, which just tells the matching engine there is more match
> to follow to find permissions.

Okay, I just found it a bit goofy to get the following:

  $ echo 'profile foo { /usr/bin/foo rix, } ' | ./apparmor_parser --dump rule-exprs -Q
  Warning from stdin (line 1): ./apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
  aare: foo   ->   foo
  aare: /usr/bin/foo   ->   /usr/bin/foo
  rule: /usr/bin/foo  ->  /usr/bin/foo (0x914245/0)(((((< 0x201>|< 0x4>)|<
  0x40>)|< 0x804000>)|< 0x10000>)|< 0x100000>)
  
  rule: \d7  ->  \a (0x4/0)< 0x4>
  
  rule: \d32  ->  \  (0x4/0)< 0x4>
 
And I worry about dumping out random garbage from the CharNode that gets
created for these.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140318/3dabe364/attachment-0001.pgp>