Re: [apparmor] [PATCH 2/2] parser: Properly parse named transition targets

2016-02-26 Thread John Johansen
On 02/26/2016 04:22 PM, Tyler Hicks wrote:
> On 2016-02-17 22:47:41, John Johansen wrote:
>> On 02/11/2016 01:57 PM, Tyler Hicks wrote:
>>> https://launchpad.net/bugs/1540666
>>>
>>> Reuse the new parse_label() function to initialize named_transition
>>> structs so that transition targets, when used with change_profile, are
>>> properly seperated into a profile namespace and profile name.
>>>
>>> Signed-off-by: Tyler Hicks 
>>
>> Acked-by: John Johansen 
>>
>> for 2.10 as well
>>
>> though we are going to have to do another patch for stacking
>> we need to be able to express
>>
>>   change_profile -> A//&:ns://B,
>>
>> and
>>   change_profile -> :ns://A//&:ns://B,
> 
> The parser doesn't have to do anything special when the '&' is in the
> middle of the transition target, right? IIUC, the parser writs that
> entire string (":ns://A//&:ns://B") to the binary policy and then kernel
> splits it up and makes sense of the '&' characters.
> 
yes, and ideally the parser won't have to do anything for a target ns either,
except that legacy encoding requires it.

The kernel now accepts either, so for anything new we should just use the
simpler form. I will also note that on older kernels pivot_root targets that
specified an ns where broken because they didn't null terminate the ns as
the exec rules do (they share the same code to find the transition for the
x table).


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor


Re: [apparmor] [PATCH] parser: Clean up pivot_root target parsing

2016-02-26 Thread John Johansen
On 02/26/2016 04:07 PM, Tyler Hicks wrote:
> Instead of reusing opt_named_transition and be forced to reconstruct the
> target path when is looks like ":odd:target", create simpler grammer
> rules that have nothing to do with named transitions and namespaces.
> 
> Signed-off-by: Tyler Hicks 

Acked-by: John Johansen 

> ---
>  parser/parser_yacc.y | 22 ++
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
> index 1f00480..7af78ce 100644
> --- a/parser/parser_yacc.y
> +++ b/parser/parser_yacc.y
> @@ -276,6 +276,7 @@ void add_local_entry(Profile *prof);
>  %type net_perms
>  %type opt_net_perm
>  %typeunix_rule
> +%typeopt_target
>  %type  opt_named_transition
>  %type  opt_unsafe
>  %type  opt_file
> @@ -1044,6 +1045,9 @@ expr:   TOK_DEFINED TOK_BOOL_VAR
>  id_or_var: TOK_ID { $$ = $1; }
>  id_or_var: TOK_SET_VAR { $$ = $1; };
>  
> +opt_target: /* nothing */ { $$ = NULL; }
> +opt_target: TOK_ARROW id_or_var { $$ = $2; };
> +
>  opt_named_transition:
>   { /* nothing */
>   parse_named_transition_target(&$$, NULL);
> @@ -1242,23 +1246,9 @@ mnt_rule: TOK_UMOUNT opt_conds opt_id TOK_END_OF_RULE
>   $$ = do_mnt_rule($2, NULL, NULL, $3, AA_MAY_UMOUNT);
>   }
>  
> -mnt_rule: TOK_PIVOTROOT opt_conds opt_id opt_named_transition TOK_END_OF_RULE
> +mnt_rule: TOK_PIVOTROOT opt_conds opt_id opt_target TOK_END_OF_RULE
>   {
> - char *name = NULL;
> - if ($4.present && $4.ns) {
> - name = (char *) malloc(strlen($4.ns) +
> -strlen($4.name) + 3);
> - if (!name) {
> - PERROR("Memory allocation error\n");
> - exit(1);
> - }
> - sprintf(name, ":%s:%s", $4.ns, $4.name);
> - free($4.ns);
> - free($4.name);
> - } else if ($4.present)
> - name = $4.name;
> -
> - $$ = do_pivot_rule($2, $3, name);
> + $$ = do_pivot_rule($2, $3, $4);
>   }
>  
>  dbus_perm: TOK_VALUE
> 


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor


Re: [apparmor] [PATCH 2/2] parser: Properly parse named transition targets

2016-02-26 Thread Tyler Hicks
On 2016-02-17 22:47:41, John Johansen wrote:
> On 02/11/2016 01:57 PM, Tyler Hicks wrote:
> > https://launchpad.net/bugs/1540666
> > 
> > Reuse the new parse_label() function to initialize named_transition
> > structs so that transition targets, when used with change_profile, are
> > properly seperated into a profile namespace and profile name.
> > 
> > Signed-off-by: Tyler Hicks 
> 
> Acked-by: John Johansen 
> 
> for 2.10 as well
> 
> though we are going to have to do another patch for stacking
> we need to be able to express
> 
>   change_profile -> A//&:ns://B,
> 
> and
>   change_profile -> :ns://A//&:ns://B,

The parser doesn't have to do anything special when the '&' is in the
middle of the transition target, right? IIUC, the parser writs that
entire string (":ns://A//&:ns://B") to the binary policy and then kernel
splits it up and makes sense of the '&' characters.

Tyler


signature.asc
Description: PGP signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor


[apparmor] [PATCH] parser: Clean up pivot_root target parsing

2016-02-26 Thread Tyler Hicks
Instead of reusing opt_named_transition and be forced to reconstruct the
target path when is looks like ":odd:target", create simpler grammer
rules that have nothing to do with named transitions and namespaces.

