From c77be398f77eba03d0b425d980d25c09e5f75326 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Fri, 25 May 2007 12:35:47 +0000 Subject: [PATCH] Add first steps to cleaning up audit. --- .../for-mainline/apparmor_ptrace-cleanup.diff | 58 +++ .../for-mainline/get-rid-of-sa-flags.diff | 133 +++++++ .../no-internal-audit-errors.diff | 21 + kernel-patches/for-mainline/series | 4 + .../for-mainline/simplify-audit-status.diff | 367 ++++++++++++++++++ 5 files changed, 583 insertions(+) create mode 100644 kernel-patches/for-mainline/apparmor_ptrace-cleanup.diff create mode 100644 kernel-patches/for-mainline/get-rid-of-sa-flags.diff create mode 100644 kernel-patches/for-mainline/no-internal-audit-errors.diff create mode 100644 kernel-patches/for-mainline/simplify-audit-status.diff diff --git a/kernel-patches/for-mainline/apparmor_ptrace-cleanup.diff b/kernel-patches/for-mainline/apparmor_ptrace-cleanup.diff new file mode 100644 index 000000000..ca15575da --- /dev/null +++ b/kernel-patches/for-mainline/apparmor_ptrace-cleanup.diff @@ -0,0 +1,58 @@ +--- + security/apparmor/lsm.c | 37 ++++++++++++++++++++----------------- + 1 file changed, 20 insertions(+), 17 deletions(-) + +--- a/security/apparmor/lsm.c ++++ b/security/apparmor/lsm.c +@@ -108,8 +108,6 @@ static int apparmor_ptrace(struct task_s + struct task_struct *child) + { + struct aa_task_context *cxt; +- struct aa_task_context *child_cxt; +- struct aa_profile *child_profile; + int error = 0; + + /* +@@ -123,22 +121,27 @@ static int apparmor_ptrace(struct task_s + + rcu_read_lock(); + cxt = aa_task_context(parent); +- child_cxt = aa_task_context(child); +- child_profile = child_cxt ? child_cxt->profile : NULL; +- if (cxt && (parent->nsproxy != child->nsproxy)) { +- aa_audit_message(NULL, GFP_ATOMIC, "REJECTING ptrace across " +- "namespace of %d by %d", +- parent->pid, child->pid); +- error = -EPERM; +- } else { +- error = aa_may_ptrace(cxt, child_profile); +- if (cxt && PROFILE_COMPLAIN(cxt->profile)) { ++ if (cxt) { ++ if (parent->nsproxy != child->nsproxy) { + aa_audit_message(cxt->profile, GFP_ATOMIC, +- "LOGPROF-HINT ptrace pid=%d child=%d " +- "(%d profile %s active %s)", +- current->pid, child->pid, current->pid, +- cxt->profile->parent->name, +- cxt->profile->name); ++ "REJECTING ptrace across " ++ "namespace of %d by %d", ++ parent->pid, child->pid); ++ error = -EPERM; ++ } else { ++ struct aa_task_context *child_cxt = ++ aa_task_context(child); ++ ++ error = aa_may_ptrace(cxt, child_cxt ? ++ child_cxt->profile : NULL); ++ if (PROFILE_COMPLAIN(cxt->profile)) { ++ aa_audit_message(cxt->profile, GFP_ATOMIC, ++ "LOGPROF-HINT ptrace pid=%d child=%d " ++ "(%d profile %s active %s)", ++ current->pid, child->pid, current->pid, ++ cxt->profile->parent->name, ++ cxt->profile->name); ++ } + } + } + rcu_read_unlock(); diff --git a/kernel-patches/for-mainline/get-rid-of-sa-flags.diff b/kernel-patches/for-mainline/get-rid-of-sa-flags.diff new file mode 100644 index 000000000..21b4d0303 --- /dev/null +++ b/kernel-patches/for-mainline/get-rid-of-sa-flags.diff @@ -0,0 +1,133 @@ +--- + security/apparmor/apparmor.h | 7 +------ + security/apparmor/main.c | 20 +------------------- + 2 files changed, 2 insertions(+), 25 deletions(-) + +--- a/security/apparmor/apparmor.h ++++ b/security/apparmor/apparmor.h +@@ -145,7 +145,7 @@ extern struct aa_profile *null_complain_ + */ + + struct aa_audit { +- unsigned short type, flags; ++ unsigned short type; + unsigned int result; + gfp_t gfp_mask; + int error_code; +@@ -175,11 +175,6 @@ struct aa_audit { + #define AA_AUDITTYPE_MSG 7 + #define AA_AUDITTYPE_SYSCALL 8 + +-/* audit flags */ +-#define AA_AUDITFLAG_AUDITSS_SYSCALL 1 /* log syscall context */ +-#define AA_AUDITFLAG_LOGERR 2 /* log operations that failed due to +- non permission errors */ +- + /* Flags for the permission check functions */ + #define AA_CHECK_FD 1 /* coming from a file descriptor */ + #define AA_CHECK_DIR 2 /* file type is directory */ +--- a/security/apparmor/main.c ++++ b/security/apparmor/main.c +@@ -367,7 +367,6 @@ int aa_audit_message(struct aa_profile * + sa.type = AA_AUDITTYPE_MSG; + sa.name = fmt; + va_start(sa.vaval, fmt); +- sa.flags = 0; + sa.gfp_mask = gfp; + sa.error_code = 0; + sa.result = 0; /* fake failure: force message to be logged */ +@@ -392,7 +391,6 @@ int aa_audit_syscallreject(struct aa_pro + + sa.type = AA_AUDITTYPE_SYSCALL; + sa.name = msg; +- sa.flags = 0; + sa.gfp_mask = gfp; + sa.error_code = 0; + sa.result = 0; /* failure */ +@@ -411,7 +409,6 @@ int aa_audit(struct aa_profile *profile, + struct audit_context *audit_cxt; + + const char *logcls; +- unsigned int flags; + int audit = 0, + complain = 0, + error = -EINVAL, +@@ -453,21 +450,13 @@ int aa_audit(struct aa_profile *profile, + logcls = complain ? "PERMITTING" : "REJECTING"; + } + +- /* In future extend w/ per-profile flags +- * (flags |= sa->profile->flags) +- */ +- flags = sa->flags; +- if (apparmor_logsyscall) +- flags |= AA_AUDITFLAG_AUDITSS_SYSCALL; +- +- + /* Force full audit syscall logging regardless of global setting if + * we are rejecting a syscall + */ + if (sa->type == AA_AUDITTYPE_SYSCALL) { + audit_cxt = current->audit_context; + } else { +- audit_cxt = (flags & AA_AUDITFLAG_AUDITSS_SYSCALL) ? ++ audit_cxt = apparmor_logsyscall ? + current->audit_context : NULL; + } + +@@ -593,7 +582,6 @@ int aa_attr(struct aa_profile *profile, + + sa.type = AA_AUDITTYPE_ATTR; + sa.iattr = iattr; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + + check = 0; +@@ -626,7 +614,6 @@ int aa_perm_xattr(struct aa_profile *pro + + sa.type = AA_AUDITTYPE_XATTR; + sa.name2 = operation; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + + if (inode && S_ISDIR(inode->i_mode)) +@@ -659,7 +646,6 @@ int aa_perm(struct aa_profile *profile, + + sa.type = AA_AUDITTYPE_FILE; + sa.mask = mask; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, check); + +@@ -686,7 +672,6 @@ int aa_perm_dir(struct aa_profile *profi + + sa.type = AA_AUDITTYPE_DIR; + sa.name2 = operation; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + + return aa_perm_dentry(profile, dentry, mnt, &sa, mask, AA_CHECK_DIR); +@@ -699,7 +684,6 @@ int aa_perm_path(struct aa_profile *prof + + sa.type = AA_AUDITTYPE_FILE; + sa.mask = mask; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + sa.name = name; + +@@ -739,7 +723,6 @@ int aa_capability(struct aa_task_context + sa.type = AA_AUDITTYPE_CAP; + sa.name = NULL; + sa.capability = cap; +- sa.flags = 0; + sa.error_code = 0; + sa.result = !error; + sa.gfp_mask = GFP_ATOMIC; +@@ -795,7 +778,6 @@ again: + aa_permerror2result(denied_mask, &sa); + + sa.type = AA_AUDITTYPE_LINK; +- sa.flags = 0; + sa.gfp_mask = GFP_KERNEL; + + error = aa_audit(profile, &sa); diff --git a/kernel-patches/for-mainline/no-internal-audit-errors.diff b/kernel-patches/for-mainline/no-internal-audit-errors.diff new file mode 100644 index 000000000..2379443d7 --- /dev/null +++ b/kernel-patches/for-mainline/no-internal-audit-errors.diff @@ -0,0 +1,21 @@ +--- + security/apparmor/main.c | 8 -------- + 1 file changed, 8 deletions(-) + +--- a/security/apparmor/main.c ++++ b/security/apparmor/main.c +@@ -432,14 +432,6 @@ int aa_audit(struct aa_profile *profile, + audit = 1; + logcls = "AUDITING"; + } +- } else if (sa->error_code < 0) { +- audit_log(current->audit_context, gfp_mask, AUDIT_APPARMOR, +- "Internal error auditing event type %d (error %d)", +- sa->type, sa->error_code); +- AA_ERROR("Internal error auditing event type %d (error %d)\n", +- sa->type, sa->error_code); +- error = sa->error_code; +- goto out; + } else if (sa->type == AA_AUDITTYPE_SYSCALL) { + /* Currently AA_AUDITTYPE_SYSCALL is for rejects only. + * Values set by aa_audit_syscallreject will get us here. diff --git a/kernel-patches/for-mainline/series b/kernel-patches/for-mainline/series index b4372296b..c8b18ff00 100644 --- a/kernel-patches/for-mainline/series +++ b/kernel-patches/for-mainline/series @@ -40,10 +40,14 @@ apparmor-audit.diff apparmor-main.diff # apparmor-main-2.diff apparmor-lsm.diff +apparmor_ptrace-cleanup.diff apparmor-module_interface.diff apparmor-misc.diff apparmor-intree.diff apparmor-lockdep.diff +get-rid-of-sa-flags.diff +no-internal-audit-errors.diff +simplify-audit-status.diff do_path_lookup-nameidata.diff sys_fchdir-nameidata.diff file_permission-nameidata.diff diff --git a/kernel-patches/for-mainline/simplify-audit-status.diff b/kernel-patches/for-mainline/simplify-audit-status.diff new file mode 100644 index 000000000..f1681ddd4 --- /dev/null +++ b/kernel-patches/for-mainline/simplify-audit-status.diff @@ -0,0 +1,367 @@ +--- + security/apparmor/apparmor.h | 7 -- + security/apparmor/main.c | 140 +++++++++++++++++-------------------------- + 2 files changed, 61 insertions(+), 86 deletions(-) + +--- a/security/apparmor/apparmor.h ++++ b/security/apparmor/apparmor.h +@@ -146,13 +146,12 @@ extern struct aa_profile *null_complain_ + + struct aa_audit { + unsigned short type; +- unsigned int result; + gfp_t gfp_mask; +- int error_code; + const char *name; + char *buffer; ++ int requested_mask, denied_mask; ++ int error_code; + union { +- int mask; + int capability; + struct { + const char *name2; +@@ -191,7 +190,7 @@ enum aa_lock_class { + extern int alloc_null_complain_profile(void); + extern void free_null_complain_profile(void); + extern int attach_nullprofile(struct aa_profile *profile); +-extern int aa_audit_message(struct aa_profile *profile, gfp_t gfp, ++extern void aa_audit_message(struct aa_profile *profile, gfp_t gfp, + const char *, ...) + __attribute__ ((format (printf, 3, 4))); + extern int aa_audit_syscallreject(struct aa_profile *profile, gfp_t gfp, +--- a/security/apparmor/main.c ++++ b/security/apparmor/main.c +@@ -38,17 +38,6 @@ static const char *capability_names[] = + */ + struct aa_profile *null_complain_profile; + +-static inline void aa_permerror2result(int perm_result, struct aa_audit *sa) +-{ +- if (perm_result == 0) { /* success */ +- sa->result = 1; +- sa->error_code = 0; +- } else { /* -ve internal error code or +ve mask of denied perms */ +- sa->result = 0; +- sa->error_code = perm_result; +- } +-} +- + /** + * aa_file_denied - check for @mask access on a file + * @profile: profile to check against +@@ -239,10 +228,9 @@ static inline void aa_put_name_buffer(ch + * an open file descriptor (AA_CHECK_FD) or not. + */ + static int aa_perm_dentry(struct aa_profile *profile, struct dentry *dentry, +- struct vfsmount *mnt, struct aa_audit *sa, int mask, +- int check) ++ struct vfsmount *mnt, struct aa_audit *sa, int check) + { +- int denied_mask, error; ++ int error; + + again: + sa->buffer = NULL; +@@ -254,14 +242,16 @@ again: + * accessed through a file descriptor. + */ + if (PTR_ERR(sa->name) == -ENOENT && (check & AA_CHECK_FD)) +- denied_mask = 0; ++ sa->denied_mask = 0; + else +- denied_mask = PTR_ERR(sa->name); ++ sa->denied_mask = PTR_ERR(sa->name); + sa->name = NULL; + } else +- denied_mask = aa_file_denied(profile, sa->name, mask); ++ sa->denied_mask = aa_file_denied(profile, sa->name, ++ sa->requested_mask); + +- aa_permerror2result(denied_mask, sa); ++ if (!sa->denied_mask) ++ sa->error_code = 0; + + error = aa_audit(profile, sa); + +@@ -358,24 +348,22 @@ void free_null_complain_profile(void) + * @flags: audit flags + * @fmt: varargs fmt + */ +-int aa_audit_message(struct aa_profile *profile, gfp_t gfp, const char *fmt, ++void aa_audit_message(struct aa_profile *profile, gfp_t gfp, const char *fmt, + ...) + { +- int ret; + struct aa_audit sa; + + sa.type = AA_AUDITTYPE_MSG; + sa.name = fmt; + va_start(sa.vaval, fmt); + sa.gfp_mask = gfp; +- sa.error_code = 0; +- sa.result = 0; /* fake failure: force message to be logged */ ++ sa.requested_mask = 0; ++ sa.denied_mask = 0; ++ sa.error_code = -EAGAIN; /* never reaches user space */ + +- ret = aa_audit(profile, &sa); ++ (void)aa_audit(profile, &sa); + + va_end(sa.vaval); +- +- return ret; + } + + /** +@@ -392,8 +380,9 @@ int aa_audit_syscallreject(struct aa_pro + sa.type = AA_AUDITTYPE_SYSCALL; + sa.name = msg; + sa.gfp_mask = gfp; +- sa.error_code = 0; +- sa.result = 0; /* failure */ ++ sa.requested_mask = 0; ++ sa.denied_mask = 0; ++ sa.error_code = -EPERM; + + return aa_audit(profile, &sa); + } +@@ -409,29 +398,16 @@ int aa_audit(struct aa_profile *profile, + struct audit_context *audit_cxt; + + const char *logcls; +- int audit = 0, +- complain = 0, +- error = -EINVAL, +- opspec_error = -EACCES; ++ int complain = 0; + + const gfp_t gfp_mask = sa->gfp_mask; + +- /* +- * sa->result: 1 success, 0 failure +- * sa->error_code: success: 0 +- * failure: +ve mask of failed permissions or -ve +- * system error +- */ +- +- if (likely(sa->result)) { ++ if (likely(!sa->error_code)) { + if (likely(!PROFILE_AUDIT(profile))) { + /* nothing to log */ +- error = 0; +- goto out; +- } else { +- audit = 1; ++ return 0; ++ } else + logcls = "AUDITING"; +- } + } else if (sa->type == AA_AUDITTYPE_SYSCALL) { + /* Currently AA_AUDITTYPE_SYSCALL is for rejects only. + * Values set by aa_audit_syscallreject will get us here. +@@ -457,16 +433,13 @@ int aa_audit(struct aa_profile *profile, + if (!ab) { + AA_ERROR("Unable to log event (%d) to audit subsys\n", + sa->type); +- if (complain) +- error = 0; +- goto out; ++ return -ENOMEM; /* FIXME: was -EINVAL if !complain: ??? */ + } + + /* messages get special handling */ + if (sa->type == AA_AUDITTYPE_MSG) { + audit_log_vformat(ab, sa->name, sa->vaval); + audit_log_end(ab); +- error = 0; + goto out; + } + +@@ -479,7 +452,6 @@ int aa_audit(struct aa_profile *profile, + sa->name2 = mangle(sa->name2, sa->buffer2); + if (!sa->name2) + return -ENAMETOOLONG; +- + } + + /* log operation */ +@@ -490,16 +462,16 @@ int aa_audit(struct aa_profile *profile, + + switch(NOFLAGS(sa->type)) { + case NOFLAGS(AA_AUDITTYPE_FILE): { +- int perm = audit ? sa->mask : sa->error_code; ++ int mask = PROFILE_AUDIT(profile) ? ++ sa->requested_mask : sa->denied_mask; + + audit_log_format(ab, "%s%s%s%s%s access to %s ", +- perm & AA_EXEC_MMAP ? "m" : "", +- perm & MAY_READ ? "r" : "", +- perm & MAY_WRITE ? "w" : "", +- perm & MAY_EXEC ? "x" : "", +- perm & AA_MAY_LINK ? "l" : "", ++ mask & AA_EXEC_MMAP ? "m" : "", ++ mask & MAY_READ ? "r" : "", ++ mask & MAY_WRITE ? "w" : "", ++ mask & MAY_EXEC ? "x" : "", ++ mask & AA_MAY_LINK ? "l" : "", + sa->name); +- opspec_error = -EPERM; + break; + } + case NOFLAGS(AA_AUDITTYPE_DIR): +@@ -532,15 +504,13 @@ int aa_audit(struct aa_profile *profile, + case NOFLAGS(AA_AUDITTYPE_CAP): + audit_log_format(ab, "access to capability '%s' ", + capability_names[sa->capability]); +- opspec_error = -EPERM; + break; + case NOFLAGS(AA_AUDITTYPE_SYSCALL): + audit_log_format(ab, "access to syscall '%s' ", sa->name); +- opspec_error = -EPERM; + break; + default: + WARN_ON(1); +- return error; ++ return -EINVAL; + } + + #undef NOFLAGS +@@ -550,12 +520,8 @@ int aa_audit(struct aa_profile *profile, + + audit_log_end(ab); + +- if (complain) +- error = 0; +- else +- error = sa->result ? 0 : opspec_error; + out: +- return error; ++ return complain ? 0 : sa->error_code; + } + + /** +@@ -575,6 +541,8 @@ int aa_attr(struct aa_profile *profile, + sa.type = AA_AUDITTYPE_ATTR; + sa.iattr = iattr; + sa.gfp_mask = GFP_KERNEL; ++ sa.requested_mask = MAY_WRITE; ++ sa.error_code = -EACCES; + + check = 0; + if (inode && S_ISDIR(inode->i_mode)) +@@ -582,7 +550,7 @@ int aa_attr(struct aa_profile *profile, + if (iattr->ia_valid & ATTR_FILE) + check |= AA_CHECK_FD; + +- error = aa_perm_dentry(profile, dentry, mnt, &sa, MAY_WRITE, check); ++ error = aa_perm_dentry(profile, dentry, mnt, &sa, check); + + return error; + } +@@ -607,11 +575,13 @@ int aa_perm_xattr(struct aa_profile *pro + sa.type = AA_AUDITTYPE_XATTR; + sa.name2 = operation; + sa.gfp_mask = GFP_KERNEL; ++ sa.requested_mask = mask; ++ sa.error_code = -EACCES; + + if (inode && S_ISDIR(inode->i_mode)) + check |= AA_CHECK_DIR; + +- error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, check); ++ error = aa_perm_dentry(profile, dentry, mnt, &sa, check); + + return error; + } +@@ -637,9 +607,10 @@ int aa_perm(struct aa_profile *profile, + goto out; + + sa.type = AA_AUDITTYPE_FILE; +- sa.mask = mask; + sa.gfp_mask = GFP_KERNEL; +- error = aa_perm_dentry(profile, dentry, mnt, &sa, mask, check); ++ sa.requested_mask = mask; ++ sa.error_code = -EACCES; /* FIXME: was -EPERM before: ??? */ ++ error = aa_perm_dentry(profile, dentry, mnt, &sa, check); + + out: + return error; +@@ -665,22 +636,23 @@ int aa_perm_dir(struct aa_profile *profi + sa.type = AA_AUDITTYPE_DIR; + sa.name2 = operation; + sa.gfp_mask = GFP_KERNEL; ++ sa.requested_mask = mask; ++ sa.error_code = -EACCES; + +- return aa_perm_dentry(profile, dentry, mnt, &sa, mask, AA_CHECK_DIR); ++ return aa_perm_dentry(profile, dentry, mnt, &sa, AA_CHECK_DIR); + } + + int aa_perm_path(struct aa_profile *profile, const char *name, int mask) + { + struct aa_audit sa; +- int denied_mask; + + sa.type = AA_AUDITTYPE_FILE; +- sa.mask = mask; ++ sa.requested_mask = mask; + sa.gfp_mask = GFP_KERNEL; + sa.name = name; + +- denied_mask = aa_file_denied(profile, name, mask); +- aa_permerror2result(denied_mask, &sa); ++ sa.denied_mask = aa_file_denied(profile, name, mask); ++ sa.error_code = sa.denied_mask ? -EACCES : 0; + + return aa_audit(profile, &sa); + } +@@ -715,8 +687,7 @@ int aa_capability(struct aa_task_context + sa.type = AA_AUDITTYPE_CAP; + sa.name = NULL; + sa.capability = cap; +- sa.error_code = 0; +- sa.result = !error; ++ sa.error_code = error; + sa.gfp_mask = GFP_ATOMIC; + + error = aa_audit(cxt->profile, &sa); +@@ -746,7 +717,7 @@ int aa_link(struct aa_profile *profile, + struct dentry *link, struct vfsmount *link_mnt, + struct dentry *target, struct vfsmount *target_mnt) + { +- int denied_mask = -EPERM, error, check = 0; ++ int error, check = 0; + struct aa_audit sa; + + again: +@@ -756,18 +727,23 @@ again: + sa.name2 = aa_get_name(target, target_mnt, &sa.buffer2, check); + + if (IS_ERR(sa.name)) { +- denied_mask = PTR_ERR(sa.name); ++ sa.requested_mask = 0; ++ sa.denied_mask = 0; ++ sa.error_code = PTR_ERR(sa.name); + sa.name = NULL; + } + if (IS_ERR(sa.name2)) { +- denied_mask = PTR_ERR(sa.name2); ++ sa.requested_mask = 0; ++ sa.denied_mask = 0; ++ sa.error_code = PTR_ERR(sa.name); + sa.name2 = NULL; + } + +- if (sa.name && sa.name2) +- denied_mask = aa_link_denied(profile, sa.name, sa.name2); +- +- aa_permerror2result(denied_mask, &sa); ++ if (sa.name && sa.name2) { ++ sa.requested_mask = AA_MAY_LINK; ++ sa.denied_mask = aa_link_denied(profile, sa.name, sa.name2); ++ sa.error_code = sa.denied_mask ? -EPERM : 0; ++ } + + sa.type = AA_AUDITTYPE_LINK; + sa.gfp_mask = GFP_KERNEL;