[apparmor] [patch 13/12] map the net permission set into a form compatible with the old dfa table
John Johansen
john.johansen at canonical.com
Sat Aug 23 04:59:29 UTC 2014
On 08/21/2014 10:40 PM, Seth Arnold wrote:
> On Thu, Aug 21, 2014 at 02:45:19PM -0700, John Johansen wrote:
>> so this should apply on top of the v2 patches and is the new direction
>> for handling the permission issues for the af_unix socket rules.
>>
>>
>> map the net permission set into a form compatible with the old dfa table
>>
>> The old dfa table format has 2 64 bit permission field used to store
>> all of allow, quiet, audit, owner/!owner and transition mask. This leaves
>> 7 bits for entry + a few other special bits.
>>
>> Since policydb entries when using old style dfa permission format
>> don't use support the !owner permission entries we can map, the
>> high net work permission bits to these entries.
>>
>> This allows us to enforce base network permissions on system with
>> only support for the old dfa table format.
>
> I'm having trouble following the thread of this code; it sounds like a
> general compatibility patch, but I expected it to be constructing
> different structures entirely. It is modifying the structures that are
> being built and I don't know what 'modern' systems will do with these: the
> permissions are mangled...
>
yes, the set of patches to change the structures is much larger and still
has problems, for us to fully land the structure changes is going to be
another thirty or forty patches, and will touch aare_rules, expr_tree,
hfa, and chfa. And while we are at it add the userspace dfa bits so we can
do proper tests of the whole mess.
Looking at time frames its not going to happen so I managed to come up with
a small patch that will let us do the minimum of what is needed.
> Even if this were adding a compatibility object to the policy, I'd expect
> that old-dfa systems weren't prepared for these permissions anyway. (Or,
> if they were prepared for these, I don't understand why these changes are
> here when there's already a unix_rule::downgrade_rule() method.)
>
Okay, we carry 2 different sets of information for network mediation.
The old af bitmask table and the new fine grained extensions in the
policydb.
We are always setting the old table, the down grade notice is that a
new fine grained rule was not created if the policy called for it.
So lets cover the cases:
* generating old policy only
- will generate policy af table
- will warn about downgrading if a new unix rule, etc is present
- can be loaded and enforced on old kernels and new kernels
* generating new policy
- will generate policy af table (this is used also for any af without
a fine grained mediation)
- will generate fine grained rules in the policydb
- will NOT warn about down grading
- can be loaded on new kernels, and MAY be loaded on kernels that
support the new ABI, but have not been patched for fine grained
unix socket mediation.
In this case the more generic downgraded unix socket rules are
used.
Yes userspace can chose to detect version differences and recompile
policy or use a different cache, but the kernel is compatible with
supporting policy compiled, and being downgraded due to a missing
extension.
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>> parser/af_unix.cc | 30 +++++++++++++++++++-----------
>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> --- 2.9-test.orig/parser/af_unix.cc
>> +++ 2.9-test/parser/af_unix.cc
>> @@ -216,6 +216,14 @@
>> }
>> }
>>
>> +static uint32_t map_perms(uint32_t mask)
>> +{
>> + return (mask & 0x7f) |
>> + ((mask & (AA_NET_GETATTR | AA_NET_SETATTR)) << (AA_OTHER_SHIFT - 8)) |
>> + ((mask & (AA_NET_ACCEPT | AA_NET_BIND | AA_NET_LISTEN)) >> 6) | /* AA_OTHER_SHIFT - 20 */
>> + ((mask & (AA_NET_SETOPT | AA_NET_GETOPT)) >> 10); /* AA_OTHER_SHIFT - 24 */
>> +}
>
> I had to break this down a bit...
> Bits 0-6, inclusive, are kept.
Yes these are the bits that are currently being used by the other
rules in the policy db mount, signals, ptrace, ...
> Bits 8-9, inclusive, are moved to 14-15.
yes
> Bits 17-19, inclusive, are moved to 11-13.
no this should be
20-22 inclusive are moved to 16-18
the bits are right the shift is wrong.
> Bits 24-25, inclusive, are moved to 14-15.
>
yes bits 24-25, should be going to position 19-20
so the shift is wrong again.
> GETATTR and GETOPT perhaps are similar enough, same with SETATTR and
> SETOPT, so overlapping bits makes sense.
>
no we are keeping them separate
> But why do we have separate bits for all these if we're just going to
> collapse them all or move them around?
>
because we are not, this is a temporary solution to fit the permissions
into the current 64 bit permission structure. 64 bits sounds like a lot
but its not when its carrying the audit, quiet, xtransition, and permissions
for both owner and !owner.
Basically its 7 permission bits owner, and 7 not owner, since the policydb
doesn't use the owner/!owner stuff we can get away with using the 14 bits
but we have to map to the format the dfa is expecting before it starts
mangling things.
We need to fix this, patches welcome
> Thanks
>
>> +
>> int unix_rule::gen_policy_re(Profile &prof)
>> {
>> std::ostringstream buffer, tmp;
>> @@ -258,8 +266,8 @@
>> if (mask & AA_NET_CREATE) {
>> buf = buffer.str();
>> if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>> - AA_NET_CREATE,
>> - audit & AA_NET_CREATE,
>> + map_perms(AA_NET_CREATE),
>> + map_perms(audit & AA_NET_CREATE),
>> dfaflags))
>> goto fail;
>> mask &= ~AA_NET_CREATE;
>> @@ -300,8 +308,8 @@
>> if (mask & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD) {
>> buf = buffer.str();
>> if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>> - mask & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD,
>> - audit & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD,
>> + map_perms(mask & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD),
>> + map_perms(audit & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD),
>> dfaflags))
>> goto fail;
>> }
>> @@ -312,8 +320,8 @@
>> tmp << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_ACCEPT;
>> buf = tmp.str();
>> if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>> - AA_NET_ACCEPT,
>> - audit & AA_NET_ACCEPT,
>> + map_perms(AA_NET_ACCEPT),
>> + map_perms(audit & AA_NET_ACCEPT),
>> dfaflags))
>> goto fail;
>> }
>> @@ -324,8 +332,8 @@
>> tmp << "..";
>> buf = tmp.str();
>> if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>> - AA_NET_LISTEN,
>> - audit & AA_NET_LISTEN,
>> + map_perms(AA_NET_LISTEN),
>> + map_perms(audit & AA_NET_LISTEN),
>> dfaflags))
>> goto fail;
>> }
>> @@ -336,8 +344,8 @@
>> tmp << "..";
>> buf = tmp.str();
>> if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>> - AA_NET_OPT,
>> - audit & AA_NET_OPT,
>> + map_perms(AA_NET_OPT),
>> + map_perms(audit & AA_NET_OPT),
>> dfaflags))
>> goto fail;
>> }
>> @@ -375,7 +383,7 @@
>> }
>>
>> buf = buffer.str();
>> - if (!prof.policy.rules->add_rule(buf.c_str(), deny, mode & AA_PEER_NET_PERMS, audit, dfaflags))
>> + if (!prof.policy.rules->add_rule(buf.c_str(), deny, map_perms(mode & AA_PEER_NET_PERMS), map_perms(audit), dfaflags))
>> goto fail;
>> }
>>
>>
>> --
>> AppArmor mailing list
>> AppArmor at lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
>>
>>
>>
More information about the AppArmor
mailing list