From 040d9e2bce0a5b321c402b79ee43a8e8d2fd3b06 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 23 Jan 2018 01:47:42 -0800 Subject: apparmor: fix display of .ns_name for containers The .ns_name should not be virtualized by the current ns view. It needs to report the ns base name as that is being used during startup as part of determining apparmor policy namespace support. BugLink: http://bugs.launchpad.net/bugs/1746463 Fixes: d9f02d9c237aa ("apparmor: fix display of ns name") Cc: Stable Reported-by: Serge Hallyn Tested-by: Serge Hallyn Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d4fa04d91439..a23b0ca19fd0 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -1189,9 +1189,7 @@ static int seq_ns_level_show(struct seq_file *seq, void *v) static int seq_ns_name_show(struct seq_file *seq, void *v) { struct aa_label *label = begin_current_label_crit_section(); - - seq_printf(seq, "%s\n", aa_ns_name(labels_ns(label), - labels_ns(label), true)); + seq_printf(seq, "%s\n", labels_ns(label)->base.name); end_current_label_crit_section(label); return 0; -- cgit v1.2.3-58-ga151 From 1d6583d9c6723d78e446dd203ffd974f6b85ab76 Mon Sep 17 00:00:00 2001 From: Pravin Shedge Date: Wed, 6 Dec 2017 23:05:59 +0530 Subject: security: apparmor: remove duplicate includes These duplicate includes have been found with scripts/checkincludes.pl but they have been removed manually to avoid removing false positives. Signed-off-by: Pravin Shedge Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index a23b0ca19fd0..00fc4f9f7f14 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -33,7 +33,6 @@ #include "include/context.h" #include "include/crypto.h" #include "include/ipc.h" -#include "include/policy_ns.h" #include "include/label.h" #include "include/policy.h" #include "include/policy_ns.h" -- cgit v1.2.3-58-ga151 From d8889d49e414b371eb235c08c3a759ab3e0cfa51 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 11 Oct 2017 01:04:48 -0700 Subject: apparmor: move context.h to cred.h Now that file contexts have been moved into file, and task context fns() and data have been split from the context, only the cred context remains in context.h so rename to cred.h to better reflect what it deals with. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 2 +- security/apparmor/capability.c | 2 +- security/apparmor/domain.c | 2 +- security/apparmor/file.c | 2 +- security/apparmor/include/context.h | 176 ------------------------------------ security/apparmor/include/cred.h | 176 ++++++++++++++++++++++++++++++++++++ security/apparmor/ipc.c | 2 +- security/apparmor/label.c | 2 +- security/apparmor/lsm.c | 2 +- security/apparmor/mount.c | 2 +- security/apparmor/policy.c | 2 +- security/apparmor/policy_ns.c | 2 +- security/apparmor/policy_unpack.c | 2 +- security/apparmor/procattr.c | 2 +- security/apparmor/resource.c | 2 +- security/apparmor/task.c | 2 +- 16 files changed, 190 insertions(+), 190 deletions(-) delete mode 100644 security/apparmor/include/context.h create mode 100644 security/apparmor/include/cred.h (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 00fc4f9f7f14..874c1bf6b84a 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -30,7 +30,7 @@ #include "include/apparmor.h" #include "include/apparmorfs.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/crypto.h" #include "include/ipc.h" #include "include/label.h" diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c index 67e347192a55..253ef6e9d445 100644 --- a/security/apparmor/capability.c +++ b/security/apparmor/capability.c @@ -19,7 +19,7 @@ #include "include/apparmor.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/audit.h" diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 56d080a6d774..cd58eef4eb8d 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -22,7 +22,7 @@ #include "include/audit.h" #include "include/apparmorfs.h" -#include "include/context.h" +#include "include/cred.h" #include "include/domain.h" #include "include/file.h" #include "include/ipc.h" diff --git a/security/apparmor/file.c b/security/apparmor/file.c index e79bf44396a3..9a67a33904b3 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -18,7 +18,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/match.h" #include "include/path.h" diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h deleted file mode 100644 index e287b7d0d4be..000000000000 --- a/security/apparmor/include/context.h +++ /dev/null @@ -1,176 +0,0 @@ -/* - * AppArmor security module - * - * This file contains AppArmor contexts used to associate "labels" to objects. - * - * Copyright (C) 1998-2008 Novell/SUSE - * Copyright 2009-2010 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_CONTEXT_H -#define __AA_CONTEXT_H - -#include -#include -#include - -#include "label.h" -#include "policy_ns.h" -#include "task.h" - -#define cred_label(X) ((X)->security) - - -/** - * aa_cred_raw_label - obtain cred's label - * @cred: cred to obtain label from (NOT NULL) - * - * Returns: confining label - * - * does NOT increment reference count - */ -static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) -{ - struct aa_label *label = cred_label(cred); - - AA_BUG(!label); - return label; -} - -/** - * aa_get_newest_cred_label - obtain the newest label on a cred - * @cred: cred to obtain label from (NOT NULL) - * - * Returns: newest version of confining label - */ -static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred) -{ - return aa_get_newest_label(aa_cred_raw_label(cred)); -} - -/** - * __aa_task_raw_label - retrieve another task's label - * @task: task to query (NOT NULL) - * - * Returns: @task's label without incrementing its ref count - * - * If @task != current needs to be called in RCU safe critical section - */ -static inline struct aa_label *__aa_task_raw_label(struct task_struct *task) -{ - return aa_cred_raw_label(__task_cred(task)); -} - -/** - * aa_current_raw_label - find the current tasks confining label - * - * Returns: up to date confining label or the ns unconfined label (NOT NULL) - * - * This fn will not update the tasks cred to the most up to date version - * of the label so it is safe to call when inside of locks. - */ -static inline struct aa_label *aa_current_raw_label(void) -{ - return aa_cred_raw_label(current_cred()); -} - -/** - * aa_get_current_label - get the newest version of the current tasks label - * - * Returns: newest version of confining label (NOT NULL) - * - * This fn will not update the tasks cred, so it is safe inside of locks - * - * The returned reference must be put with aa_put_label() - */ -static inline struct aa_label *aa_get_current_label(void) -{ - struct aa_label *l = aa_current_raw_label(); - - if (label_is_stale(l)) - return aa_get_newest_label(l); - return aa_get_label(l); -} - -#define __end_current_label_crit_section(X) end_current_label_crit_section(X) - -/** - * end_label_crit_section - put a reference found with begin_current_label.. - * @label: label reference to put - * - * Should only be used with a reference obtained with - * begin_current_label_crit_section and never used in situations where the - * task cred may be updated - */ -static inline void end_current_label_crit_section(struct aa_label *label) -{ - if (label != aa_current_raw_label()) - aa_put_label(label); -} - -/** - * __begin_current_label_crit_section - current's confining label - * - * Returns: up to date confining label or the ns unconfined label (NOT NULL) - * - * safe to call inside locks - * - * The returned reference must be put with __end_current_label_crit_section() - * This must NOT be used if the task cred could be updated within the - * critical section between __begin_current_label_crit_section() .. - * __end_current_label_crit_section() - */ -static inline struct aa_label *__begin_current_label_crit_section(void) -{ - struct aa_label *label = aa_current_raw_label(); - - if (label_is_stale(label)) - label = aa_get_newest_label(label); - - return label; -} - -/** - * begin_current_label_crit_section - current's confining label and update it - * - * Returns: up to date confining label or the ns unconfined label (NOT NULL) - * - * Not safe to call inside locks - * - * The returned reference must be put with end_current_label_crit_section() - * This must NOT be used if the task cred could be updated within the - * critical section between begin_current_label_crit_section() .. - * end_current_label_crit_section() - */ -static inline struct aa_label *begin_current_label_crit_section(void) -{ - struct aa_label *label = aa_current_raw_label(); - - if (label_is_stale(label)) { - label = aa_get_newest_label(label); - if (aa_replace_current_label(label) == 0) - /* task cred will keep the reference */ - aa_put_label(label); - } - - return label; -} - -static inline struct aa_ns *aa_get_current_ns(void) -{ - struct aa_label *label; - struct aa_ns *ns; - - label = __begin_current_label_crit_section(); - ns = aa_get_ns(labels_ns(label)); - __end_current_label_crit_section(label); - - return ns; -} - -#endif /* __AA_CONTEXT_H */ diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h new file mode 100644 index 000000000000..e287b7d0d4be --- /dev/null +++ b/security/apparmor/include/cred.h @@ -0,0 +1,176 @@ +/* + * AppArmor security module + * + * This file contains AppArmor contexts used to associate "labels" to objects. + * + * Copyright (C) 1998-2008 Novell/SUSE + * Copyright 2009-2010 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_CONTEXT_H +#define __AA_CONTEXT_H + +#include +#include +#include + +#include "label.h" +#include "policy_ns.h" +#include "task.h" + +#define cred_label(X) ((X)->security) + + +/** + * aa_cred_raw_label - obtain cred's label + * @cred: cred to obtain label from (NOT NULL) + * + * Returns: confining label + * + * does NOT increment reference count + */ +static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) +{ + struct aa_label *label = cred_label(cred); + + AA_BUG(!label); + return label; +} + +/** + * aa_get_newest_cred_label - obtain the newest label on a cred + * @cred: cred to obtain label from (NOT NULL) + * + * Returns: newest version of confining label + */ +static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred) +{ + return aa_get_newest_label(aa_cred_raw_label(cred)); +} + +/** + * __aa_task_raw_label - retrieve another task's label + * @task: task to query (NOT NULL) + * + * Returns: @task's label without incrementing its ref count + * + * If @task != current needs to be called in RCU safe critical section + */ +static inline struct aa_label *__aa_task_raw_label(struct task_struct *task) +{ + return aa_cred_raw_label(__task_cred(task)); +} + +/** + * aa_current_raw_label - find the current tasks confining label + * + * Returns: up to date confining label or the ns unconfined label (NOT NULL) + * + * This fn will not update the tasks cred to the most up to date version + * of the label so it is safe to call when inside of locks. + */ +static inline struct aa_label *aa_current_raw_label(void) +{ + return aa_cred_raw_label(current_cred()); +} + +/** + * aa_get_current_label - get the newest version of the current tasks label + * + * Returns: newest version of confining label (NOT NULL) + * + * This fn will not update the tasks cred, so it is safe inside of locks + * + * The returned reference must be put with aa_put_label() + */ +static inline struct aa_label *aa_get_current_label(void) +{ + struct aa_label *l = aa_current_raw_label(); + + if (label_is_stale(l)) + return aa_get_newest_label(l); + return aa_get_label(l); +} + +#define __end_current_label_crit_section(X) end_current_label_crit_section(X) + +/** + * end_label_crit_section - put a reference found with begin_current_label.. + * @label: label reference to put + * + * Should only be used with a reference obtained with + * begin_current_label_crit_section and never used in situations where the + * task cred may be updated + */ +static inline void end_current_label_crit_section(struct aa_label *label) +{ + if (label != aa_current_raw_label()) + aa_put_label(label); +} + +/** + * __begin_current_label_crit_section - current's confining label + * + * Returns: up to date confining label or the ns unconfined label (NOT NULL) + * + * safe to call inside locks + * + * The returned reference must be put with __end_current_label_crit_section() + * This must NOT be used if the task cred could be updated within the + * critical section between __begin_current_label_crit_section() .. + * __end_current_label_crit_section() + */ +static inline struct aa_label *__begin_current_label_crit_section(void) +{ + struct aa_label *label = aa_current_raw_label(); + + if (label_is_stale(label)) + label = aa_get_newest_label(label); + + return label; +} + +/** + * begin_current_label_crit_section - current's confining label and update it + * + * Returns: up to date confining label or the ns unconfined label (NOT NULL) + * + * Not safe to call inside locks + * + * The returned reference must be put with end_current_label_crit_section() + * This must NOT be used if the task cred could be updated within the + * critical section between begin_current_label_crit_section() .. + * end_current_label_crit_section() + */ +static inline struct aa_label *begin_current_label_crit_section(void) +{ + struct aa_label *label = aa_current_raw_label(); + + if (label_is_stale(label)) { + label = aa_get_newest_label(label); + if (aa_replace_current_label(label) == 0) + /* task cred will keep the reference */ + aa_put_label(label); + } + + return label; +} + +static inline struct aa_ns *aa_get_current_ns(void) +{ + struct aa_label *label; + struct aa_ns *ns; + + label = __begin_current_label_crit_section(); + ns = aa_get_ns(labels_ns(label)); + __end_current_label_crit_section(label); + + return ns; +} + +#endif /* __AA_CONTEXT_H */ diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index d7b137d4eb74..527ea1557120 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -17,7 +17,7 @@ #include "include/audit.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/ipc.h" #include "include/sig_names.h" diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 69c7451becef..523250e34837 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -16,7 +16,7 @@ #include #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/label.h" #include "include/policy.h" #include "include/secid.h" diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 7577cd982230..ef6334e11597 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -30,7 +30,7 @@ #include "include/apparmorfs.h" #include "include/audit.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/ipc.h" #include "include/path.h" diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index 8c558cbce930..6e8c7ac0b33d 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -18,7 +18,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/domain.h" #include "include/file.h" #include "include/match.h" diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index a158af1f1b38..a8e096a88e62 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -82,7 +82,7 @@ #include "include/apparmor.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/ipc.h" #include "include/match.h" diff --git a/security/apparmor/policy_ns.c b/security/apparmor/policy_ns.c index b1e629cba70b..b0f9dc3f765a 100644 --- a/security/apparmor/policy_ns.c +++ b/security/apparmor/policy_ns.c @@ -21,7 +21,7 @@ #include #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy_ns.h" #include "include/label.h" #include "include/policy.h" diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index ece0c246cfe6..40c8dc617b13 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -23,7 +23,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/crypto.h" #include "include/match.h" #include "include/path.h" diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index d81617379d63..80c34ed373c3 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -13,7 +13,7 @@ */ #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/policy_ns.h" #include "include/domain.h" diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c index cf4d234febe9..d022137143b9 100644 --- a/security/apparmor/resource.c +++ b/security/apparmor/resource.c @@ -16,7 +16,7 @@ #include #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/resource.h" #include "include/policy.h" diff --git a/security/apparmor/task.c b/security/apparmor/task.c index 36eb8707ad89..44b9b938e06d 100644 --- a/security/apparmor/task.c +++ b/security/apparmor/task.c @@ -16,7 +16,7 @@ * should return to the previous cred if it has not been modified. */ -#include "include/context.h" +#include "include/cred.h" #include "include/task.h" /** -- cgit v1.2.3-58-ga151 From 9fcf78cca198600b27c44b4e50f00f8af3927f17 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 8 Oct 2017 18:26:19 -0700 Subject: apparmor: update domain transitions that are subsets of confinement at nnp Domain transition so far have been largely blocked by no new privs, unless the transition has been provably a subset of the previous confinement. There was a couple problems with the previous implementations, - transitions that weren't explicitly a stack but resulted in a subset of confinement were disallowed - confinement subsets were only calculated from the previous confinement instead of the confinement being enforced at the time of no new privs, so transitions would have to get progressively tighter. Fix this by detecting and storing a reference to the task's confinement at the "time" no new privs is set. This reference is then used to determine whether a transition is a subsystem of the confinement at the time no new privs was set. Unfortunately the implementation is less than ideal in that we have to detect no new privs after the fact when a task attempts a domain transition. This is adequate for the currently but will not work in a stacking situation where no new privs could be conceivably be set in both the "host" and in the container. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 + security/apparmor/domain.c | 163 +++++++++++++++++++++++---------------- security/apparmor/include/task.h | 4 + security/apparmor/task.c | 7 ++ 4 files changed, 110 insertions(+), 65 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 874c1bf6b84a..07623fb41e32 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2156,6 +2156,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("change_profile", 1), AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), + AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), AA_SFS_FILE_STRING("version", "1.2"), { } }; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index cd58eef4eb8d..9d1936519cfd 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -592,22 +592,6 @@ static struct aa_label *profile_transition(struct aa_profile *profile, if (!new) goto audit; - /* Policy has specified a domain transitions. if no_new_privs and - * confined and not transitioning to the current domain fail. - * - * NOTE: Domain transitions from unconfined and to stritly stacked - * subsets are allowed even when no_new_privs is set because this - * aways results in a further reduction of permissions. - */ - if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && - !profile_unconfined(profile) && - !aa_label_is_subset(new, &profile->label)) { - error = -EPERM; - info = "no new privs"; - nonewprivs = true; - perms.allow &= ~MAY_EXEC; - goto audit; - } if (!(perms.xindex & AA_X_UNSAFE)) { if (DEBUG_ON) { @@ -684,21 +668,6 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec, perms.allow &= ~AA_MAY_ONEXEC; goto audit; } - /* Policy has specified a domain transitions. if no_new_privs and - * confined and not transitioning to the current domain fail. - * - * NOTE: Domain transitions from unconfined and to stritly stacked - * subsets are allowed even when no_new_privs is set because this - * aways results in a further reduction of permissions. - */ - if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && - !profile_unconfined(profile) && - !aa_label_is_subset(onexec, &profile->label)) { - error = -EPERM; - info = "no new privs"; - perms.allow &= ~AA_MAY_ONEXEC; - goto audit; - } if (!(perms.xindex & AA_X_UNSAFE)) { if (DEBUG_ON) { @@ -800,6 +769,17 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) label = aa_get_newest_label(cred_label(bprm->cred)); + /* + * Detect no new privs being set, and store the label it + * occurred under. Ideally this would happen when nnp + * is set but there isn't a good way to do that yet. + * + * Testing for unconfined must be done before the subset test + */ + if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && !unconfined(label) && + !ctx->nnp) + ctx->nnp = aa_get_label(label); + /* buffer freed below, name is pointer into buffer */ get_buffers(buffer); /* Test for onexec first as onexec override other x transitions. */ @@ -820,7 +800,20 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) goto done; } - /* TODO: Add ns level no_new_privs subset test */ + /* Policy has specified a domain transitions. If no_new_privs and + * confined ensure the transition is to confinement that is subset + * of the confinement when the task entered no new privs. + * + * NOTE: Domain transitions from unconfined and to stacked + * subsets are allowed even when no_new_privs is set because this + * aways results in a further reduction of permissions. + */ + if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && + !unconfined(label) && !aa_label_is_subset(new, ctx->nnp)) { + error = -EPERM; + info = "no new privs"; + goto audit; + } if (bprm->unsafe & LSM_UNSAFE_SHARE) { /* FIXME: currently don't mediate shared state */ @@ -1047,30 +1040,28 @@ build: int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; - struct aa_task_ctx *ctx; + struct aa_task_ctx *ctx = task_ctx(current); struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; struct aa_perms perms = {}; const char *info = NULL; int error = 0; - /* - * Fail explicitly requested domain transitions if no_new_privs. - * There is no exception for unconfined as change_hat is not - * available. - */ - if (task_no_new_privs(current)) { - /* not an apparmor denial per se, so don't log it */ - AA_DEBUG("no_new_privs - change_hat denied"); - return -EPERM; - } - /* released below */ cred = get_current_cred(); - ctx = task_ctx(current); label = aa_get_newest_cred_label(cred); previous = aa_get_newest_label(ctx->previous); + /* + * Detect no new privs being set, and store the label it + * occurred under. Ideally this would happen when nnp + * is set but there isn't a good way to do that yet. + * + * Testing for unconfined must be done before the subset test + */ + if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp) + ctx->nnp = aa_get_label(label); + if (unconfined(label)) { info = "unconfined can not change_hat"; error = -EPERM; @@ -1091,6 +1082,18 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) if (error) goto fail; + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(new, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + if (flags & AA_CHANGE_TEST) goto out; @@ -1100,6 +1103,18 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* kill task in case of brute force attacks */ goto kill; } else if (previous && !(flags & AA_CHANGE_TEST)) { + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(previous, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + /* Return to saved label. Kill task if restore fails * to avoid brute force attacks */ @@ -1142,21 +1157,6 @@ static int change_profile_perms_wrapper(const char *op, const char *name, const char *info = NULL; int error = 0; - /* - * Fail explicitly requested domain transitions when no_new_privs - * and not unconfined OR the transition results in a stack on - * the current label. - * Stacking domain transitions and transitions from unconfined are - * allowed even when no_new_privs is set because this aways results - * in a reduction of permissions. - */ - if (task_no_new_privs(current) && !stack && - !profile_unconfined(profile) && - !aa_label_is_subset(target, &profile->label)) { - info = "no new privs"; - error = -EPERM; - } - if (!error) error = change_profile_perms(profile, target, stack, request, profile->file.start, perms); @@ -1190,10 +1190,23 @@ int aa_change_profile(const char *fqname, int flags) const char *info = NULL; const char *auditname = fqname; /* retain leading & if stack */ bool stack = flags & AA_CHANGE_STACK; + struct aa_task_ctx *ctx = task_ctx(current); int error = 0; char *op; u32 request; + label = aa_get_current_label(); + + /* + * Detect no new privs being set, and store the label it + * occurred under. Ideally this would happen when nnp + * is set but there isn't a good way to do that yet. + * + * Testing for unconfined must be done before the subset test + */ + if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp) + ctx->nnp = aa_get_label(label); + if (!fqname || !*fqname) { AA_DEBUG("no profile name"); return -EINVAL; @@ -1281,14 +1294,28 @@ check: if (flags & AA_CHANGE_TEST) goto out; + /* stacking is always a subset, so only check the nonstack case */ + if (!stack) { + new = fn_label_build_in_ns(label, profile, GFP_KERNEL, + aa_get_label(target), + aa_get_label(&profile->label)); + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(new, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + } + if (!(flags & AA_CHANGE_ONEXEC)) { /* only transition profiles in the current ns */ if (stack) new = aa_label_merge(label, target, GFP_KERNEL); - else - new = fn_label_build_in_ns(label, profile, GFP_KERNEL, - aa_get_label(target), - aa_get_label(&profile->label)); if (IS_ERR_OR_NULL(new)) { info = "failed to build target label"; error = PTR_ERR(new); @@ -1297,9 +1324,15 @@ check: goto audit; } error = aa_replace_current_label(new); - } else + } else { + if (new) { + aa_put_label(new); + new = NULL; + } + /* full transition will be built in exec path */ error = aa_set_current_onexec(target, stack); + } audit: error = fn_for_each_in_ns(label, profile, diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h index d222197db299..55edaa1d83f8 100644 --- a/security/apparmor/include/task.h +++ b/security/apparmor/include/task.h @@ -18,11 +18,13 @@ /* * struct aa_task_ctx - information for current task label change + * @nnp: snapshot of label at time of no_new_privs * @onexec: profile to transition to on next exec (MAY BE NULL) * @previous: profile the task may return to (MAY BE NULL) * @token: magic value the task must know for returning to @previous_profile */ struct aa_task_ctx { + struct aa_label *nnp; struct aa_label *onexec; struct aa_label *previous; u64 token; @@ -52,6 +54,7 @@ static inline struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags) static inline void aa_free_task_ctx(struct aa_task_ctx *ctx) { if (ctx) { + aa_put_label(ctx->nnp); aa_put_label(ctx->previous); aa_put_label(ctx->onexec); @@ -68,6 +71,7 @@ static inline void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old) { *new = *old; + aa_get_label(new->nnp); aa_get_label(new->previous); aa_get_label(new->onexec); } diff --git a/security/apparmor/task.c b/security/apparmor/task.c index 44b9b938e06d..c6b78a14da91 100644 --- a/security/apparmor/task.c +++ b/security/apparmor/task.c @@ -45,6 +45,7 @@ struct aa_label *aa_get_task_label(struct task_struct *task) int aa_replace_current_label(struct aa_label *label) { struct aa_label *old = aa_current_raw_label(); + struct aa_task_ctx *ctx = task_ctx(current); struct cred *new; AA_BUG(!label); @@ -59,6 +60,12 @@ int aa_replace_current_label(struct aa_label *label) if (!new) return -ENOMEM; + if (ctx->nnp && label_is_stale(ctx->nnp)) { + struct aa_label *tmp = ctx->nnp; + + ctx->nnp = aa_get_newest_label(tmp); + aa_put_label(tmp); + } if (unconfined(label) || (labels_ns(old) != labels_ns(label))) /* * if switching to unconfined or a different label namespace -- cgit v1.2.3-58-ga151 From cf91600071a973c28cebf314e618610a20ec4d6d Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 5 Feb 2018 09:58:29 +0100 Subject: apparmor: cleanup create_aafs() error path Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 07623fb41e32..8cdab3c5bc63 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2458,34 +2458,26 @@ static int __init aa_create_aafs(void) dent = securityfs_create_file(".load", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_load); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subload(root_ns) = dent; dent = securityfs_create_file(".replace", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_replace); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subreplace(root_ns) = dent; dent = securityfs_create_file(".remove", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_remove); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subremove(root_ns) = dent; dent = securityfs_create_file("revision", 0444, aa_sfs_entry.dentry, NULL, &aa_fs_ns_revision_fops); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subrevision(root_ns) = dent; /* policy tree referenced by magic policy symlink */ @@ -2499,10 +2491,8 @@ static int __init aa_create_aafs(void) /* magic symlink similar to nsfs redirects based on task policy */ dent = securityfs_create_symlink("policy", aa_sfs_entry.dentry, NULL, &policy_link_iops); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; error = aa_mk_null_file(aa_sfs_entry.dentry); if (error) @@ -2514,6 +2504,8 @@ static int __init aa_create_aafs(void) aa_info_message("AppArmor Filesystem Enabled"); return 0; +dent_error: + error = PTR_ERR(dent); error: aa_destroy_aafs(); AA_ERROR("Error creating AppArmor securityfs\n"); -- cgit v1.2.3-58-ga151 From a0781209cb894e5115bb00c269b1d94c4b632d6a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 5 Feb 2018 18:26:46 +0100 Subject: apparmor: cleanup: simplify code to get ns symlink name ns_get_name() is called in only one place and can be folded in. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 8cdab3c5bc63..1e63ff2e5b85 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -119,9 +119,7 @@ static int aafs_count; static int aafs_show_path(struct seq_file *seq, struct dentry *dentry) { - struct inode *inode = d_inode(dentry); - - seq_printf(seq, "%s:[%lu]", AAFS_NAME, inode->i_ino); + seq_printf(seq, "%s:[%lu]", AAFS_NAME, d_inode(dentry)->i_ino); return 0; } @@ -2392,29 +2390,18 @@ static const char *policy_get_link(struct dentry *dentry, return NULL; } -static int ns_get_name(char *buf, size_t size, struct aa_ns *ns, - struct inode *inode) -{ - int res = snprintf(buf, size, "%s:[%lu]", AAFS_NAME, inode->i_ino); - - if (res < 0 || res >= size) - res = -ENOENT; - - return res; -} - static int policy_readlink(struct dentry *dentry, char __user *buffer, int buflen) { - struct aa_ns *ns; char name[32]; int res; - ns = aa_get_current_ns(); - res = ns_get_name(name, sizeof(name), ns, d_inode(dentry)); - if (res >= 0) + res = snprintf(name, sizeof(name), "%s:[%lu]", AAFS_NAME, + d_inode(dentry)->i_ino); + if (res > 0 && res < sizeof(name)) res = readlink_copy(buffer, buflen, name); - aa_put_ns(ns); + else + res = -ENOENT; return res; } -- cgit v1.2.3-58-ga151 From 73f488cd903938e78979d50e081a0314ad142351 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 12 Dec 2017 15:28:05 -0800 Subject: apparmor: convert attaching profiles via xattrs to use dfa matching This converts profile attachment based on xattrs to a fixed extended conditional using dfa matching. This has a couple of advantages - pattern matching can be used for the xattr match - xattrs can be optional for an attachment or marked as required - the xattr attachment conditional will be able to be combined with other extended conditionals when the flexible extended conditional work lands. The xattr fixed extended conditional is appended to the xmatch conditional. If an xattr attachment is specified the profile xmatch will be generated regardless of whether there is a pattern match on the executable name. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/apparmorfs.c | 5 ++++ security/apparmor/domain.c | 52 ++++++++++++++++++++++++++------------ security/apparmor/include/policy.h | 2 -- security/apparmor/policy.c | 6 +---- security/apparmor/policy_unpack.c | 35 +------------------------ 5 files changed, 43 insertions(+), 57 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 1e63ff2e5b85..35e6b240fb14 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2147,6 +2147,10 @@ static struct aa_sfs_entry aa_sfs_entry_signal[] = { { } }; +static struct aa_sfs_entry aa_sfs_entry_attach[] = { + AA_SFS_FILE_BOOLEAN("xattr", 1), + { } +}; static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("change_hat", 1), AA_SFS_FILE_BOOLEAN("change_hatv", 1), @@ -2155,6 +2159,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), + AA_SFS_DIR("attach_conditions", aa_sfs_entry_attach), AA_SFS_FILE_STRING("version", "1.2"), { } }; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6bcafe8d226d..6a1279f11fcc 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -306,11 +306,12 @@ static int change_profile_perms(struct aa_profile *profile, * aa_xattrs_match - check whether a file matches the xattrs defined in profile * @bprm: binprm struct for the process to validate * @profile: profile to match against (NOT NULL) + * @state: state to start match in * * Returns: number of extended attributes that matched, or < 0 on error */ static int aa_xattrs_match(const struct linux_binprm *bprm, - struct aa_profile *profile) + struct aa_profile *profile, unsigned int state) { int i; size_t size; @@ -321,27 +322,40 @@ static int aa_xattrs_match(const struct linux_binprm *bprm, if (!bprm || !profile->xattr_count) return 0; + /* transition from exec match to xattr set */ + state = aa_dfa_null_transition(profile->xmatch, state); + d = bprm->file->f_path.dentry; for (i = 0; i < profile->xattr_count; i++) { size = vfs_getxattr_alloc(d, profile->xattrs[i], &value, value_size, GFP_KERNEL); - if (size < 0) { - ret = -EINVAL; - goto out; - } + if (size >= 0) { + u32 perm; - /* Check the xattr value, not just presence */ - if (profile->xattr_lens[i]) { - if (profile->xattr_lens[i] != size) { + /* Check the xattr value, not just presence */ + state = aa_dfa_match_len(profile->xmatch, state, value, + size); + perm = dfa_user_allow(profile->xmatch, state); + if (!(perm & MAY_EXEC)) { ret = -EINVAL; goto out; } - - if (memcmp(value, profile->xattr_values[i], size)) { + } + /* transition to next element */ + state = aa_dfa_null_transition(profile->xmatch, state); + if (size < 0) { + /* + * No xattr match, so verify if transition to + * next element was valid. IFF so the xattr + * was optional. + */ + if (!state) { ret = -EINVAL; goto out; } + /* don't count missing optional xattr as matched */ + ret--; } } @@ -403,13 +417,16 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, perm = dfa_user_allow(profile->xmatch, state); /* any accepting state means a valid match. */ if (perm & MAY_EXEC) { - int ret = aa_xattrs_match(bprm, profile); + int ret = aa_xattrs_match(bprm, profile, state); /* Fail matching if the xattrs don't match */ if (ret < 0) continue; - /* The new match isn't more specific + /* + * TODO: allow for more flexible best match + * + * The new match isn't more specific * than the current best match */ if (profile->xmatch_len == len && @@ -428,9 +445,11 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, xattrs = ret; conflict = false; } - } else if (!strcmp(profile->base.name, name) && - aa_xattrs_match(bprm, profile) >= 0) - /* exact non-re match, no more searching required */ + } else if (!strcmp(profile->base.name, name)) + /* + * old exact non-re match, without conditionals such + * as xattrs. no more searching required + */ return profile; } @@ -652,7 +671,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile, * met, and fail execution otherwise */ label_for_each(i, new, component) { - if (aa_xattrs_match(bprm, component) < 0) { + if (aa_xattrs_match(bprm, component, state) < + 0) { error = -EACCES; info = "required xattrs not present"; perms.allow &= ~MAY_EXEC; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 02bde92ebb5c..c93b9ed55490 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -151,8 +151,6 @@ struct aa_profile { int xattr_count; char **xattrs; - size_t *xattr_lens; - char **xattr_values; struct aa_rlimit rlimits; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 7fee546ba10d..c07493ce2376 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -228,13 +228,9 @@ void aa_free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); - for (i = 0; i < profile->xattr_count; i++) { + for (i = 0; i < profile->xattr_count; i++) kzfree(profile->xattrs[i]); - kzfree(profile->xattr_values[i]); - } kzfree(profile->xattrs); - kzfree(profile->xattr_lens); - kzfree(profile->xattr_values); kzfree(profile->dirname); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 98d019185e57..8a31ddd474d7 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -540,8 +540,7 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) size = unpack_array(e, NULL); profile->xattr_count = size; - profile->xattrs = kcalloc(size, sizeof(char *), - GFP_KERNEL); + profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL); if (!profile->xattrs) goto fail; for (i = 0; i < size; i++) { @@ -554,38 +553,6 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) goto fail; } - if (unpack_nameX(e, AA_STRUCT, "xattr_values")) { - int i, size; - - size = unpack_array(e, NULL); - - /* Must be the same number of xattr values as xattrs */ - if (size != profile->xattr_count) - goto fail; - - profile->xattr_lens = kcalloc(size, sizeof(size_t), - GFP_KERNEL); - if (!profile->xattr_lens) - goto fail; - - profile->xattr_values = kcalloc(size, sizeof(char *), - GFP_KERNEL); - if (!profile->xattr_values) - goto fail; - - for (i = 0; i < size; i++) { - profile->xattr_lens[i] = unpack_blob(e, - &profile->xattr_values[i], NULL); - profile->xattr_values[i] = - kvmemdup(profile->xattr_values[i], - profile->xattr_lens[i]); - } - - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) - goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) - goto fail; - } return 1; fail: -- cgit v1.2.3-58-ga151 From 21f606610502ef56f9180b1529fc7e02957564c8 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 18 Nov 2017 19:43:13 -0800 Subject: apparmor: improve overlapping domain attachment resolution Overlapping domain attachments using the current longest left exact match fail in some simple cases, and with the fix to ensure consistent behavior by failing unresolvable attachments it becomes important to do a better job. eg. under the current match the following are unresolvable where the alternation is clearly a better match under the most specific left match rule. /** /{bin/,}usr/ Use a counting match that detects when a loop in the state machine is enter, and return the match count to provide a better specific left match resolution. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 + security/apparmor/domain.c | 30 ++++++---- security/apparmor/include/match.h | 19 ++++++ security/apparmor/match.c | 122 +++++++++++++++++++++++++++++++++++++- 4 files changed, 158 insertions(+), 14 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 35e6b240fb14..3dcc122234c8 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2159,6 +2159,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), + AA_SFS_FILE_BOOLEAN("computed_longest_left", 1), AA_SFS_DIR("attach_conditions", aa_sfs_entry_attach), AA_SFS_FILE_STRING("version", "1.2"), { } diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6a1279f11fcc..57cc892e05a2 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -385,10 +385,13 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, struct list_head *head, const char **info) { - int len = 0, xattrs = 0; + int candidate_len = 0, candidate_xattrs = 0; bool conflict = false; struct aa_profile *profile, *candidate = NULL; + AA_BUG(!name); + AA_BUG(!head); + list_for_each_entry_rcu(profile, head, base.list) { if (profile->label.flags & FLAG_NULL && &profile->label == ns_unconfined(profile->ns)) @@ -406,19 +409,20 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * match. */ if (profile->xmatch) { - unsigned int state; + unsigned int state, count; u32 perm; - if (profile->xmatch_len < len) - continue; - - state = aa_dfa_match(profile->xmatch, - DFA_START, name); + state = aa_dfa_leftmatch(profile->xmatch, DFA_START, + name, &count); perm = dfa_user_allow(profile->xmatch, state); /* any accepting state means a valid match. */ if (perm & MAY_EXEC) { - int ret = aa_xattrs_match(bprm, profile, state); + int ret; + + if (count < candidate_len) + continue; + ret = aa_xattrs_match(bprm, profile, state); /* Fail matching if the xattrs don't match */ if (ret < 0) continue; @@ -429,10 +433,10 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * The new match isn't more specific * than the current best match */ - if (profile->xmatch_len == len && - ret <= xattrs) { + if (count == candidate_len && + ret <= candidate_xattrs) { /* Match is equivalent, so conflict */ - if (ret == xattrs) + if (ret == candidate_xattrs) conflict = true; continue; } @@ -441,8 +445,8 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * xattrs, or a longer match */ candidate = profile; - len = profile->xmatch_len; - xattrs = ret; + candidate_len = profile->xmatch_len; + candidate_xattrs = ret; conflict = false; } } else if (!strcmp(profile->base.name, name)) diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index e0de00bd16a8..958d2b52a7b7 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -138,6 +138,25 @@ unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, void aa_dfa_free_kref(struct kref *kref); +#define WB_HISTORY_SIZE 8 +struct match_workbuf { + unsigned int count; + unsigned int pos; + unsigned int len; + unsigned int size; /* power of 2, same as history size */ + unsigned int history[WB_HISTORY_SIZE]; +}; +#define DEFINE_MATCH_WB(N) \ +struct match_workbuf N = { \ + .count = 0, \ + .pos = 0, \ + .len = 0, \ + .size = WB_HISTORY_SIZE, \ +} + +unsigned int aa_dfa_leftmatch(struct aa_dfa *dfa, unsigned int start, + const char *str, unsigned int *count); + /** * aa_get_dfa - increment refcount on dfa @p * @dfa: dfa (MAYBE NULL) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 7ae6ed9d69dd..dd4c995c5e25 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -556,7 +556,6 @@ unsigned int aa_dfa_match_until(struct aa_dfa *dfa, unsigned int start, return state; } - /** * aa_dfa_matchn_until - traverse @dfa until accept or @n bytes consumed * @dfa: the dfa to match @str against (NOT NULL) @@ -618,3 +617,124 @@ unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, *retpos = str; return state; } + +#define inc_wb_pos(wb) \ +do { \ + wb->pos = (wb->pos + 1) & (wb->size - 1); \ + wb->len = (wb->len + 1) & (wb->size - 1); \ +} while (0) + +/* For DFAs that don't support extended tagging of states */ +static bool is_loop(struct match_workbuf *wb, unsigned int state, + unsigned int *adjust) +{ + unsigned int pos = wb->pos; + unsigned int i; + + if (wb->history[pos] < state) + return false; + + for (i = 0; i <= wb->len; i++) { + if (wb->history[pos] == state) { + *adjust = i; + return true; + } + if (pos == 0) + pos = wb->size; + pos--; + } + + *adjust = i; + return true; +} + +static unsigned int leftmatch_fb(struct aa_dfa *dfa, unsigned int start, + const char *str, struct match_workbuf *wb, + unsigned int *count) +{ + 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; + + AA_BUG(!dfa); + AA_BUG(!str); + AA_BUG(!wb); + AA_BUG(!count); + + *count = 0; + if (state == 0) + return 0; + + /* current state is , 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) { + unsigned int adjust; + + wb->history[wb->pos] = state; + pos = base_idx(base[state]) + equiv[(u8) *str++]; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (is_loop(wb, state, &adjust)) { + state = aa_dfa_match(dfa, state, str); + *count -= adjust; + goto out; + } + inc_wb_pos(wb); + (*count)++; + } + } else { + /* default is direct to next state */ + while (*str) { + unsigned int adjust; + + wb->history[wb->pos] = state; + pos = base_idx(base[state]) + (u8) *str++; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (is_loop(wb, state, &adjust)) { + state = aa_dfa_match(dfa, state, str); + *count -= adjust; + goto out; + } + inc_wb_pos(wb); + (*count)++; + } + } + +out: + if (!state) + *count = 0; + return state; +} + +/** + * aa_dfa_leftmatch - 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) + * @count: current count of longest left. + * + * 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. + * + * Returns: final state reached after input is consumed + */ +unsigned int aa_dfa_leftmatch(struct aa_dfa *dfa, unsigned int start, + const char *str, unsigned int *count) +{ + DEFINE_MATCH_WB(wb); + + /* TODO: match for extended state dfas */ + + return leftmatch_fb(dfa, start, str, &wb, count); +} -- cgit v1.2.3-58-ga151 From 56974a6fcfef69ee0825bd66ed13e92070ac5224 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 18 Jul 2017 23:18:33 -0700 Subject: apparmor: add base infastructure for socket mediation version 2 - Force an abi break. Network mediation will only be available in v8 abi complaint policy. Provide a basic mediation of sockets. This is not a full net mediation but just whether a spcific family of socket can be used by an application, along with setting up some basic infrastructure for network mediation to follow. the user space rule hav the basic form of NETWORK RULE = [ QUALIFIERS ] 'network' [ DOMAIN ] [ TYPE | PROTOCOL ] DOMAIN = ( 'inet' | 'ax25' | 'ipx' | 'appletalk' | 'netrom' | 'bridge' | 'atmpvc' | 'x25' | 'inet6' | 'rose' | 'netbeui' | 'security' | 'key' | 'packet' | 'ash' | 'econet' | 'atmsvc' | 'sna' | 'irda' | 'pppox' | 'wanpipe' | 'bluetooth' | 'netlink' | 'unix' | 'rds' | 'llc' | 'can' | 'tipc' | 'iucv' | 'rxrpc' | 'isdn' | 'phonet' | 'ieee802154' | 'caif' | 'alg' | 'nfc' | 'vsock' | 'mpls' | 'ib' | 'kcm' ) ',' TYPE = ( 'stream' | 'dgram' | 'seqpacket' | 'rdm' | 'raw' | 'packet' ) PROTOCOL = ( 'tcp' | 'udp' | 'icmp' ) eg. network, network inet, Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/.gitignore | 1 + security/apparmor/Makefile | 43 +++- security/apparmor/apparmorfs.c | 2 + security/apparmor/file.c | 30 +++ security/apparmor/include/apparmor.h | 3 +- security/apparmor/include/audit.h | 6 + security/apparmor/include/net.h | 106 ++++++++++ security/apparmor/include/perms.h | 5 +- security/apparmor/include/policy.h | 11 + security/apparmor/lib.c | 5 +- security/apparmor/lsm.c | 392 +++++++++++++++++++++++++++++++++++ security/apparmor/net.c | 187 +++++++++++++++++ security/apparmor/policy_unpack.c | 3 +- 13 files changed, 786 insertions(+), 8 deletions(-) create mode 100644 security/apparmor/include/net.h create mode 100644 security/apparmor/net.c (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/.gitignore b/security/apparmor/.gitignore index 9cdec70d72b8..d5b291e94264 100644 --- a/security/apparmor/.gitignore +++ b/security/apparmor/.gitignore @@ -1,5 +1,6 @@ # # Generated include files # +net_names.h capability_names.h rlim_names.h diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile index 380c8e08174a..ff23fcfefe19 100644 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@ -5,11 +5,44 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor.o apparmor-y := apparmorfs.o audit.o capability.o task.o ipc.o lib.o match.o \ path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \ - resource.o secid.o file.o policy_ns.o label.o mount.o + resource.o secid.o file.o policy_ns.o label.o mount.o net.o apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o -clean-files := capability_names.h rlim_names.h +clean-files := capability_names.h rlim_names.h net_names.h +# Build a lower case string table of address family names +# Transform lines from +# #define AF_LOCAL 1 /* POSIX name for AF_UNIX */ +# #define AF_INET 2 /* Internet IP Protocol */ +# to +# [1] = "local", +# [2] = "inet", +# +# and build the securityfs entries for the mapping. +# Transforms lines from +# #define AF_INET 2 /* Internet IP Protocol */ +# to +# #define AA_SFS_AF_MASK "local inet" +quiet_cmd_make-af = GEN $@ +cmd_make-af = echo "static const char *address_family_names[] = {" > $@ ;\ + sed $< >>$@ -r -n -e "/AF_MAX/d" -e "/AF_LOCAL/d" -e "/AF_ROUTE/d" -e \ + 's/^\#define[ \t]+AF_([A-Z0-9_]+)[ \t]+([0-9]+)(.*)/[\2] = "\L\1",/p';\ + echo "};" >> $@ ;\ + printf '%s' '\#define AA_SFS_AF_MASK "' >> $@ ;\ + sed -r -n -e "/AF_MAX/d" -e "/AF_LOCAL/d" -e "/AF_ROUTE/d" -e \ + 's/^\#define[ \t]+AF_([A-Z0-9_]+)[ \t]+([0-9]+)(.*)/\L\1/p'\ + $< | tr '\n' ' ' | sed -e 's/ $$/"\n/' >> $@ + +# Build a lower case string table of sock type names +# Transform lines from +# SOCK_STREAM = 1, +# to +# [1] = "stream", +quiet_cmd_make-sock = GEN $@ +cmd_make-sock = echo "static const char *sock_type_names[] = {" >> $@ ;\ + sed $^ >>$@ -r -n \ + -e 's/^\tSOCK_([A-Z0-9_]+)[\t]+=[ \t]+([0-9]+)(.*)/[\2] = "\L\1",/p';\ + echo "};" >> $@ # Build a lower case string table of capability names # Transforms lines from @@ -62,6 +95,7 @@ cmd_make-rlim = echo "static const char *const rlim_names[RLIM_NLIMITS] = {" \ tr '\n' ' ' | sed -e 's/ $$/"\n/' >> $@ $(obj)/capability.o : $(obj)/capability_names.h +$(obj)/net.o : $(obj)/net_names.h $(obj)/resource.o : $(obj)/rlim_names.h $(obj)/capability_names.h : $(srctree)/include/uapi/linux/capability.h \ $(src)/Makefile @@ -69,3 +103,8 @@ $(obj)/capability_names.h : $(srctree)/include/uapi/linux/capability.h \ $(obj)/rlim_names.h : $(srctree)/include/uapi/asm-generic/resource.h \ $(src)/Makefile $(call cmd,make-rlim) +$(obj)/net_names.h : $(srctree)/include/linux/socket.h \ + $(srctree)/include/linux/net.h \ + $(src)/Makefile + $(call cmd,make-af) + $(call cmd,make-sock) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 3dcc122234c8..10d16e3abed9 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2169,6 +2169,7 @@ static struct aa_sfs_entry aa_sfs_entry_versions[] = { AA_SFS_FILE_BOOLEAN("v5", 1), AA_SFS_FILE_BOOLEAN("v6", 1), AA_SFS_FILE_BOOLEAN("v7", 1), + AA_SFS_FILE_BOOLEAN("v8", 1), { } }; @@ -2204,6 +2205,7 @@ static struct aa_sfs_entry aa_sfs_entry_features[] = { AA_SFS_DIR("policy", aa_sfs_entry_policy), AA_SFS_DIR("domain", aa_sfs_entry_domain), AA_SFS_DIR("file", aa_sfs_entry_file), + AA_SFS_DIR("network_v8", aa_sfs_entry_network), AA_SFS_DIR("mount", aa_sfs_entry_mount), AA_SFS_DIR("namespaces", aa_sfs_entry_ns), AA_SFS_FILE_U64("capability", VFS_CAP_FLAGS_MASK), diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 9a67a33904b3..224b2fef93ca 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -21,6 +21,7 @@ #include "include/cred.h" #include "include/file.h" #include "include/match.h" +#include "include/net.h" #include "include/path.h" #include "include/policy.h" #include "include/label.h" @@ -560,6 +561,32 @@ static int __file_path_perm(const char *op, struct aa_label *label, return error; } +static int __file_sock_perm(const char *op, struct aa_label *label, + struct aa_label *flabel, struct file *file, + u32 request, u32 denied) +{ + struct socket *sock = (struct socket *) file->private_data; + int error; + + AA_BUG(!sock); + + /* revalidation due to label out of date. No revocation at this time */ + if (!denied && aa_label_is_subset(flabel, label)) + return 0; + + /* TODO: improve to skip profiles cached in flabel */ + error = aa_sock_file_perm(label, op, request, sock); + if (denied) { + /* TODO: improve to skip profiles checked above */ + /* check every profile in file label to is cached */ + last_error(error, aa_sock_file_perm(flabel, op, request, sock)); + } + if (!error) + update_file_ctx(file_ctx(file), label, request); + + return error; +} + /** * aa_file_perm - do permission revalidation check & audit for @file * @op: operation being checked @@ -604,6 +631,9 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, error = __file_path_perm(op, label, flabel, file, request, denied); + else if (S_ISSOCK(file_inode(file)->i_mode)) + error = __file_sock_perm(op, label, flabel, file, request, + denied); done: rcu_read_unlock(); diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h index 829082c35faa..73d63b58d875 100644 --- a/security/apparmor/include/apparmor.h +++ b/security/apparmor/include/apparmor.h @@ -24,12 +24,13 @@ #define AA_CLASS_UNKNOWN 1 #define AA_CLASS_FILE 2 #define AA_CLASS_CAP 3 -#define AA_CLASS_NET 4 +#define AA_CLASS_DEPRECATED 4 #define AA_CLASS_RLIMITS 5 #define AA_CLASS_DOMAIN 6 #define AA_CLASS_MOUNT 7 #define AA_CLASS_PTRACE 9 #define AA_CLASS_SIGNAL 10 +#define AA_CLASS_NET 14 #define AA_CLASS_LABEL 16 #define AA_CLASS_LAST AA_CLASS_LABEL diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 41ad2c947bf4..9c9be9c98c15 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -134,6 +134,12 @@ struct apparmor_audit_data { int signal; int unmappedsig; }; + struct { + int type, protocol; + struct sock *peer_sk; + void *addr; + int addrlen; + } net; }; }; struct { diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h new file mode 100644 index 000000000000..ec7228e857a9 --- /dev/null +++ b/security/apparmor/include/net.h @@ -0,0 +1,106 @@ +/* + * AppArmor security module + * + * This file contains AppArmor network mediation definitions. + * + * Copyright (C) 1998-2008 Novell/SUSE + * 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 + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#ifndef __AA_NET_H +#define __AA_NET_H + +#include +#include + +#include "apparmorfs.h" +#include "label.h" +#include "perms.h" +#include "policy.h" + +#define AA_MAY_SEND AA_MAY_WRITE +#define AA_MAY_RECEIVE AA_MAY_READ + +#define AA_MAY_SHUTDOWN AA_MAY_DELETE + +#define AA_MAY_CONNECT AA_MAY_OPEN +#define AA_MAY_ACCEPT 0x00100000 + +#define AA_MAY_BIND 0x00200000 +#define AA_MAY_LISTEN 0x00400000 + +#define AA_MAY_SETOPT 0x01000000 +#define AA_MAY_GETOPT 0x02000000 + +#define NET_PERMS_MASK (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CREATE | \ + AA_MAY_SHUTDOWN | AA_MAY_BIND | AA_MAY_LISTEN | \ + AA_MAY_CONNECT | AA_MAY_ACCEPT | AA_MAY_SETATTR | \ + AA_MAY_GETATTR | AA_MAY_SETOPT | AA_MAY_GETOPT) + +#define NET_FS_PERMS (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CREATE | \ + AA_MAY_SHUTDOWN | AA_MAY_CONNECT | AA_MAY_RENAME |\ + AA_MAY_SETATTR | AA_MAY_GETATTR | AA_MAY_CHMOD | \ + AA_MAY_CHOWN | AA_MAY_CHGRP | AA_MAY_LOCK | \ + AA_MAY_MPROT) + +#define NET_PEER_MASK (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CONNECT | \ + AA_MAY_ACCEPT) +struct aa_sk_ctx { + struct aa_label *label; + struct aa_label *peer; +}; + +#define SK_CTX(X) ((X)->sk_security) +#define SOCK_ctx(X) SOCK_INODE(X)->i_security +#define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P) \ + struct lsm_network_audit NAME ## _net = { .sk = (SK), \ + .family = (F)}; \ + DEFINE_AUDIT_DATA(NAME, \ + ((SK) && (F) != AF_UNIX) ? LSM_AUDIT_DATA_NET : \ + LSM_AUDIT_DATA_NONE, \ + OP); \ + NAME.u.net = &(NAME ## _net); \ + aad(&NAME)->net.type = (T); \ + aad(&NAME)->net.protocol = (P) + +#define DEFINE_AUDIT_SK(NAME, OP, SK) \ + DEFINE_AUDIT_NET(NAME, OP, SK, (SK)->sk_family, (SK)->sk_type, \ + (SK)->sk_protocol) + + +#define af_select(FAMILY, FN, DEF_FN) \ +({ \ + int __e; \ + switch ((FAMILY)) { \ + default: \ + __e = DEF_FN; \ + } \ + __e; \ +}) + +extern struct aa_sfs_entry aa_sfs_entry_network[]; + +void audit_net_cb(struct audit_buffer *ab, void *va); +int aa_profile_af_perm(struct aa_profile *profile, struct common_audit_data *sa, + u32 request, u16 family, int type); +int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family, + int type, int protocol); +static inline int aa_profile_af_sk_perm(struct aa_profile *profile, + struct common_audit_data *sa, + u32 request, + struct sock *sk) +{ + return aa_profile_af_perm(profile, sa, request, sk->sk_family, + sk->sk_type); +} +int aa_sk_perm(const char *op, u32 request, struct sock *sk); + +int aa_sock_file_perm(struct aa_label *label, const char *op, u32 request, + struct socket *sock); + +#endif /* __AA_NET_H */ diff --git a/security/apparmor/include/perms.h b/security/apparmor/include/perms.h index d7b7e7115160..38aa6247d00f 100644 --- a/security/apparmor/include/perms.h +++ b/security/apparmor/include/perms.h @@ -138,9 +138,10 @@ extern struct aa_perms allperms; void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask); -void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask); +void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, + u32 mask); void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, - u32 chrsmask, const char **names, u32 namesmask); + u32 chrsmask, const char * const *names, u32 namesmask); void aa_apply_modes_to_perms(struct aa_profile *profile, struct aa_perms *perms); void aa_compute_perms(struct aa_dfa *dfa, unsigned int state, diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index c93b9ed55490..ffe12a2366e0 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -30,6 +30,7 @@ #include "file.h" #include "lib.h" #include "label.h" +#include "net.h" #include "perms.h" #include "resource.h" @@ -224,6 +225,16 @@ static inline unsigned int PROFILE_MEDIATES_SAFE(struct aa_profile *profile, return 0; } +static inline unsigned int PROFILE_MEDIATES_AF(struct aa_profile *profile, + u16 AF) { + unsigned int state = PROFILE_MEDIATES(profile, AA_CLASS_NET); + __be16 be_af = cpu_to_be16(AF); + + if (!state) + return 0; + return aa_dfa_match_len(profile->policy.dfa, state, (char *) &be_af, 2); +} + /** * aa_get_profile - increment refcount on profile @p * @p: profile (MAYBE NULL) diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 4d5e98e49d5e..068a9f471f77 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -211,7 +211,8 @@ void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask) *str = '\0'; } -void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask) +void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, + u32 mask) { const char *fmt = "%s"; unsigned int i, perm = 1; @@ -229,7 +230,7 @@ void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask) } void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, - u32 chrsmask, const char **names, u32 namesmask) + u32 chrsmask, const char * const *names, u32 namesmask) { char str[33]; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ef6334e11597..956edebf83eb 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -33,6 +33,7 @@ #include "include/cred.h" #include "include/file.h" #include "include/ipc.h" +#include "include/net.h" #include "include/path.h" #include "include/label.h" #include "include/policy.h" @@ -743,6 +744,373 @@ static int apparmor_task_kill(struct task_struct *target, struct siginfo *info, return error; } +/** + * apparmor_sk_alloc_security - allocate and attach the sk_security field + */ +static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags) +{ + struct aa_sk_ctx *ctx; + + ctx = kzalloc(sizeof(*ctx), flags); + if (!ctx) + return -ENOMEM; + + SK_CTX(sk) = ctx; + + return 0; +} + +/** + * apparmor_sk_free_security - free the sk_security field + */ +static void apparmor_sk_free_security(struct sock *sk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + SK_CTX(sk) = NULL; + aa_put_label(ctx->label); + aa_put_label(ctx->peer); + kfree(ctx); +} + +/** + * apparmor_clone_security - clone the sk_security field + */ +static void apparmor_sk_clone_security(const struct sock *sk, + struct sock *newsk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + struct aa_sk_ctx *new = SK_CTX(newsk); + + new->label = aa_get_label(ctx->label); + new->peer = aa_get_label(ctx->peer); +} + +/** + * apparmor_socket_create - check perms before creating a new socket + */ +static int apparmor_socket_create(int family, int type, int protocol, int kern) +{ + struct aa_label *label; + int error = 0; + + AA_BUG(in_interrupt()); + + label = begin_current_label_crit_section(); + if (!(kern || unconfined(label))) + error = af_select(family, + create_perm(label, family, type, protocol), + aa_af_perm(label, OP_CREATE, AA_MAY_CREATE, + family, type, protocol)); + end_current_label_crit_section(label); + + return error; +} + +/** + * apparmor_socket_post_create - setup the per-socket security struct + * + * Note: + * - kernel sockets currently labeled unconfined but we may want to + * move to a special kernel label + * - socket may not have sk here if created with sock_create_lite or + * sock_alloc. These should be accept cases which will be handled in + * sock_graft. + */ +static int apparmor_socket_post_create(struct socket *sock, int family, + int type, int protocol, int kern) +{ + struct aa_label *label; + + if (kern) { + struct aa_ns *ns = aa_get_current_ns(); + + label = aa_get_label(ns_unconfined(ns)); + aa_put_ns(ns); + } else + label = aa_get_current_label(); + + if (sock->sk) { + struct aa_sk_ctx *ctx = SK_CTX(sock->sk); + + aa_put_label(ctx->label); + ctx->label = aa_get_label(label); + } + aa_put_label(label); + + return 0; +} + +/** + * apparmor_socket_bind - check perms before bind addr to socket + */ +static int apparmor_socket_bind(struct socket *sock, + struct sockaddr *address, int addrlen) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!address); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + bind_perm(sock, address, addrlen), + aa_sk_perm(OP_BIND, AA_MAY_BIND, sock->sk)); +} + +/** + * apparmor_socket_connect - check perms before connecting @sock to @address + */ +static int apparmor_socket_connect(struct socket *sock, + struct sockaddr *address, int addrlen) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!address); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + connect_perm(sock, address, addrlen), + aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk)); +} + +/** + * apparmor_socket_list - check perms before allowing listen + */ +static int apparmor_socket_listen(struct socket *sock, int backlog) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + listen_perm(sock, backlog), + aa_sk_perm(OP_LISTEN, AA_MAY_LISTEN, sock->sk)); +} + +/** + * apparmor_socket_accept - check perms before accepting a new connection. + * + * Note: while @newsock is created and has some information, the accept + * has not been done. + */ +static int apparmor_socket_accept(struct socket *sock, struct socket *newsock) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!newsock); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + accept_perm(sock, newsock), + aa_sk_perm(OP_ACCEPT, AA_MAY_ACCEPT, sock->sk)); +} + +static int aa_sock_msg_perm(const char *op, u32 request, struct socket *sock, + struct msghdr *msg, int size) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!msg); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + msg_perm(op, request, sock, msg, size), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_socket_sendmsg - check perms before sending msg to another socket + */ +static int apparmor_socket_sendmsg(struct socket *sock, + struct msghdr *msg, int size) +{ + return aa_sock_msg_perm(OP_SENDMSG, AA_MAY_SEND, sock, msg, size); +} + +/** + * apparmor_socket_recvmsg - check perms before receiving a message + */ +static int apparmor_socket_recvmsg(struct socket *sock, + struct msghdr *msg, int size, int flags) +{ + return aa_sock_msg_perm(OP_RECVMSG, AA_MAY_RECEIVE, sock, msg, size); +} + +/* revaliation, get/set attr, shutdown */ +static int aa_sock_perm(const char *op, u32 request, struct socket *sock) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + sock_perm(op, request, sock), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_socket_getsockname - check perms before getting the local address + */ +static int apparmor_socket_getsockname(struct socket *sock) +{ + return aa_sock_perm(OP_GETSOCKNAME, AA_MAY_GETATTR, sock); +} + +/** + * apparmor_socket_getpeername - check perms before getting remote address + */ +static int apparmor_socket_getpeername(struct socket *sock) +{ + return aa_sock_perm(OP_GETPEERNAME, AA_MAY_GETATTR, sock); +} + +/* revaliation, get/set attr, opt */ +static int aa_sock_opt_perm(const char *op, u32 request, struct socket *sock, + int level, int optname) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + opt_perm(op, request, sock, level, optname), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_getsockopt - check perms before getting socket options + */ +static int apparmor_socket_getsockopt(struct socket *sock, int level, + int optname) +{ + return aa_sock_opt_perm(OP_GETSOCKOPT, AA_MAY_GETOPT, sock, + level, optname); +} + +/** + * apparmor_setsockopt - check perms before setting socket options + */ +static int apparmor_socket_setsockopt(struct socket *sock, int level, + int optname) +{ + return aa_sock_opt_perm(OP_SETSOCKOPT, AA_MAY_SETOPT, sock, + level, optname); +} + +/** + * apparmor_socket_shutdown - check perms before shutting down @sock conn + */ +static int apparmor_socket_shutdown(struct socket *sock, int how) +{ + return aa_sock_perm(OP_SHUTDOWN, AA_MAY_SHUTDOWN, sock); +} + +/** + * apparmor_socket_sock_recv_skb - check perms before associating skb to sk + * + * Note: can not sleep may be called with locks held + * + * dont want protocol specific in __skb_recv_datagram() + * to deny an incoming connection socket_sock_rcv_skb() + */ +static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) +{ + return 0; +} + + +static struct aa_label *sk_peer_label(struct sock *sk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + if (ctx->peer) + return ctx->peer; + + return ERR_PTR(-ENOPROTOOPT); +} + +/** + * apparmor_socket_getpeersec_stream - get security context of peer + * + * Note: for tcp only valid if using ipsec or cipso on lan + */ +static int apparmor_socket_getpeersec_stream(struct socket *sock, + char __user *optval, + int __user *optlen, + unsigned int len) +{ + char *name; + int slen, error = 0; + struct aa_label *label; + struct aa_label *peer; + + label = begin_current_label_crit_section(); + peer = sk_peer_label(sock->sk); + if (IS_ERR(peer)) { + error = PTR_ERR(peer); + goto done; + } + slen = aa_label_asxprint(&name, labels_ns(label), peer, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED, GFP_KERNEL); + /* don't include terminating \0 in slen, it breaks some apps */ + if (slen < 0) { + error = -ENOMEM; + } else { + if (slen > len) { + error = -ERANGE; + } else if (copy_to_user(optval, name, slen)) { + error = -EFAULT; + goto out; + } + if (put_user(slen, optlen)) + error = -EFAULT; +out: + kfree(name); + + } + +done: + end_current_label_crit_section(label); + + return error; +} + +/** + * apparmor_socket_getpeersec_dgram - get security label of packet + * @sock: the peer socket + * @skb: packet data + * @secid: pointer to where to put the secid of the packet + * + * Sets the netlabel socket state on sk from parent + */ +static int apparmor_socket_getpeersec_dgram(struct socket *sock, + struct sk_buff *skb, u32 *secid) + +{ + /* TODO: requires secid support */ + return -ENOPROTOOPT; +} + +/** + * apparmor_sock_graft - Initialize newly created socket + * @sk: child sock + * @parent: parent socket + * + * Note: could set off of SOCK_CTX(parent) but need to track inode and we can + * just set sk security information off of current creating process label + * Labeling of sk for accept case - probably should be sock based + * instead of task, because of the case where an implicitly labeled + * socket is shared by different tasks. + */ +static void apparmor_sock_graft(struct sock *sk, struct socket *parent) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + if (!ctx->label) + ctx->label = aa_get_current_label(); +} + static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme), @@ -777,6 +1145,30 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(getprocattr, apparmor_getprocattr), LSM_HOOK_INIT(setprocattr, apparmor_setprocattr), + LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security), + LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security), + LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security), + + LSM_HOOK_INIT(socket_create, apparmor_socket_create), + LSM_HOOK_INIT(socket_post_create, apparmor_socket_post_create), + LSM_HOOK_INIT(socket_bind, apparmor_socket_bind), + LSM_HOOK_INIT(socket_connect, apparmor_socket_connect), + LSM_HOOK_INIT(socket_listen, apparmor_socket_listen), + LSM_HOOK_INIT(socket_accept, apparmor_socket_accept), + LSM_HOOK_INIT(socket_sendmsg, apparmor_socket_sendmsg), + LSM_HOOK_INIT(socket_recvmsg, apparmor_socket_recvmsg), + LSM_HOOK_INIT(socket_getsockname, apparmor_socket_getsockname), + LSM_HOOK_INIT(socket_getpeername, apparmor_socket_getpeername), + LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt), + LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt), + LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown), + LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb), + LSM_HOOK_INIT(socket_getpeersec_stream, + apparmor_socket_getpeersec_stream), + LSM_HOOK_INIT(socket_getpeersec_dgram, + apparmor_socket_getpeersec_dgram), + LSM_HOOK_INIT(sock_graft, apparmor_sock_graft), + LSM_HOOK_INIT(cred_alloc_blank, apparmor_cred_alloc_blank), LSM_HOOK_INIT(cred_free, apparmor_cred_free), LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare), diff --git a/security/apparmor/net.c b/security/apparmor/net.c new file mode 100644 index 000000000000..bb24cfa0a164 --- /dev/null +++ b/security/apparmor/net.c @@ -0,0 +1,187 @@ +/* + * AppArmor security module + * + * This file contains AppArmor network mediation + * + * Copyright (C) 1998-2008 Novell/SUSE + * 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 + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#include "include/apparmor.h" +#include "include/audit.h" +#include "include/cred.h" +#include "include/label.h" +#include "include/net.h" +#include "include/policy.h" + +#include "net_names.h" + + +struct aa_sfs_entry aa_sfs_entry_network[] = { + AA_SFS_FILE_STRING("af_mask", AA_SFS_AF_MASK), + { } +}; + +static const char * const net_mask_names[] = { + "unknown", + "send", + "receive", + "unknown", + + "create", + "shutdown", + "connect", + "unknown", + + "setattr", + "getattr", + "setcred", + "getcred", + + "chmod", + "chown", + "chgrp", + "lock", + + "mmap", + "mprot", + "unknown", + "unknown", + + "accept", + "bind", + "listen", + "unknown", + + "setopt", + "getopt", + "unknown", + "unknown", + + "unknown", + "unknown", + "unknown", + "unknown", +}; + + +/* audit callback for net specific fields */ +void audit_net_cb(struct audit_buffer *ab, void *va) +{ + struct common_audit_data *sa = va; + + audit_log_format(ab, " family="); + if (address_family_names[sa->u.net->family]) + audit_log_string(ab, address_family_names[sa->u.net->family]); + else + audit_log_format(ab, "\"unknown(%d)\"", sa->u.net->family); + audit_log_format(ab, " sock_type="); + if (sock_type_names[aad(sa)->net.type]) + audit_log_string(ab, sock_type_names[aad(sa)->net.type]); + else + audit_log_format(ab, "\"unknown(%d)\"", aad(sa)->net.type); + audit_log_format(ab, " protocol=%d", aad(sa)->net.protocol); + + if (aad(sa)->request & NET_PERMS_MASK) { + audit_log_format(ab, " requested_mask="); + aa_audit_perm_mask(ab, aad(sa)->request, NULL, 0, + net_mask_names, NET_PERMS_MASK); + + if (aad(sa)->denied & NET_PERMS_MASK) { + audit_log_format(ab, " denied_mask="); + aa_audit_perm_mask(ab, aad(sa)->denied, NULL, 0, + net_mask_names, NET_PERMS_MASK); + } + } + if (aad(sa)->peer) { + audit_log_format(ab, " peer="); + aa_label_xaudit(ab, labels_ns(aad(sa)->label), aad(sa)->peer, + FLAGS_NONE, GFP_ATOMIC); + } +} + +/* Generic af perm */ +int aa_profile_af_perm(struct aa_profile *profile, struct common_audit_data *sa, + u32 request, u16 family, int type) +{ + struct aa_perms perms = { }; + unsigned int state; + __be16 buffer[2]; + + AA_BUG(family >= AF_MAX); + AA_BUG(type < 0 || type >= SOCK_MAX); + + if (profile_unconfined(profile)) + return 0; + state = PROFILE_MEDIATES(profile, AA_CLASS_NET); + if (!state) + return 0; + + buffer[0] = cpu_to_be16(family); + buffer[1] = cpu_to_be16((u16) type); + state = aa_dfa_match_len(profile->policy.dfa, state, (char *) &buffer, + 4); + aa_compute_perms(profile->policy.dfa, state, &perms); + aa_apply_modes_to_perms(profile, &perms); + + return aa_check_perms(profile, &perms, request, sa, audit_net_cb); +} + +int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family, + int type, int protocol) +{ + struct aa_profile *profile; + DEFINE_AUDIT_NET(sa, op, NULL, family, type, protocol); + + return fn_for_each_confined(label, profile, + aa_profile_af_perm(profile, &sa, request, family, + type)); +} + +static int aa_label_sk_perm(struct aa_label *label, const char *op, u32 request, + struct sock *sk) +{ + struct aa_profile *profile; + DEFINE_AUDIT_SK(sa, op, sk); + + AA_BUG(!label); + AA_BUG(!sk); + + if (unconfined(label)) + return 0; + + return fn_for_each_confined(label, profile, + aa_profile_af_sk_perm(profile, &sa, request, sk)); +} + +int aa_sk_perm(const char *op, u32 request, struct sock *sk) +{ + struct aa_label *label; + int error; + + AA_BUG(!sk); + AA_BUG(in_interrupt()); + + /* TODO: switch to begin_current_label ???? */ + label = begin_current_label_crit_section(); + error = aa_label_sk_perm(label, op, request, sk); + end_current_label_crit_section(label); + + return error; +} + + +int aa_sock_file_perm(struct aa_label *label, const char *op, u32 request, + struct socket *sock) +{ + AA_BUG(!label); + AA_BUG(!sock); + AA_BUG(!sock->sk); + + return aa_label_sk_perm(label, op, request, sock->sk); +} diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 8a31ddd474d7..b9e6b2cafa69 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -37,7 +37,8 @@ #define v5 5 /* base version */ #define v6 6 /* per entry policydb mediation check */ -#define v7 7 /* full network masking */ +#define v7 7 +#define v8 8 /* full network masking */ /* * The AppArmor interface treats data as a type byte followed by the -- cgit v1.2.3-58-ga151 From b9590ad4c4f2fedc364016613f2af74ea7758bea Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 3 Mar 2018 01:59:02 -0800 Subject: apparmor: remove POLICY_MEDIATES_SAFE The unpack code now makes sure every profile has a dfa so the safe version of POLICY_MEDIATES is no longer needed. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 2 +- security/apparmor/include/policy.h | 12 +----------- 2 files changed, 2 insertions(+), 12 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 10d16e3abed9..701cb3e5ec3b 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -619,7 +619,7 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms, tmp = aa_compute_fperms(dfa, state, &cond); } } else if (profile->policy.dfa) { - if (!PROFILE_MEDIATES_SAFE(profile, *match_str)) + if (!PROFILE_MEDIATES(profile, *match_str)) return; /* no change to current perms */ dfa = profile->policy.dfa; state = aa_dfa_match_len(dfa, profile->policy.start[0], diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index ffe12a2366e0..ab64c6b5db5a 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -214,17 +214,7 @@ static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p) return labels_profile(aa_get_newest_label(&p->label)); } -#define PROFILE_MEDIATES(P, T) ((P)->policy.start[(T)]) -/* safe version of POLICY_MEDIATES for full range input */ -static inline unsigned int PROFILE_MEDIATES_SAFE(struct aa_profile *profile, - unsigned char class) -{ - if (profile->policy.dfa) - return aa_dfa_match_len(profile->policy.dfa, - profile->policy.start[0], &class, 1); - return 0; -} - +#define PROFILE_MEDIATES(P, T) ((P)->policy.start[(unsigned char) (T)]) static inline unsigned int PROFILE_MEDIATES_AF(struct aa_profile *profile, u16 AF) { unsigned int state = PROFILE_MEDIATES(profile, AA_CLASS_NET); -- cgit v1.2.3-58-ga151 From 1180b4c757aab5506f1be367000364dd5cf5cd02 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 15 Mar 2018 22:31:38 -0700 Subject: apparmor: fix dangling symlinks to policy rawdata after replacement When policy replacement occurs the symlinks in the profile directory need to be updated to point to the new rawdata, otherwise once the old rawdata is removed the symlink becomes broken. Fix this by dynamically generating the symlink everytime it is read. These links are used enough that their value needs to be cached and this way we can avoid needing locking to read and update the link value. Fixes: a481f4d917835 ("apparmor: add custom apparmorfs that will be used by policy namespace files") BugLink: http://bugs.launchpad.net/bugs/1755563 Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 126 +++++++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 31 deletions(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 701cb3e5ec3b..62301ddbbe5e 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -310,6 +310,7 @@ static struct dentry *aafs_create_dir(const char *name, struct dentry *parent) * @name: name of dentry to create * @parent: parent directory for this dentry * @target: if symlink, symlink target string + * @private: private data * @iops: struct of inode_operations that should be used * * If @target parameter is %NULL, then the @iops parameter needs to be @@ -318,17 +319,17 @@ static struct dentry *aafs_create_dir(const char *name, struct dentry *parent) static struct dentry *aafs_create_symlink(const char *name, struct dentry *parent, const char *target, + void *private, const struct inode_operations *iops) { struct dentry *dent; char *link = NULL; if (target) { - link = kstrdup(target, GFP_KERNEL); if (!link) return ERR_PTR(-ENOMEM); } - dent = aafs_create(name, S_IFLNK | 0444, parent, NULL, link, NULL, + dent = aafs_create(name, S_IFLNK | 0444, parent, private, link, NULL, iops); if (IS_ERR(dent)) kfree(link); @@ -1479,26 +1480,95 @@ static int profile_depth(struct aa_profile *profile) return depth; } -static int gen_symlink_name(char *buffer, size_t bsize, int depth, - const char *dirname, const char *fname) +static char *gen_symlink_name(int depth, const char *dirname, const char *fname) { + char *buffer, *s; int error; + int size = depth * 6 + strlen(dirname) + strlen(fname) + 11; + + s = buffer = kmalloc(size, GFP_KERNEL); + if (!buffer) + return ERR_PTR(-ENOMEM); for (; depth > 0; depth--) { - if (bsize < 7) - return -ENAMETOOLONG; - strcpy(buffer, "../../"); - buffer += 6; - bsize -= 6; + strcpy(s, "../../"); + s += 6; + size -= 6; } - error = snprintf(buffer, bsize, "raw_data/%s/%s", dirname, fname); - if (error >= bsize || error < 0) - return -ENAMETOOLONG; + error = snprintf(s, size, "raw_data/%s/%s", dirname, fname); + if (error >= size || error < 0) + return ERR_PTR(-ENAMETOOLONG); - return 0; + return buffer; +} + +static void rawdata_link_cb(void *arg) +{ + kfree(arg); +} + +static const char *rawdata_get_link_base(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done, + const char *name) +{ + struct aa_proxy *proxy = inode->i_private; + struct aa_label *label; + struct aa_profile *profile; + char *target; + int depth; + + if (!dentry) + return ERR_PTR(-ECHILD); + + label = aa_get_label_rcu(&proxy->label); + profile = labels_profile(label); + depth = profile_depth(profile); + target = gen_symlink_name(depth, profile->rawdata->name, name); + aa_put_label(label); + + if (IS_ERR(target)) + return target; + + set_delayed_call(done, rawdata_link_cb, target); + + return target; } +static const char *rawdata_get_link_sha1(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "sha1"); +} + +static const char *rawdata_get_link_abi(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "abi"); +} + +static const char *rawdata_get_link_data(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "raw_data"); +} + +static const struct inode_operations rawdata_link_sha1_iops = { + .get_link = rawdata_get_link_sha1, +}; + +static const struct inode_operations rawdata_link_abi_iops = { + .get_link = rawdata_get_link_abi, +}; +static const struct inode_operations rawdata_link_data_iops = { + .get_link = rawdata_get_link_data, +}; + + /* * Requires: @profile->ns->lock held */ @@ -1569,34 +1639,28 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent) } if (profile->rawdata) { - char target[64]; - int depth = profile_depth(profile); - - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "sha1"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_sha1", dir, target, NULL); + dent = aafs_create_symlink("raw_sha1", dir, NULL, + profile->label.proxy, + &rawdata_link_sha1_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_HASH] = dent; - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "abi"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_abi", dir, target, NULL); + dent = aafs_create_symlink("raw_abi", dir, NULL, + profile->label.proxy, + &rawdata_link_abi_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_ABI] = dent; - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "raw_data"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_data", dir, target, NULL); + dent = aafs_create_symlink("raw_data", dir, NULL, + profile->label.proxy, + &rawdata_link_data_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_DATA] = dent; } -- cgit v1.2.3-58-ga151 From 588558eb6d0e0b6edfa65a67e906c2ffeba63ff1 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 27 Mar 2018 14:35:58 +0100 Subject: apparmor: fix memory leak on buffer on error exit path Currently on the error exit path the allocated buffer is not free'd causing a memory leak. Fix this by kfree'ing it. Detected by CoverityScan, CID#1466876 ("Resource leaks") Fixes: 1180b4c757aa ("apparmor: fix dangling symlinks to policy rawdata after replacement") Signed-off-by: Colin Ian King Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'security/apparmor/apparmorfs.c') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 62301ddbbe5e..f4308683c0af 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -1497,8 +1497,10 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname) } error = snprintf(s, size, "raw_data/%s/%s", dirname, fname); - if (error >= size || error < 0) + if (error >= size || error < 0) { + kfree(buffer); return ERR_PTR(-ENAMETOOLONG); + } return buffer; } -- cgit v1.2.3-58-ga151