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 <[email protected]> >> --- >> 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 >> [email protected] >> Modify settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/apparmor >> >> >> -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
