diff mbox series

[RFC,5/5] apparmor: parse profiles in sandbox mode

Message ID 20240222131230.635-6-petrtesarik@huaweicloud.com (mailing list archive)
State RFC
Headers show
Series PoC: convert AppArmor parser to SandBox Mode | expand

Commit Message

Petr Tesarik Feb. 22, 2024, 1:12 p.m. UTC 
From: Petr Tesarik <petr.tesarik1@huawei-partners.com>

Run aa_unpack() in sandbox mode. Map the input data read-only and then walk
the resulting list created in sandbox mode.

Hashes are calculated in kernel mode, because crypto routines are not
sandboxed. The fixups should sanitize the parameters of AppArmor functions
and they should be defined close to the target functions. Both requires
extending the generic API and adding some more arch hooks, which would grow
this patch series too much.

For demonstration purposes, the fixups blindly trust the input from sandbox
mode and are hard-wired in the arch code.

Signed-off-by: Petr Tesarik <petr.tesarik1@huawei-partners.com>
---
 arch/x86/kernel/sbm/core.c | 15 +++++++++++++++
 security/apparmor/crypto.c |  7 ++++---
 security/apparmor/policy.c | 29 ++++++++++++++++++++++-------
 security/apparmor/secid.c  |  3 ++-
 4 files changed, 43 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/arch/x86/kernel/sbm/core.c b/arch/x86/kernel/sbm/core.c
index 3cf3842292b9..3268c00da873 100644
--- a/arch/x86/kernel/sbm/core.c
+++ b/arch/x86/kernel/sbm/core.c
@@ -40,6 +40,16 @@  extern char __nosbm_text_start[], __nosbm_text_end[];
  * HACK: PROOF-OF-CONCEPT FIXUP CODE STARTS HERE
  */
 
+/*
+ * HACK: These declarations are needed to make a proxy call, but in the
+ * final version, AppArmor itself will define the proxies. At least, do
+ * not make the functions callable from here. All we need are their
+ * entry point addresses.
+ */
+extern char aa_alloc_secid[];
+extern char aa_calc_hash[];
+extern char aa_calc_profile_hash[];
+
 typedef unsigned long (*sbm_proxy_call_fn)(struct x86_sbm_state *,
 					   unsigned long func,
 					   struct pt_regs *);
@@ -135,6 +145,11 @@  static const struct sbm_fixup fixups[] =
 	{ vzalloc, proxy_alloc1 },
 	{ vfree, proxy_free },
 
+	/* AppArmor */
+	{ aa_alloc_secid, x86_sbm_proxy_call },
+	{ aa_calc_hash, x86_sbm_proxy_call },
+	{ aa_calc_profile_hash, x86_sbm_proxy_call },
+
 	{ }
 };
 
diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
index aad486b2fca6..db349cd4e467 100644
--- a/security/apparmor/crypto.c
+++ b/security/apparmor/crypto.c
@@ -12,6 +12,7 @@ 
  */
 
 #include <crypto/hash.h>
+#include <linux/sbm.h>
 
 #include "include/apparmor.h"
 #include "include/crypto.h"
@@ -25,7 +26,7 @@  unsigned int aa_hash_size(void)
 	return apparmor_hash_size;
 }
 
-char *aa_calc_hash(void *data, size_t len)
+char * __nosbm aa_calc_hash(void *data, size_t len)
 {
 	SHASH_DESC_ON_STACK(desc, apparmor_tfm);
 	char *hash;
@@ -58,8 +59,8 @@  char *aa_calc_hash(void *data, size_t len)
 	return ERR_PTR(error);
 }
 
-int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
-			 size_t len)
+int __nosbm aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
+				 size_t len)
 {
 	SHASH_DESC_ON_STACK(desc, apparmor_tfm);
 	int error;
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 957654d253dd..f2b9bf851be0 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -74,6 +74,7 @@ 
 #include <linux/cred.h>
 #include <linux/rculist.h>
 #include <linux/user_namespace.h>
+#include <linux/sbm.h>
 
 #include "include/apparmor.h"
 #include "include/capability.h"
@@ -1040,6 +1041,11 @@  static struct aa_profile *update_to_newest_parent(struct aa_profile *new)
 	return newest;
 }
 
