[apparmor] [patch 02/12] parser: Add support for unix domain socket rules.

[Tyler Hicks]

I've found a few issues with this patch through testing.

 1) It segfaults on bare unix rules ("unix,"). However, this is fixed
    later in patch 12. I have not looked into the cause or the fix.

 2) It doesn't allow the policy author to only specify a permission. For
    example you can't create a blanket rule for all AF_UNIX create
    permissions ("unix create,"). The create operation is denied unless
    you specify the type ("unix create type=stream,"). This can be
    manually demonstrated using a simple profile to confine dbus-send
    ("profile test { file, dbus, unix create, }") but I also hope to
    have regression tests ready within the next day or two to test these
    types of things.

 3) It messes up the stub rule creation and causes some rule types to
    not be enforced. I'll point out where I think the problem area is
    below and I'll also respond with a proposed fix.

On 2014-08-25 17:06:07, john.johansen at canonical.com wrote:
> This patch implements parsing of fine grained mediation for unix domain
> sockets, that have abstract and anonymous paths. Sockets with file
> system paths are handled by regular file access rules.

<snip>

> --- 2.9-test.orig/parser/parser_regex.c
> +++ 2.9-test/parser/parser_regex.c
> @@ -665,14 +665,17 @@
>  	return TRUE;
>  }
>  
> -#define MAKE_STR(X) #X
> -#define CLASS_STR(X) "\\d" MAKE_STR(X)
> +#define MAKE_STR(A) #A
> +#define CLASS_STR(X) "\\000\\d" MAKE_STR(X)
> +#define CLASS_SUB_STR(X, Y) MAKE_STR(X) MAKE_STR(Y)

This changes doesn't look correct and I've verified that it causes some
rule types to not be enforced. I'll make a guess at what the magic byte
sequence should be and respond with a tested patch. However, I'd like
for you to verify that the byte sequence I picked is correct.

Tyler

>  
>  static const char *mediates_file = CLASS_STR(AA_CLASS_FILE);
>  static const char *mediates_mount = CLASS_STR(AA_CLASS_MOUNT);
>  static const char *mediates_dbus =  CLASS_STR(AA_CLASS_DBUS);
>  static const char *mediates_signal =  CLASS_STR(AA_CLASS_SIGNAL);
>  static const char *mediates_ptrace =  CLASS_STR(AA_CLASS_PTRACE);
> +static const char *mediates_extended_net = CLASS_STR(AA_CLASS_NET);
> +static const char *mediates_net_unix = CLASS_SUB_STR(AA_CLASS_NET, AF_UNIX);
>  
>  int process_profile_policydb(Profile *prof)
>  {
> @@ -689,7 +692,7 @@
>  	 * to be supported
>  	 */
>  
> -	/* note: this activates unix domain sockets mediation on connect */
> +	/* note: this activates fs based unix domain sockets mediation on connect */
>  	if (kernel_abi_version > 5 &&
>  	    !prof->policy.rules->add_rule(mediates_file, 0, AA_MAY_READ, 0, dfaflags))
>  		goto out;
> @@ -705,6 +708,10 @@
>  	if (kernel_supports_ptrace &&
>  	    !prof->policy.rules->add_rule(mediates_ptrace, 0, AA_MAY_READ, 0, dfaflags))
>  		goto out;
> +	if (kernel_supports_unix &&
> +	    (!prof->policy.rules->add_rule(mediates_extended_net, 0, AA_MAY_READ, 0, dfaflags) ||
> +	     !prof->policy.rules->add_rule(mediates_net_unix, 0, AA_MAY_READ, 0, dfaflags)))
> +		goto out;
>  
>  	if (prof->policy.rules->rule_count > 0) {
>  		prof->policy.dfa = prof->policy.rules->create_dfa(&prof->policy.size, dfaflags);
-------------- 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/20140826/4c995261/attachment.pgp>