From: Deven Bowers <[email protected]>
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 <[email protected]>
Signed-off-by: Fan Wu <[email protected]>
---
v2:
+ Split evaluation loop, access control hooks, and evaluation loop from policy parser and userspace interface to pass mailing list character limit
v3:
+ Move ipe_load_properties to patch 04.
+ Remove useless 0-initializations Prefix extern variables with ipe_
+ Remove kernel module parameters, as these are exposed through sysctls.
+ Add more prose to the IPE base config option help text.
+ Use GFP_KERNEL for audit_log_start.
+ Remove unnecessary caching system.
+ Remove comments from headers
+ Use rcu_access_pointer for rcu-pointer null check
+ Remove usage of reqprot; use prot only.
+Move policy load and activation audit event to 03/12
v4:
+ Remove sysctls in favor of securityfs nodes
+ Re-add kernel module parameters, as these are now exposed through securityfs.
+ Refactor property audit loop to a separate function.
v5:
+ fix minor grammatical errors
+ do not group rule by curly-brace in audit record,
+ reconstruct the exact rule.
v6:
+ No changes
v7:
+ Further split lsm creation into a separate commit from the evaluation loop and audit system, for easier review.
+ Propagating changes to support the new ipe_context structure in the evaluation loop.
v8:
+ Remove ipe_hook enumeration; hooks can be correlated via syscall record.
v9:
+ Remove ipe_context related code and simplify the evaluation loop.
v10:
+ Split eval part and boot_verified part
v11:
+ Fix code style issues
---
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/Makefile b/security/ipe/Makefile
index c09aec4904f2..57fe922cf1fc 100644
--- a/security/ipe/Makefile
+++ b/security/ipe/Makefile
@@ -6,6 +6,7 @@
#
obj-$(CONFIG_SECURITY_IPE) += \
+ eval.o \
ipe.o \
policy.o \
policy_parser.o \
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;
+ 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);
+ 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;
+}
diff --git a/security/ipe/eval.h b/security/ipe/eval.h
new file mode 100644
index 000000000000..6b434515968f
--- /dev/null
+++ b/security/ipe/eval.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#ifndef _IPE_EVAL_H
+#define _IPE_EVAL_H
+
+#include <linux/file.h>
+#include <linux/types.h>
+
+#include "policy.h"
+
+extern struct ipe_policy __rcu *ipe_active_policy;
+
+struct ipe_eval_ctx {
+ enum ipe_op_type op;
+
+ const struct file *file;
+};
+
+int ipe_evaluate_event(const struct ipe_eval_ctx *const ctx);
+
+#endif /* _IPE_EVAL_H */
--
2.25.1
On Oct 4, 2023 Fan Wu <[email protected]> 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 <[email protected]>
> Signed-off-by: Fan Wu <[email protected]>
> ---
> v2:
> + Split evaluation loop, access control hooks, and evaluation loop from policy parser and userspace interface to pass mailing list character limit
>
> v3:
> + Move ipe_load_properties to patch 04.
> + Remove useless 0-initializations Prefix extern variables with ipe_
> + Remove kernel module parameters, as these are exposed through sysctls.
> + Add more prose to the IPE base config option help text.
> + Use GFP_KERNEL for audit_log_start.
> + Remove unnecessary caching system.
> + Remove comments from headers
> + Use rcu_access_pointer for rcu-pointer null check
> + Remove usage of reqprot; use prot only.
> +Move policy load and activation audit event to 03/12
>
> v4:
> + Remove sysctls in favor of securityfs nodes
> + Re-add kernel module parameters, as these are now exposed through securityfs.
> + Refactor property audit loop to a separate function.
>
> v5:
> + fix minor grammatical errors
> + do not group rule by curly-brace in audit record,
> + reconstruct the exact rule.
>
> v6:
> + No changes
>
> v7:
> + Further split lsm creation into a separate commit from the evaluation loop and audit system, for easier review.
> + Propagating changes to support the new ipe_context structure in the evaluation loop.
>
> v8:
> + Remove ipe_hook enumeration; hooks can be correlated via syscall record.
>
> v9:
> + Remove ipe_context related code and simplify the evaluation loop.
>
> v10:
> + Split eval part and boot_verified part
>
> v11:
> + Fix code style issues
> ---
> 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?
> + 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?
> + 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
On 10/23/2023 8:52 PM, Paul Moore wrote:
> On Oct 4, 2023 Fan Wu <[email protected]> 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 <[email protected]>
>> Signed-off-by: Fan Wu <[email protected]>
...
>> ---
>> 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