[apparmor] [RFC, PATCH 1/3] apparmor: add a cache entry expiration time aging out capability audit cache

[Ryan Lee]

Noting for additional clarity that I have withdrawn this RFC patchset
in favor of a later PATCH v2 with the same subject line.

On Fri, Sep 13, 2024 at 4:21 PM Ryan Lee <ryan.lee at canonical.com> 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. This
> first patch hardcodes a small duration for the timeout period.
>
> Signed-off-by: Ryan Lee <ryan.lee at 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)
>