Re: [PATCH v3] ipe: add errno field to IPE policy load auditing
On 3/4/2025 4:04 PM, Jasjiv Singh wrote: > > > On 3/3/2025 2:11 PM, Fan Wu wrote: >> On Fri, Feb 28, 2025 at 3:11 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: >>> >>> * 0: no error >>> * -EPERM: Insufficient permission >>> * -EEXIST: Same name policy already deployed >>> * -EBADMSG: policy is invalid >>> * -ENOMEM: out of memory (OOM) >>> * -ERANGE: policy version number overflow >>> * -EINVAL: policy version parsing error >>> >> >> These error codes are not exhaustive. We recently introduced the >> secondary keyring and platform keyring to sign policy so the policy >> loading could return -ENOKEY or -EKEYREJECT. And also the update >> policy can return -ESTALE when the policy version is old. >> This is my fault that I forgot we should also update the documentation >> of the newly introduced error codes. Could you please go through the >> whole loading code and find all possible error codes? Also this is a >> good chance to update the current stale function documents. >> >> ... >> > > So, I looked into error codes when the policy loads. In ipe_new_policy, > the verify_pkcs7_signature can return a lot of errno codes (ex: ENOKEY, > EKEYREJECTED, EBADMSG, etc.) while parsing the pkcs7 and other functions > as well. Also, In ipe_new_policyfs_node used in new_policy(), I see the same > issue with securityfs_create_dir and securityfs_create_file as they > return the errno directly from API to. So, what should we return? > > For other functions: I have complied the errno list: > > * -ENOENT: Policy is not found while updating > * -EEXIST: Same name policy already deployed > * -ERANGE: Policy version number overflow > * -EINVAL: Policy version parsing error > * -EPERM: Insufficient permission > * -ESTALE: Policy version is old > * -ENOMEM: Out of memory (OOM) > * -EBADMSG: Policy is invalid > > - Jasjiv > >>> >>> Signed-off-by: Jasjiv Singh >>> --- >>> Documentation/admin-guide/LSM/ipe.rst | 19 ++- >>> security/ipe/audit.c | 15 --- >>> security/ipe/fs.c | 16 +++- >>> security/ipe/policy_fs.c | 18 +- >>> 4 files changed, 50 insertions(+), 18 deletions(-) >>> >>> diff --git a/Documentation/admin-guide/LSM/ipe.rst >>> b/Documentation/admin-guide/LSM/ipe.rst >>> index f93a467db628..5dbf54471fab 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=c03e 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 >>>| >>> >>> +++---+--
[PATCH v1 2/2] audit: record AUDIT_ANOM_* events regardless of presence of rules
When no audit rules are in place, AUDIT_ANOM_{LINK,CREAT} events
reported in audit_log_path_denied() are unconditionally dropped due to
an explicit check for the existence of any audit rules. Given this is a
report of a security violation, allow it to be recorded regardless of
the existence of any audit rules.
To test,
mkdir -p /root/tmp
chmod 1777 /root/tmp
touch /root/tmp/test.txt
useradd test
chown test /root/tmp/test.txt
{echo C0644 12 test.txt; printf 'hello\ntest1\n'; printf \\000;} | \
scp -t /root/tmp
Check with
ausearch -m ANOM_CREAT -ts recent
Link: https://issues.redhat.com/browse/RHEL-9065
Signed-off-by: Richard Guy Briggs
---
kernel/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 53e3bddcc327..0cf2827882fc 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2285,7 +2285,7 @@ void audit_log_path_denied(int type, const char
*operation)
{
struct audit_buffer *ab;
- if (!audit_enabled || audit_dummy_context())
+ if (!audit_enabled)
return;
/* Generate log with subject, operation, outcome. */
--
2.43.5
Re: [PATCH v3] ipe: add errno field to IPE policy load auditing
On Tue, Mar 4, 2025 at 4:04 PM Jasjiv Singh wrote: > > > > On 3/3/2025 2:11 PM, Fan Wu wrote: > > On Fri, Feb 28, 2025 at 3:11 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: > >> > >> * 0: no error > >> * -EPERM: Insufficient permission > >> * -EEXIST: Same name policy already deployed > >> * -EBADMSG: policy is invalid > >> * -ENOMEM: out of memory (OOM) > >> * -ERANGE: policy version number overflow > >> * -EINVAL: policy version parsing error > >> > > > > These error codes are not exhaustive. We recently introduced the > > secondary keyring and platform keyring to sign policy so the policy > > loading could return -ENOKEY or -EKEYREJECT. And also the update > > policy can return -ESTALE when the policy version is old. > > This is my fault that I forgot we should also update the documentation > > of the newly introduced error codes. Could you please go through the > > whole loading code and find all possible error codes? Also this is a > > good chance to update the current stale function documents. > > > > ... > > > > So, I looked into error codes when the policy loads. In ipe_new_policy, > the verify_pkcs7_signature can return a lot of errno codes (ex: ENOKEY, > EKEYREJECTED, EBADMSG, etc.) while parsing the pkcs7 and other functions > as well. Also, In ipe_new_policyfs_node used in new_policy(), I see the same > issue with securityfs_create_dir and securityfs_create_file as they > return the errno directly from API to. So, what should we return? I think the key here is we need to document the errors in the ipe's context. For example, ENOKEY means the key used to sign the ipe policy is not found in the keyring, EKEYREJECTED means ipe signature verification failed. If an error does not have specific meaning for ipe then probably we don't need to document it. > > For other functions: I have complied the errno list: > > * -ENOENT: Policy is not found while updating This one means policy was deleted while updating, this only happens the update happened just after the policy deletion. > * -EEXIST: Same name policy already deployed > * -ERANGE: Policy version number overflow > * -EINVAL: Policy version parsing error > * -EPERM: Insufficient permission > * -ESTALE: Policy version is old Maybe make this one clearer, how about trying to update an ipe policy with an older version policy. -Fan
[PATCH v1 0/2] override audit silence norule for fs cases
The audit subsystem normally suppresses output when there are no rules present to avoid overwhelming the user with unwanted messages. It could be argued that another security subsystem would generally want to override that default. Allow them through for fsnotify and filesystem security violations. Richard Guy Briggs (2): audit: record fanotify event regardless of presence of rules audit: record AUDIT_ANOM_* events regardless of presence of rules include/linux/audit.h | 8 +--- kernel/audit.c| 2 +- kernel/auditsc.c | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) -- 2.43.5
[PATCH v1 1/2] audit: record fanotify event regardless of presence of rules
When no audit rules are in place, fanotify event results are
unconditionally dropped due to an explicit check for the existence of
any audit rules. Given this is a report from another security
sub-system, allow it to be recorded regardless of the existence of any
audit rules.
To test, install and run the fapolicyd daemon with default config. Then
as an unprivileged user, create and run a very simple binary that should
be denied. Then check for an event with
ausearch -m FANOTIFY -ts recent
Link: https://issues.redhat.com/browse/RHEL-1367
Signed-off-by: Richard Guy Briggs
---
include/linux/audit.h | 8 +---
kernel/auditsc.c | 2 +-
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 0050ef288ab3..d0c6f23503a1 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -418,7 +418,7 @@ 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_fanotify(u32 response, struct
fanotify_response_info_audit_rule *friar);
+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);
extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
@@ -525,12 +525,6 @@ static inline void audit_log_kern_module(char *name)
__audit_log_kern_module(name);
}
-static inline void audit_fanotify(u32 response, struct
fanotify_response_info_audit_rule *friar)
-{
- if (!audit_dummy_context())
- __audit_fanotify(response, friar);
-}
-
static inline void audit_tk_injoffset(struct timespec64 offset)
{
/* ignore no-op events */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0627e74585ce..936825114bae 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2880,7 +2880,7 @@ void __audit_log_kern_module(char *name)
context->type = AUDIT_KERN_MODULE;
}
-void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule
*friar)
+void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule
*friar)
{
/* {subj,obj}_trust values are {0,1,2}: no,yes,unknown */
switch (friar->hdr.type) {
--
2.43.5
Re: [PATCH v3] ipe: add errno field to IPE policy load auditing
On 3/5/2025 1:23 PM, Fan Wu wrote:
> On Tue, Mar 4, 2025 at 4:04 PM Jasjiv Singh
> wrote:
>>
>>
>>
>> On 3/3/2025 2:11 PM, Fan Wu wrote:
>>> On Fri, Feb 28, 2025 at 3:11 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:
* 0: no error
* -EPERM: Insufficient permission
* -EEXIST: Same name policy already deployed
* -EBADMSG: policy is invalid
* -ENOMEM: out of memory (OOM)
* -ERANGE: policy version number overflow
* -EINVAL: policy version parsing error
>>>
>>> These error codes are not exhaustive. We recently introduced the
>>> secondary keyring and platform keyring to sign policy so the policy
>>> loading could return -ENOKEY or -EKEYREJECT. And also the update
>>> policy can return -ESTALE when the policy version is old.
>>> This is my fault that I forgot we should also update the documentation
>>> of the newly introduced error codes. Could you please go through the
>>> whole loading code and find all possible error codes? Also this is a
>>> good chance to update the current stale function documents.
>>>
>>> ...
>>>
>>
>> So, I looked into error codes when the policy loads. In ipe_new_policy,
>> the verify_pkcs7_signature can return a lot of errno codes (ex: ENOKEY,
>> EKEYREJECTED, EBADMSG, etc.) while parsing the pkcs7 and other functions
>> as well. Also, In ipe_new_policyfs_node used in new_policy(), I see the same
>> issue with securityfs_create_dir and securityfs_create_file as they
>> return the errno directly from API to. So, what should we return?
>
> I think the key here is we need to document the errors in the ipe's
> context. For example, ENOKEY means the key used to sign the ipe policy
> is not found in the keyring, EKEYREJECTED means ipe signature
> verification failed. If an error does not have specific meaning for
> ipe then probably we don't need to document it.
>
>>
>> For other functions: I have complied the errno list:
>>
>> * -ENOENT: Policy is not found while updating
>
> This one means policy was deleted while updating, this only happens
> the update happened just after the policy deletion.
>
>> * -EEXIST: Same name policy already deployed
>> * -ERANGE: Policy version number overflow
>> * -EINVAL: Policy version parsing error
>> * -EPERM: Insufficient permission
>> * -ESTALE: Policy version is old
>
> Maybe make this one clearer, how about trying to update an ipe policy
> with an older version policy.
>
> -Fan
Thanks, I have compiled the list based on your comments.
* -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
How does that that look? I will update the documentation with this list
in the next patch along with suggestions you mentioned.
Moving the memdup failure discussion here:
>>> diff --git a/security/ipe/fs.c b/security/ipe/fs.c
>>> index 5b6d19fb844a..da51264a1d0f 100644
>>> --- a/security/ipe/fs.c
>>> +++ b/security/ipe/fs.c
>>> @@ -141,12 +141,16 @@ static ssize_t new_policy(struct file *f, const char
>>> __user *data,
>>> char *copy = NULL;
>>> int rc = 0;
>>>
>>> - if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
>>> - return -EPERM;
>>> + if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN)) {
>>> + rc = -EPERM;
>>> + goto out;
>>> + }
>>>
>>> copy = memdup_user_nul(data, len);
>>> - if (IS_ERR(copy))
>>> - return PTR_ERR(copy);
>>> + if (IS_ERR(copy)) {
>>> + rc = PTR_ERR(copy);
>>> + goto out;
>>> + }
>>>
>>> p = ipe_new_policy(NULL, 0, copy, len);
>>> if (IS_ERR(p)) {
>>> @@ -161,8 +165,10 @@ static ssize_t new_policy(struct file *f, const char
>>> __user *data,
>>> ipe_audit_policy_load(p);
>>>
>>> out:
>>> - if (rc < 0)
>>> + if (rc < 0) {
>>> ipe_free_policy(p);
>>> + ipe_audit_policy_load(ERR_PTR(rc));
>>> + }
>>> kfree(copy);
>>> return (rc < 0) ? rc : len;
>>> }
>>
>> In case of mem
Re: [PATCH v3] ipe: add errno field to IPE policy load auditing
On Wed, Mar 5, 2025 at 3:27 PM Jasjiv Singh
wrote:
>
>
>
> On 3/5/2025 1:23 PM, Fan Wu wrote:
> > On Tue, Mar 4, 2025 at 4:04 PM Jasjiv Singh
> > wrote:
> >>
> >>
> >>
> >> On 3/3/2025 2:11 PM, Fan Wu wrote:
> >>> On Fri, Feb 28, 2025 at 3:11 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:
>
> * 0: no error
> * -EPERM: Insufficient permission
> * -EEXIST: Same name policy already deployed
> * -EBADMSG: policy is invalid
> * -ENOMEM: out of memory (OOM)
> * -ERANGE: policy version number overflow
> * -EINVAL: policy version parsing error
>
> >>>
> >>> These error codes are not exhaustive. We recently introduced the
> >>> secondary keyring and platform keyring to sign policy so the policy
> >>> loading could return -ENOKEY or -EKEYREJECT. And also the update
> >>> policy can return -ESTALE when the policy version is old.
> >>> This is my fault that I forgot we should also update the documentation
> >>> of the newly introduced error codes. Could you please go through the
> >>> whole loading code and find all possible error codes? Also this is a
> >>> good chance to update the current stale function documents.
> >>>
> >>> ...
> >>>
> >>
> >> So, I looked into error codes when the policy loads. In ipe_new_policy,
> >> the verify_pkcs7_signature can return a lot of errno codes (ex: ENOKEY,
> >> EKEYREJECTED, EBADMSG, etc.) while parsing the pkcs7 and other functions
> >> as well. Also, In ipe_new_policyfs_node used in new_policy(), I see the
> >> same
> >> issue with securityfs_create_dir and securityfs_create_file as they
> >> return the errno directly from API to. So, what should we return?
> >
> > I think the key here is we need to document the errors in the ipe's
> > context. For example, ENOKEY means the key used to sign the ipe policy
> > is not found in the keyring, EKEYREJECTED means ipe signature
> > verification failed. If an error does not have specific meaning for
> > ipe then probably we don't need to document it.
> >
> >>
> >> For other functions: I have complied the errno list:
> >>
> >> * -ENOENT: Policy is not found while updating
> >
> > This one means policy was deleted while updating, this only happens
> > the update happened just after the policy deletion.
> >
> >> * -EEXIST: Same name policy already deployed
> >> * -ERANGE: Policy version number overflow
> >> * -EINVAL: Policy version parsing error
> >> * -EPERM: Insufficient permission
> >> * -ESTALE: Policy version is old
> >
> > Maybe make this one clearer, how about trying to update an ipe policy
> > with an older version policy.
> >
> > -Fan
>
> Thanks, I have compiled the list based on your comments.
>
> * -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
>
> How does that that look? I will update the documentation with this list
> in the next patch along with suggestions you mentioned.
>
This looks good to me, make sure to also update the function
documentations in the code.
>
> Moving the memdup failure discussion here:
>
> >>> diff --git a/security/ipe/fs.c b/security/ipe/fs.c
> >>> index 5b6d19fb844a..da51264a1d0f 100644
> >>> --- a/security/ipe/fs.c
> >>> +++ b/security/ipe/fs.c
> >>> @@ -141,12 +141,16 @@ static ssize_t new_policy(struct file *f, const
> >>> char __user *data,
> >>> char *copy = NULL;
> >>> int rc = 0;
> >>>
> >>> - if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> >>> - return -EPERM;
> >>> + if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN)) {
> >>> + rc = -EPERM;
> >>> + goto out;
> >>> + }
> >>>
> >>> copy = memdup_user_nul(data, len);
> >>> - if (IS_ERR(copy))
> >>> - return PTR_ERR(copy);
> >>> + if (IS_ERR(copy)) {
> >>> + rc = PTR_ERR(copy);
> >>> + goto out;
> >>> + }
> >>>
> >>> p = ipe_new_policy(NULL, 0, copy, len);
> >>> if (IS_ERR(p)) {
> >>> @
