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

[Tim Gardner]

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.

rtg
-- 
Tim Gardner tim.gardner at canonical.com