Re: [AppArmor 38/41] AppArmor: Module and LSM hooks

2007-04-16 Thread John Johansen
On Thu, Apr 12, 2007 at 11:21:01AM +0100, Alan Cox wrote:
> > +
> > +   /**
> > +* parent can ptrace child when
> > +* - parent is unconfined
> > +* - parent is in complain mode
> > +* - parent and child are confined by the same profile
> > +*/
> 
> Your profiles are name based. That means the same profile in a different
> namespace does different things. It would be a very odd case where it
> mattered but surely the parent ptrace child rule should also require that
> the parent and child are in the same namespace when using apparmor name
> based security.
> 
you are right we should be requiring parent and child are in the same
namespace.  This has been fixed.

> > +static int apparmor_capget(struct task_struct *task,
> > +   kernel_cap_t *effective,
> > +   kernel_cap_t *inheritable,
> > +   kernel_cap_t *permitted)
> > +{
> > +   return cap_capget(task, effective, inheritable, permitted);
> > +}
> 
> Pointless function should go away.
> 
yes we had a few of those thanks for pointing it out.

> > +static int apparmor_sysctl(struct ctl_table *table, int op)
> > +{
> > +   int error = 0;
> > +
> > +   if ((op & 002) && !capable(CAP_SYS_ADMIN))
> > +   error = aa_reject_syscall(current, GFP_KERNEL,
> > + "sysctl (write)");
> > +
> > +   return error;
> 
> The usual file permission security override is DAC not ADMIN. What is the
> logic of this choice.
> 
This was a very course grain check that was done to restrict access to
sysctl's that could be potentially used to elevated priledge.  The check
is inconsistent with AppArmor's model and we should be modelling
sysctl accesses as pathname access, and then we could be using standard
mediation.

thanks for the review
john



pgpu5UrkM9BxV.pgp
Description: PGP signature


Re: [AppArmor 38/41] AppArmor: Module and LSM hooks

2007-04-12 Thread Alan Cox
> +
> + /**
> +  * parent can ptrace child when
> +  * - parent is unconfined
> +  * - parent is in complain mode
> +  * - parent and child are confined by the same profile
> +  */

Your profiles are name based. That means the same profile in a different
namespace does different things. It would be a very odd case where it
mattered but surely the parent ptrace child rule should also require that
the parent and child are in the same namespace when using apparmor name
based security.

> +static int apparmor_capget(struct task_struct *task,
> + kernel_cap_t *effective,
> + kernel_cap_t *inheritable,
> + kernel_cap_t *permitted)
> +{
> + return cap_capget(task, effective, inheritable, permitted);
> +}

Pointless function should go away.

> +static int apparmor_sysctl(struct ctl_table *table, int op)
> +{
> + int error = 0;
> +
> + if ((op & 002) && !capable(CAP_SYS_ADMIN))
> + error = aa_reject_syscall(current, GFP_KERNEL,
> +   "sysctl (write)");
> +
> + return error;

The usual file permission security override is DAC not ADMIN. What is the
logic of this choice.

> +}
> +
> +static int apparmor_syslog(int type)
> +{
> + return cap_syslog(type);
> +}

More pointless functions to delete.


-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[AppArmor 38/41] AppArmor: Module and LSM hooks

2007-04-12 Thread jjohansen
Module parameters, LSM hooks, initialization and teardown.

Signed-off-by: John Johansen <[EMAIL PROTECTED]>
Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]>

---
 security/apparmor/lsm.c |  829 
 1 file changed, 829 insertions(+)

