apparmor: move ptrace checks to using labels
authorJohn Johansen <john.johansen@canonical.com>
Fri, 9 Jun 2017 21:22:14 +0000 (14:22 -0700)
committerJohn Johansen <john.johansen@canonical.com>
Sun, 11 Jun 2017 00:11:41 +0000 (17:11 -0700)
Signed-off-by: John Johansen <john.johansen@canonical.com>
security/apparmor/domain.c
security/apparmor/include/apparmor.h
security/apparmor/include/ipc.h
security/apparmor/ipc.c
security/apparmor/lsm.c

index 8d6797c..fab8923 100644 (file)
@@ -51,14 +51,16 @@ void aa_free_domain_entries(struct aa_domain *domain)
 
 /**
  * may_change_ptraced_domain - check if can change profile on ptraced task
- * @to_profile: profile to change to  (NOT NULL)
+ * @to_label: profile to change to  (NOT NULL)
+ * @info: message if there is an error
  *
  * Check if current is ptraced and if so if the tracing task is allowed
  * to trace the new domain
  *
  * Returns: %0 or error if change not allowed
  */
-static int may_change_ptraced_domain(struct aa_profile *to_profile)
+static int may_change_ptraced_domain(struct aa_label *to_label,
+                                    const char **info)
 {
        struct task_struct *tracer;
        struct aa_label *tracerl = NULL;
@@ -74,13 +76,14 @@ static int may_change_ptraced_domain(struct aa_profile *to_profile)
        if (!tracer || unconfined(tracerl))
                goto out;
 
-       error = aa_may_ptrace(labels_profile(tracerl), to_profile,
-                             PTRACE_MODE_ATTACH);
+       error = aa_may_ptrace(tracerl, to_label, PTRACE_MODE_ATTACH);
 
 out:
        rcu_read_unlock();
        aa_put_label(tracerl);
 
+       if (error)
+               *info = "ptrace prevents transition";
        return error;
 }
 
@@ -477,7 +480,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
        }
 
        if (bprm->unsafe & LSM_UNSAFE_PTRACE) {
-               error = may_change_ptraced_domain(new_profile);
+               error = may_change_ptraced_domain(&new_profile->label, &info);
                if (error)
                        goto audit;
        }
@@ -661,7 +664,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
                        }
                }
 
-               error = may_change_ptraced_domain(hat);
+               error = may_change_ptraced_domain(&hat->label, &info);
                if (error) {
                        info = "ptraced";
                        error = -EPERM;
@@ -782,7 +785,7 @@ int aa_change_profile(const char *fqname, int flags)
        }
 
        /* check if tracing task is allowed to trace target domain */
-       error = may_change_ptraced_domain(target);
+       error = may_change_ptraced_domain(&target->label, &info);
        if (error) {
                info = "ptrace prevents transition";
                goto audit;
index c4a9004..aaf893f 100644 (file)
@@ -27,6 +27,7 @@
 #define AA_CLASS_NET           4
 #define AA_CLASS_RLIMITS       5
 #define AA_CLASS_DOMAIN                6
+#define AA_CLASS_PTRACE                9
 #define AA_CLASS_LABEL         16
 
 #define AA_CLASS_LAST          AA_CLASS_LABEL
index 288ca76..fb3e751 100644 (file)
@@ -4,7 +4,7 @@
  * This file contains AppArmor ipc mediation function definitions.
  *
  * Copyright (C) 1998-2008 Novell/SUSE
- * Copyright 2009-2010 Canonical Ltd.
+ * Copyright 2009-2017 Canonical Ltd.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
 
 struct aa_profile;
 
-int aa_may_ptrace(struct aa_profile *tracer, struct aa_profile *tracee,
-                 unsigned int mode);
+#define AA_PTRACE_TRACE                MAY_WRITE
+#define AA_PTRACE_READ         MAY_READ
 
-int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
-             unsigned int mode);
+int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
+                 u32 request);
 
 #endif /* __AA_IPC_H */
index 7678d94..f816493 100644 (file)
@@ -4,7 +4,7 @@
  * This file contains AppArmor ipc mediation
  *
  * Copyright (C) 1998-2008 Novell/SUSE
- * Copyright 2009-2010 Canonical Ltd.
+ * Copyright 2009-2017 Canonical Ltd.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
 static void audit_ptrace_cb(struct audit_buffer *ab, void *va)
 {
        struct common_audit_data *sa = va;
+
        audit_log_format(ab, " peer=");
        aa_label_xaudit(ab, labels_ns(aad(sa)->label), aad(sa)->peer,
                        FLAGS_NONE, GFP_ATOMIC);
 }
 
