Re: [PATCH RFC v11 3/19] ipe: add evaluation loop

From: Fan Wu
Date: Wed Oct 25 2023 - 20:15:34 EST




On 10/23/2023 8:52 PM, Paul Moore wrote:
On Oct 4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:

IPE must have a centralized function to evaluate incoming callers
against IPE's policy. This iteration of the policy for against the rules
for that specific caller is known as the evaluation loop.

Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
...
---
security/ipe/Makefile | 1 +
security/ipe/eval.c | 96 +++++++++++++++++++++++++++++++++++++++++++
security/ipe/eval.h | 24 +++++++++++
3 files changed, 121 insertions(+)
create mode 100644 security/ipe/eval.c
create mode 100644 security/ipe/eval.h

...

diff --git a/security/ipe/eval.c b/security/ipe/eval.c
new file mode 100644
index 000000000000..5533c359bbeb
--- /dev/null
+++ b/security/ipe/eval.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include <linux/fs.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/file.h>
+#include <linux/sched.h>
+#include <linux/rcupdate.h>
+
+#include "ipe.h"
+#include "eval.h"
+#include "policy.h"
+
+struct ipe_policy __rcu *ipe_active_policy;
+
+/**
+ * evaluate_property - Analyze @ctx against a property.
+ * @ctx: Supplies a pointer to the context to be evaluated.
+ * @p: Supplies a pointer to the property to be evaluated.
+ *
+ * Return:
+ * * true - The current @ctx match the @p
+ * * false - The current @ctx doesn't match the @p
+ */
+static bool evaluate_property(const struct ipe_eval_ctx *const ctx,
+ struct ipe_prop *p)
+{
+ return false;
+}
+
+/**
+ * ipe_evaluate_event - Analyze @ctx against the current active policy.
+ * @ctx: Supplies a pointer to the context to be evaluated.
+ *
+ * This is the loop where all policy evaluation happens against IPE policy.
+ *
+ * Return:
+ * * 0 - OK
+ * * -EACCES - @ctx did not pass evaluation.
+ * * !0 - Error
+ */
+int ipe_evaluate_event(const struct ipe_eval_ctx *const ctx)
+{
+ bool match = false;
+ enum ipe_action_type action;
+ struct ipe_policy *pol = NULL;
+ const struct ipe_rule *rule = NULL;
+ const struct ipe_op_table *rules = NULL;
+ struct ipe_prop *prop = NULL;
+
+ rcu_read_lock();
+
+ pol = rcu_dereference(ipe_active_policy);
+ if (!pol) {
+ rcu_read_unlock();
+ return 0;
+ }
+
+ if (ctx->op == IPE_OP_INVALID) {
+ rcu_read_unlock();
+ if (pol->parsed->global_default_action == IPE_ACTION_DENY)
+ return -EACCES;

Assuming that the RCU lock protects @pol, shouldn't it be held until
after the global_default_action comparison?

Yes for this part the unlock should be moved after the comparison. Thanks for spotting this.

+ return 0;
+ }
+
+ rules = &pol->parsed->rules[ctx->op];
+
+ list_for_each_entry(rule, &rules->rules, next) {
+ match = true;
+
+ list_for_each_entry(prop, &rule->props, next) {
+ match = match && evaluate_property(ctx, prop);

The @match variable will always be true on the right side above, or am
I missing something?

Yes the "match &&" are completely unnecessary. I will remove them.

-Fan
+ if (!match)
+ break;
+ }
+
+ if (match)
+ break;
+ }
+
+ if (match)
+ action = rule->action;
+ else if (rules->default_action != IPE_ACTION_INVALID)
+ action = rules->default_action;
+ else
+ action = pol->parsed->global_default_action;
+
+ rcu_read_unlock();
+ if (action == IPE_ACTION_DENY)
+ return -EACCES;
+
+ return 0;
+}

--
paul-moore.com