Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing
On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh 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 > --- > 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. -Fan .
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 wrote: > On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh > 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 > > --- > > 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
[PATCH v2] audit,module: restore audit logging in load failure case
The move of the module sanity check to earlier skipped the audit logging
call in the case of failure and to a place where the previously used
context is unavailable.
Add an audit logging call for the module loading failure case and get
the module name when possible.
Link: https://issues.redhat.com/browse/RHEL-52839
Fixes: 02da2cbab452 ("module: move check_modinfo() early to early_mod_check()")
Signed-off-by: Richard Guy Briggs
---
Changelog:
v2
- use info->name for both audit_log_kern_module() calls and add const
---
include/linux/audit.h | 9 -
kernel/audit.h| 2 +-
kernel/auditsc.c | 2 +-
kernel/module/main.c | 6 --
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 0050ef288ab3..a394614ccd0b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -417,7 +417,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
extern void __audit_log_capset(const struct cred *new, const struct cred *old);
extern void __audit_mmap_fd(int fd, int flags);
extern void __audit_openat2_how(struct open_how *how);
-extern void __audit_log_kern_module(char *name);
+extern void __audit_log_kern_module(const char *name);
extern void __audit_fanotify(u32 response, struct
fanotify_response_info_audit_rule *friar);
extern void __audit_tk_injoffset(struct timespec64 offset);
extern void __audit_ntp_log(const struct audit_ntp_data *ad);
@@ -519,7 +519,7 @@ static inline void audit_openat2_how(struct open_how *how)
__audit_openat2_how(how);
}
-static inline void audit_log_kern_module(char *name)
+static inline void audit_log_kern_module(const char *name)
{
if (!audit_dummy_context())
__audit_log_kern_module(name);
@@ -677,9 +677,8 @@ static inline void audit_mmap_fd(int fd, int flags)
static inline void audit_openat2_how(struct open_how *how)
{ }
-static inline void audit_log_kern_module(char *name)
-{
-}
+static inline void audit_log_kern_module(const char *name)
+{ }
static inline void audit_fanotify(u32 response, struct
fanotify_response_info_audit_rule *friar)
{ }
diff --git a/kernel/audit.h b/kernel/audit.h
index 0211cb307d30..2a24d01c5fb0 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -200,7 +200,7 @@ struct audit_context {
int argc;
} execve;
struct {
- char*name;
+ const char *name;
} module;
struct {
struct audit_ntp_data ntp_data;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9c853cde9abe..7bc0462e86f3 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2866,7 +2866,7 @@ void __audit_openat2_how(struct open_how *how)
context->type = AUDIT_OPENAT2;
}
-void __audit_log_kern_module(char *name)
+void __audit_log_kern_module(const char *name)
{
struct audit_context *context = audit_context();
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 1fb9ad289a6f..efa62ace1b23 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3346,7 +3346,7 @@ static int load_module(struct load_info *info, const char
__user *uargs,
module_allocated = true;
- audit_log_kern_module(mod->name);
+ audit_log_kern_module(info->name);
/* Reserve our place in the list. */
err = add_unformed_module(mod);
@@ -3506,8 +3506,10 @@ static int load_module(struct load_info *info, const
char __user *uargs,
* failures once the proper module was allocated and
* before that.
*/
- if (!module_allocated)
+ if (!module_allocated) {
+ audit_log_kern_module(info->name ? info->name :
"(unavailable)");
mod_stat_bump_becoming(info, flags);
+ }
free_copy(info, flags);
return err;
}
--
2.43.5
