linux-kernel - Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhRPU1C1-B=PUwUcheOmYhSDzHJMmcpg3j9z0DPiMOHydg@mail.gmail.com>
Date: Mon, 17 Mar 2025 17:03:51 -0400
From: Paul Moore <paul@...l-moore.com>
To: Fan Wu <wufan@...nel.org>
Cc: Jasjiv Singh <jasjivsingh@...ux.microsoft.com>, corbet@....net, jmorris@...ei.org, 
	serge@...lyn.com, eparis@...hat.com, linux-doc@...r.kernel.org, 
	linux-security-module@...r.kernel.org, audit@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing

On Mon, Mar 17, 2025 at 4:59 PM Fan Wu <wufan@...nel.org> wrote:
> On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh
> <jasjivsingh@...ux.microsoft.com> wrote:
> >
> > Users of IPE require a way to identify when and why an operation fails,
> > allowing them to both respond to violations of policy and be notified
> > of potentially malicious actions on their systems with respect to IPE.
> >
> > This patch introduces a new error field to the AUDIT_IPE_POLICY_LOAD event
> > to log policy loading failures. Currently, IPE only logs successful policy
> > loads, but not failures. Tracking failures is crucial to detect malicious
> > attempts and ensure a complete audit trail for security events.
> >
> > The new error field will capture the following error codes:
> >
> > * -ENOKEY: Key used to sign the IPE policy not found in the keyring
> > * -ESTALE: Attempting to update an IPE policy with an older version
> > * -EKEYREJECTED: IPE signature verification failed
> > * -ENOENT: Policy was deleted while updating
> > * -EEXIST: Same name policy already deployed
> > * -ERANGE: Policy version number overflow
> > * -EINVAL: Policy version parsing error
> > * -EPERM: Insufficient permission
> > * -ENOMEM: Out of memory (OOM)
> > * -EBADMSG: Policy is invalid
> >
> > Here are some examples of the updated audit record types:
> >
> > AUDIT_IPE_POLICY_LOAD(1422):
> > audit:  AUDIT1422 policy_name="Test_Policy" policy_version=0.0.1
> > policy_digest=sha256:84EFBA8FA71E62AE0A537FAB962F8A2BD1053964C4299DCA
> > 92BFFF4DB82E86D3 auid=1000 ses=3 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=? policy_version=? policy_digest=?
> > auid=1000 ses=3 lsm=ipe res=0 errno=-74
> >
> > The above record shows a policy load failure due to an invalid policy
> > (-EBADMSG).
> >
> > 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 | 69 +++++++++++++++++++--------
> >  security/ipe/audit.c                  | 19 ++++++--
> >  security/ipe/fs.c                     | 25 ++++++----
> >  security/ipe/policy.c                 | 17 ++++---
> >  security/ipe/policy_fs.c              | 28 ++++++++---
> >  5 files changed, 113 insertions(+), 45 deletions(-)
> >
>
> ...
>
> > diff --git a/security/ipe/policy.c b/security/ipe/policy.c
> > index b628f696e32b..1c58c29886e8 100644
> > --- a/security/ipe/policy.c
> > +++ b/security/ipe/policy.c
> > @@ -84,8 +84,11 @@ static int set_pkcs7_data(void *ctx, const void *data, size_t len,
> >   * ipe_new_policy.
> >   *
> >   * Context: Requires root->i_rwsem to be held.
> > - * Return: %0 on success. If an error occurs, the function will return
> > - * the -errno.
> > + * Return:
> > + * * %0        - Success
> > + * * %-ENOENT  - Policy was deleted while updating
> > + * * %-EINVAL  - Policy name mismatch
> > + * * %-ESTALE  - Policy version too old
> >   */
> >  int ipe_update_policy(struct inode *root, const char *text, size_t textlen,
> >                       const char *pkcs7, size_t pkcs7len)
> > @@ -146,10 +149,12 @@ int ipe_update_policy(struct inode *root, const char *text, size_t textlen,
> >   *
> >   * Return:
> >   * * a pointer to the ipe_policy structure     - Success
> > - * * %-EBADMSG                                 - Policy is invalid
> > - * * %-ENOMEM                                  - Out of memory (OOM)
> > - * * %-ERANGE                                  - Policy version number overflow
> > - * * %-EINVAL                                  - Policy version parsing error
> > + * * %-EBADMSG                         - Policy is invalid
> > + * * %-ENOMEM                          - Out of memory (OOM)
> > + * * %-ERANGE                          - Policy version number overflow
> > + * * %-EINVAL                          - Policy version parsing error
> > + * * %-ENOKEY                          - Policy signing key not found
> > + * * %-EKEYREJECTED                    - Policy signature verification failed
> >   */
>
> The indentation here is not aligned.
>
> I don't see any other issue, if there is no objection from the audit
> folks, I will pull this into ipe's tree.

No objections from me.

--
paul-moore.com