| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAKtyLkFg2+8ciy4DM=g+vcTVvuRPNL2SHbN+m9ObErxtYXZYPw@mail.gmail.com>
Date: Wed, 19 Feb 2025 13:49:43 -0800
From: Fan Wu <wufan@...nel.org>
To: Jasjiv Singh <jasjivsingh@...ux.microsoft.com>
Cc: corbet@....net, jmorris@...ei.org, serge@...lyn.com, eparis@...hat.com,
paul@...l-moore.com, linux-doc@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-audit@...hat.com,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] ipe: add errno field to IPE policy load auditing
On Fri, Feb 14, 2025 at 1:42 PM Jasjiv Singh
<jasjivsingh@...ux.microsoft.com> 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.
>
> 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 <jasjivsingh@...ux.microsoft.com>
> ---
> 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.
> #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.
> 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
>
>
Powered by blists - more mailing lists