--- /dev/null
+++ b/security/apparmor/lsm.c
@@ -0,0 +1,829 @@
+/*
+ * Copyright (C) 1998-2007 Novell/SUSE
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * AppArmor LSM interface
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "apparmor.h"
+#include "inline.h"
+
+static int param_set_aabool(const char *val, struct kernel_param *kp);
+static int param_get_aabool(char *buffer, struct kernel_param *kp);
+#define param_check_aabool(name, p) __param_check(name, p, int)
+
+static int param_set_aauint(const char *val, struct kernel_param *kp);
+static int param_get_aauint(char *buffer, struct kernel_param *kp);
+#define param_check_aauint(name, p) __param_check(name, p, int)
+
+/* Flag values, also controllable via /sys/module/apparmor/parameters
+ * We define special types as we want to do additional mediation.
+ *
+ * Complain mode -- in complain mode access failures result in auditing only
+ * and task is allowed access.  audit events are processed by userspace to
+ * generate policy.  Default is 'enforce' (0).
+ * Value is also togglable per profile and referenced when global value is
+ * enforce.
+ */
+int apparmor_complain = 0;
+module_param_named(complain, apparmor_complain, aabool, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(apparmor_complain, "Toggle AppArmor complain mode");
+
+/* Debug mode */
+int apparmor_debug = 0;
+module_param_named(debug, apparmor_debug, aabool, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(apparmor_debug, "Toggle AppArmor debug mode");
+
+/* Audit mode */
+int apparmor_audit = 0;
+module_param_named(audit, apparmor_audit, aabool, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(apparmor_audit, "Toggle AppArmor audit mode");
+
+/* Syscall logging mode */
+int apparmor_logsyscall = 0;
+module_param_named(logsyscall, apparmor_logsyscall, aabool, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(apparmor_logsyscall, "Toggle AppArmor logsyscall mode");
+
+/* Maximum pathname length before accesses will start getting rejected */
+unsigned int apparmor_path_max = 2 * PATH_MAX;
+module_param_named(path_max, apparmor_path_max, aauint, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(apparmor_path_max, "Maximum pathname length allowed");
+
+static int param_set_aabool(const char *val, struct kernel_param *kp)
+{
+   if (aa_task_context(current))
+   return -EPERM;
+   return param_set_bool(val, kp);
+}
+
+static int param_get_aabool(char *buffer, struct kernel_param *kp)
+{
+   if (aa_task_context(current))
+   return -EPERM;
+   return param_get_bool(buffer, kp);
+}
+
+static int param_set_aauint(const char *val, struct kernel_param *kp)
+{
+   if (aa_task_context(current))
+   return -EPERM;
+   return param_set_uint(val, kp);
+}
+
+static int param_get_aauint(char *buffer, struct kernel_param *kp)
+{
+   if (aa_task_context(current))
+   return -EPERM;
+   return param_get_uint(buffer, kp);
+}
+
+static int aa_reject_syscall(struct task_struct *task, gfp_t flags,
+const char *name)
+{
+   struct aa_profile *profile = aa_get_profile(task);
+   int error = 0;
+
+   if (profile) {
+   error = aa_audit_syscallreject(profile, flags, name);
+   aa_put_profile(profile);
+   }
+
+   return error;
+}
+
+static int apparmor_ptrace(struct task_struct *parent,
+  struct task_struct *child)
+{
+   struct aa_task_context *cxt;
+   struct aa_task_context *child_cxt;
+   struct aa_profile *child_profile;
+   int error = 0;
+
+   /**
+* parent can ptrace child when
+* - parent is unconfined
+* - parent is in complain mode
+* - parent and child are confined by the same profile
+*/
+
+   rcu_read_lock();
+   cxt = aa_task_context(parent);
+   child_cxt = aa_task_context(child);
+   child_profile = child_cxt ? child_cxt->profile : NULL;
+   error = aa_may_ptrace(cxt, child_profile);
+   if (cxt && PROFILE_COMPLAIN(cxt->profile)) {
+   LOG_HINT(cxt->profile, GFP_ATOMIC, HINT_PTRACE,
+"pid=%d child=%d\n",
+current->pid, child->pid);
+   }
+   rcu_read_unlock();
+
+   return error;
+}
+
+static int apparmor_capget(struct task_struct *task,
+   kernel_cap_t *effective,
+   kernel_cap_t *inheritable,
+   kernel_cap_t