From f5c4927c851eae3fff9268932a3ad478679afd71 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 28 Aug 2020 08:35:45 -0700 Subject: [PATCH] parser: convert remaining pwarn() to flag controlled warns Make all warnings that go through pwarn() controllable by warning flags. This adds several new warning control flags, documented in --help=warn Convert --debug-cache to be unified with warning flags. So it can be set by either --debug-cache or --warn=debug-cache Also add an "all" option to be able to turn on all warnings. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600 Signed-off-by: John Johansen --- parser/parser.h | 34 +++++++++++++++++++++++++++------- parser/parser_common.c | 4 ++-- parser/parser_interface.c | 2 +- parser/parser_main.c | 33 +++++++++++++++++++++------------ parser/parser_misc.c | 6 +++--- parser/parser_regex.c | 2 +- parser/parser_yacc.y | 17 ++++++++--------- parser/policy_cache.c | 17 ++++++----------- 8 files changed, 69 insertions(+), 46 deletions(-) diff --git a/parser/parser.h b/parser/parser.h index 445e33f0e..6d9427791 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -52,11 +52,31 @@ class rule_t; extern int parser_token; -#define WARN_RULE_NOT_ENFORCED 1 -#define WARN_RULE_DOWNGRADED 2 -#define WARN_ABI 4 -#define WARN_DEPRECATED 8 -#define WARN_DEV (WARN_RULE_NOT_ENFORCED | WARN_RULE_DOWNGRADED | WARN_ABI | WARN_DEPRECATED) +#define WARN_RULE_NOT_ENFORCED 0x1 +#define WARN_RULE_DOWNGRADED 0x2 +#define WARN_ABI 0x4 +#define WARN_DEPRECATED 0x8 +#define WARN_CONFIG 0x10 +#define WARN_CACHE 0x20 +#define WARN_DEBUG_CACHE 0x40 +#define WARN_JOBS 0x80 +#define WARN_DANGEROUS 0x100 +#define WARN_UNEXPECTED 0x200 +#define WARN_FORMAT 0x400 +#define WARN_MISSING 0x800 +#define WARN_OVERRIDE 0x1000 + +#define WARN_DEV (WARN_RULE_NOT_ENFORCED | WARN_RULE_DOWNGRADED | WARN_ABI | \ + WARN_DEPRECATED | WARN_DANGEROUS | WARN_UNEXPECTED | \ + WARN_FORMAT | WARN_MISSING | WARN_OVERRIDE | WARN_DEBUG_CACHE) + +#define DEFAULT_WARNINGS (WARN_CONFIG | WARN_CACHE | WARN_JOBS | \ + WARN_UNEXPECTED | WARN_OVERRIDE) + +#define WARN_ALL (WARN_RULE_NOT_ENFORCED | WARN_RULE_DOWNGRADED | WARN_ABI | \ + WARN_DEPRECATED | WARN_CONFIG | WARN_CACHE | \ + WARN_DEBUG_CACHE | WARN_JOBS | WARN_DANGEROUS | \ + WARN_UNEXPECTED | WARN_FORMAT | WARN_MISSING | WARN_OVERRIDE) extern dfaflags_t warnflags; @@ -331,10 +351,10 @@ extern char *profile_ns; extern char *current_filename; extern FILE *ofile; extern int read_implies_exec; -extern void pwarn(const char *fmt, ...) __attribute__((__format__(__printf__, 1, 2))); +extern void pwarnf(const char *fmt, ...) __attribute__((__format__(__printf__, 1, 2))); extern void common_warn_once(const char *name, const char *msg, const char **warned_name); -#define pwarn_onflag(F, args...) do { if (warnflags & (F)) pwarn(args); } while (0) +#define pwarn(F, args...) do { if (warnflags & (F)) pwarnf(args); } while (0) /* from parser_main (cannot be used in tst builds) */ extern int force_complain; diff --git a/parser/parser_common.c b/parser/parser_common.c index e018e59aa..ff78dd2f2 100644 --- a/parser/parser_common.c +++ b/parser/parser_common.c @@ -84,7 +84,7 @@ int current_lineno = 1; int option = OPTION_ADD; dfaflags_t dfaflags = (dfaflags_t)(DFA_CONTROL_TREE_NORMAL | DFA_CONTROL_TREE_SIMPLE | DFA_CONTROL_MINIMIZE | DFA_CONTROL_DIFF_ENCODE); -dfaflags_t warnflags = 0; +dfaflags_t warnflags = DEFAULT_WARNINGS; const char *progname = __FILE__; char *profile_ns = NULL; @@ -99,7 +99,7 @@ int read_implies_exec = 1; int read_implies_exec = 0; #endif -void pwarn(const char *fmt, ...) +void pwarnf(const char *fmt, ...) { va_list arg; char *newfmt; diff --git a/parser/parser_interface.c b/parser/parser_interface.c index 79b58b51e..7c379bf89 100644 --- a/parser/parser_interface.c +++ b/parser/parser_interface.c @@ -471,7 +471,7 @@ void sd_serialize_profile(std::ostringstream &buf, Profile *profile, } sd_write_arrayend(buf); } else if (profile->net.allow && (warnflags & WARN_RULE_NOT_ENFORCED)) - pwarn(_("profile %s network rules not enforced\n"), profile->name); + pwarn(WARN_RULE_NOT_ENFORCED, _("profile %s network rules not enforced\n"), profile->name); if (profile->policy.dfa) { sd_write_struct(buf, "policydb"); diff --git a/parser/parser_main.c b/parser/parser_main.c index bc24885d7..96b407017 100644 --- a/parser/parser_main.c +++ b/parser/parser_main.c @@ -80,7 +80,6 @@ int skip_mode_force = 0; int abort_on_error = 0; /* stop processing profiles if error */ int skip_bad_cache_rebuild = 0; int mru_skip_cache = 1; -int debug_cache = 0; /* for jobs_max and jobs * LONG_MAX : no limit @@ -251,7 +250,17 @@ optflag_table_t warnflag_table[] = { { 0, "rule-downgraded", "warn if a rule is downgraded to a lesser but still enforcing rule", WARN_RULE_DOWNGRADED }, { 0, "abi", "warn if there are abi issues in the profile", WARN_ABI }, { 0, "deprecated", "warn if something in the profile is deprecated", WARN_DEPRECATED }, + { 0, "config", "enable configuration warnings", WARN_CONFIG }, + { 0, "cache", "enable regular cache warnings", WARN_CACHE }, + { 0, "debug-cache", "enable warnings for debug cache file checks", WARN_DEBUG_CACHE }, + { 0, "jobs", "enable job control warnings", WARN_JOBS }, + { 0, "dangerous", "warn on dangerous policy", WARN_DANGEROUS }, + { 0, "unexpected", "warn when an unexpected condition is found", WARN_UNEXPECTED }, + { 0, "format", "warn on unnecessary or confusing formating", WARN_FORMAT }, + { 0, "missing", "warn when missing qualifier and a default is used", WARN_MISSING }, + { 0, "override", "warn when overriding", WARN_OVERRIDE }, { 0, "dev", "turn on warnings that are useful for profile development", WARN_DEV }, + { 0, "all", "turn on all warnings", WARN_ALL}, { 0, NULL, NULL, 0 }, }; @@ -688,7 +697,7 @@ static int process_arg(int c, char *optarg) } break; case ARG_DEBUG_CACHE: - debug_cache = 1; + warnflags |= WARN_DEBUG_CACHE; break; case 'j': jobs = process_jobs_arg("-j", optarg); @@ -766,7 +775,7 @@ static int process_config_file(const char *name) f = fopen(name, "r"); if (!f) { - pwarn("config file '%s' not found\n", name); + pwarn(WARN_CONFIG, "config file '%s' not found\n", name); return 0; } @@ -1001,7 +1010,7 @@ int process_profile(int option, aa_kernel_interface *kernel_interface, } } else { if (write_cache) - pwarn("%s: cannot use or update cache, disable, or force-complain via stdin\n", progname); + pwarn(WARN_CACHE, "%s: cannot use or update cache, disable, or force-complain via stdin\n", progname); skip_cache = write_cache = 0; } @@ -1034,7 +1043,7 @@ int process_profile(int option, aa_kernel_interface *kernel_interface, basename, O_RDONLY); if (fd != -1) - pwarn(_("Could not get cachename for '%s'\n"), basename); + pwarn(WARN_CACHE, _("Could not get cachename for '%s'\n"), basename); } else { valid_read_cache(cachename); } @@ -1107,12 +1116,12 @@ int process_profile(int option, aa_kernel_interface *kernel_interface, if (pc && write_cache && !force_complain) { writecachename = cache_filename(pc, 0, basename); if (!writecachename) { - pwarn("Cache write disabled: Cannot create cache file name '%s': %m\n", basename); + pwarn(WARN_CACHE, "Cache write disabled: Cannot create cache file name '%s': %m\n", basename); write_cache = 0; } cachetmp = setup_cache_tmp(&cachetmpname, writecachename); if (cachetmp == -1) { - pwarn("Cache write disabled: Cannot create setup tmp cache file '%s': %m\n", writecachename); + pwarn(WARN_CACHE, "Cache write disabled: Cannot create setup tmp cache file '%s': %m\n", writecachename); write_cache = 0; } } @@ -1121,7 +1130,7 @@ int process_profile(int option, aa_kernel_interface *kernel_interface, if (retval == 0 && write_cache) { if (cachetmp == -1) { unlink(cachetmpname); - pwarn("Warning failed to create cache: %s\n", + pwarn(WARN_CACHE, "Warning failed to create cache: %s\n", basename); } else { install_cache(cachetmpname, writecachename); @@ -1259,7 +1268,7 @@ static void setup_parallel_compile(void) jobs_max = compute_jobs(maxn, jobs_max); if (jobs > jobs_max) { - pwarn("%s: Warning capping number of jobs to %ld * # of cpus == '%ld'", + pwarn(WARN_JOBS, "%s: Warning capping number of jobs to %ld * # of cpus == '%ld'", progname, jobs_max, jobs); jobs = jobs_max; } else if (jobs < jobs_max) @@ -1429,7 +1438,7 @@ int main(int argc, char *argv[]) } if (create_cache_dir) - pwarn_onflag(WARN_DEPRECATED, _("The --create-cache-dir option is deprecated. Please use --write-cache.\n")); + pwarn(WARN_DEPRECATED, _("The --create-cache-dir option is deprecated. Please use --write-cache.\n")); retval = aa_policy_cache_new(&policy_cache, kernel_features, AT_FDCWD, cacheloc[0], max_caches); if (retval) { @@ -1454,9 +1463,9 @@ int main(int argc, char *argv[]) for (i = 1; i < cacheloc_n; i++) { if (aa_policy_cache_add_ro_dir(policy_cache, AT_FDCWD, cacheloc[i])) { - pwarn("Cache: failed to add read only location '%s', does not contain valid cache directory for the specified feature set\n", cacheloc[i]); + pwarn(WARN_CACHE, "Cache: failed to add read only location '%s', does not contain valid cache directory for the specified feature set\n", cacheloc[i]); } else if (show_cache) - pwarn("Cache: added readonly location '%s'\n", cacheloc[i]); + pwarn(WARN_CACHE, "Cache: added readonly location '%s'\n", cacheloc[i]); } } } diff --git a/parser/parser_misc.c b/parser/parser_misc.c index 8930894f8..94167ac19 100644 --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -276,7 +276,7 @@ static int capable_add_cap(const char *str, int len, unsigned int cap, struct capability_table *ent = find_cap_entry_by_name(name); if (ent) { if (ent->cap != cap) { - pwarn("feature capability '%s:%d' does not equal expected %d. Ignoring ...\n", name, cap, ent->cap); + pwarn(WARN_UNEXPECTED, "feature capability '%s:%d' does not equal expected %d. Ignoring ...\n", name, cap, ent->cap); /* TODO: make warn to error config */ return 0; } @@ -522,7 +522,7 @@ static int warned_uppercase = 0; void warn_uppercase(void) { if (!warned_uppercase) { - pwarn_onflag(WARN_DEPRECATED, _("Uppercase qualifiers \"RWLIMX\" are deprecated, please convert to lowercase\n" + pwarn(WARN_DEPRECATED, _("Uppercase qualifiers \"RWLIMX\" are deprecated, please convert to lowercase\n" "See the apparmor.d(5) manpage for details.\n")); warned_uppercase = 1; } @@ -598,7 +598,7 @@ reeval: case COD_UNSAFE_UNCONFINED_CHAR: tmode = AA_EXEC_UNSAFE; - pwarn(_("Unconfined exec qualifier (%c%c) allows some dangerous environment variables " + pwarn(WARN_DANGEROUS, _("Unconfined exec qualifier (%c%c) allows some dangerous environment variables " "to be passed to the unconfined process; 'man 5 apparmor.d' for details.\n"), COD_UNSAFE_UNCONFINED_CHAR, COD_EXEC_CHAR); /* fall through */ diff --git a/parser/parser_regex.c b/parser/parser_regex.c index b84c7720b..cc509d26b 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -382,7 +382,7 @@ pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, int glob, /* quoting mark used for something that * does not need to be quoted; give a * warning */ - pwarn("Character %c was quoted " + pwarn(WARN_FORMAT, "Character %c was quoted" "unnecessarily, dropped preceding" " quote ('\\') character\n", *sptr); diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 9930721cb..8569f37b5 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -305,7 +305,7 @@ list: preamble strlen(default_features_abi))) { yyerror(_("Failed to setup default policy feature abi")); } - pwarn_onflag(WARN_ABI, _("%s: File '%s' missing feature abi, falling back to default policy feature abi\n"), progname, current_filename); + pwarn(WARN_ABI, _("%s: File '%s' missing feature abi, falling back to default policy feature abi\n"), progname, current_filename); } } if (!add_cap_feature_mask(policy_features, @@ -358,7 +358,7 @@ profile_base: TOK_ID opt_id_or_var opt_cond_list flags TOK_OPEN rules TOK_CLOSE * --namespace-string command line option */ if (prof->ns && strcmp(prof->ns, profile_ns)) - pwarn("%s: -n %s overriding policy specified namespace :%s:\n", + pwarn(WARN_OVERRIDE, "%s: -n %s overriding policy specified namespace :%s:\n", progname, profile_ns, prof->ns); free(prof->ns); @@ -401,7 +401,7 @@ profile: opt_profile_flag profile_base PDEBUG("Matched: %s { ... }\n", $2->name); if ($2->name[0] == '/') - pwarn_onflag(WARN_DEPRECATED, _("The use of file paths as profile names is deprecated. See man apparmor.d for more information\n")); + pwarn(WARN_DEPRECATED, _("The use of file paths as profile names is deprecated. See man apparmor.d for more information\n")); if ($2->name[0] != '/' && !($1 || $2->ns)) yyerror(_("Profile names must begin with a '/', namespace or keyword 'profile' or 'hat'.")); @@ -951,7 +951,7 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE opt_id TOK_END_OF_RULE else if (tmp < 0LL) yyerror("RLIMIT '%s' invalid value %s\n", $4, $6); if (!$7) - pwarn(_("RLIMIT 'cpu' no units specified using default units of seconds\n")); + pwarn(WARN_MISSING, _("RLIMIT 'cpu' no units specified using default units of seconds\n")); value = tmp; break; #ifdef RLIMIT_RTTIME @@ -963,7 +963,7 @@ rules: rules TOK_SET TOK_RLIMIT TOK_ID TOK_LE TOK_VALUE opt_id TOK_END_OF_RULE if (tmp < 0LL) yyerror("RLIMIT '%s' invalid value %s %s\n", $4, $6, $7 ? $7 : ""); if (!$7) - pwarn(_("RLIMIT 'rttime' no units specified using default units of microseconds\n")); + pwarn(WARN_MISSING, _("RLIMIT 'rttime' no units specified using default units of microseconds\n")); value = tmp; break; #endif @@ -1570,9 +1570,8 @@ change_profile: TOK_CHANGE_PROFILE opt_exec_mode opt_id opt_named_transition TOK if (exec_mode == EXEC_MODE_UNSAFE) mode |= ALL_AA_EXEC_UNSAFE; else if (exec_mode == EXEC_MODE_SAFE && - !features_supports_stacking && - warnflags & WARN_RULE_DOWNGRADED) { - pwarn("downgrading change_profile safe rule to unsafe due to lack of necessary kernel support\n"); + !features_supports_stacking) { + pwarn(WARN_RULE_DOWNGRADED, "downgrading change_profile safe rule to unsafe due to lack of necessary kernel support\n"); /** * No need to do anything because 'unsafe' exec * mode is the only supported mode of @@ -1813,7 +1812,7 @@ static void abi_features(char *filename, bool search) } if (policy_features) { if (!aa_features_is_equal(tmp_features, policy_features)) { - pwarn_onflag(WARN_ABI, _("%s: %s features abi '%s' differs from policy declared feature abi, using the features abi declared in policy\n"), progname, current_filename, filename); + pwarn(WARN_ABI, _("%s: %s features abi '%s' differs from policy declared feature abi, using the features abi declared in policy\n"), progname, current_filename, filename); } aa_features_unref(tmp_features); } else { diff --git a/parser/policy_cache.c b/parser/policy_cache.c index 2864eec85..341e0331f 100644 --- a/parser/policy_cache.c +++ b/parser/policy_cache.c @@ -46,15 +46,13 @@ bool valid_cached_file_version(const char *cachename) } size_t res = fread(buffer, 1, HEADER_STRING_SIZE + VERSION_STRING_SIZE, f); if (res < HEADER_STRING_SIZE + VERSION_STRING_SIZE) { - if (debug_cache) - pwarn("%s: cache file '%s' invalid size\n", progname, cachename); + pwarn(WARN_DEBUG_CACHE, "%s: cache file '%s' invalid size\n", progname, cachename); return false; } /* 12 byte header that is always the same and then 4 byte version # */ if (memcmp(buffer, header_string, HEADER_STRING_SIZE) != 0) { - if (debug_cache) - pwarn("%s: cache file '%s' has wrong header\n", progname, cachename); + pwarn(WARN_DEBUG_CACHE, "%s: cache file '%s' has wrong header\n", progname, cachename); return false; } @@ -63,8 +61,7 @@ bool valid_cached_file_version(const char *cachename) parser_abi_version, kernel_abi_version)); if (memcmp(buffer + HEADER_STRING_SIZE, &version, VERSION_STRING_SIZE) != 0) { - if (debug_cache) - pwarn("%s: cache file '%s' has wrong version\n", progname, cachename); + pwarn(WARN_DEBUG_CACHE, "%s: cache file '%s' has wrong version\n", progname, cachename); return false; } @@ -89,8 +86,7 @@ void update_mru_tstamp(FILE *file, const char *name) if (tstamp_is_null(cache_tstamp)) return; if (tstamp_cmp(stat_file.st_mtim, cache_tstamp) > 0) { - if (debug_cache) - pwarn("%s: file '%s' is newer than cache file\n", progname, name); + pwarn(WARN_DEBUG_CACHE, "%s: file '%s' is newer than cache file\n", progname, name); mru_skip_cache = 1; } } @@ -124,8 +120,7 @@ void valid_read_cache(const char *cachename) } else { if (!cond_clear_cache) write_cache = 0; - if (debug_cache) - pwarn("%s: Invalid or missing cache file '%s' (%s)\n", progname, cachename, strerror(errno)); + pwarn(WARN_DEBUG_CACHE, "%s: Invalid or missing cache file '%s' (%s)\n", progname, cachename, strerror(errno)); } } } @@ -184,7 +179,7 @@ void install_cache(const char *cachetmpname, const char *cachename) } if (rename(cachetmpname, cachename) < 0) { - pwarn("Warning failed to write cache: %s\n", cachename); + pwarn(WARN_CACHE, "Warning failed to write cache: %s\n", cachename); unlink(cachetmpname); } else if (show_cache) {