From a29e23283139c0a5cb334b21db5e97fa7f239bde Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 29 Apr 2020 02:18:24 -0700 Subject: [PATCH] parser: feature abi: setup parser to intersect policy and kernel features The features abi adds the ability to track the policy abi separate from the kernel. This allow the compiler to determine whether policy was developed with a certain feature in mind, eg. unix rules. This allows the compiler to know whether it should tell the kernel to enforce the feature if the kernel supports the rule but the policy doesn't use it. To find if a feature is supported we take the intersection of what is supported by the policy and what is supported by the kernel. Policy encoding features like whether to diff_encode policy are not influenced by policy so these remain kernel only features. In addition to adding the above intersection of policy rename --compile-features to --policy-features as better represents what it represents. --compile-features is left as a hidden item for backwards compatibility. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491 Signed-off-by: John Johansen Acked-by: Steve Beattie --- parser/af_unix.cc | 4 +-- parser/apparmor_parser.pod | 14 ++++++++ parser/dbus.cc | 2 +- parser/mount.cc | 2 +- parser/parser.h | 16 +++++----- parser/parser_common.c | 16 +++++----- parser/parser_interface.c | 3 +- parser/parser_main.c | 65 ++++++++++++++++++++++---------------- parser/parser_regex.c | 14 ++++---- parser/parser_yacc.y | 8 +++-- parser/ptrace.cc | 2 +- parser/signal.cc | 2 +- 12 files changed, 88 insertions(+), 60 deletions(-) diff --git a/parser/af_unix.cc b/parser/af_unix.cc index c80a9f532..65d8772f9 100644 --- a/parser/af_unix.cc +++ b/parser/af_unix.cc @@ -322,8 +322,8 @@ int unix_rule::gen_policy_re(Profile &prof) * rules ability */ downgrade_rule(prof); - if (!kernel_supports_unix) { - if (kernel_supports_network) { + if (!features_supports_unix) { + if (features_supports_network) { /* only warn if we are building against a kernel * that requires downgrading */ if (warnflags & WARN_RULE_DOWNGRADED) diff --git a/parser/apparmor_parser.pod b/parser/apparmor_parser.pod index 0cf68542f..a1c699bb8 100644 --- a/parser/apparmor_parser.pod +++ b/parser/apparmor_parser.pod @@ -184,16 +184,30 @@ defined as an absolute paths. Set the location of the apparmor security filesystem (default is "/sys/kernel/security/apparmor"). +=item --policy-features n + +Specify the feature set that the policy was developed under. + +=item --kernel-features n + +Specify the feature set of the kernel that the policy is being compiled for. If not specified this will be determined by the system's kernel. + =item -M n, --features-file n Use the features file located at path "n" (default is /etc/apparmor.d/cache/.features). If the --cache-loc option is present, the ".features" file in the specified cache directory is used. +Note: this sets both the --kernel-features and --policy-features to be the +same. + =item -m n, --match-string n Only use match features "n". +Note: this sets both the --kernel-features and --policy-features to be the +same. + =item -n n, --namespace-string n Force a profile to load in the namespace "n". diff --git a/parser/dbus.cc b/parser/dbus.cc index 56bc2a3f9..b511578cd 100644 --- a/parser/dbus.cc +++ b/parser/dbus.cc @@ -219,7 +219,7 @@ int dbus_rule::gen_policy_re(Profile &prof) pattern_t ptype; int pos; - if (!kernel_supports_dbus) { + if (!features_supports_dbus) { warn_once(prof.name); return RULE_NOT_SUPPORTED; } diff --git a/parser/mount.cc b/parser/mount.cc index e02a7dc2b..cf08245c7 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -593,7 +593,7 @@ int mnt_rule::gen_policy_re(Profile &prof) int count = 0; unsigned int tmpflags, tmpinv_flags; - if (!kernel_supports_mount) { + if (!features_supports_mount) { warn_once(prof.name); return RULE_NOT_SUPPORTED; } diff --git a/parser/parser.h b/parser/parser.h index 920ff0518..9f8d4216f 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -300,16 +300,16 @@ extern int perms_create; extern int net_af_max_override; extern int kernel_load; extern int kernel_supports_setload; -extern int kernel_supports_network; +extern int features_supports_network; extern int kernel_supports_policydb; extern int kernel_supports_diff_encode; -extern int kernel_supports_mount; -extern int kernel_supports_dbus; -extern int kernel_supports_signal; -extern int kernel_supports_ptrace; -extern int kernel_supports_unix; -extern int kernel_supports_stacking; -extern int kernel_supports_domain_xattr; +extern int features_supports_mount; +extern int features_supports_dbus; +extern int features_supports_signal; +extern int features_supports_ptrace; +extern int features_supports_unix; +extern int features_supports_stacking; +extern int features_supports_domain_xattr; extern int kernel_supports_oob; extern int conf_verbose; extern int conf_quiet; diff --git a/parser/parser_common.c b/parser/parser_common.c index 3e097959a..aedda6372 100644 --- a/parser/parser_common.c +++ b/parser/parser_common.c @@ -65,16 +65,16 @@ int perms_create = 0; /* perms contain create flag */ int net_af_max_override = -1; /* use kernel to determine af_max */ int kernel_load = 1; int kernel_supports_setload = 0; /* kernel supports atomic set loads */ -int kernel_supports_network = 0; /* kernel supports network rules */ -int kernel_supports_unix = 0; /* kernel supports unix socket rules */ +int features_supports_network = 0; /* kernel supports network rules */ +int features_supports_unix = 0; /* kernel supports unix socket rules */ int kernel_supports_policydb = 0; /* kernel supports new policydb */ -int kernel_supports_mount = 0; /* kernel supports mount rules */ -int kernel_supports_dbus = 0; /* kernel supports dbus rules */ +int features_supports_mount = 0; /* kernel supports mount rules */ +int features_supports_dbus = 0; /* kernel supports dbus rules */ int kernel_supports_diff_encode = 0; /* kernel supports diff_encode */ -int kernel_supports_signal = 0; /* kernel supports signal rules */ -int kernel_supports_ptrace = 0; /* kernel supports ptrace rules */ -int kernel_supports_stacking = 0; /* kernel supports stacking */ -int kernel_supports_domain_xattr = 0; /* x attachment cond */ +int features_supports_signal = 0; /* kernel supports signal rules */ +int features_supports_ptrace = 0; /* kernel supports ptrace rules */ +int features_supports_stacking = 0; /* kernel supports stacking */ +int features_supports_domain_xattr = 0; /* x attachment cond */ int kernel_supports_oob = 0; /* out of band transitions */ int conf_verbose = 0; int conf_quiet = 0; diff --git a/parser/parser_interface.c b/parser/parser_interface.c index 32f5279e2..3558cba9e 100644 --- a/parser/parser_interface.c +++ b/parser/parser_interface.c @@ -458,7 +458,8 @@ void sd_serialize_profile(std::ostringstream &buf, Profile *profile, sd_serialize_rlimits(buf, &profile->rlimits); - if (profile->net.allow && kernel_supports_network) { + /* choice to support / downgrade needs to already have been made */ + if (profile->net.allow && features_supports_network) { size_t i; sd_write_array(buf, "net_allowed_af", get_af_max()); for (i = 0; i < get_af_max(); i++) { diff --git a/parser/parser_main.c b/parser/parser_main.c index a98c0a461..c92d214c5 100644 --- a/parser/parser_main.c +++ b/parser/parser_main.c @@ -108,7 +108,7 @@ static const char *cacheloc[MAX_CACHE_LOCS]; static int cacheloc_n = 0; static bool print_cache_dir = false; -static aa_features *compile_features = NULL; +static aa_features *policy_features = NULL; static aa_features *kernel_features = NULL; static const char *config_file = "/etc/apparmor/parser.conf"; @@ -161,7 +161,8 @@ struct option long_options[] = { {"max-jobs", 1, 0, 136}, /* no short option */ {"print-cache-dir", 0, 0, 137}, /* no short option */ {"kernel-features", 1, 0, 138}, /* no short option */ - {"compile-features", 1, 0, 139}, /* no short option */ + {"policy-features", 1, 0, 139}, /* no short option */ + {"compile-features", 1, 0, 139}, /* original name of policy-features */ {"print-config-file", 0, 0, 140}, /* no short option */ {"config-file", 1, 0, EARLY_ARG_CONFIG_FILE}, /* early option, no short option */ @@ -195,7 +196,7 @@ static void display_usage(const char *command) "-f n, --subdomainfs n Set location of apparmor filesystem\n" "-m n, --match-string n Use only features n\n" "-M n, --features-file n Set compile & kernel features to file n\n" - "--compile-features n Compile features set in file n\n" + "--policy-features n Policy features set in file n\n" "--kernel-features n Kernel features set in file n\n" "-n n, --namespace n Set Namespace for the profile\n" "-X, --readimpliesX Map profile read permissions to mr\n" @@ -526,25 +527,30 @@ static int process_arg(int c, char *optarg) } break; case 'm': - if (aa_features_new_from_string(&compile_features, + if (policy_features) + aa_features_unref(policy_features); + if (kernel_features) + aa_features_unref(kernel_features); + if (aa_features_new_from_string(&policy_features, optarg, strlen(optarg))) { fprintf(stderr, "Failed to parse features string: %m\n"); exit(1); } + kernel_features = aa_features_ref(policy_features); break; case 'M': - if (compile_features) - aa_features_unref(compile_features); + if (policy_features) + aa_features_unref(policy_features); if (kernel_features) aa_features_unref(kernel_features); - if (aa_features_new(&compile_features, AT_FDCWD, optarg)) { + if (aa_features_new(&policy_features, AT_FDCWD, optarg)) { fprintf(stderr, "Failed to load features from '%s': %m\n", optarg); exit(1); } - kernel_features = aa_features_ref(compile_features); + kernel_features = aa_features_ref(policy_features); break; case 138: if (kernel_features) @@ -557,9 +563,9 @@ static int process_arg(int c, char *optarg) } break; case 139: - if (compile_features) - aa_features_unref(compile_features); - if (aa_features_new(&compile_features, AT_FDCWD, optarg)) { + if (policy_features) + aa_features_unref(policy_features); + if (aa_features_new(&policy_features, AT_FDCWD, optarg)) { fprintf(stderr, "Failed to load compile features from '%s': %m\n", optarg); @@ -743,6 +749,11 @@ int have_enough_privilege(void) return 0; } +int features_intersect(aa_features *a, aa_features *b, const char *str) +{ + return aa_features_supports(a, str) && aa_features_supports(b, str); +} + static void set_features_by_match_file(void) { autofclose FILE *ms = fopen(MATCH_FILE, "r"); @@ -754,7 +765,7 @@ static void set_features_by_match_file(void) goto no_match; if (strstr(match_string, " perms=c")) perms_create = 1; - kernel_supports_network = 1; + features_supports_network = 1; return; } no_match: @@ -764,7 +775,7 @@ no_match: static void set_supported_features(aa_features *kernel_features unused) { /* has process_args() already assigned a match string? */ - if (!compile_features && aa_features_new_from_kernel(&compile_features) == -1) { + if (!policy_features && aa_features_new_from_kernel(&policy_features) == -1) { set_features_by_match_file(); return; } @@ -774,28 +785,28 @@ static void set_supported_features(aa_features *kernel_features unused) * rule down grades for a give kernel */ perms_create = 1; - kernel_supports_policydb = aa_features_supports(compile_features, "file"); - kernel_supports_network = aa_features_supports(compile_features, "network"); - kernel_supports_unix = aa_features_supports(compile_features, + kernel_supports_policydb = aa_features_supports(kernel_features, "file"); + features_supports_network = features_intersect(kernel_features, policy_features, "network"); + features_supports_unix = features_intersect(kernel_features, policy_features, "network/af_unix"); - kernel_supports_mount = aa_features_supports(compile_features, "mount"); - kernel_supports_dbus = aa_features_supports(compile_features, "dbus"); - kernel_supports_signal = aa_features_supports(compile_features, "signal"); - kernel_supports_ptrace = aa_features_supports(compile_features, "ptrace"); - kernel_supports_setload = aa_features_supports(compile_features, + features_supports_mount = features_intersect(kernel_features, policy_features, "mount"); + features_supports_dbus = features_intersect(kernel_features, policy_features, "dbus"); + features_supports_signal = features_intersect(kernel_features, policy_features, "signal"); + features_supports_ptrace = features_intersect(kernel_features, policy_features, "ptrace"); + kernel_supports_setload = aa_features_supports(kernel_features, "policy/set_load"); - kernel_supports_diff_encode = aa_features_supports(compile_features, + kernel_supports_diff_encode = aa_features_supports(kernel_features, "policy/diff_encode"); - kernel_supports_stacking = aa_features_supports(compile_features, + features_supports_stacking = aa_features_supports(policy_features, "domain/stack"); - kernel_supports_domain_xattr = aa_features_supports(compile_features, + features_supports_domain_xattr = features_intersect(kernel_features, policy_features, "domain/attach_conditions/xattr"); - kernel_supports_oob = aa_features_supports(compile_features, + kernel_supports_oob = aa_features_supports(kernel_features, "policy/outofband"); - if (aa_features_supports(compile_features, "policy/versions/v7")) + if (aa_features_supports(kernel_features, "policy/versions/v7")) kernel_abi_version = 7; - else if (aa_features_supports(compile_features, "policy/versions/v6")) + else if (aa_features_supports(kernel_features, "policy/versions/v6")) kernel_abi_version = 6; if (!kernel_supports_diff_encode) diff --git a/parser/parser_regex.c b/parser/parser_regex.c index 86174d5f9..90bf2c429 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -524,7 +524,7 @@ static int process_profile_name_xmatch(Profile *prof) } } if (prof->xattrs.list) { - if (!(kernel_supports_domain_xattr && kernel_supports_oob)) { + if (!(features_supports_domain_xattr && kernel_supports_oob)) { warn_once_xattr(name); free_cond_entry_list(prof->xattrs); goto build; @@ -689,7 +689,7 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry) /* allow change_profile for all execs */ vec[0] = "/[^/\\x00][^\\x00]*"; - if (!kernel_supports_stacking) { + if (!features_supports_stacking) { bool stack; if (!parse_label(&stack, &ns, &name, @@ -881,19 +881,19 @@ int process_profile_policydb(Profile *prof) if (kernel_abi_version > 5 && !prof->policy.rules->add_rule(mediates_file, 0, AA_MAY_READ, 0, dfaflags)) goto out; - if (kernel_supports_mount && + if (features_supports_mount && !prof->policy.rules->add_rule(mediates_mount, 0, AA_MAY_READ, 0, dfaflags)) goto out; - if (kernel_supports_dbus && + if (features_supports_dbus && !prof->policy.rules->add_rule(mediates_dbus, 0, AA_MAY_READ, 0, dfaflags)) goto out; - if (kernel_supports_signal && + if (features_supports_signal && !prof->policy.rules->add_rule(mediates_signal, 0, AA_MAY_READ, 0, dfaflags)) goto out; - if (kernel_supports_ptrace && + if (features_supports_ptrace && !prof->policy.rules->add_rule(mediates_ptrace, 0, AA_MAY_READ, 0, dfaflags)) goto out; - if (kernel_supports_unix && + if (features_supports_unix && (!prof->policy.rules->add_rule(mediates_extended_net, 0, AA_MAY_READ, 0, dfaflags) || !prof->policy.rules->add_rule(mediates_net_unix, 0, AA_MAY_READ, 0, dfaflags))) goto out; diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index e44c7f3aa..d5a9852d9 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -711,8 +711,10 @@ rules: rules opt_prefix network_rule yyerror(_("Memory allocation error.")); list_for_each_safe($3, entry, tmp) { - /* map to extended mediation if available */ - if (entry->family == AF_UNIX && kernel_supports_unix) { + /* map to extended mediation, let rule backend do + * downgrade if needed + */ + if (entry->family == AF_UNIX) { unix_rule *rule = new unix_rule(entry->type, $2.audit, $2.deny); if (!rule) yyerror(_("Memory allocation error.")); @@ -1531,7 +1533,7 @@ 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 && - !kernel_supports_stacking && + !features_supports_stacking && warnflags & WARN_RULE_DOWNGRADED) { pwarn("downgrading change_profile safe rule to unsafe due to lack of necessary kernel support\n"); /** diff --git a/parser/ptrace.cc b/parser/ptrace.cc index bc7122b46..e76d69247 100644 --- a/parser/ptrace.cc +++ b/parser/ptrace.cc @@ -128,7 +128,7 @@ int ptrace_rule::gen_policy_re(Profile &prof) * the compile could be used on another kernel unchanged?? * Current caching doesn't support this but in the future maybe */ - if (!kernel_supports_ptrace) { + if (!features_supports_ptrace) { warn_once(prof.name); return RULE_NOT_SUPPORTED; } diff --git a/parser/signal.cc b/parser/signal.cc index c823871d5..2b0992a1a 100644 --- a/parser/signal.cc +++ b/parser/signal.cc @@ -264,7 +264,7 @@ int signal_rule::gen_policy_re(Profile &prof) * it. We may want to switch this so that a compile could be * used for full support on kernels that don't support the feature */ - if (!kernel_supports_signal) { + if (!features_supports_signal) { warn_once(prof.name); return RULE_NOT_SUPPORTED; }