[apparmor] C based aa-exec

[Seth Arnold]

On Thu, Jul 11, 2013 at 03:53:05PM -0700, John Johansen wrote:
> So this is a C based version of aa-exec to replace the perl version.

I like it better already... but... :)

> ---
> 
> /*
>  *   Copyright (C) 2013 Canonical Ltd.
>  *
>  *   This program is free software; you can redistribute it and/or
>  *   modify it under the terms of version 2 of the GNU General Public
>  *   License published by the Free Software Foundation.
>  */
> 
> #define _GNU_SOURCE
> 
> #include <errno.h>
> #include <getopt.h>
> #include <libintl.h>
> #include <locale.h>
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/apparmor.h>
> 
> #define _(s) gettext(s)
> 
> #define aa_verbose(X...)		\
> do {					\
> 	if (opt_verbose) {		\
> 		fprintf(g_outfile, X);	\
> 	}				\
> } while (0)
> 
> #define aa_error(X, Y...)				\
> do {							\
> 	if (X) {					\
> 		fprintf(g_outfile, "aborting: " Y);	\
> 		exit(-1);				\
> 	}						\
> } while (0)
> 
> /* Internationalization support. Can be defined in Makefile */
> #ifndef PACKAGE
> #define PACKAGE "apparmor"
> #endif
> #ifndef LOCALEDIR
> #define LOCALEDIR ""
> #endif
> 
> const char *g_version	= "3.0";
> const char *g_copyright	= "Copyright (C) 2013 Canonical Ltd.";
> FILE *g_outfile;
> char *g_progname;
> int opt_immediate;
> int opt_verbose;
> char *opt_debug;
> char *opt_file;
> char *opt_profile;
> char *opt_namespace;
> 
> const char *g_options =
> 	"Options:\n"
> 	"--------\n"
> 	"-p PROFILE, --profile=PROFILE		PROFILE to confine <prog> with\n"
> 	"-n NAMESPACE, --namespace=NAMESPACE	NAMESPACE to confine <prog> in\n"
> 	"-f FILE, --file=FILE			profile file to load\n"
> 	"-i, --immediate			change profile immediately instead of at exec\n"
> 	"-v, --verbose				show messages with stats\n"
> 	"-d FILE, --debug=FILE			dump verbose output to FILE\n"
> 	"-h, --help				display this help\n";
> 
> const char *short_options = "d:hp:n:f:iv";
> struct option long_options[] = {
> 	{"debug", 		1, 0, 'd'},
> 	{"help",		0, 0, 'h'},
> 	{"profile",		1, 0, 'p'},
> 	{"namespace",		1, 0, 'n'},
> 	{"file",		1, 0, 'f'},
> 	{"immediate",		0, 0, 'i'},
> 	{"verbose",		0, 0, 'v'},
> 	{"Version",		0, 0, 'V'},
> 	{NULL, 0, 0, 0},
> };
> 
> static void display_version(void)
> {
> 	printf("%s version %s\n%s\n", g_progname, g_version, g_copyright);
> }
> 
> static void display_usage(void)
> {
> 	display_version();
> 	printf("\nUsage: %s [options] [--] <prog> <args>\n\n%s", g_progname,
> 	       g_options);
> }
> 
> static void process_args(int argc, char *argv[])
> {
> 	int c, o;
> 	int count = 0;
> 
> 	while ((c = getopt_long(argc, argv, short_options, long_options, &o)) != -1) {
> 		switch (c) {
> 		case 0:
> 			display_usage();
> 			exit(0);
> 			break;
> 		case 'V':
> 			display_version();
> 			break;
> 		case 'd':
> 			opt_verbose = 1;
> 			opt_debug = optarg;
> 			break;
> 		case 'h':
> 			display_usage();
> 			exit(0);
> 			break;
> 		case 'p':
> 			opt_profile = optarg;
> 			break;
> 		case 'n':
> 			opt_namespace = optarg;
> 			break;
> 		case 'f':
> 			opt_file = optarg;
> 			break;
> 		case 'i':
> 			opt_immediate = 1;
> 			break;
> 		case 'v':
> 			opt_verbose = 1;
> 			break;
> 		default:
> 			printf("Unknown option %c\n\n", c);
> 			display_usage();
> 			exit(0);
> 			break;
> 		}
> 		count++;
> 	}
> }
> 
> int main(int argc, char *argv[])
> {
> 	int rc = 0;
> 	char *profile = NULL;
> 
> 	/* name of executable, for error reporting and usage display */
> 	g_progname = argv[0];
> 	g_outfile = stderr;
> 
> 	setlocale(LC_MESSAGES, "");
> 	bindtextdomain(PACKAGE, LOCALEDIR);
> 	textdomain(PACKAGE);

None of the newly-added strings are marked for translation. Getting the
strerror(3) messages translated will probably be useful for the user,
but having our strings available for translation would be kind. (Even if
in the usual case none are ever seen..)

> 	process_args(argc, argv);
> 
> 	if (opt_debug) {
> 		FILE *tmp = fopen(opt_debug, "w+");
> 		aa_error(tmp == NULL, "could not open debug file %s\n", opt_debug);
> 		aa_error(setvbuf(tmp, NULL, _IONBF, 0) != 0, "could not set debug file %s to unbuffered\n", opt_debug);
> 		g_outfile = tmp;
> 	}
> 


The following if() statement means this won't actually load a profile:

aa-exec -f foo_profile -- executable

Someone might want to load a profile that has a path-based attachment
knowing that the executable will eventually exec() that program...
(Think of the old firefox.sh script..)

> 	if (opt_namespace || opt_profile) {
> 		if (opt_namespace) {
> 			rc = asprintf(&profile, ":%s:%s", opt_namespace, opt_profile ? opt_profile : "");
> 			aa_error(rc == -1, "%s", strerror(errno));
> 
> 		} else if (opt_profile) {
> 			rc = asprintf(&profile, "%s", opt_profile);
> 			aa_error(rc == -1, "%s", strerror(errno));
> 		}
> 
> 		if (opt_file) {
> 			char *cmd;
> 			rc = asprintf(&cmd, "apparmor_parser -r %s", opt_file);
> 			aa_error(rc == -1, "%s", strerror(errno));
> 			aa_verbose("%s\n", cmd);
> 			rc = system(cmd);
> 			aa_error(rc == -1, "could not load file %s", opt_file);
> 		}

I'm not particularly keen on the use of system(3) to launch
apparmor_parser here -- if the given filename is shell-unsafe, this might
be a problem. (Not a security problem from command-line use, of course,
but if the file-to-load comes from a process that isn't as trusted as
root, it'll be an issue.)

Of course, I assume future versions of the parser will make it so this
can be replaced with a library call instead, but in the meantime, doing
the full and annoying argv = { "/sbin/apparmor_parser", "-r", opt_file};
dance would be safer.

> 
> 		if (opt_immediate) {
> 			aa_verbose("aa_change_profile(\"%s\")\n", profile);
> 			rc = aa_change_profile(profile);
> 		} else {
> 			aa_verbose("aa_change_onexec(\"%s\")\n", profile);
> 			rc = aa_change_onexec(profile);
> 		}
> 
> 		if (rc == -1) {
> 			if (errno == ENOENT || errno == EACCES)
> 				aa_error(1, "\'%s\' does not exist", profile);
> 			else if (errno == EINVAL)
> 				aa_error(1, "AppArmor interface not available");
> 			else
> 				aa_error(1, "%s", strerror(errno));
> 		}
> 	}
> 
> 	aa_verbose("exec %s\n", argv[optind]);
> 	(void)execvpe(argv[optind], &argv[optind], environ);
> 	/* exec failed, kill outselves to flag parent */

"ourselves"

> 	rc = errno;
> 	fprintf(g_outfile, "FAIL: exec to '%s' failed (%s)\n", argv[optind],
> 		strerror(errno));
> 	return rc;
> }

Just a note that "return rc" isn't quite the same as "kill ourselves"
-- this will look like a standard error return from the child program.
It might be better to use abort(3) instead, so our parent can see with
WTERMSIG() the reason for the non-standard exit...

(At least with bash the abnormal termination via abort(3) is obvious..)

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130711/722aeee9/attachment.pgp>