[apparmor] [patch 13/12] map the net permission set into a form compatible with the old dfa table

[Seth Arnold]

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...

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.)

> 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.
Bits 8-9, inclusive, are moved to 14-15.
Bits 17-19, inclusive, are moved to 11-13.
Bits 24-25, inclusive, are moved to 14-15.

GETATTR and GETOPT perhaps are similar enough, same with SETATTR and
SETOPT, so overlapping bits makes sense.

But why do we have separate bits for all these if we're just going to
collapse them all or move them around?

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
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140821/a8a83b8b/attachment-0001.pgp>