[patch][trusty] Revert "UBUNTU: SAUCE: apparmor: fix unix domain sockets to be mediated on connection"

[John Johansen]

On 01/28/2014 03:08 AM, Tim Gardner wrote:
> On 01/28/2014 10:42 AM, John Johansen wrote:
>> This reverts commit 059c1f0963799ae6ac778863a82ba117e8041b54.
>>
>> http://bugs.launchpad.net/bugs/1270215
>>
>> Precise policy was not setup to deal with mediation of unix domain
>> sockets at connection, as such this patch causes policy failures on
>> precise. This bug could be fixed by updating policy but that would
>> still cause custom policy to break, so as with lts-saucy this feature
>> should be removed for lts-trusty on precise.
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>>   security/apparmor/lsm.c | 48 ++++++++++++------------------------------------
>>   1 file changed, 12 insertions(+), 36 deletions(-)
>>
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index b83e92b..b320317 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -787,29 +787,10 @@ do { \
>>   // sk->sk_socket is NULL when orphaned/being shutdown
>>   // socket->sk set on graft, and sock_init_data if (socket exists)
>>
>> -#define UNIX_ANONYMOUS(U) (!unix_sk(U)->addr)
>> -#define UNIX_FS(U) (!UNIX_ANONYMOUS(U) && unix_sk(U)->addr->name->sun_path[0])
>> -
>> -static int unix_fs_perm(int op, struct aa_label *label, struct sock *sk,
>> -            u32 mask)
>> -{
>> -    if (!unconfined(label) && UNIX_FS(sk)) {
>> -        struct unix_sock *u = unix_sk(sk);
>> -
>> -        /* the sunpath may not be valid for this ns so use the path */
>> -        struct path_cond cond = { u->path.dentry->d_inode->i_uid,
>> -                      u->path.dentry->d_inode->i_mode
>> -        };
>> -
>> -        return aa_path_perm(op, label, &u->path, 0, mask, &cond);
>> -    }
>> -    return 0;
>> -}
>> -
>>   /**
>>    * apparmor_unix_stream_connect - check perms before making unix domain conn
>>    *
>> - * other is locked when this hook is called
>> + * only used for alt unix socket namespace ???
>>    */
>>   static int apparmor_unix_stream_connect(struct sock *sock, struct sock *other,
>>                       struct sock *newsk)
>> @@ -817,16 +798,16 @@ static int apparmor_unix_stream_connect(struct sock *sock, struct sock *other,
>>       struct aa_sk_cxt *sock_cxt = SK_CXT(sock);
>>       struct aa_sk_cxt *other_cxt = SK_CXT(other);
>>       struct aa_sk_cxt *new_cxt = SK_CXT(newsk);
>> -    struct aa_label *label = __aa_get_current_label();
>>
>> -    int error = unix_fs_perm(OP_CONNECT, label, other,
>> -                 MAY_READ | MAY_WRITE);
>> -    __aa_put_current_label(label);
>>
>> -    if (error)
>> +#if 0
>> +    if (!perms to connect sock to other)
>> +
>>           return error;
>> +#endif
>>
>> -    /* Cross reference the peer labels for SO_PEERSEC */
>> +// ??? label not updated after connection??? it would be good if the label
>> +// was updated as the task labeling is updated
>>       if (new_cxt->peer) {
>>           //printk("%s: new_cxt->peer\n", __FUNCTION__);
>>           aa_put_label(new_cxt->peer);
>> @@ -849,21 +830,16 @@ static int apparmor_unix_stream_connect(struct sock *sock, struct sock *other,
>>   /**
>>    * apparmor_unix_may_send - check perms before conn or sending unix dgrams
>>    *
>> - * other is locked when this hook is called
>> + * Only used for alt unix socket namespace ????
>>    */
>>   static int apparmor_unix_may_send(struct socket *sock, struct socket *other)
>>   {
>> -    struct aa_sk_cxt *other_cxt = SK_CXT(other->sk);
>> -    struct aa_label *label = __aa_get_current_label();
>> -    int e, error ;
>> +  //  ??? how do these play in with regular perm checks, conditional?
>>
>> -    error = unix_fs_perm(OP_SENDMSG, label, other->sk, MAY_WRITE);
>> -    e = unix_fs_perm(OP_SENDMSG, other_cxt->label, sock->sk, MAY_READ);
>> -    if (e)
>> -        error = e;
>> -    __aa_put_current_label(label);
>> +//    print_sk(sock->sk);
>> +//    print_sk(other->sk);
>>
>> -    return error;
>> +    return 0;
>>   }
>>
>>   /**
>>
> 
> John - I assume this is really intended for application against the LTS branch only ? Is there a simpler patch such as a config option that is only turned on for the LTS version ? That way I don't have to carry a big delta between mainline and the LTS rebase.
> 
sorry yes, this is meant for lts-trust on precise

There is not a config option to disable this in the code atm. For the next sync of apparmor to trusty I can include a config option to disable new features that are not detected via policy load.