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 <[email protected]>

Acked-by: Tyler Hicks <[email protected]>

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

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to