Signed-off-by: Tyler Hicks 
---
 parser/parser_yacc.y | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
index 1f00480..7af78ce 100644
--- a/parser/parser_yacc.y
+++ b/parser/parser_yacc.y
@@ -276,6 +276,7 @@ void add_local_entry(Profile *prof);
 %type   net_perms
 %type   opt_net_perm
 %type  unix_rule
+%type  opt_target
 %type  opt_named_transition
 %type  opt_unsafe
 %type  opt_file
@@ -1044,6 +1045,9 @@ expr: TOK_DEFINED TOK_BOOL_VAR
 id_or_var: TOK_ID { $$ = $1; }
 id_or_var: TOK_SET_VAR { $$ = $1; };
 
+opt_target: /* nothing */ { $$ = NULL; }
+opt_target: TOK_ARROW id_or_var { $$ = $2; };
+
 opt_named_transition:
{ /* nothing */
parse_named_transition_target(&$$, NULL);
@@ -1242,23 +1246,9 @@ mnt_rule: TOK_UMOUNT opt_conds opt_id TOK_END_OF_RULE
$$ = do_mnt_rule($2, NULL, NULL, $3, AA_MAY_UMOUNT);
}
 
-mnt_rule: TOK_PIVOTROOT opt_conds opt_id opt_named_transition TOK_END_OF_RULE
+mnt_rule: TOK_PIVOTROOT opt_conds opt_id opt_target TOK_END_OF_RULE
{
-   char *name = NULL;
-   if ($4.present && $4.ns) {
-   name = (char *) malloc(strlen($4.ns) +
-  strlen($4.name) + 3);
-   if (!name) {
-   PERROR("Memory allocation error\n");
-   exit(1);
-   }
-   sprintf(name, ":%s:%s", $4.ns, $4.name);
-   free($4.ns);
-   free($4.name);
-   } else if ($4.present)
-   name = $4.name;
-
-   $$ = do_pivot_rule($2, $3, name);
+   $$ = do_pivot_rule($2, $3, $4);
}
 
 dbus_perm: TOK_VALUE
-- 
2.7.0


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor


Re: [apparmor] [profile: plugin-container] the dbus machine-id: deny or allow 'r'?

2016-02-26 Thread Simon McVittie
On 25/02/16 17:18, daniel curtis wrote:
> Anyway, there is a one thing which wonders me:
> '/var/lib/dbus/machine-id'. According to the DENIED messages in a log
> files, there is something like this:
> 
> name="/var/lib/dbus/machine-id", denied mask 'r'

The systemd machine ID (always /etc/machine-id) and the older D-Bus
machine ID (traditionally /var/lib/dbus/machine-id) are both a
randomly-generated unique ID (UUID, GUID) for the machine, intended to
be used in the situations where a hostname would traditionally be used.
Because hostnames are user-facing and partially cosmetic, they could get
changed by deliberate reconfiguration or a misconfigured DHCP client;
and because they are user-facing and short, they are non-unique (for
example consider how many machines have hostnames like "ubuntu" or
"localhost"). The machine ID is intended to avoid both of those issues.

Anything that needs a unique identifier for the machine, and does not
need to be prevented from identifying the machine for privacy reasons,
should read /etc/machine-id, its compile-time-configured
${localstatedir}/dbus/machine-id, and /var/lib/dbus/machine-id (ideally
in that order). It isn't *only* for D-Bus. The designers of D-Bus
introduced it partially because a couple of D-Bus features needed it,
and partially as a general "API" for the rest of the system.

For example, gnome-settings-daemon stores multi-display configurations
keyed by machine ID, so that you can share an NFS home directory between
machine A (where the HDMI monitor is to the left of the VGA monitor) and
machine B (where the HDMI monitor is above the laptop display), and each
machine gets its correct monitor configuration.

When connecting to the session bus, D-Bus implementations might need the
machine ID as part of the X11 autolaunch and DBUS_COOKIE_SHA1 protocols,
but X11 autolaunch is a bad idea (set DBUS_SESSION_BUS_ADDRESS correctly
instead) and DBUS_COOKIE_SHA1 should never be necessary on Linux (the
session bus now defaults to only allowing the simpler and more secure
EXTERNAL authentication). A modern Linux system shouldn't need it for
either of those features.

Connecting to the system bus doesn't require the machine ID at all, if I
remember correctly.

> "Some programs may request access to the DBus system bus socket, but may
> not actually need it for normal functioning. In these cases, (...) the
> same may be the case for the dbus machine-id:
> 
> deny /var/lib/dbus/machine-id r,"

I don't think that's specifically saying that you should allow reading
the machine ID, or that you should deny reading the machine ID.
Similarly, it is not saying that you should allow or deny connecting to
the system bus socket (see  for what that
means) or the session bus socket ().

What it is saying is that neither is necessarily always correct, and
you'll need to make your own decision, based on an assessment of what
the program does, what its fallback behaviour would be for denial, and
your own security policy (i.e. how much functionality you would be
willing to give up for increased privacy).

There are various possible reasons why plugin-container might read the
machine ID; I don't know which one applies.

In practice, if plugin-container is able to determine IP addresses,
network device MAC addresses, hardware serial numbers, the username, the
fully-qualified hostname and other identifying information, then there
is little point in preventing it from reading the machine ID, because it
probably already has enough information to identify a machine uniquely.
If it is sufficiently locked-down to have only a small subset of that
information, then there might be some benefit in preventing it from
reading the machine ID as well.

Regards,
a D-Bus maintainer
-- 
Simon McVittie
Collabora Ltd. 


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor