[apparmor] [patch 18/12] v3 unix socket rules

[Tyler Hicks]

On 2014-09-05 16:29:59, John Johansen wrote:
> So a revised version of the patch
> 
> v2 - properly handle the accept permission, look for the bits with local_mask
> 
> Do not output local permissions for rules that have peer_conditionals
> 
> while it is not possible to specify a rule with local conditionals with
> peer conditionals
> eg.
>    unix listen peer=(addr=@foo),
> 
> a rule such as
>    unix peer=(addr=@foo),
> 
> is possible, and was setting all permissions for local as well as the peer
> condition permissions.
> 
> Currently this means the create permission must be specified in a separate
> rule from a rule with a peer= condition, if create is to be allowed. This
> isn't too much of an issue but it does mean rule such as
>   unix connect peer=(addr=@foo),
> 
> Can not imply the ability to create a socket. Which may indeed be the
> behavior if we wish to enforce that the socket was created in another
> process and passed in. Is this what we want to do?
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>

Acked-by: Tyler Hicks <tyhicks at canonical.com>

I've been testing this patch quite a bit, too.

I'm going to commit this patch to lp:apparmor since I have a patch (that
has been acked already) which builds on it and I don't want us to lose
track of either of them.

Tyler

> 
> ---
> 
> === modified file 'parser/af_unix.cc'
> --- parser/af_unix.cc	2014-09-05 15:49:33 +0000
> +++ parser/af_unix.cc	2014-09-05 23:17:06 +0000
> @@ -334,7 +334,7 @@
>  	}
>  
>  	write_to_prot(buffer);
> -	if (mask & AA_NET_CREATE) {
> +	if ((mask & AA_NET_CREATE) && !has_peer_conds()) {
>  		buf = buffer.str();
>  		if (!prof.policy.rules->add_rule(buf.c_str(), deny,
>  						 map_perms(AA_NET_CREATE),
> @@ -355,16 +355,18 @@
>  		buffer << "\\x00";
>  
>  		/* create already masked off */
> -		if (mask & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD) {
> +		int local_mask = has_peer_conds() ? AA_NET_ACCEPT :
> +					AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD;
> +		if (mask & local_mask) {
>  			buf = buffer.str();
>  			if (!prof.policy.rules->add_rule(buf.c_str(), deny,
> -							 map_perms(mask & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD),
> -							 map_perms(audit & AA_LOCAL_NET_PERMS & ~AA_LOCAL_NET_CMD),
> +							 map_perms(mask & local_mask),
> +							 map_perms(audit & local_mask),
>  							 dfaflags))
>  				goto fail;
>  		}
>  
> -		if (mask & AA_NET_LISTEN) {
> +		if ((mask & AA_NET_LISTEN) && !has_peer_conds()) {
>  			std::ostringstream tmp(buffer.str());
>  			tmp.seekp(0, ios_base::end);
>  			tmp << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_LISTEN;
> @@ -377,7 +379,7 @@
>  							 dfaflags))
>  				goto fail;
>  		}
> -		if (mask & AA_NET_OPT) {
> +		if ((mask & AA_NET_OPT) && !has_peer_conds()) {
>  			std::ostringstream tmp(buffer.str());
>  			tmp.seekp(0, ios_base::end);
>  			tmp << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_OPT;
> 
-------------- 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/20140922/8f52981b/attachment.pgp>