[apparmor] [PATCH v2 4/6] utils: Add the --namespace option to C based aa-exec

[Tyler Hicks]

On 2015-12-17 14:30:58, John Johansen wrote:
> On 12/16/2015 07:25 PM, Tyler Hicks wrote:
> > Switch to the policy in the namespace specified by the --namespace
> > option.
> > 
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> > ---
> >  binutils/aa_exec.c | 55 +++++++++++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 46 insertions(+), 9 deletions(-)
> > 
> > diff --git a/binutils/aa_exec.c b/binutils/aa_exec.c
> > index 9bcd62f..a57b4ec 100644
> > --- a/binutils/aa_exec.c
> > +++ b/binutils/aa_exec.c
> > @@ -19,6 +19,7 @@
> >  #include <errno.h>
> >  #include <getopt.h>
> >  #include <libintl.h>
> > +#include <limits.h>
> >  #include <stdio.h>
> >  #include <stdarg.h>
> >  #include <stdlib.h>
> > @@ -28,6 +29,7 @@
> >  #define _(s) gettext(s)
> >  
> >  static const char *opt_profile = NULL;
> > +static const char *opt_namespace = NULL;
> >  static bool opt_debug = false;
> >  static bool opt_immediate = false;
> >  static bool opt_verbose = false;
> > @@ -49,6 +51,7 @@ static void usage(const char *name, bool error)
> >  		"\n"
> >  		"OPTIONS:\n"
> >  		"  -p PROFILE, --profile=PROFILE		PROFILE to confine <prog> with\n"
> > +		"  -n NAMESPACE, --namespace=NAMESPACE	NAMESPACE to confine <prog> in\n"
> >  		"  -d, --debug				show messages with debugging information\n"
> >  		"  -i, --immediate			change profile immediately instead of at exec\n"
> >  		"  -v, --verbose				show messages with stats\n"
> > @@ -112,11 +115,12 @@ static char **parse_args(int argc, char **argv)
> >  		{"debug", no_argument, 0, 'd'},
> >  		{"help", no_argument, 0, 'h'},
> >  		{"profile", required_argument, 0, 'p'},
> > +		{"namespace", required_argument, 0, 'n'},
> >  		{"immediate", no_argument, 0, 'i'},
> >  		{"verbose", no_argument, 0, 'v'},
> >  	};
> >  
> > -	while ((opt = getopt_long(argc, argv, "+dhp:iv", long_opts, NULL)) != -1) {
> > +	while ((opt = getopt_long(argc, argv, "+dhp:n:iv", long_opts, NULL)) != -1) {
> >  		switch (opt) {
> >  		case 'd':
> >  			opt_debug = true;
> > @@ -127,6 +131,9 @@ static char **parse_args(int argc, char **argv)
> >  		case 'p':
> >  			opt_profile = optarg;
> >  			break;
> > +		case 'n':
> > +			opt_namespace = optarg;
> > +			break;
> >  		case 'i':
> >  			opt_immediate = true;
> >  			break;
> > @@ -145,28 +152,58 @@ static char **parse_args(int argc, char **argv)
> >  	return argv + optind;
> >  }
> >  
> > +static void build_name(char *name, size_t name_len,
> > +		       const char *namespace, const char *profile)
> > +{
> > +	size_t required_len = 1; /* reserve 1 byte for NUL-terminator */
> > +
> > +	if (namespace)
> > +		required_len += 1 + strlen(namespace) + 1; /* :<NAMESPACE>: */
> > +
> > +	if (profile)
> > +		required_len += strlen(profile);
> > +
> > +	if (required_len > name_len)
> > +		error("name too long (%zu > %zu)", required_len, name_len);
> > +
> > +	name[0] = '\0';
> > +
> > +	if (namespace) {
> > +		strcat(name, ":");
> > +		strcat(name, namespace);
> > +		strcat(name, ":");
> > +	}
> > +
> while this does work, the interface accepts
>   :<namespace name>:<profile name>
> and
>   :<namespace name>://<profile name>
> 
> if there is an error we are exposing this to the user instead of the more standard
>   :<namespace name>://
> 
> I'm not sure it is worth changing, I like the shorter form for the kernel but I
> think in generally anything exposed to the user should probably try to be
> consistent, and probably stick with the :// syntax as that is what is used
> beyond apparmor

It is a simple change to move to the double slash syntax.

> 
> also I smell an opportunity for a library fn

Yeah, I'll keep this in mind as I'm working on the userspace API early
next year.

> 
> > +	if (profile)
> > +		strcat(name, profile);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> > +	char name[PATH_MAX];
> >  	int rc = 0;
> >  
> >  	argv = parse_args(argc, argv);
> >  
> > -	if (!opt_profile)
> > +	if (opt_namespace || opt_profile)
> > +		build_name(name, sizeof(name), opt_namespace, opt_profile);
> > +	else
> >  		goto exec;
> >  
> >  	if (opt_immediate) {
> > -		verbose("aa_change_profile(\"%s\")", opt_profile);
> > -		rc = aa_change_profile(opt_profile);
> > -		debug("%d = aa_change_profile(\"%s\")", rc, opt_profile);
> > +		verbose("aa_change_profile(\"%s\")", name);
> > +		rc = aa_change_profile(name);
> > +		debug("%d = aa_change_profile(\"%s\")", rc, name);
> >  	} else {
> > -		verbose("aa_change_onexec(\"%s\")", opt_profile);
> > -		rc = aa_change_onexec(opt_profile);
> > -		debug("%d = aa_change_onexec(\"%s\")", rc, opt_profile);
> > +		verbose("aa_change_onexec(\"%s\")", name);
> > +		rc = aa_change_onexec(name);
> > +		debug("%d = aa_change_onexec(\"%s\")", rc, name);
> >  	}
> >  
> >  	if (rc) {
> >  		if (errno == ENOENT || errno == EACCES) {
> > -			error("profile '%s' does not exist", opt_profile);
> > +			error("%s '%s' does not exist\n",
> > +			      opt_profile ? "profile" : "namespace", name);
> 
> this is where we expose it to the user

Ok, does this extra change to this patch get your ack?


diff --git a/binutils/aa_exec.c b/binutils/aa_exec.c
index a57b4ec..7e73f45 100644
--- a/binutils/aa_exec.c
+++ b/binutils/aa_exec.c
@@ -158,7 +158,7 @@ static void build_name(char *name, size_t name_len,
 	size_t required_len = 1; /* reserve 1 byte for NUL-terminator */
 
 	if (namespace)
-		required_len += 1 + strlen(namespace) + 1; /* :<NAMESPACE>: */
+		required_len += 1 + strlen(namespace) + 3; /* :<NAMESPACE>:// */
 
 	if (profile)
 		required_len += strlen(profile);
@@ -171,7 +171,7 @@ static void build_name(char *name, size_t name_len,
 	if (namespace) {
 		strcat(name, ":");
 		strcat(name, namespace);
-		strcat(name, ":");
+		strcat(name, "://");
 	}
 
 	if (profile)
-------------- 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/20151217/4ca0b863/attachment.pgp>