For review:
The following patch series adds mount mediation to apparmor. As part of
this it does some rework/cleanup up on the path name lookup and dfa matching
code, and adds base support for the extended policydb to policy loading.
This patch series assume the pull request containing Kees Cook's apparmorfs
patches have been applied.
When __d_path and d_absolute_path fail due to the name being outside of
the current namespace no name is reported. Use dentry_path to provide
some hint as to which file was being accessed.
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/path.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index c31ce83..b3cf4cd 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -94,18 +94,21 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
} else
res = d_absolute_path(path, buf, buflen);
- *name = res;
/* handle error conditions - and still allow a partial path to
* be returned.
*/
if (IS_ERR(res)) {
- error = PTR_ERR(res);
- *name = buf;
- goto out;
- }
- if (!our_mnt(path->mnt))
+ res = dentry_path_raw(path->dentry, buf, buflen);
+ if (IS_ERR(res)) {
+ error = PTR_ERR(res);
+ *name = buf;
+ goto out;
+ };
+ } else if (!our_mnt(path->mnt))
connected = 0;
+ *name = res;
+
ok:
/* Handle two cases:
* 1. A deleted dentry && profile is not allowing mediation of deleted
--
1.7.9
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/path.c | 16 ++++++----------
1 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index b3cf4cd..95a589e 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -83,21 +83,18 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
struct path root;
get_fs_root(current->fs, &root);
res = __d_path(path, &root, buf, buflen);
- if (res && !IS_ERR(res)) {
- /* everything's fine */
- *name = res;
- path_put(&root);
- goto ok;
- }
path_put(&root);
- connected = 0;
- } else
+ } else {
res = d_absolute_path(path, buf, buflen);
+ if (!our_mnt(path->mnt))
+ connected = 0;
+ }
/* handle error conditions - and still allow a partial path to
* be returned.
*/
- if (IS_ERR(res)) {
+ if (!res || IS_ERR(res)) {
+ connected = 0;
res = dentry_path_raw(path->dentry, buf, buflen);
if (IS_ERR(res)) {
error = PTR_ERR(res);
@@ -109,7 +106,6 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
*name = res;
-ok:
/* Handle two cases:
* 1. A deleted dentry && profile is not allowing mediation of deleted
* 2. On some filesystems, newly allocated dentries appear to the
--
1.7.9
Add the base support for the new policy extensions. This does not bring
any additional functionality, or change current semantics.
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/include/apparmor.h | 13 +++++++++++++
security/apparmor/include/policy.h | 13 +++++++++++++
security/apparmor/policy.c | 1 +
security/apparmor/policy_unpack.c | 22 ++++++++++++++++++++++
4 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
index 248c408..40aedd9 100644
--- a/security/apparmor/include/apparmor.h
+++ b/security/apparmor/include/apparmor.h
@@ -19,6 +19,19 @@
#include "match.h"
+/*
+ * Class of mediation types in the AppArmor policy db
+ */
+#define AA_CLASS_ENTRY 0
+#define AA_CLASS_UNKNOWN 1
+#define AA_CLASS_FILE 2
+#define AA_CLASS_CAP 3
+#define AA_CLASS_NET 4
+#define AA_CLASS_RLIMITS 5
+#define AA_CLASS_DOMAIN 6
+
+#define AA_CLASS_LAST AA_CLASS_DOMAIN
+
/* Control parameters settable through module/boot flags */
extern enum audit_mode aa_g_audit;
extern bool aa_g_audit_header;
diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index aeda5cf..9e18e96 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -129,6 +129,17 @@ struct aa_namespace {
struct list_head sub_ns;
};
+/* struct aa_policydb - match engine for a policy
+ * dfa: dfa pattern match
+ * start: set of start states for the different classes of data
+ */
+struct aa_policydb {
+ /* Generic policy DFA specific rule types will be subsections of it */
+ struct aa_dfa *dfa;
+ unsigned int start[AA_CLASS_LAST + 1];
+
+};
+
/* struct aa_profile - basic confinement data
* @base - base components of the profile (name, refcount, lists, lock ...)
* @parent: parent of profile
@@ -143,6 +154,7 @@ struct aa_namespace {
* @flags: flags controlling profile behavior
* @path_flags: flags controlling path generation behavior
* @size: the memory consumed by this profiles rules
+ * @policy: general match rules governing policy
* @file: The set of rules governing basic file access and domain transitions
* @caps: capabilities for the profile
* @rlimits: rlimits for the profile
@@ -179,6 +191,7 @@ struct aa_profile {
u32 path_flags;
int size;
+ struct aa_policydb policy;
struct aa_file_rules file;
struct aa_caps caps;
struct aa_rlimit rlimits;
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 537e5dc..8b7febb 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -752,6 +752,7 @@ static void free_profile(struct aa_profile *profile)
aa_free_sid(profile->sid);
aa_put_dfa(profile->xmatch);
+ aa_put_dfa(profile->policy.dfa);
aa_put_profile(profile->replacedby);
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 6137b10..c7a6d03 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -562,6 +562,28 @@ static struct aa_profile *unpack_profile(struct aa_ext *e)
if (!unpack_rlimits(e, profile))
goto fail;
+ if (unpack_nameX(e, AA_STRUCT, "policydb")) {
+ /* generic policy dfa - optional and may be NULL */
+ profile->policy.dfa = unpack_dfa(e);
+ if (IS_ERR(profile->policy.dfa)) {
+ error = PTR_ERR(profile->policy.dfa);
+ profile->policy.dfa = NULL;
+ goto fail;
+ }
+ if (!unpack_u32(e, &profile->policy.start[0], "start"))
+ /* default start state */
+ profile->policy.start[0] = DFA_START;
+ /* setup class index */
+ for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) {
+ profile->policy.start[i] =
+ aa_dfa_next(profile->policy.dfa,
+ profile->policy.start[0],
+ i);
+ }
+ if (!unpack_nameX(e, AA_STRUCTEND, NULL))
+ goto fail;
+ }
+
/* get file rules */
profile->file.dfa = unpack_dfa(e);
if (IS_ERR(profile->file.dfa)) {
--
1.7.9
Add the ability for apparmor to do mediation of mount operations. Mount
rules require an updated apparmor_parser (2.8 series) for policy compilation.
The basic form of the rules are.
[audit] [deny] mount [conds]* [device] [ -> [conds] path],
[audit] [deny] remount [conds]* [path],
[audit] [deny] umount [conds]* [path],
[audit] [deny] pivotroot [oldroot=<value>] <path>
remount is just a short cut for mount options=remount
where [conds] can be
fstype=<expr>
options=<expr>
Example mount commands
mount, # allow all mounts, but not umount or pivotroot
mount fstype=procfs, # allow mounting procfs anywhere
mount options=(bind, ro) /foo -> /bar, # readonly bind mount
mount /dev/sda -> /mnt,
mount /dev/sd** -> /mnt/**,
mount fstype=overlayfs options=(rw,upperdir=/tmp/upper/,lowerdir=/) -> /mnt/
umount,
umount /m*,
See the apparmor userspace for full documentation
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
include/linux/lsm_audit.h | 7 +
security/apparmor/Makefile | 3 +-
security/apparmor/apparmorfs.c | 13 +
security/apparmor/audit.c | 4 +
security/apparmor/domain.c | 2 +-
security/apparmor/include/apparmor.h | 3 +-
security/apparmor/include/audit.h | 4 +
security/apparmor/include/domain.h | 2 +
security/apparmor/include/mount.h | 53 +++
security/apparmor/lsm.c | 59 ++++
security/apparmor/mount.c | 600 ++++++++++++++++++++++++++++++++++
11 files changed, 746 insertions(+), 4 deletions(-)
create mode 100644 security/apparmor/include/mount.h
create mode 100644 security/apparmor/mount.c
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 88e78de..7611a42 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -119,6 +119,13 @@ struct common_audit_data {
unsigned long max;
} rlim;
struct {
+ const char *src_name;
+ const char *type;
+ const char *trans;
+ const char *data;
+ unsigned long flags;
+ } mnt;
+ struct {
const char *target;
u32 request;
u32 denied;
diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile
index 86103ce..30cf9b2 100644
--- a/security/apparmor/Makefile
+++ b/security/apparmor/Makefile
@@ -4,11 +4,10 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor.o
apparmor-y := apparmorfs.o audit.o capability.o context.o ipc.o lib.o match.o \
path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \
- resource.o sid.o file.o
+ resource.o sid.o file.o mount.o
clean-files := capability_names.h rlim_names.h
-
# Build a lower case string table of capability names
# Transforms lines from
# #define CAP_DAC_OVERRIDE 1
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 16c15ec..f38a259 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -198,9 +198,22 @@ static struct aa_fs_entry aa_fs_entry_domain[] = {
{ }
};
+static struct aa_fs_entry aa_fs_entry_mount[] = {
+ AA_FS_FILE_STRING("mask", "mount umount"),
+ { }
+};
+
+static struct aa_fs_entry aa_fs_entry_namespaces[] = {
+ AA_FS_FILE_BOOLEAN("profile", 1),
+ AA_FS_FILE_BOOLEAN("pivot_root", 1),
+ { }
+};
+
static struct aa_fs_entry aa_fs_entry_features[] = {
AA_FS_DIR("domain", aa_fs_entry_domain),
AA_FS_DIR("file", aa_fs_entry_file),
+ AA_FS_DIR("mount", aa_fs_entry_mount),
+ AA_FS_DIR("namespaces", aa_fs_entry_namespaces),
AA_FS_FILE_U64("capability", VFS_CAP_FLAGS_MASK),
AA_FS_DIR("rlimit", aa_fs_entry_rlimit),
{ }
diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 61344b5..d63cfb6 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -44,6 +44,10 @@ const char *op_table[] = {
"file_mmap",
"file_mprotect",
+ "pivotroot",
+ "mount",
+ "umount",
+
"create",
"post_create",
"bind",
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 7c69599..6fc18d1 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -242,7 +242,7 @@ static const char *next_name(int xtype, const char *name)
*
* Returns: refcounted profile, or NULL on failure (MAYBE NULL)
*/
-static struct aa_profile *x_table_lookup(struct aa_profile *profile, u32 xindex)
+struct aa_profile *x_table_lookup(struct aa_profile *profile, u32 xindex)
{
struct aa_profile *new_profile = NULL;
struct aa_namespace *ns = profile->ns;
diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
index 40aedd9..e243d96 100644
--- a/security/apparmor/include/apparmor.h
+++ b/security/apparmor/include/apparmor.h
@@ -29,8 +29,9 @@
#define AA_CLASS_NET 4
#define AA_CLASS_RLIMITS 5
#define AA_CLASS_DOMAIN 6
+#define AA_CLASS_MOUNT 7
-#define AA_CLASS_LAST AA_CLASS_DOMAIN
+#define AA_CLASS_LAST AA_CLASS_MOUNT
/* Control parameters settable through module/boot flags */
extern enum audit_mode aa_g_audit;
diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h
index 9317cd8..0e8689d 100644
--- a/security/apparmor/include/audit.h
+++ b/security/apparmor/include/audit.h
@@ -73,6 +73,10 @@ enum aa_ops {
OP_FMMAP,
OP_FMPROT,
+ OP_PIVOTROOT,
+ OP_MOUNT,
+ OP_UMOUNT,
+
OP_CREATE,
OP_POST_CREATE,
OP_BIND,
diff --git a/security/apparmor/include/domain.h b/security/apparmor/include/domain.h
index de04464..a3f70c5 100644
--- a/security/apparmor/include/domain.h
+++ b/security/apparmor/include/domain.h
@@ -23,6 +23,8 @@ struct aa_domain {
char **table;
};
+struct aa_profile *x_table_lookup(struct aa_profile *profile, u32 xindex);
+
int apparmor_bprm_set_creds(struct linux_binprm *bprm);
int apparmor_bprm_secureexec(struct linux_binprm *bprm);
void apparmor_bprm_committing_creds(struct linux_binprm *bprm);
diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h
new file mode 100644
index 0000000..6f936bc
--- /dev/null
+++ b/security/apparmor/include/mount.h
@@ -0,0 +1,53 @@
+/*
+ * AppArmor security module
+ *
+ * This file contains AppArmor file mediation function definitions.
+ *
+ * Copyright 2012 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
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ */
+
+#ifndef __AA_MOUNT_H
+#define __AA_MOUNT_H
+
+#include <linux/fs.h>
+#include <linux/path.h>
+
+#include "domain.h"
+#include "policy.h"
+
+/* mount perms */
+#define AA_MAY_PIVOTROOT 0x01
+#define AA_MAY_MOUNT 0x02
+#define AA_MAY_UMOUNT 0x04
+
+#define AA_MS_IGNORE_MASK (MS_KERNMOUNT | MS_NOSEC | MS_ACTIVE | MS_BORN)
+
+
+int aa_remount(struct aa_profile *profile, struct path *path,
+ unsigned long flags, void *data);
+
+int aa_bind_mount(struct aa_profile *profile, struct path *path,
+ const char *old_name, unsigned long flags);
+
+
+int aa_mount_change_type(struct aa_profile *profile, struct path *path,
+ unsigned long flags);
+
+int aa_move_mount(struct aa_profile *profile, struct path *path,
+ const char *old_name);
+
+int aa_new_mount(struct aa_profile *profile, const char *dev_name,
+ struct path *path, const char *type, unsigned long flags,
+ void *data);
+
+int aa_umount(struct aa_profile *profile, struct vfsmount *mnt, int flags);
+
+int aa_pivotroot(struct aa_profile *profile, struct path *old_path,
+ struct path *new_path);
+
+#endif /* __AA_MOUNT_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 97ce8fa..08e0fa5 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -35,6 +35,7 @@
#include "include/path.h"
#include "include/policy.h"
#include "include/procattr.h"
+#include "include/mount.h"
/* Flag indicating whether initialization completed */
int apparmor_initialized __initdata;
@@ -511,6 +512,60 @@ static int apparmor_file_mprotect(struct vm_area_struct *vma,
!(vma->vm_flags & VM_SHARED) ? MAP_PRIVATE : 0);
}
+static int apparmor_sb_mount(char *dev_name, struct path *path, char *type,
+ unsigned long flags, void *data)
+{
+ struct aa_profile *profile;
+ int error = 0;
+
+ /* Discard magic */
+ if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
+ flags &= ~MS_MGC_MSK;
+
+ flags &= ~AA_MS_IGNORE_MASK;
+
+ profile = __aa_current_profile();
+ if (!unconfined(profile)) {
+ if (flags & MS_REMOUNT)
+ error = aa_remount(profile, path, flags, data);
+ else if (flags & MS_BIND)
+ error = aa_bind_mount(profile, path, dev_name, flags);
+ else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE |
+ MS_UNBINDABLE))
+ error = aa_mount_change_type(profile, path, flags);
+ else if (flags & MS_MOVE)
+ error = aa_move_mount(profile, path, dev_name);
+ else
+ error = aa_new_mount(profile, dev_name, path, type,
+ flags, data);
+ }
+ return error;
+}
+
+static int apparmor_sb_umount(struct vfsmount *mnt, int flags)
+{
+ struct aa_profile *profile;
+ int error = 0;
+
+ profile = __aa_current_profile();
+ if (!unconfined(profile))
+ error = aa_umount(profile, mnt, flags);
+
+ return error;
+}
+
+static int apparmor_sb_pivotroot(struct path *old_path, struct path *new_path)
+{
+ struct aa_profile *profile;
+ int error = 0;
+
+ profile = __aa_current_profile();
+ if (!unconfined(profile))
+ error = aa_pivotroot(profile, old_path, new_path);
+
+ return error;
+}
+
static int apparmor_getprocattr(struct task_struct *task, char *name,
char **value)
{
@@ -628,6 +683,10 @@ static struct security_operations apparmor_ops = {
.capget = apparmor_capget,
.capable = apparmor_capable,
+ .sb_mount = apparmor_sb_mount,
+ .sb_umount = apparmor_sb_umount,
+ .sb_pivotroot = apparmor_sb_pivotroot,
+
.path_link = apparmor_path_link,
.path_unlink = apparmor_path_unlink,
.path_symlink = apparmor_path_symlink,
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
new file mode 100644
index 0000000..1352d60
--- /dev/null
+++ b/security/apparmor/mount.c
@@ -0,0 +1,600 @@
+/*
+ * AppArmor security module
+ *
+ * This file contains AppArmor mediation of files
+ *
+ * Copyright (C) 1998-2008 Novell/SUSE
+ * Copyright 2009-2012 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
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ */
+
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/namei.h>
+
+#include "include/apparmor.h"
+#include "include/audit.h"
+#include "include/context.h"
+#include "include/domain.h"
+#include "include/file.h"
+#include "include/match.h"
+#include "include/mount.h"
+#include "include/path.h"
+#include "include/policy.h"
+
+
+static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
+{
+ if (flags & MS_RDONLY)
+ audit_log_format(ab, "ro");
+ else
+ audit_log_format(ab, "rw");
+ if (flags & MS_NOSUID)
+ audit_log_format(ab, ", nosuid");
+ if (flags & MS_NODEV)
+ audit_log_format(ab, ", nodev");
+ if (flags & MS_NOEXEC)
+ audit_log_format(ab, ", noexec");
+ if (flags & MS_SYNCHRONOUS)
+ audit_log_format(ab, ", sync");
+ if (flags & MS_REMOUNT)
+ audit_log_format(ab, ", remount");
+ if (flags & MS_MANDLOCK)
+ audit_log_format(ab, ", mand");
+ if (flags & MS_DIRSYNC)
+ audit_log_format(ab, ", dirsync");
+ if (flags & MS_NOATIME)
+ audit_log_format(ab, ", noatime");
+ if (flags & MS_NODIRATIME)
+ audit_log_format(ab, ", nodiratime");
+ if (flags & MS_BIND)
+ audit_log_format(ab, flags & MS_REC ? ", rbind" : ", bind");
+ if (flags & MS_MOVE)
+ audit_log_format(ab, ", move");
+ if (flags & MS_SILENT)
+ audit_log_format(ab, ", silent");
+ if (flags & MS_POSIXACL)
+ audit_log_format(ab, ", acl");
+ if (flags & MS_UNBINDABLE)
+ audit_log_format(ab, flags & MS_REC ? ", runbindable" :
+ ", unbindable");
+ if (flags & MS_PRIVATE)
+ audit_log_format(ab, flags & MS_REC ? ", rprivate" :
+ ", private");
+ if (flags & MS_UNBINDABLE)
+ audit_log_format(ab, flags & MS_REC ? ", rslave" :
+ ", slave");
+ if (flags & MS_UNBINDABLE)
+ audit_log_format(ab, flags & MS_REC ? ", rshared" :
+ ", shared");
+ if (flags & MS_RELATIME)
+ audit_log_format(ab, ", relatime");
+ if (flags & MS_I_VERSION)
+ audit_log_format(ab, ", iversion");
+ if (flags & MS_STRICTATIME)
+ audit_log_format(ab, ", strictatime");
+ if (flags & MS_NOUSER)
+ audit_log_format(ab, ", nouser");
+}
+
+/**
+ * mount_audit_cb - call back for mount specific audit fields
+ * @ab: audit_buffer (NOT NULL)
+ * @va: audit struct to audit values of (NOT NULL)
+ */
+static void mount_audit_cb(struct audit_buffer *ab, void *va)
+{
+ struct common_audit_data *sa = va;
+
+ if (sa->aad.mnt.type) {
+ audit_log_format(ab, " fstype=");
+ audit_log_untrustedstring(ab, sa->aad.mnt.type);
+ }
+ if (sa->aad.mnt.src_name) {
+ audit_log_format(ab, " src_name=");
+ audit_log_untrustedstring(ab, sa->aad.mnt.src_name);
+ }
+ if (sa->aad.mnt.trans) {
+ audit_log_format(ab, " trans=");
+ audit_log_untrustedstring(ab, sa->aad.mnt.trans);
+ }
+ if (sa->aad.mnt.flags || sa->aad.op == OP_MOUNT) {
+ audit_log_format(ab, " flags=\"");
+ audit_mnt_flags(ab, sa->aad.mnt.flags);
+ audit_log_format(ab, "\"");
+ }
+ if (sa->aad.mnt.data) {
+ audit_log_format(ab, " options=");
+ audit_log_untrustedstring(ab, sa->aad.mnt.data);
+ }
+}
+
+/**
+ * aa_audit_file - handle the auditing of file operations
+ * @profile: the profile being enforced (NOT NULL)
+ * @gfp: allocation flags
+ * @op: operation being mediated
+ * @name: name of object being mediated (MAYBE NULL)
+ * @src_name: src_name of object being mediated (MAYBE_NULL)
+ * @type: type of filesystem
+ * @trans: name of trans (MAYBE NULL)
+ * @flags: filesystem idependent mount flags
+ * @data: filesystem mount flags
+ * @request: permissions requested
+ * @perms: the permissions computed for the request (NOT NULL)
+ * @info: extra information message (MAYBE NULL)
+ * @error: 0 if operation allowed else failure error code
+ *
+ * Returns: %0 or error on failure
+ */
+int aa_audit_mount(struct aa_profile *profile, gfp_t gfp, int op,
+ const char *name, const char *src_name, const char *type,
+ const char *trans, unsigned long flags, void *data,
+ u32 request, struct file_perms *perms, const char *info,
+ int error)
+{
+ int audit_type = AUDIT_APPARMOR_AUTO;
+ struct common_audit_data sa;
+
+ if (likely(!error)) {
+ u32 mask = perms->audit;
+
+ if (unlikely(AUDIT_MODE(profile) == AUDIT_ALL))
+ mask = 0xffff;
+
+ /* mask off perms that are not being force audited */
+ request &= mask;
+
+ if (likely(!request))
+ return 0;
+ audit_type = AUDIT_APPARMOR_AUDIT;
+ } else {
+ /* only report permissions that were denied */
+ request = request & ~perms->allow;
+
+ if (request & perms->kill)
+ audit_type = AUDIT_APPARMOR_KILL;
+
+ /* quiet known rejects, assumes quiet and kill do not overlap */
+ if ((request & perms->quiet) &&
+ AUDIT_MODE(profile) != AUDIT_NOQUIET &&
+ AUDIT_MODE(profile) != AUDIT_ALL)
+ request &= ~perms->quiet;
+
+ if (!request)
+ return COMPLAIN_MODE(profile) ? 0 : error;
+ }
+
+ COMMON_AUDIT_DATA_INIT(&sa, NONE);
+ sa.aad.op = op,
+ sa.aad.name = name;
+ sa.aad.info = info;
+ sa.aad.error = error;
+ sa.aad.mnt.src_name = src_name;
+ sa.aad.mnt.type = type;
+ sa.aad.mnt.trans = trans;
+ sa.aad.mnt.flags = flags;
+ sa.aad.mnt.data = data;
+
+ return aa_audit(audit_type, profile, gfp, &sa, mount_audit_cb);
+}
+
+
+/**
+ * match_mnt_flags - Do an ordered match on mount flags
+ * @dfa: dfa to match against
+ * @state: state to start in
+ * @flags: mount flags to match against
+ *
+ * Mount flags are encoded as an ordered match. This is done instead of
+ * checking against a simple bitmask, to allow for logical operations
+ * on the flags.
+ *
+ * Returns: next state after flags match
+ */
+static unsigned int match_mnt_flags(struct aa_dfa *dfa, unsigned int state,
+ unsigned long flags)
+{
+ unsigned int i;
+
+ for (i = 0; i <= 31 ; ++i) {
+ if ((1 << i) & flags)
+ state = aa_dfa_next(dfa, state, i + 1);
+ }
+ return state;
+}
+
+/**
+ * compute_mnt_perms - compute mount permission associated with @state
+ * @dfa: dfa to match against (NOT NULL)
+ * @state: state match finished in
+ *
+ * Returns: mount permissions
+ */
+static struct file_perms compute_mnt_perms(struct aa_dfa *dfa,
+ unsigned int state)
+{
+ struct file_perms perms;
+
+ perms.kill = 0;
+ perms.allow = dfa_user_allow(dfa, state);
+ perms.audit = dfa_user_audit(dfa, state);
+ perms.quiet = dfa_user_quiet(dfa, state);
+ perms.xindex = dfa_user_xindex(dfa, state);
+
+ return perms;
+}
+
+static int path_flags(struct aa_profile *profile, struct path *path)
+{
+ return profile->path_flags |
+ S_ISDIR(path->dentry->d_inode->i_mode) ? PATH_IS_DIR : 0;
+}
+
+int aa_remount(struct aa_profile *profile, struct path *path,
+ unsigned long flags, void *data)
+{
+ struct file_perms perms;
+ const char *name, *info = NULL;
+ char *buffer = NULL;
+ int binarydata, error;
+
+ binarydata = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
+
+ error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip device */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip type */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = match_mnt_flags(profile->policy.dfa, state, flags);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ if (data && !binarydata)
+ state = aa_dfa_match(profile->policy.dfa, state,
+ data);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_MOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_MOUNT, name,
+ NULL, NULL, NULL, flags, data, AA_MAY_MOUNT,
+ &perms, info, error);
+ kfree(buffer);
+
+ return error;
+}
+
+int aa_bind_mount(struct aa_profile *profile, struct path *path,
+ const char *dev_name, unsigned long flags)
+{
+ struct file_perms perms = { };
+ char *buffer = NULL, *old_buffer = NULL;
+ const char *name, *old_name, *info = NULL;
+ struct path old_path;
+ int error;
+
+ if (!dev_name || !*dev_name)
+ return -EINVAL;
+
+ flags &= MS_REC | MS_BIND;
+
+ error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
+ if (error)
+ goto audit;
+
+ error = aa_path_name(&old_path, path_flags(profile, &old_path),
+ &old_buffer, &old_name, &info);
+ path_put(&old_path);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = aa_dfa_match(profile->policy.dfa, state, old_name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip type */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = match_mnt_flags(profile->policy.dfa, state, flags);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_MOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_MOUNT, name,
+ old_name, NULL, NULL, flags, NULL,
+ AA_MAY_MOUNT, &perms, info, error);
+
+ kfree(buffer);
+ kfree(old_buffer);
+
+ return error;
+}
+
+int aa_mount_change_type(struct aa_profile *profile, struct path *path,
+ unsigned long flags)
+{
+ struct file_perms perms = { };
+ char *buffer = NULL;
+ const char *name, *info = NULL;
+ int error;
+
+ flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
+ MS_UNBINDABLE);
+
+ error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip device */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip type */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = match_mnt_flags(profile->policy.dfa, state, flags);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_MOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_MOUNT, name,
+ NULL, NULL, NULL, flags, NULL,
+ AA_MAY_MOUNT, &perms, info, error);
+ kfree(buffer);
+
+ return error;
+}
+
+int aa_move_mount(struct aa_profile *profile, struct path *path,
+ const char *orig_name)
+{
+ struct file_perms perms = { };
+ char *buffer = NULL, *old_buffer = NULL;
+ const char *name, *old_name, *info = NULL;
+ struct path old_path;
+ int error;
+
+ if (!orig_name || !*orig_name)
+ return -EINVAL;
+
+ error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
+ if (error)
+ goto audit;
+
+ error = aa_path_name(&old_path, path_flags(profile, &old_path),
+ &old_buffer, &old_name, &info);
+ path_put(&old_path);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = aa_dfa_match(profile->policy.dfa, state, old_name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ /* skip type */
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = match_mnt_flags(profile->policy.dfa, state, MS_MOVE);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_MOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_MOUNT, name,
+ old_name, NULL, NULL, MS_MOVE, NULL,
+ AA_MAY_MOUNT, &perms, info, error);
+
+ kfree(buffer);
+ kfree(old_buffer);
+
+ return error;
+}
+
+int aa_new_mount(struct aa_profile *profile, const char *orig_dev_name,
+ struct path *path, const char *type, unsigned long flags,
+ void *data)
+{
+ struct file_system_type *fstype = NULL;
+ struct file_perms perms = { };
+ char *buffer = NULL, *dev_buffer = NULL;
+ const char *name, *dev_name, *info = NULL;
+ struct path dev_path;
+ int binary_data, error;
+
+ fstype = get_fs_type(type);
+ if (!fstype) {
+ error = -ENODEV;
+ goto out;
+ }
+ binary_data = fstype->fs_flags & FS_BINARY_MOUNTDATA;
+
+ if (fstype->fs_flags & FS_REQUIRES_DEV) {
+ if (!dev_name) {
+ error = -ENOENT;
+ goto out;
+ }
+
+ error = kern_path(orig_dev_name, LOOKUP_FOLLOW, &dev_path);
+ if (error)
+ goto audit;
+
+ error = aa_path_name(&dev_path, path_flags(profile, &dev_path),
+ &dev_buffer, &dev_name, &info);
+ path_put(&dev_path);
+ if (error)
+ goto audit;
+ } else
+ dev_name = orig_dev_name;
+
+ error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ if (dev_name)
+ state = aa_dfa_match(profile->policy.dfa, state,
+ dev_name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = aa_dfa_match(profile->policy.dfa, state, type);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = match_mnt_flags(profile->policy.dfa, state, flags);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ if (data && !binary_data)
+ state = aa_dfa_match(profile->policy.dfa, state,
+ data);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_MOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_MOUNT, name, dev_name,
+ type, NULL, flags, data, AA_MAY_MOUNT,
+ &perms, info, error);
+ kfree(buffer);
+ kfree(dev_buffer);
+
+out:
+ if (fstype)
+ put_filesystem(fstype);
+
+ return error;
+
+}
+
+int aa_umount(struct aa_profile *profile, struct vfsmount *mnt, int flags)
+{
+ struct file_perms perms = { };
+ char *buffer = NULL;
+ const char *name, *info = NULL;
+ int error;
+
+ struct path path = { mnt, mnt->mnt_root };
+ error = aa_path_name(&path, path_flags(profile, &path), &buffer, &name,
+ &info);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ name);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_UMOUNT & ~perms.allow)
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_UMOUNT, name, NULL,
+ NULL, NULL, 0, NULL, AA_MAY_UMOUNT,
+ &perms, info, error);
+ kfree(buffer);
+
+ return error;
+}
+
+int aa_pivotroot(struct aa_profile *profile, struct path *old_path,
+ struct path *new_path)
+{
+ struct file_perms perms = { };
+ struct aa_profile *target = NULL;
+ char *old_buffer = NULL, *new_buffer = NULL;
+ const char *old_name, *new_name, *info = NULL;
+ int error;
+
+ error = aa_path_name(old_path, path_flags(profile, old_path),
+ &old_buffer, &old_name, &info);
+ if (error)
+ goto audit;
+
+ error = aa_path_name(new_path, path_flags(profile, new_path),
+ &new_buffer, &new_name, &info);
+ if (error)
+ goto audit;
+
+ if (profile->policy.dfa) {
+ unsigned int state;
+ state = aa_dfa_match(profile->policy.dfa,
+ profile->policy.start[AA_CLASS_MOUNT],
+ old_name);
+ state = aa_dfa_null_transition(profile->policy.dfa, state);
+ state = aa_dfa_match(profile->policy.dfa, state, new_name);
+ perms = compute_mnt_perms(profile->policy.dfa, state);
+ }
+
+ if (AA_MAY_PIVOTROOT & perms.allow) {
+ if ((perms.xindex & AA_X_TYPE_MASK) == AA_X_TABLE) {
+ target = x_table_lookup(profile, perms.xindex);
+ if (!target)
+ error = -ENOENT;
+ else
+ error = aa_replace_current_profile(target);
+ }
+ } else
+ error = -EACCES;
+
+audit:
+ error = aa_audit_mount(profile, GFP_KERNEL, OP_PIVOTROOT, new_name,
+ old_name, NULL,
+ target ? target->base.name : NULL, 0, NULL,
+ AA_MAY_PIVOTROOT, &perms, info, error);
+
+ aa_put_profile(target);
+ kfree(old_buffer);
+ kfree(new_buffer);
+
+ return error;
+}
--
1.7.9
Profiles that want name lookup past the chroot to the namespace root
must be marked as such, all other profiles should be chroot relative.
Currently the autogenerated null (learning), and unconfined profiles are
not marked as such. Make sure they are properly flagged. This should not
affect behavior except for auto-generated profiles when a chroot is entered.
Profiles loaded from userspace will not be affected as they provide their
own value for the flag.
This change does not affect mediation as it only changes the path reported by
the unconfined (none mediating), an null learning profiles.
Also ensure that if a profile is ever loaded with out path flags set, that
it defaults to being chroot relative.
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/policy.c | 3 +++
security/apparmor/policy_unpack.c | 3 ++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 4f0eade..537e5dc 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -655,6 +655,9 @@ struct aa_profile *aa_alloc_profile(const char *hname)
return NULL;
}
+ /* default to chroot relative paths */
+ profile->path_flags = PATH_CHROOT_REL;
+
/* refcount released by caller */
return profile;
}
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 5c46acf..6137b10 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -25,6 +25,7 @@
#include "include/audit.h"
#include "include/context.h"
#include "include/match.h"
+#include "include/path.h"
#include "include/policy.h"
#include "include/policy_unpack.h"
#include "include/sid.h"
@@ -523,7 +524,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e)
profile->path_flags |= profile->flags & PFLAG_MEDIATE_DELETED;
else
/* set a default value if path_flags field is not present */
- profile->path_flags = PFLAG_MEDIATE_DELETED;
+ profile->path_flags = PFLAG_MEDIATE_DELETED | PATH_CHROOT_REL;
if (!unpack_u32(e, &(profile->caps.allow.cap[0]), NULL))
goto fail;
--
1.7.9
Move the path name lookup failure messages into the main path name lookup
routine, as the information is useful in more than just aa_path_perm.
Also rename aa_get_name to aa_path_name as it is not getting a reference
counted object with a corresponding put fn.
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/domain.c | 5 ++---
security/apparmor/file.c | 18 +++++++-----------
security/apparmor/include/path.h | 3 ++-
security/apparmor/path.c | 22 ++++++++++++++++++----
4 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c1e18ba..7c69599 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -372,13 +372,12 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
state = profile->file.start;
/* buffer freed below, name is pointer into buffer */
- error = aa_get_name(&bprm->file->f_path, profile->path_flags, &buffer,
- &name);
+ error = aa_path_name(&bprm->file->f_path, profile->path_flags, &buffer,
+ &name, &info);
if (error) {
if (profile->flags &
(PFLAG_IX_ON_NAME_ERROR | PFLAG_UNCONFINED))
error = 0;
- info = "Exec failed name resolution";
name = bprm->filename;
goto audit;
}
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index bba875c..3022c0f 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -278,22 +278,16 @@ int aa_path_perm(int op, struct aa_profile *profile, struct path *path,
int error;
flags |= profile->path_flags | (S_ISDIR(cond->mode) ? PATH_IS_DIR : 0);
- error = aa_get_name(path, flags, &buffer, &name);
+ error = aa_path_name(path, flags, &buffer, &name, &info);
if (error) {
if (error == -ENOENT && is_deleted(path->dentry)) {
/* Access to open files that are deleted are
* give a pass (implicit delegation)
*/
error = 0;
+ info = NULL;
perms.allow = request;
- } else if (error == -ENOENT)
- info = "Failed name lookup - deleted entry";
- else if (error == -ESTALE)
- info = "Failed name lookup - disconnected path";
- else if (error == -ENAMETOOLONG)
- info = "Failed name lookup - name too long";
- else
- info = "Failed name lookup";
+ }
} else {
aa_str_perms(profile->file.dfa, profile->file.start, name, cond,
&perms);
@@ -364,12 +358,14 @@ int aa_path_link(struct aa_profile *profile, struct dentry *old_dentry,
lperms = nullperms;
/* buffer freed below, lname is pointer in buffer */
- error = aa_get_name(&link, profile->path_flags, &buffer, &lname);
+ error = aa_path_name(&link, profile->path_flags, &buffer, &lname,
+ &info);
if (error)
goto audit;
/* buffer2 freed below, tname is pointer in buffer2 */
- error = aa_get_name(&target, profile->path_flags, &buffer2, &tname);
+ error = aa_path_name(&target, profile->path_flags, &buffer2, &tname,
+ &info);
if (error)
goto audit;
diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h
index 27b327a..286ac75 100644
--- a/security/apparmor/include/path.h
+++ b/security/apparmor/include/path.h
@@ -26,6 +26,7 @@ enum path_flags {
PATH_MEDIATE_DELETED = 0x10000, /* mediate deleted paths */
};
-int aa_get_name(struct path *path, int flags, char **buffer, const char **name);
+int aa_path_name(struct path *path, int flags, char **buffer,
+ const char **name, const char **info);
#endif /* __AA_PATH_H */
diff --git a/security/apparmor/path.c b/security/apparmor/path.c
index 95a589e..41e19a1 100644
--- a/security/apparmor/path.c
+++ b/security/apparmor/path.c
@@ -157,7 +157,7 @@ out:
* Returns: %0 else error on failure
*/
static int get_name_to_buffer(struct path *path, int flags, char *buffer,
- int size, char **name)
+ int size, char **name, const char **info)
{
int adjust = (flags & PATH_IS_DIR) ? 1 : 0;
int error = d_namespace_path(path, buffer, size - adjust, name, flags);
@@ -169,15 +169,27 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer,
*/
strcpy(&buffer[size - 2], "/");
+ if (info && error) {
+ if (error == -ENOENT)
+ *info = "Failed name lookup - deleted entry";
+ else if (error == -ESTALE)
+ *info = "Failed name lookup - disconnected path";
+ else if (error == -ENAMETOOLONG)
+ *info = "Failed name lookup - name too long";
+ else
+ *info = "Failed name lookup";
+ }
+
return error;
}
/**
- * aa_get_name - compute the pathname of a file
+ * aa_path_name - compute the pathname of a file
* @path: path the file (NOT NULL)
* @flags: flags controlling path name generation
* @buffer: buffer that aa_get_name() allocated (NOT NULL)
* @name: Returns - the generated path name if !error (NOT NULL)
+ * @info: Returns - information on why the path lookup failed (MAYBE NULL)
*
* @name is a pointer to the beginning of the pathname (which usually differs
* from the beginning of the buffer), or NULL. If there is an error @name
@@ -190,7 +202,8 @@ static int get_name_to_buffer(struct path *path, int flags, char *buffer,
*
* Returns: %0 else error code if could retrieve name
*/
-int aa_get_name(struct path *path, int flags, char **buffer, const char **name)
+int aa_path_name(struct path *path, int flags, char **buffer, const char **name,
+ const char **info)
{
char *buf, *str = NULL;
int size = 256;
@@ -204,7 +217,7 @@ int aa_get_name(struct path *path, int flags, char **buffer, const char **name)
if (!buf)
return -ENOMEM;
- error = get_name_to_buffer(path, flags, buf, size, &str);
+ error = get_name_to_buffer(path, flags, buf, size, &str, info);
if (error != -ENAMETOOLONG)
break;
@@ -212,6 +225,7 @@ int aa_get_name(struct path *path, int flags, char **buffer, const char **name)
size <<= 1;
if (size > aa_g_path_max)
return -ENAMETOOLONG;
+ *info = NULL;
}
*buffer = buf;
*name = str;
--
1.7.9
Update aa_dfa_match so that it doesn't result in an input string being
walked twice (once to get its length and another time to match)
Add a single step functions
aa_dfa_next
Signed-off-by: John Johansen <[email protected]>
Acked-by: Kees Cook <[email protected]>
---
security/apparmor/include/apparmor.h | 2 +-
security/apparmor/include/match.h | 3 +
security/apparmor/match.c | 80 ++++++++++++++++++++++++++++++++-
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h
index df36495..248c408 100644
--- a/security/apparmor/include/apparmor.h
+++ b/security/apparmor/include/apparmor.h
@@ -81,7 +81,7 @@ static inline unsigned int aa_dfa_null_transition(struct aa_dfa *dfa,
unsigned int start)
{
/* the null transition only needs the string's null terminator byte */
- return aa_dfa_match_len(dfa, start, "", 1);
+ return aa_dfa_next(dfa, start, 0);
}
static inline bool mediated_filesystem(struct inode *inode)
diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index a4a8639..775843e 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -116,6 +116,9 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start,
const char *str, int len);
unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start,
const char *str);
+unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state,
+ const char c);
+
void aa_dfa_free_kref(struct kref *kref);
/**
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 94de6b4..90971a8 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -335,12 +335,12 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start,
}
/**
- * aa_dfa_next_state - traverse @dfa to find state @str stops at
+ * aa_dfa_match - traverse @dfa to find state @str stops at
* @dfa: the dfa to match @str against (NOT NULL)
* @start: the state of the dfa to start matching in
* @str: the null terminated string of bytes to match against the dfa (NOT NULL)
*
- * aa_dfa_next_state will match @str against the dfa and return the state it
+ * aa_dfa_match will match @str against the dfa and return the state it
* finished matching in. The final state can be used to look up the accepting
* label, or as the start state of a continuing match.
*
@@ -349,5 +349,79 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start,
unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start,
const char *str)
{
- return aa_dfa_match_len(dfa, start, str, strlen(str));
+ u16 *def = DEFAULT_TABLE(dfa);
+ u32 *base = BASE_TABLE(dfa);
+ u16 *next = NEXT_TABLE(dfa);
+ u16 *check = CHECK_TABLE(dfa);
+ unsigned int state = start, pos;
+
+ if (state == 0)
+ return 0;
+
+ /* current state is <state>, matching character *str */
+ if (dfa->tables[YYTD_ID_EC]) {
+ /* Equivalence class table defined */
+ u8 *equiv = EQUIV_TABLE(dfa);
+ /* default is direct to next state */
+ while (*str) {
+ pos = base[state] + equiv[(u8) *str++];
+ if (check[pos] == state)
+ state = next[pos];
+ else
+ state = def[state];
+ }
+ } else {
+ /* default is direct to next state */
+ while (*str) {
+ pos = base[state] + (u8) *str++;
+ if (check[pos] == state)
+ state = next[pos];
+ else
+ state = def[state];
+ }
+ }
+
+ return state;
+}
+
+/**
+ * aa_dfa_next - step one character to the next state in the dfa
+ * @dfa: the dfa to tranverse (NOT NULL)
+ * @state: the state to start in
+ * @c: the input character to transition on
+ *
+ * aa_dfa_match will step through the dfa by one input character @c
+ *
+ * Returns: state reach after input @c
+ */
+unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state,
+ const char c)
+{
+ u16 *def = DEFAULT_TABLE(dfa);
+ u32 *base = BASE_TABLE(dfa);
+ u16 *next = NEXT_TABLE(dfa);
+ u16 *check = CHECK_TABLE(dfa);
+ unsigned int pos;
+
+ /* current state is <state>, matching character *str */
+ if (dfa->tables[YYTD_ID_EC]) {
+ /* Equivalence class table defined */
+ u8 *equiv = EQUIV_TABLE(dfa);
+ /* default is direct to next state */
+
+ pos = base[state] + equiv[(u8) c];
+ if (check[pos] == state)
+ state = next[pos];
+ else
+ state = def[state];
+ } else {
+ /* default is direct to next state */
+ pos = base[state] + (u8) c;
+ if (check[pos] == state)
+ state = next[pos];
+ else
+ state = def[state];
+ }
+
+ return state;
}
--
1.7.9
Only checking "[PATCH 7/7] AppArmor: Add the ability to mediate mount".
John Johansen wrote:
> --- a/security/apparmor/include/apparmor.h
> +++ b/security/apparmor/include/apparmor.h
> @@ -29,8 +29,9 @@
> #define AA_CLASS_NET 4
> #define AA_CLASS_RLIMITS 5
> #define AA_CLASS_DOMAIN 6
> +#define AA_CLASS_MOUNT 7
>
> -#define AA_CLASS_LAST AA_CLASS_DOMAIN
> +#define AA_CLASS_LAST AA_CLASS_MOUNT
Maybe use of enum is easier to maintain?
> --- /dev/null
> +++ b/security/apparmor/mount.c
> +static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
> +{
(...snipped...)
> + if (flags & MS_UNBINDABLE)
> + audit_log_format(ab, flags & MS_REC ? ", runbindable" :
> + ", unbindable");
> + if (flags & MS_PRIVATE)
> + audit_log_format(ab, flags & MS_REC ? ", rprivate" :
> + ", private");
> + if (flags & MS_UNBINDABLE)
> + audit_log_format(ab, flags & MS_REC ? ", rslave" :
> + ", slave");
MS_UNBINDABLE -> MS_SLAVE
> + if (flags & MS_UNBINDABLE)
> + audit_log_format(ab, flags & MS_REC ? ", rshared" :
> + ", shared");
MS_UNBINDABLE -> MS_SHARED
(...snipped...)
> +}
> +/**
> + * mount_audit_cb - call back for mount specific audit fields
> + * @ab: audit_buffer (NOT NULL)
> + * @va: audit struct to audit values of (NOT NULL)
> + */
> +static void mount_audit_cb(struct audit_buffer *ab, void *va)
> +{
> + struct common_audit_data *sa = va;
(...snipped...)
> + if (sa->aad.mnt.data) {
> + audit_log_format(ab, " options=");
> + audit_log_untrustedstring(ab, sa->aad.mnt.data);
> + }
data might be binary. Also, there is no guarantee that
memchr(data, '\0', PAGE_SIZE) != NULL even if data is not binary.
I think auditing this argument should be avoided.
> +}
> +/**
> + * aa_audit_file - handle the auditing of file operations
> + * @profile: the profile being enforced (NOT NULL)
> + * @gfp: allocation flags
> + * @op: operation being mediated
> + * @name: name of object being mediated (MAYBE NULL)
> + * @src_name: src_name of object being mediated (MAYBE_NULL)
> + * @type: type of filesystem
aa_remount() passes @type == NULL.
Maybe use of gcc's __attribute__((nonnull(...))) helps catching this kind of
error.
> + * @trans: name of trans (MAYBE NULL)
> + * @flags: filesystem idependent mount flags
> + * @data: filesystem mount flags
> + * @request: permissions requested
> + * @perms: the permissions computed for the request (NOT NULL)
> + * @info: extra information message (MAYBE NULL)
> + * @error: 0 if operation allowed else failure error code
> + *
> + * Returns: %0 or error on failure
> + */
> +int aa_remount(struct aa_profile *profile, struct path *path,
> + unsigned long flags, void *data)
> +{
> + struct file_perms perms;
Please explicitly initialize perms, or
> + const char *name, *info = NULL;
> + char *buffer = NULL;
> + int binarydata, error;
> +
> + binarydata = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
> +
> + error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
> + &info);
> + if (error)
> + goto audit;
> +
this error path will call aa_audit_mount() with perms uninitialized.
> +int aa_mount_change_type(struct aa_profile *profile, struct path *path,
> + unsigned long flags)
> +{
> + struct file_perms perms = { };
> + char *buffer = NULL;
> + const char *name, *info = NULL;
> + int error;
> +
> + flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
> + MS_UNBINDABLE);
Do you really want to drop these flags from audit_mnt_flags()?
> +int aa_move_mount(struct aa_profile *profile, struct path *path,
> + const char *orig_name)
> +{
> + struct file_perms perms = { };
> + char *buffer = NULL, *old_buffer = NULL;
> + const char *name, *old_name, *info = NULL;
Please explicitly initialize old_name with NULL, or
> + struct path old_path;
> + int error;
> +
> + if (!orig_name || !*orig_name)
> + return -EINVAL;
> +
> + error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
> + &info);
> + if (error)
> + goto audit;
this error path will call aa_audit_mount() with old_name uninitialized.
> +int aa_new_mount(struct aa_profile *profile, const char *orig_dev_name,
> + struct path *path, const char *type, unsigned long flags,
> + void *data)
> +{
> + struct file_system_type *fstype = NULL;
> + struct file_perms perms = { };
> + char *buffer = NULL, *dev_buffer = NULL;
> + const char *name, *dev_name, *info = NULL;
Please explicitly initialize dev_name with NULL, or
> + struct path dev_path;
> + int binary_data, error;
> +
> + fstype = get_fs_type(type);
get_fs_type() does not accept type == NULL but type != NULL is not checked.
> + if (!fstype) {
> + error = -ENODEV;
> + goto out;
> + }
> + binary_data = fstype->fs_flags & FS_BINARY_MOUNTDATA;
> +
> + if (fstype->fs_flags & FS_REQUIRES_DEV) {
> + if (!dev_name) {
dev_name -> orig_dev_name
kern_path() does not accept orig_dev_name == NULL.
> + error = -ENOENT;
> + goto out;
> + }
> +
> + error = kern_path(orig_dev_name, LOOKUP_FOLLOW, &dev_path);
> + if (error)
> + goto audit;
> +
this error path will call aa_audit_mount() with dev_name uninitialized.
> + error = aa_path_name(&dev_path, path_flags(profile, &dev_path),
> + &dev_buffer, &dev_name, &info);
> + path_put(&dev_path);
> + if (error)
> + goto audit;
> + } else
> + dev_name = orig_dev_name;
> +
(...snipped...)
> +}
> +int aa_pivotroot(struct aa_profile *profile, struct path *old_path,
> + struct path *new_path)
> +{
> + struct file_perms perms = { };
> + struct aa_profile *target = NULL;
> + char *old_buffer = NULL, *new_buffer = NULL;
> + const char *old_name, *new_name, *info = NULL;
Please explicitly initialize new_name with NULL, or
> + int error;
> +
> + error = aa_path_name(old_path, path_flags(profile, old_path),
> + &old_buffer, &old_name, &info);
> + if (error)
> + goto audit;
this error path will call aa_audit_mount() with new_name uninitialized.
(...snipped...)
> +}
On 02/27/2012 06:29 PM, Tetsuo Handa wrote:
> Only checking "[PATCH 7/7] AppArmor: Add the ability to mediate mount".
>
>
>
> John Johansen wrote:
>> --- a/security/apparmor/include/apparmor.h
>> +++ b/security/apparmor/include/apparmor.h
>> @@ -29,8 +29,9 @@
>> #define AA_CLASS_NET 4
>> #define AA_CLASS_RLIMITS 5
>> #define AA_CLASS_DOMAIN 6
>> +#define AA_CLASS_MOUNT 7
>>
>> -#define AA_CLASS_LAST AA_CLASS_DOMAIN
>> +#define AA_CLASS_LAST AA_CLASS_MOUNT
>
> Maybe use of enum is easier to maintain?
>
meh, I'm indifferent either works I don't really see enums as easier, but certainly
no worse
>
>
>> --- /dev/null
>> +++ b/security/apparmor/mount.c
>> +static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
>> +{
> (...snipped...)
>> + if (flags & MS_UNBINDABLE)
>> + audit_log_format(ab, flags & MS_REC ? ", runbindable" :
>> + ", unbindable");
>> + if (flags & MS_PRIVATE)
>> + audit_log_format(ab, flags & MS_REC ? ", rprivate" :
>> + ", private");
>> + if (flags & MS_UNBINDABLE)
>> + audit_log_format(ab, flags & MS_REC ? ", rslave" :
>> + ", slave");
>
> MS_UNBINDABLE -> MS_SLAVE
>
>> + if (flags & MS_UNBINDABLE)
>> + audit_log_format(ab, flags & MS_REC ? ", rshared" :
>> + ", shared");
>
> MS_UNBINDABLE -> MS_SHARED
>
> (...snipped...)
gah, color me embarrassed and a little puzzled I know I fixed that error once
already. Not sure if it did get saved, was not git added or maybe something
else ...
>> +}
>
>
>
>> +/**
>> + * mount_audit_cb - call back for mount specific audit fields
>> + * @ab: audit_buffer (NOT NULL)
>> + * @va: audit struct to audit values of (NOT NULL)
>> + */
>> +static void mount_audit_cb(struct audit_buffer *ab, void *va)
>> +{
>> + struct common_audit_data *sa = va;
> (...snipped...)
>> + if (sa->aad.mnt.data) {
>> + audit_log_format(ab, " options=");
>> + audit_log_untrustedstring(ab, sa->aad.mnt.data);
>> + }
>
> data might be binary. Also, there is no guarantee that
> memchr(data, '\0', PAGE_SIZE) != NULL even if data is not binary.
> I think auditing this argument should be avoided.
>
Hrmm we should only be setting it/checking it if its not binary data.
But of course while I got the checking bit in there, I missed the
conditional on setting.
You are right about the no null termination not being guarenteed
though we should be using audit_log_n_untrustedstring, and capping
at the first of null byte or data page.
Its size can be an issue and that is why its deliberately the last element
that gets added. I think that we do want to audit it though because it
can be quite useful.
>> +}
>
>
>
>> +/**
>> + * aa_audit_file - handle the auditing of file operations
>> + * @profile: the profile being enforced (NOT NULL)
>> + * @gfp: allocation flags
>> + * @op: operation being mediated
>> + * @name: name of object being mediated (MAYBE NULL)
>> + * @src_name: src_name of object being mediated (MAYBE_NULL)
>> + * @type: type of filesystem
>
> aa_remount() passes @type == NULL.
right, I did grab the type in remount from the fs, though perhaps I should have,
NULL should be valid here as it just assigns it to mnt.type which is conditionally
logged in mount_audit_cb, so the comment needs to updated
Really the whole audit setup for these routines needs to be reworked, to clean
them up. I have been toying with a couple of ways to make this cleaner and
avoid calling function with a huge list of parameters like, is being done here.
> Maybe use of gcc's __attribute__((nonnull(...))) helps catching this kind of
> error.
>
yeah, actually that isn't a bad idea
>> + * @trans: name of trans (MAYBE NULL)
>> + * @flags: filesystem idependent mount flags
>> + * @data: filesystem mount flags
>> + * @request: permissions requested
>> + * @perms: the permissions computed for the request (NOT NULL)
>> + * @info: extra information message (MAYBE NULL)
>> + * @error: 0 if operation allowed else failure error code
>> + *
>> + * Returns: %0 or error on failure
>> + */
>
>
>
>> +int aa_remount(struct aa_profile *profile, struct path *path,
>> + unsigned long flags, void *data)
>> +{
>> + struct file_perms perms;
>
> Please explicitly initialize perms, or
>
yikes! yes. I missed this one when reworking how the permissions where handled
>> + const char *name, *info = NULL;
>> + char *buffer = NULL;
>> + int binarydata, error;
>> +
>> + binarydata = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
>> +
>> + error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
>> + &info);
>> + if (error)
>> + goto audit;
>> +
>
> this error path will call aa_audit_mount() with perms uninitialized.
>
>
>
>> +int aa_mount_change_type(struct aa_profile *profile, struct path *path,
>> + unsigned long flags)
>> +{
>> + struct file_perms perms = { };
>> + char *buffer = NULL;
>> + const char *name, *info = NULL;
>> + int error;
>> +
>> + flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
>> + MS_UNBINDABLE);
>
> Do you really want to drop these flags from audit_mnt_flags()?
>
hrmm, it was intentional, though something I was on the fence about. Basically
this is currently the set of valid flags for do_change_type() and the question was
whether to have apparmor rejecting on invalid flags or do_change_type()
Of course this also means that apparmor needs to be kept in sync with what is
valid flags in do_change_type(). So perhaps it is better to just have apparmor issue
rejects here even if do_change_type would fail it later.
>
>
>> +int aa_move_mount(struct aa_profile *profile, struct path *path,
>> + const char *orig_name)
>> +{
>> + struct file_perms perms = { };
>> + char *buffer = NULL, *old_buffer = NULL;
>> + const char *name, *old_name, *info = NULL;
>
> Please explicitly initialize old_name with NULL, or
>
yep, thanks
>> + struct path old_path;
>> + int error;
>> +
>> + if (!orig_name || !*orig_name)
>> + return -EINVAL;
>> +
>> + error = aa_path_name(path, path_flags(profile, path), &buffer, &name,
>> + &info);
>> + if (error)
>> + goto audit;
>
> this error path will call aa_audit_mount() with old_name uninitialized.
>
>
>
>> +int aa_new_mount(struct aa_profile *profile, const char *orig_dev_name,
>> + struct path *path, const char *type, unsigned long flags,
>> + void *data)
>> +{
>> + struct file_system_type *fstype = NULL;
>> + struct file_perms perms = { };
>> + char *buffer = NULL, *dev_buffer = NULL;
>> + const char *name, *dev_name, *info = NULL;
>
> Please explicitly initialize dev_name with NULL, or
>
sigh, yep.
>> + struct path dev_path;
>> + int binary_data, error;
>> +
>> + fstype = get_fs_type(type);
>
> get_fs_type() does not accept type == NULL but type != NULL is not checked.
>
indeed, thanks
>> + if (!fstype) {
>> + error = -ENODEV;
>> + goto out;
>> + }
>> + binary_data = fstype->fs_flags & FS_BINARY_MOUNTDATA;
>> +
>> + if (fstype->fs_flags & FS_REQUIRES_DEV) {
>> + if (!dev_name) {
>
> dev_name -> orig_dev_name
>
> kern_path() does not accept orig_dev_name == NULL.
>
right
>> + error = -ENOENT;
>> + goto out;
>> + }
>> +
>> + error = kern_path(orig_dev_name, LOOKUP_FOLLOW, &dev_path);
>> + if (error)
>> + goto audit;
>> +
>
> this error path will call aa_audit_mount() with dev_name uninitialized.
>
yep
>> + error = aa_path_name(&dev_path, path_flags(profile, &dev_path),
>> + &dev_buffer, &dev_name, &info);
>> + path_put(&dev_path);
>> + if (error)
>> + goto audit;
>> + } else
>> + dev_name = orig_dev_name;
>> +
> (...snipped...)
>> +}
>
>
>
>> +int aa_pivotroot(struct aa_profile *profile, struct path *old_path,
>> + struct path *new_path)
>> +{
>> + struct file_perms perms = { };
>> + struct aa_profile *target = NULL;
>> + char *old_buffer = NULL, *new_buffer = NULL;
>> + const char *old_name, *new_name, *info = NULL;
>
> Please explicitly initialize new_name with NULL, or
>
yep
>> + int error;
>> +
>> + error = aa_path_name(old_path, path_flags(profile, old_path),
>> + &old_buffer, &old_name, &info);
>> + if (error)
>> + goto audit;
>
> this error path will call aa_audit_mount() with new_name uninitialized.
>
> (...snipped...)
>> +}
thanks for the review Tetsuo