When auditing capabilities, AppArmor uses a per-CPU, per-profile cache such that the same capability for the same profile doesn't get repeatedly audited, with the original goal of reducing audit logspam. However, this cache does not have an expiration time, resulting in confusion when a profile is shared across binaries (for example) and an expected DENIED audit entry doesn't appear, despite the cache entry having been populated much longer ago. This confusion was exacerbated by the per-CPU nature of the cache resulting in the expected entries sporadically appearing when the later denial+audit occurred on a different CPU.
To resolve this, record the last time a capability was audited for a profile and add a timestamp expiration check before doing the audit. This first patch hardcodes a small duration for the timeout period. Signed-off-by: Ryan Lee <ryan....@canonical.com> --- security/apparmor/capability.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c index 7c0f66f1b297..64005b3d0fcc 100644 --- a/security/apparmor/capability.c +++ b/security/apparmor/capability.c @@ -12,6 +12,7 @@ #include <linux/errno.h> #include <linux/gfp.h> #include <linux/security.h> +#include <linux/timekeeping.h> #include "include/apparmor.h" #include "include/capability.h" @@ -33,6 +34,8 @@ struct aa_sfs_entry aa_sfs_entry_caps[] = { struct audit_cache { struct aa_profile *profile; kernel_cap_t caps; + /* Capabilities go from 0 to CAP_LAST_CAP */ + u64 ktime_ns_last_audited[CAP_LAST_CAP+1]; }; static DEFINE_PER_CPU(struct audit_cache, audit_cache); @@ -65,6 +68,8 @@ static void audit_cb(struct audit_buffer *ab, void *va) static int audit_caps(struct apparmor_audit_data *ad, struct aa_profile *profile, int cap, int error) { + const u64 AUDIT_CACHE_TIMEOUT_NS = 100*1000; /* 100 us */ + u64 audit_cache_expiration; struct aa_ruleset *rules = list_first_entry(&profile->rules, typeof(*rules), list); struct audit_cache *ent; @@ -90,7 +95,9 @@ static int audit_caps(struct apparmor_audit_data *ad, struct aa_profile *profile /* Do simple duplicate message elimination */ ent = &get_cpu_var(audit_cache); - if (profile == ent->profile && cap_raised(ent->caps, cap)) { + audit_cache_expiration = ent->ktime_ns_last_audited[cap] + AUDIT_CACHE_TIMEOUT_NS; + if (profile == ent->profile && cap_raised(ent->caps, cap) + && ktime_get_ns() <= audit_cache_expiration) { put_cpu_var(audit_cache); if (COMPLAIN_MODE(profile)) return complain_error(error); @@ -99,6 +106,7 @@ static int audit_caps(struct apparmor_audit_data *ad, struct aa_profile *profile aa_put_profile(ent->profile); ent->profile = aa_get_profile(profile); cap_raise(ent->caps, cap); + ent->ktime_ns_last_audited[cap] = ktime_get_ns(); } put_cpu_var(audit_cache); -- Major items I'm seeking input on (reason for RFC designation): - Whether storing a timestamp per capability is the best approach or whether we should do something else - Whether to hardcode the expiration offset or whether to expose it as a sysctl (see PATCH 3/3 of this series)