diff --git a/parser/immunix.h b/parser/immunix.h index c45b7e2ed..1d715dcc8 100644 --- a/parser/immunix.h +++ b/parser/immunix.h @@ -34,7 +34,8 @@ #define POS_KERN_COD_EXEC_UNCONSTRAINED (POS_KERN_COD_EXEC_INHERIT + 1) #define POS_KERN_COD_EXEC_PROFILE (POS_KERN_COD_EXEC_UNCONSTRAINED + 1) #define POS_KERN_COD_EXEC_MMAP (POS_KERN_COD_EXEC_PROFILE + 1) -#define POS_KERN_COD_FILE_MAX POS_KERN_COD_EXEC_MMAP +#define POS_KERN_COD_EXEC_UNSAFE (POS_KERN_COD_EXEC_MMAP + 1) +#define POS_KERN_COD_FILE_MAX POS_KERN_COD_EXEC_UNSAFE #define POS_KERN_COD_NET_MIN (POS_KERN_COD_FILE_MAX + 1) #define POS_KERN_COD_TCP_CONNECT POS_KERN_COD_NET_MIN @@ -62,6 +63,7 @@ #define KERN_COD_EXEC_UNCONSTRAINED (0x01 << POS_KERN_COD_EXEC_UNCONSTRAINED) #define KERN_COD_EXEC_PROFILE (0x01 << POS_KERN_COD_EXEC_PROFILE) #define KERN_COD_EXEC_MMAP (0x01 << POS_KERN_COD_EXEC_MMAP) +#define KERN_COD_EXEC_UNSAFE (0x01 << POS_KERN_COD_EXEC_UNSAFE) #define KERN_EXEC_MODIFIERS(X) (X & (KERN_COD_EXEC_INHERIT | \ KERN_COD_EXEC_UNCONSTRAINED | \ KERN_COD_EXEC_PROFILE)) diff --git a/parser/parser.h b/parser/parser.h index d8f2134e2..eecf8512e 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -105,8 +105,10 @@ struct var_string { #define COD_EXEC_CHAR 'x' #define COD_INHERIT_CHAR 'i' #define COD_LINK_CHAR 'l' -#define COD_UNCONSTRAINED_CHAR 'u' -#define COD_PROFILE_CHAR 'p' +#define COD_UNCONSTRAINED_CHAR 'U' +#define COD_UNSAFE_UNCONSTRAINED_CHAR 'u' +#define COD_PROFILE_CHAR 'P' +#define COD_UNSAFE_PROFILE_CHAR 'p' #define COD_MMAP_CHAR 'm' #define OPTION_ADD 1 @@ -121,9 +123,6 @@ struct var_string { #endif #define NPDEBUG(fmt, args...) /* Do nothing */ -/* FIXME: PWARN needs to become a true function so we can i18n-ize calls - * to it */ -#define PWARN(fmt, args...) fprintf(stderr, _("Warning (line %d): " fmt), current_lineno, ## args) #define PERROR(fmt, args...) fprintf(stderr, fmt, ## args) #ifndef TRUE @@ -147,6 +146,7 @@ extern char *profilename; /* from parser_main */ extern int force_complain; +extern void pwarn(char *fmt, ...) __attribute__((__format__(__printf__, 1, 2))); extern int yyparse(void); extern void yyerror(char *msg, ...); diff --git a/parser/parser_main.c b/parser/parser_main.c index a2f08759b..7732864c9 100644 --- a/parser/parser_main.c +++ b/parser/parser_main.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -55,13 +56,16 @@ const char *parser_title = "Novell/SUSE AppArmor parser"; const char *parser_copyright = "Copyright (C) 1999, 2000, 2003, 2004, 2005, 2006 Novell Inc."; char *progname; +int option = OPTION_ADD; int force_complain = 0; int names_only = 0; int dump_vars = 0; int dump_expanded_vars = 0; +int conf_quiet = 0; char *subdomainbase = NULL; char *profilename; char *match_string = NULL; +extern int current_lineno; struct option long_options[] = { {"add", 0, 0, 'a'}, @@ -81,6 +85,7 @@ struct option long_options[] = { {"names", 0, 0, 'N'}, /* undocumented only emit profilenames */ {"stdout", 0, 0, 'S'}, {"match-string", 1, 0, 'm'}, + {"quiet", 0, 0, 'q'}, {NULL, 0, 0, 0}, }; @@ -110,16 +115,38 @@ static void display_usage(char *command) "-b n, --base n Set base dir and cwd\n" "-f n, --subdomainfs n Set location of apparmor filesystem\n" "-S, --stdout Write output to stdout\n" - "-m n, --match-string n Use only match features n\n", command); + "-m n, --match-string n Use only match features n\n" + "-q, --quiet Don't emit warnings\n", command); +} + +void pwarn(char *fmt, ...) +{ + va_list arg; + char *newfmt; + int rc; + + if (conf_quiet || names_only || option == OPTION_REMOVE) + return; + + rc = asprintf(&newfmt, "Warning (%s line %d): %s", + profilename ? profilename : "stdin", + current_lineno, + fmt); + if (!newfmt) + return; + + va_start(arg, fmt); + vfprintf(stderr, newfmt, arg); + va_end(arg); } static int process_args(int argc, char *argv[]) { int c, o; - int option = OPTION_ADD; int count = 0; + option = OPTION_ADD; - while ((c = getopt_long(argc, argv, "adf:hrRvpI:b:CNSm:", long_options, &o)) != -1) + while ((c = getopt_long(argc, argv, "adf:hrRvpI:b:CNSm:q", long_options, &o)) != -1) { switch (c) { case 0: @@ -182,6 +209,9 @@ static int process_args(int argc, char *argv[]) case 'm': match_string = strdup(optarg); break; + case 'q': + conf_quiet = 1; + break; default: display_usage(progname); exit(0); diff --git a/parser/parser_merge.c b/parser/parser_merge.c index a3440461c..79837df69 100644 --- a/parser/parser_merge.c +++ b/parser/parser_merge.c @@ -27,6 +27,7 @@ #include "parser.h" + static inline int count_net_entries(struct codomain *cod) { struct cod_net_entry *list; @@ -73,14 +74,23 @@ static int process_file_entries(struct codomain *cod) qsort(table, count, sizeof(struct cod_entry *), file_comp); table[count] = NULL; +#define CHECK_CONFLICT_UNSAFE(a, b) \ + (((a & KERN_COD_EXEC_UNSAFE) ^ (b & KERN_COD_EXEC_UNSAFE)) && \ + (KERN_EXEC_MODIFIERS(a) & ~KERN_COD_EXEC_INHERIT) && \ + (KERN_EXEC_MODIFIERS(b) & ~KERN_COD_EXEC_INHERIT)) + /* walk the sorted table merging similar entries */ for (cur = table[0], next = table[1], n = 1; next != NULL; n++, next = table[n]) { if (file_comp(&cur, &next) == 0) { + int conflict = CHECK_CONFLICT_UNSAFE(cur->mode, next->mode); + PDEBUG("%s: cur_mode: %x next_mode: %x conflict %d\n", + __FUNCTION__, cur->mode, next->mode, conflict); cur->mode |= next->mode; /* check for merged x consistency */ - if ((KERN_COD_MAY_EXEC & cur->mode) && - (KERN_EXEC_MODIFIERS(cur->mode) & - (KERN_EXEC_MODIFIERS(cur->mode) - 1))) { + if (KERN_COD_MAY_EXEC & cur->mode && + ((KERN_EXEC_MODIFIERS(cur->mode) & + (KERN_EXEC_MODIFIERS(cur->mode) - 1)) || + conflict)) { PERROR(_("profile %s: has merged rule %s with multiple x modifiers\n"), cod->name, cur->name); return 0; diff --git a/parser/parser_misc.c b/parser/parser_misc.c index 543162ea8..9d272b211 100644 --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -261,6 +261,16 @@ int str_to_boolean(const char *value) return retval; } +static int warned_uppercase = 0; + +static void warn_uppercase(void) +{ + if (!warned_uppercase) { + pwarn("Uppercase qualifiers \"RWLIMX\" are deprecated, please convert to lowercase\n" + "See the apparmor.d(5) manpage for details.\n"); + warned_uppercase = 1; + } +} int parse_mode(const char *str_mode) { /* The 'check' int is a bit of a kludge, but we need some context @@ -276,9 +286,11 @@ int parse_mode(const char *str_mode) p = str_mode; while (*p) { - char this = tolower(*p); - char next = tolower(*(p + 1)); + char this = *p; + char next = *(p + 1); + char lower; +reeval: switch (this) { case COD_READ_CHAR: PDEBUG("Parsing mode: found READ\n"); @@ -297,24 +309,34 @@ int parse_mode(const char *str_mode) case COD_INHERIT_CHAR: PDEBUG("Parsing mode: found INHERIT\n"); - if (next != COD_EXEC_CHAR) { + if (next != COD_EXEC_CHAR && tolower(next) != COD_EXEC_CHAR) { yyerror(_("Exec qualifier 'i' must be followed by 'x'")); } else if (IS_DIFF_QUAL(this)) { yyerror(_("Exec qualifier 'i' invalid, conflicting qualifier already specified")); } else { + if (next != tolower(next)) + warn_uppercase(); mode |= (KERN_COD_EXEC_INHERIT | KERN_COD_MAY_EXEC); p++; /* skip 'x' */ } break; + case COD_UNSAFE_UNCONSTRAINED_CHAR: + mode |= KERN_COD_EXEC_UNSAFE; + pwarn("Unconstrained exec qualifier (%c%c) allows some dangerous environment variables\n" + "to be passed to the unconfined process; see the apparmor.d(5) manpage for details.\n", + COD_UNSAFE_UNCONSTRAINED_CHAR, COD_EXEC_CHAR); + /* fall through */ case COD_UNCONSTRAINED_CHAR: PDEBUG("Parsing mode: found UNCONSTRAINED\n"); - if (next != COD_EXEC_CHAR) { + if (next != COD_EXEC_CHAR && tolower(next) != COD_EXEC_CHAR) { yyerror(_("Exec qualifier 'u' must be followed by 'x'")); } else if (IS_DIFF_QUAL(this)) { yyerror(_("Exec qualifier 'u' invalid, conflicting qualifier already specified")); } else { + if (next != tolower(next)) + warn_uppercase(); mode |= (KERN_COD_EXEC_UNCONSTRAINED | KERN_COD_MAY_EXEC); @@ -322,13 +344,18 @@ int parse_mode(const char *str_mode) } break; + case COD_UNSAFE_PROFILE_CHAR: + mode |= KERN_COD_EXEC_UNSAFE; + /* fall through */ case COD_PROFILE_CHAR: PDEBUG("Parsing mode: found PROFILE\n"); - if (next != COD_EXEC_CHAR) { + if (next != COD_EXEC_CHAR && tolower(next) != COD_EXEC_CHAR) { yyerror(_("Exec qualifier 'p' must be followed by 'x'")); } else if (IS_DIFF_QUAL(this)) { yyerror(_("Exec qualifier 'p' invalid, conflicting qualifier already specified")); } else { + if (next != tolower(next)) + warn_uppercase(); mode |= (KERN_COD_EXEC_PROFILE | KERN_COD_MAY_EXEC); p++; /* skip 'x' */ @@ -345,8 +372,28 @@ int parse_mode(const char *str_mode) yyerror(_("Invalid mode, 'x' must be preceded by exec qualifier 'i', 'p', or 'u'")); break; + /* error cases */ + default: - yyerror(_("Internal: unexpected mode character in input")); + lower = tolower(this); + switch (lower) { + case COD_READ_CHAR: + case COD_WRITE_CHAR: + case COD_LINK_CHAR: + case COD_INHERIT_CHAR: + case COD_MMAP_CHAR: + case COD_EXEC_CHAR: + PDEBUG("Parsing mode: found invalid upper case char %c\n", this); + warn_uppercase(); + this = lower; + goto reeval; + break; + default: + yyerror(_("Internal: unexpected mode character '%c' in input"), + this); + break; + } + break; } p++; @@ -510,21 +557,29 @@ void debug_cod_entries(struct cod_entry *list) printf("Mode:\t"); if (item->mode & KERN_COD_MAY_READ) - printf("r"); + printf("%c", COD_READ_CHAR); if (item->mode & KERN_COD_MAY_WRITE) - printf("w"); - if (item->mode & KERN_COD_MAY_EXEC) - printf("x"); + printf("%c", COD_WRITE_CHAR); if (item->mode & KERN_COD_MAY_LINK) - printf("l"); + printf("%c", COD_LINK_CHAR); if (item->mode & KERN_COD_EXEC_INHERIT) - printf("i"); - if (item->mode & KERN_COD_EXEC_UNCONSTRAINED) - printf("u"); - if (item->mode & KERN_COD_EXEC_PROFILE) - printf("p"); + printf("%c", COD_INHERIT_CHAR); + if (item->mode & KERN_COD_EXEC_UNCONSTRAINED) { + if (item->mode & KERN_COD_EXEC_UNSAFE) + printf("%c", COD_UNSAFE_UNCONSTRAINED_CHAR); + else + printf("%c", COD_UNCONSTRAINED_CHAR); + } + if (item->mode & KERN_COD_EXEC_PROFILE) { + if (item->mode & KERN_COD_EXEC_UNSAFE) + printf("%c", COD_UNSAFE_PROFILE_CHAR); + else + printf("%c", COD_PROFILE_CHAR); + } if (item->mode & KERN_COD_EXEC_MMAP) printf("%c", COD_MMAP_CHAR); + if (item->mode & KERN_COD_MAY_EXEC) + printf("%c", COD_EXEC_CHAR); if (item->name) printf("\tName:\t(%s)\n", item->name); @@ -712,6 +767,3 @@ int main(void) return rc; } #endif /* UNIT_TEST */ - - - diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 8e957101a..0dc9a621f 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -814,7 +814,7 @@ ports: TOK_COLON TOK_NUM TOK_RANGE TOK_NUM if ((*ports)[0] > (*ports)[1]) { unsigned short tmp; - PWARN("expected first port number to be less than the second, swapping (%ld,%ld)\n", + pwarn("expected first port number to be less than the second, swapping (%ld,%ld)\n", $2, $4); tmp = (*ports)[0]; (*ports)[0] = (*ports)[1]; diff --git a/parser/tst/simple_tests/profile_basic_ok1.sd b/parser/tst/simple_tests/profile_basic_ok1.sd index 029e5ff8e..ceb24a388 100644 --- a/parser/tst/simple_tests/profile_basic_ok1.sd +++ b/parser/tst/simple_tests/profile_basic_ok1.sd @@ -8,12 +8,12 @@ /does/not/exist { #include - /usr/X11R6/lib/lib*so* rRr, + /usr/X11R6/lib/lib*so* rrr, /does/not/exist r, - /var/log/messages wWw, - /tmp/sd*.foo rwRWwrlL, - /bin/cat PxpxpXPXpx, - /bin/ls iXIXIxIX, - /bin/echo uXUXuxUxuX, + /var/log/messages www, + /tmp/sd*.foo rwrwwrll, + /bin/cat pxpxpxpxpx, + /bin/ls ixixixix, + /bin/echo uxuxuxuxux, }