[apparmor] [PATCH v2] apparmor: add a cache entry expiration time aging out capability audit cache

[John Johansen]

On 9/20/24 12:53, Ryan Lee wrote:
> 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.
> 
> v1 -> v2:
>   - Hardcode a longer timeout and drop the patches making it a sysctl,
>     after discussion with John Johansen.
>   - Cache the expiration time instead of the last-audited time. This value
>     can never be zero, which lets us drop the kernel_cap_t caps field from
>     the cache struct.
> 
> Signed-off-by: Ryan Lee <ryan.lee at canonical.com>

Acked-by: John Johansen <john.johansen at canonical.com>

I have pulled this into my tree

> ---
>   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,7 @@ 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_expiration[CAP_LAST_CAP+1];
>   };
>   
>   static DEFINE_PER_CPU(struct audit_cache, audit_cache);
> @@ -64,6 +67,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 = 1000*1000*1000; /* 1 second */
> +
>   	struct aa_ruleset *rules = list_first_entry(&profile->rules,
>   						    typeof(*rules), list);
>   	struct audit_cache *ent;
> @@ -90,7 +94,8 @@ 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)) {
> +	/* If the capability was never raised the timestamp check would also catch that */
> +	if (profile == ent->profile && ktime_get_ns() <= ent->ktime_ns_expiration[cap]) {
>   		put_cpu_var(audit_cache);
>   		if (COMPLAIN_MODE(profile))
>   			return complain_error(error);
> @@ -99,6 +104,6 @@ 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_expiration[cap] = ktime_get_ns() + AUDIT_CACHE_TIMEOUT_NS;
>   	}
>   	put_cpu_var(audit_cache);
>