+static SBM_DEFINE_CALL(aa_unpack, struct aa_loaddata *, udata,
+		       struct list_head *, lh, const char **, ns);
+static SBM_DEFINE_THUNK(aa_unpack, struct aa_loaddata *, udata,
+		       struct list_head *, lh, const char **, ns);
+
 /**
  * aa_replace_profiles - replace profile(s) on the profile list
  * @policy_ns: namespace load is occurring on
@@ -1063,12 +1069,20 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 	struct aa_loaddata *rawdata_ent;
 	const char *op;
 	ssize_t count, error;
-	LIST_HEAD(lh);
+	struct list_head lh, *sbm_lh;
+	struct sbm sbm;
 
 	op = mask & AA_MAY_REPLACE_POLICY ? OP_PROF_REPL : OP_PROF_LOAD;
 	aa_get_loaddata(udata);
 	/* released below */
-	error = aa_unpack(udata, &lh, &ns_name);
+	sbm_init(&sbm);
+	SBM_MAP_READONLY(&sbm, udata->data, udata->size);
+	/* TODO: Handling of list heads could be improved */
+	sbm_lh = SBM_COPY_OUT(&sbm, &lh, sizeof(lh));
+	INIT_LIST_HEAD(sbm_lh);
+	error = sbm_call(&sbm, aa_unpack,
+			 SBM_COPY_INOUT(&sbm, udata, sizeof(*udata)), sbm_lh,
+			 SBM_COPY_OUT(&sbm, &ns_name, sizeof(ns_name)));
 	if (error)
 		goto out;
 
@@ -1078,7 +1092,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 	 *       fail. Sort ent list and take ns locks in hierarchy order
 	 */
 	count = 0;
-	list_for_each_entry(ent, &lh, list) {
+	list_for_each_entry(ent, sbm_lh, list) {
 		if (ns_name) {
 			if (ent->ns_name &&
 			    strcmp(ent->ns_name, ns_name) != 0) {
@@ -1128,7 +1142,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 		}
 	}
 	/* setup parent and ns info */
-	list_for_each_entry(ent, &lh, list) {
+	list_for_each_entry(ent, sbm_lh, list) {
 		struct aa_policy *policy;
 		struct aa_profile *p;
 
@@ -1159,7 +1173,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 		policy = __lookup_parent(ns, ent->new->base.hname);
 		if (!policy) {
 			/* first check for parent in the load set */
-			p = __list_lookup_parent(&lh, ent->new);
+			p = __list_lookup_parent(sbm_lh, ent->new);
 			if (!p) {
 				/*
 				 * fill in missing parent with null
@@ -1198,7 +1212,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 			goto fail_lock;
 		}
 	}
-	list_for_each_entry(ent, &lh, list) {
+	list_for_each_entry(ent, sbm_lh, list) {
 		if (!ent->old) {
 			struct dentry *parent;
 			if (rcu_access_pointer(ent->new->parent)) {
@@ -1220,7 +1234,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 	__aa_bump_ns_revision(ns);
 	if (aa_g_export_binary)
 		__aa_loaddata_update(udata, ns->revision);
-	list_for_each_entry_safe(ent, tmp, &lh, list) {
+	list_for_each_entry_safe(ent, tmp, sbm_lh, list) {
 		list_del_init(&ent->list);
 		op = (!ent->old && !ent->rename) ? OP_PROF_LOAD : OP_PROF_REPL;
 
@@ -1265,6 +1279,7 @@  ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
 	mutex_unlock(&ns->lock);
 
 out:
+	sbm_destroy(&sbm);
 	aa_put_ns(ns);
 	aa_put_loaddata(udata);
 	kfree(ns_name);
diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
index 83d3d1e6d9dc..4190666d2dee 100644
--- a/security/apparmor/secid.c
+++ b/security/apparmor/secid.c
@@ -16,6 +16,7 @@ 
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/xarray.h>
+#include <linux/sbm.h>
 
 #include "include/cred.h"
 #include "include/lib.h"
@@ -116,7 +117,7 @@  void apparmor_release_secctx(char *secdata, u32 seclen)
  * Returns: 0 with @label->secid initialized
  *          <0 returns error with @label->secid set to AA_SECID_INVALID
  */
-int aa_alloc_secid(struct aa_label *label, gfp_t gfp)
+int __nosbm aa_alloc_secid(struct aa_label *label, gfp_t gfp)
 {
 	unsigned long flags;
 	int ret;