-/**
- * aa_audit_ptrace - do auditing for ptrace
- * @profile: profile being enforced  (NOT NULL)
- * @target: profile being traced (NOT NULL)
- * @error: error condition
- *
- * Returns: %0 or error code
- */
-static int aa_audit_ptrace(struct aa_profile *profile,
-                          struct aa_profile *target, int error)
+static int cross_ptrace_perm(struct aa_profile *tracer,
+                            struct aa_profile *tracee, u32 request,
+                            struct common_audit_data *sa)
 {
-       DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
+       /* policy uses the old style capability check for ptrace */
+       if (profile_unconfined(tracer) || tracer == tracee)
+               return 0;
 
-       aad(&sa)->peer = &target->label;
-       aad(&sa)->error = error;
+       aad(sa)->label = &tracer->label;
+       aad(sa)->peer = &tracee->label;
+       aad(sa)->request = 0;
+       aad(sa)->error = aa_capable(&tracer->label, CAP_SYS_PTRACE, 1);
 
-       return aa_audit(AUDIT_APPARMOR_AUTO, profile, &sa, audit_ptrace_cb);
+       return aa_audit(AUDIT_APPARMOR_AUTO, tracer, sa, audit_ptrace_cb);
 }
 
 /**
  * aa_may_ptrace - test if tracer task can trace the tracee
- * @tracer: profile of the task doing the tracing  (NOT NULL)
- * @tracee: task to be traced
- * @mode: whether PTRACE_MODE_READ || PTRACE_MODE_ATTACH
+ * @tracer: label of the task doing the tracing  (NOT NULL)
+ * @tracee: task label to be traced
+ * @request: permission request
  *
  * Returns: %0 else error code if permission denied or error
  */
-int aa_may_ptrace(struct aa_profile *tracer, struct aa_profile *tracee,
-                 unsigned int mode)
+int aa_may_ptrace(struct aa_label *tracer, struct aa_label *tracee,
+                 u32 request)
 {
-       /* TODO: currently only based on capability, not extended ptrace
-        *       rules,
-        *       Test mode for PTRACE_MODE_READ || PTRACE_MODE_ATTACH
-        */
+       DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_PTRACE);
 
-       if (profile_unconfined(tracer) || tracer == tracee)
-               return 0;
-       /* log this capability request */
-       return aa_capable(&tracer->label, CAP_SYS_PTRACE, 1);
+       return xcheck_labels_profiles(tracer, tracee, cross_ptrace_perm,
+                                     request, &sa);
 }
 
-/**
- * aa_ptrace - do ptrace permission check and auditing
- * @tracer: task doing the tracing (NOT NULL)
- * @tracee: task being traced (NOT NULL)
- * @mode: ptrace mode either PTRACE_MODE_READ || PTRACE_MODE_ATTACH
- *
- * Returns: %0 else error code if permission denied or error
- */
-int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
-             unsigned int mode)
-{
-       /*
-        * tracer can ptrace tracee when
-        * - tracer is unconfined ||
-        *   - tracer is in complain mode
-        *   - tracer has rules allowing it to trace tracee currently this is:
-        *       - confined by the same profile ||
-        *       - tracer profile has CAP_SYS_PTRACE
-        */
 
-       struct aa_label *tracer_l = aa_get_task_label(tracer);
-       int error = 0;
-
-       if (!unconfined(tracer_l)) {
-               struct aa_label *tracee_l = aa_get_task_label(tracee);
-
-               error = aa_may_ptrace(labels_profile(tracer_l),
-                                     labels_profile(tracee_l),
-                                     mode);
-               error = aa_audit_ptrace(labels_profile(tracer_l),
-                                       labels_profile(tracee_l),
-                                       error);
-
-               aa_put_label(tracee_l);
-       }
-       aa_put_label(tracer_l);
-
-       return error;
-}
index c3e98f7..bf28b48 100644 (file)
@@ -104,12 +104,31 @@ static void apparmor_cred_transfer(struct cred *new, const struct cred *old)
 static int apparmor_ptrace_access_check(struct task_struct *child,
                                        unsigned int mode)
 {
-       return aa_ptrace(current, child, mode);
+       struct aa_label *tracer, *tracee;
+       int error;
+
+       tracer = begin_current_label_crit_section();
+       tracee = aa_get_task_label(child);
+       error = aa_may_ptrace(tracer, tracee,
+                 mode == PTRACE_MODE_READ ? AA_PTRACE_READ : AA_PTRACE_TRACE);
+       aa_put_label(tracee);
+       end_current_label_crit_section(tracer);
+
+       return error;
 }
 
 static int apparmor_ptrace_traceme(struct task_struct *parent)
 {
-       return aa_ptrace(parent, current, PTRACE_MODE_ATTACH);
+       struct aa_label *tracer, *tracee;
+       int error;
+
+       tracee = begin_current_label_crit_section();
+       tracer = aa_get_task_label(parent);
+       error = aa_may_ptrace(tracer, tracee, AA_PTRACE_TRACE);
+       aa_put_label(tracer);
+       end_current_label_crit_section(tracee);
+
+       return error;
 }
 
 /* Derived from security/commoncap.c:cap_capget */