On Tue, Apr 15, 2014 at 10:22:11AM -0700, john.johan...@canonical.com wrote: > This is not the cleanup this code needs, but a quick hack to add the > -M flag so we can specify a feature file (or directory) to use for > the compile. > > It mostly just moves around existing code and adds the -M option, > though it does introduce a few changes. > > While I didn't do it in this patch I propose we drop support for > the match file without create support. This is several years old > now and would clean things up a lot. > > Note: that the manually input -m or -M drop support for it already > I just can't see a good way to support a single input stream indicating > the result/existance of two separate files. > > This needs more work but is needed to support tests and the policy_mediates > frame work depends on the policydb getting generated with the special > stub rules to indicate whether policy was compiled expecting a certain > feature. But this can break the current tests, at least once a bug > in the policy rule counting is fixed in a follow on patch. > > Signed-off-by: John Johansen <john.johan...@canonical.com>
This looks like a very useful step forward. Probably iterating towards something nicer is the better approach :) Acked-by: Seth Arnold <seth.arn...@canonical.com> Thanks > > --- > parser/parser_main.c | 198 > +++++++++++++------------- > parser/tst/features_files/features.dbus | 34 ++++ > parser/tst/features_files/features.mount | 34 ++++ > parser/tst/features_files/features.mount+dbus | 37 ++++ > parser/tst/features_files/features.nopolicydb | 31 ++++ > parser/tst/minimize.sh | 16 +- > 6 files changed, 247 insertions(+), 103 deletions(-) > > --- 2.9-test.orig/parser/parser_main.c > +++ 2.9-test/parser/parser_main.c > @@ -53,8 +53,8 @@ > #define OLD_MODULE_NAME "subdomain" > #define PROC_MODULES "/proc/modules" > #define DEFAULT_APPARMORFS "/sys/kernel/security/" MODULE_NAME > -#define MATCH_STRING "/sys/kernel/security/" MODULE_NAME "/matching" > -#define FLAGS_FILE "/sys/kernel/security/" MODULE_NAME "/features" > +#define MATCH_FILE "/sys/kernel/security/" MODULE_NAME "/matching" > +#define FEATURES_FILE "/sys/kernel/security/" MODULE_NAME "/features" > #define MOUNTED_FS "/proc/mounts" > #define AADFA "pattern=aadfa" > > @@ -79,16 +79,17 @@ > int skip_mode_force = 0; > struct timespec mru_tstamp; > > -#define FLAGS_STRING_SIZE 8192 > -char *match_string = NULL; > -char *flags_string = NULL; > +#define FEATURES_STRING_SIZE 8192 > +char *features_string = NULL; > char *cacheloc = NULL; > > /* per-profile settings */ > int force_complain = 0; > > +static int load_features(const char *name); > + > /* Make sure to update BOTH the short and long_options */ > -static const char *short_options = "adf:h::rRVvI:b:BCD:NSm:qQn:XKTWkL:O:po:"; > +static const char *short_options = > "adf:h::rRVvI:b:BCD:NSm:M:qQn:XKTWkL:O:po:"; > struct option long_options[] = { > {"add", 0, 0, 'a'}, > {"binary", 0, 0, 'B'}, > @@ -106,6 +107,7 @@ > {"stdout", 0, 0, 'S'}, > {"ofile", 1, 0, 'o'}, > {"match-string", 1, 0, 'm'}, > + {"features-file", 1, 0, 'M'}, > {"quiet", 0, 0, 'q'}, > {"skip-kernel-load", 0, 0, 'Q'}, > {"verbose", 0, 0, 'v'}, > @@ -153,7 +155,8 @@ > "-b n, --base n Set base dir and cwd\n" > "-I n, --Include n Add n to the search path\n" > "-f n, --subdomainfs n Set location of apparmor filesystem\n" > - "-m n, --match-string n Use only match features n\n" > + "-m n, --match-string n Use only features n\n" > + "-M n, --features-file n Use only features in file n\n" > "-n n, --namespace n Set Namespace for the profile\n" > "-X, --readimpliesX Map profile read permissions to mr\n" > "-k, --show-cache Report cache hit/miss details\n" > @@ -520,7 +523,14 @@ > } > break; > case 'm': > - match_string = strdup(optarg); > + features_string = strdup(optarg); > + break; > + case 'M': > + if (load_features(optarg) == -1) { > + fprintf(stderr, "Failed to load features from '%s'\n", > + optarg); > + exit(1); > + } > break; > case 'q': > conf_verbose = 0; > @@ -733,104 +743,107 @@ > struct features_struct fst = { buffer, size, pos }; > > if (dirat_for_each(NULL, filename, &fst, features_dir_cb)) { > - PDEBUG("Failed evaluating .features\n"); > + PDEBUG("Failed evaluating %s\n", filename); > exit(1); > } > > return fst.pos; > } > > -/* match_string == NULL --> no match_string available > - match_string != NULL --> either a matching string specified on the > - command line, or the kernel supplied a match string */ > -static void get_match_string(void) { > +static char *load_features_file(const char *name) { > + char *buffer; > + FILE *f = NULL; > + size_t size; > > - FILE *ms = NULL; > - struct stat stat_file; > + f = fopen(name, "r"); > + if (!f) > + return NULL; > > - /* has process_args() already assigned a match string? */ > - if (match_string) > - goto out; > + buffer = (char *) malloc(FEATURES_STRING_SIZE); > + if (!buffer) > + goto fail; > > - if (stat(FLAGS_FILE, &stat_file) == -1) > - goto out; > + size = fread(buffer, 1, FEATURES_STRING_SIZE - 1, f); > + if (!size || ferror(f)) > + goto fail; > + buffer[size] = 0; > > - if (S_ISDIR(stat_file.st_mode)) { > - /* if we have a features directory default to */ > - perms_create = 1; > + fclose(f); > + return buffer; > > - flags_string = (char *) malloc(FLAGS_STRING_SIZE); > - handle_features_dir(FLAGS_FILE, &flags_string, > FLAGS_STRING_SIZE, flags_string); > - if (strstr(flags_string, "network")) > - kernel_supports_network = 1; > - else > - kernel_supports_network = 0; > - if (strstr(flags_string, "mount")) > - kernel_supports_mount = 1; > - if (strstr(flags_string, "dbus")) > - kernel_supports_dbus = 1; > - return; > - } > +fail: > + int save = errno; > + free(buffer); > + if (f) > + fclose(f); > + errno = save; > + return NULL; > +} > > - ms = fopen(MATCH_STRING, "r"); > - if (!ms) > - goto out; > +static int load_features(const char *name) > +{ > + struct stat stat_file; > > - match_string = (char *) malloc(1000); > - if (!match_string) { > - goto out; > - } > + if (stat(name, &stat_file) == -1) > + return -1; > > - if (!fgets(match_string, 1000, ms)) { > - free(match_string); > - match_string = NULL; > + if (S_ISDIR(stat_file.st_mode)) { > + /* if we have a features directory default to */ > + features_string = (char *) malloc(FEATURES_STRING_SIZE); > + handle_features_dir(name, &features_string, > FEATURES_STRING_SIZE, features_string); > + } else { > + features_string = load_features_file(name); > + if (!features_string) > + return -1; > } > > -out: > - if (match_string) { > + return 0; > +} > + > +static void set_features_by_match_file(void) > +{ > + FILE *ms = fopen(MATCH_FILE, "r"); > + if (ms) { > + char *match_string = (char *) malloc(1000); > + if (!match_string) > + goto no_match; > + if (!fgets(match_string, 1000, ms)) { > + free(match_string); > + goto no_match; > + } > if (strstr(match_string, " perms=c")) > perms_create = 1; > - } else { > - perms_create = 1; > - kernel_supports_network = 0; > + free(match_string); > + goto out; > } > +no_match: > + perms_create = 1; > + kernel_supports_network = 0; > > +out: > if (ms) > fclose(ms); > - return; > } > > -static void get_flags_string(char **flags, const char *flags_file) { > - char *pos; > - FILE *f = NULL; > - size_t size; > - > - /* abort if missing or already set */ > - if (!flags || *flags) > - return; > - > - f = fopen(flags_file, "r"); > - if (!f) > - return; > +static void set_supported_features(void) { > > - *flags = (char *) malloc(FLAGS_STRING_SIZE); > - if (!*flags) > - goto fail; > - > - size = fread(*flags, 1, FLAGS_STRING_SIZE - 1, f); > - if (!size || ferror(f)) > - goto fail; > - (*flags)[size] = 0; > + /* has process_args() already assigned a match string? */ > + if (!features_string) { > + if (load_features(FEATURES_FILE) == -1) { > + set_features_by_match_file(); > + return; > + } > + } > > - fclose(f); > - return; > + perms_create = 1; > > -fail: > - free(*flags); > - *flags = NULL; > - if (f) > - fclose(f); > - return; > + /* TODO: make this real parsing and config setting */ > + if (strstr(features_string, "network")) > + kernel_supports_network = 1; > + if (strstr(features_string, "mount")) > + kernel_supports_mount = 1; > + if (strstr(features_string, "dbus")) > + kernel_supports_dbus = 1; > } > > int process_binary(int option, const char *profilename) > @@ -1211,28 +1224,23 @@ > char *cache_flags = NULL; > > /* Get the match string to determine type of regex support needed */ > - get_match_string(); > - /* Get kernel features string */ > - get_flags_string(&flags_string, FLAGS_FILE); > + set_supported_features(); > + > /* Gracefully handle AppArmor kernel without compatibility patch */ > - if (!flags_string) { > + if (!features_string) { > PERROR("Cache read/write disabled: %s interface file missing. " > "(Kernel needs AppArmor 2.4 compatibility patch.)\n", > - FLAGS_FILE); > + FEATURES_FILE); > write_cache = 0; > skip_read_cache = 1; > return; > - } else if (strstr(flags_string, "network")) > - kernel_supports_network = 1; > - else > - kernel_supports_network = 0; > - > + } > > > /* > * Deal with cache directory versioning: > * - If cache/.features is missing, create it if --write-cache. > - * - If cache/.features exists, and does not match flags_string, > + * - If cache/.features exists, and does not match features_string, > * force cache reading/writing off. > */ > if (asprintf(&cache_features_path, "%s/.features", cacheloc) == -1) { > @@ -1240,16 +1248,16 @@ > exit(1); > } > > - get_flags_string(&cache_flags, cache_features_path); > + cache_flags = load_features_file(cache_features_path); > if (cache_flags) { > - if (strcmp(flags_string, cache_flags) != 0) { > + if (strcmp(features_string, cache_flags) != 0) { > if (write_cache && cond_clear_cache) { > if (create_cache(cacheloc, cache_features_path, > - flags_string)) > + features_string)) > skip_read_cache = 1; > } else { > if (show_cache) > - PERROR("Cache read/write disabled: %s > does not match %s\n", FLAGS_FILE, cache_features_path); > + PERROR("Cache read/write disabled: %s > does not match %s\n", FEATURES_FILE, cache_features_path); > write_cache = 0; > skip_read_cache = 1; > } > @@ -1257,7 +1265,7 @@ > free(cache_flags); > cache_flags = NULL; > } else if (write_cache) { > - create_cache(cacheloc, cache_features_path, flags_string); > + create_cache(cacheloc, cache_features_path, features_string); > } > > free(cache_features_path); > --- /dev/null > +++ 2.9-test/parser/tst/features_files/features.dbus > @@ -0,0 +1,34 @@ > +dbus {mask {acquire send receive > +} > +} > +caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid > setuid setpcap linux_immutable net_bind_service net_broadcast net_admin > net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace > sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config > mknod lease audit_write audit_control setfcap mac_override mac_admin syslog > wake_alarm block_suspend > +} > +} > +rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks > sigpending msgqueue nice rtprio rttime > +} > +} > +capability {0xffffff > +} > +namespaces {pivot_root {yes > +} > +profile {yes > +} > +} > +network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge > atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc > rds sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet > ieee802154 caif alg nfc vsock max > +} > +} > +file {mask {create read write exec append mmap_exec link lock > +} > +} > +domain {change_profile {yes > +} > +change_onexec {yes > +} > +change_hatv {yes > +} > +change_hat {yes > +} > +} > +policy {set_load {yes > +} > +} > --- /dev/null > +++ 2.9-test/parser/tst/features_files/features.mount > @@ -0,0 +1,34 @@ > +caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid > setuid setpcap linux_immutable net_bind_service net_broadcast net_admin > net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace > sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config > mknod lease audit_write audit_control setfcap mac_override mac_admin syslog > wake_alarm block_suspend > +} > +} > +rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks > sigpending msgqueue nice rtprio rttime > +} > +} > +capability {0xffffff > +} > +namespaces {pivot_root {yes > +} > +profile {yes > +} > +} > +mount {mask {mount umount > +} > +} > +network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge > atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc > rds sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet > ieee802154 caif alg nfc vsock max > +} > +} > +file {mask {create read write exec append mmap_exec link lock > +} > +} > +domain {change_profile {yes > +} > +change_onexec {yes > +} > +change_hatv {yes > +} > +change_hat {yes > +} > +} > +policy {set_load {yes > +} > +} > --- /dev/null > +++ 2.9-test/parser/tst/features_files/features.mount+dbus > @@ -0,0 +1,37 @@ > +dbus {mask {acquire send receive > +} > +} > +caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid > setuid setpcap linux_immutable net_bind_service net_broadcast net_admin > net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace > sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config > mknod lease audit_write audit_control setfcap mac_override mac_admin syslog > wake_alarm block_suspend > +} > +} > +rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks > sigpending msgqueue nice rtprio rttime > +} > +} > +capability {0xffffff > +} > +namespaces {pivot_root {yes > +} > +profile {yes > +} > +} > +mount {mask {mount umount > +} > +} > +network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge > atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc > rds sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet > ieee802154 caif alg nfc vsock max > +} > +} > +file {mask {create read write exec append mmap_exec link lock > +} > +} > +domain {change_profile {yes > +} > +change_onexec {yes > +} > +change_hatv {yes > +} > +change_hat {yes > +} > +} > +policy {set_load {yes > +} > +} > --- /dev/null > +++ 2.9-test/parser/tst/features_files/features.nopolicydb > @@ -0,0 +1,31 @@ > +caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid > setuid setpcap linux_immutable net_bind_service net_broadcast net_admin > net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace > sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config > mknod lease audit_write audit_control setfcap mac_override mac_admin syslog > wake_alarm block_suspend > +} > +} > +rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks > sigpending msgqueue nice rtprio rttime > +} > +} > +capability {0xffffff > +} > +namespaces {pivot_root {yes > +} > +profile {yes > +} > +} > +network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge > atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc > rds sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet > ieee802154 caif alg nfc vsock max > +} > +} > +file {mask {create read write exec append mmap_exec link lock > +} > +} > +domain {change_profile {yes > +} > +change_onexec {yes > +} > +change_hatv {yes > +} > +change_hat {yes > +} > +} > +policy {set_load {yes > +} > +} > --- 2.9-test.orig/parser/tst/minimize.sh > +++ 2.9-test/parser/tst/minimize.sh > @@ -78,7 +78,7 @@ > # {a} (0x 40030/0/0/0) > > echo -n "Minimize profiles basic perms " > -if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, /** w, }" | > ${APPARMOR_PARSER} -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep > '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > +if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, /** w, }" | > ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D > dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > echo "failed" > exit 1; > fi > @@ -93,7 +93,7 @@ > # {9} (0x 12804a/0/2800a/0) > # {c} (0x 40030/0/0/0) > echo -n "Minimize profiles audit perms " > -if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit /** w, }" | > ${APPARMOR_PARSER} -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep > '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > +if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit /** w, }" | > ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D > dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > echo "failed" > exit 1; > fi > @@ -112,7 +112,7 @@ > # {c} (0x 40030/0/0/0) > > echo -n "Minimize profiles deny perms " > -if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, deny /** w, }" | > ${APPARMOR_PARSER} -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep > '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > +if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, deny /** w, }" | > ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D > dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 6 ] ; then > echo "failed" > exit 1; > fi > @@ -130,7 +130,7 @@ > # {c} (0x 40030/0/0/0) > > echo -n "Minimize profiles audit deny perms " > -if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | > ${APPARMOR_PARSER} -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep > '^{.*} (.*)$' | wc -l` -ne 5 ] ; then > +if [ `echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | > ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D > dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 5 ] ; then > echo "failed" > exit 1; > fi > @@ -162,7 +162,7 @@ > # > > echo -n "Minimize profiles xtrans " > -if [ `echo "/t { /b px, /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -QT > -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` > -ne 3 ] ; then > +if [ `echo "/t { /b px, /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M > features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep > -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 3 ] ; then > echo "failed" > exit 1; > fi > @@ -170,7 +170,7 @@ > > # same test as above + audit > echo -n "Minimize profiles audit xtrans " > -if [ `echo "/t { /b px, audit /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} > -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc > -l` -ne 3 ] ; then > +if [ `echo "/t { /b px, audit /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} > -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | > grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 3 ] ; then > echo "failed" > exit 1; > fi > @@ -183,7 +183,7 @@ > # {3} (0x 0/fe17f85/0/14005) > > echo -n "Minimize profiles deny xtrans " > -if [ `echo "/t { /b px, deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} > -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc > -l` -ne 1 ] ; then > +if [ `echo "/t { /b px, deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M > features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep > -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 1 ] ; then > echo "failed" > exit 1; > fi > @@ -195,7 +195,7 @@ > # {3} (0x 0/fe17f85/0/0) > > echo -n "Minimize profiles audit deny xtrans " > -if [ `echo "/t { /b px, audit deny /* xr, /a Cx -> foo, }" | > ${APPARMOR_PARSER} -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep > '^{.*} (.*)$' | wc -l` -ne 0 ] ; then > +if [ `echo "/t { /b px, audit deny /* xr, /a Cx -> foo, }" | > ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D > dfa-states 2>&1 | grep -v '<==' | grep '^{.*} (.*)$' | wc -l` -ne 0 ] ; then > echo "failed" > exit 1; > fi > > > -- > AppArmor mailing list > AppArmor@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor >
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor