Thread (11 messages) 11 messages, 2 authors, 2025-03-06

Re: [RFC PATCH] ipe: add errno field to IPE policy load auditing

From: Fan Wu <wufan@kernel.org>
Date: 2025-02-19 21:49:54
Also in: linux-doc, lkml

On Fri, Feb 14, 2025 at 1:42 PM Jasjiv Singh
[off-list ref] wrote:
...
AUDIT_IPE_POLICY_LOAD(1422):

audit: AUDIT1422 policy_name="boot_verified" policy_version=0.0.0
  policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F2676
  auid=4294967295 ses=4294967295 lsm=ipe res=1 errno=0
The above record shows a new policy has been successfully loaded into
the kernel with the policy name, version, and hash with the errno=0.

AUDIT_IPE_POLICY_LOAD(1422) with error:

audit: AUDIT1422 policy_name="boot_verified" policy_version=0.0.0
  policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F2676
  auid=4294967295 ses=4294967295 lsm=ipe res=0 errno=-74
This doesn't seem to be right in the error case, I suggest copying a
real record from a running system.
quoted hunk ↗ jump to hunk
The above record shows a policy load failure due to an invalid policy.

By adding this error field, we ensure that all policy load attempts,
whether successful or failed, are logged, providing a comprehensive
audit trail for IPE policy management.

Signed-off-by: Jasjiv Singh <redacted>
---
 Documentation/admin-guide/LSM/ipe.rst | 17 ++++++++++++-----
 security/ipe/audit.c                  | 17 ++++++++++++++---
 security/ipe/policy.c                 |  4 +++-
 3 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index f93a467db628..2143165f48c9 100644
--- a/Documentation/admin-guide/LSM/ipe.rst
+++ b/Documentation/admin-guide/LSM/ipe.rst
@@ -423,7 +423,7 @@ Field descriptions:

 Event Example::

-   type=1422 audit(1653425529.927:53): policy_name="boot_verified" policy_version=0.0.0 policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F26765076DD8EED7B8F4DB auid=4294967295 ses=4294967295 lsm=ipe res=1
+   type=1422 audit(1653425529.927:53): policy_name="boot_verified" policy_version=0.0.0 policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F26765076DD8EED7B8F4DB auid=4294967295 ses=4294967295 lsm=ipe res=1 errno=0
    type=1300 audit(1653425529.927:53): arch=c000003e syscall=1 success=yes exit=2567 a0=3 a1=5596fcae1fb0 a2=a07 a3=2 items=0 ppid=184 pid=229 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=4294967295 comm="python3" exe="/usr/bin/python3.10" key=(null)
    type=1327 audit(1653425529.927:53): PROCTITLE proctitle=707974686F6E3300746573742F6D61696E2E7079002D66002E2E
@@ -436,11 +436,11 @@ Field descriptions:
 +----------------+------------+-----------+---------------------------------------------------+
 | Field          | Value Type | Optional? | Description of Value                              |
 +================+============+===========+===================================================+
-| policy_name    | string     | No        | The policy_name                                   |
+| policy_name    | string     | Yes       | The policy_name                                   |
 +----------------+------------+-----------+---------------------------------------------------+
-| policy_version | string     | No        | The policy_version                                |
+| policy_version | string     | Yes       | The policy_version                                |
 +----------------+------------+-----------+---------------------------------------------------+
-| policy_digest  | string     | No        | The policy hash                                   |
+| policy_digest  | string     | Yes       | The policy hash                                   |
 +----------------+------------+-----------+---------------------------------------------------+
 | auid           | integer    | No        | The login user ID                                 |
 +----------------+------------+-----------+---------------------------------------------------+
@@ -450,7 +450,14 @@ Field descriptions:
 +----------------+------------+-----------+---------------------------------------------------+
 | res            | integer    | No        | The result of the audited operation(success/fail) |
 +----------------+------------+-----------+---------------------------------------------------+
-
+| errno          | integer    | No        | The result of the policy error as follows:        |
+|                |            |           |                                                   |
+|                |            |           | +  0: no error                                    |
+|                |            |           | +  -EBADMSG: policy is invalid                    |
+|                |            |           | +  -ENOMEM: out of memory (OOM)                   |
+|                |            |           | +  -ERANGE: policy version number overflow        |
+|                |            |           | +  -EINVAL: policy version parsing error          |
++----------------+------------+-----------+---------------------------------------------------+

 1404 AUDIT_MAC_STATUS
 ^^^^^^^^^^^^^^^^^^^^^
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
index f05f0caa4850..f810f7004498 100644
--- a/security/ipe/audit.c
+++ b/security/ipe/audit.c
@@ -21,6 +21,8 @@

 #define AUDIT_POLICY_LOAD_FMT "policy_name=\"%s\" policy_version=%hu.%hu.%hu "\
                              "policy_digest=" IPE_AUDIT_HASH_ALG ":"
+#define AUDIT_POLICY_LOAD_NULL_FMT "policy_name=? policy_version=? "\
+                                  "policy_digest=?"
How about AUDIT_POLICY_LOAD_FAIL_FMT instead.
quoted hunk ↗ jump to hunk
 #define AUDIT_OLD_ACTIVE_POLICY_FMT "old_active_pol_name=\"%s\" "\
                                    "old_active_pol_version=%hu.%hu.%hu "\
                                    "old_policy_digest=" IPE_AUDIT_HASH_ALG ":"
@@ -253,6 +255,8 @@ void ipe_audit_policy_activation(const struct ipe_policy *const op,
  */
 void ipe_audit_policy_load(const struct ipe_policy *const p)
 {
+       int res = 0;
+       int err = 0;
I would try to avoid using these two variables since this function is
fairly short. Also please use Reverse XMAS tree declarations in
future.
quoted hunk ↗ jump to hunk
        struct audit_buffer *ab;

        ab = audit_log_start(audit_context(), GFP_KERNEL,
@@ -260,10 +264,17 @@ void ipe_audit_policy_load(const struct ipe_policy *const p)
        if (!ab)
                return;

-       audit_policy(ab, AUDIT_POLICY_LOAD_FMT, p);
-       audit_log_format(ab, " auid=%u ses=%u lsm=ipe res=1",
+       if (!IS_ERR(p)) {
+               audit_policy(ab, AUDIT_POLICY_LOAD_FMT, p);
+               res = 1;
+       } else {
+               audit_log_format(ab, AUDIT_POLICY_LOAD_NULL_FMT);
+               err = PTR_ERR(p);
+       }
+
+       audit_log_format(ab, " auid=%u ses=%u lsm=ipe res=%d errno=%d",
                         from_kuid(&init_user_ns, audit_get_loginuid(current)),
-                        audit_get_sessionid(current));
+                        audit_get_sessionid(current), res, err);

        audit_log_end(ab);
 }
diff --git a/security/ipe/policy.c b/security/ipe/policy.c
index b628f696e32b..0f616e9fbe61 100644
--- a/security/ipe/policy.c
+++ b/security/ipe/policy.c
@@ -202,7 +202,9 @@ struct ipe_policy *ipe_new_policy(const char *text, size_t textlen,
        return new;
 err:
        ipe_free_policy(new);
-       return ERR_PTR(rc);
+       new = ERR_PTR(rc);
+       ipe_audit_policy_load(new);
+       return new;
Auditing failure here is not correct. ipe_new_policy() can succeed
while the following security fs nodes creation can fail. Similarly
insufficient permission error is not audited.
I suggest auditing the failure cases in new_policy() and update_policy().

-Fan
 }

 /**
--
2.34.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help