On Fri, Mar 7, 2025 at 2:07 PM Jasjiv Singh <jasjivsingh@xxxxxxxxxxxxxxxxxxx> wrote: ... > Signed-off-by: Jasjiv Singh <jasjivsingh@xxxxxxxxxxxxxxxxxxx> > --- > Documentation/admin-guide/LSM/ipe.rst | 69 +++++++++++++++++++-------- > security/ipe/audit.c | 21 ++++++-- > security/ipe/fs.c | 19 ++++++-- > security/ipe/policy.c | 11 ++++- > security/ipe/policy_fs.c | 29 ++++++++--- > 5 files changed, 111 insertions(+), 38 deletions(-) > > diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst > index f93a467db628..0615941de6e0 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 > > @@ -433,24 +433,55 @@ This record will always be emitted in conjunction with a ``AUDITSYSCALL`` record > > Field descriptions: > > -+----------------+------------+-----------+---------------------------------------------------+ > -| Field | Value Type | Optional? | Description of Value | > -+================+============+===========+===================================================+ > -| policy_name | string | No | The policy_name | > -+----------------+------------+-----------+---------------------------------------------------+ > -| policy_version | string | No | The policy_version | > -+----------------+------------+-----------+---------------------------------------------------+ > -| policy_digest | string | No | The policy hash | > -+----------------+------------+-----------+---------------------------------------------------+ > -| auid | integer | No | The login user ID | > -+----------------+------------+-----------+---------------------------------------------------+ > -| ses | integer | No | The login session ID | > -+----------------+------------+-----------+---------------------------------------------------+ > -| lsm | string | No | The lsm name associated with the event | > -+----------------+------------+-----------+---------------------------------------------------+ > -| res | integer | No | The result of the audited operation(success/fail) | > -+----------------+------------+-----------+---------------------------------------------------+ > - > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| Field | Value Type | Optional? | Description of Value | > ++================+============+===========+=============================================================+ > +| policy_name | string | Yes | The policy_name | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| policy_version | string | Yes | The policy_version | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| policy_digest | string | Yes | The policy hash | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| auid | integer | No | The login user ID | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| ses | integer | No | The login session ID | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| lsm | string | No | The lsm name associated with the event | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| res | integer | No | The result of the audited operation(success/fail) | > ++----------------+------------+-----------+-------------------------------------------------------------+ > +| errno | integer | No | Error code from policy loading operations (see table below) | > ++----------------+------------+-----------+-------------------------------------------------------------+ > + > +Policy error codes (errno): > + > +The following table lists the error codes that may appear in the errno field while loading or updating the policy: > + > ++----------------+--------------------------------------------------------+ > +| Error Code | Description | > ++================+========================================================+ > +| 0 | No error | Nit: How about using "Success" here to match with the function comments. > ++----------------+--------------------------------------------------------+ > +| -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 | > ++----------------+--------------------------------------------------------+ > +| -ENOKEY | Key used to sign the IPE policy not found in keyring | > ++----------------+--------------------------------------------------------+ > +| -EKEYREJECTED | IPE signature verification failed | More accurately should be the IPE policy signature. > ++----------------+--------------------------------------------------------+ > +| -ESTALE | Attempting to update an IPE policy with older version | > ++----------------+--------------------------------------------------------+ > +| -ENOENT | Policy was deleted while updating | > ++----------------+--------------------------------------------------------+ > > 1404 AUDIT_MAC_STATUS > ^^^^^^^^^^^^^^^^^^^^^ > diff --git a/security/ipe/audit.c b/security/ipe/audit.c > index f05f0caa4850..ac9d68b68b8b 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_FAIL_FMT "policy_name=? policy_version=? "\ > + "policy_digest=?" > #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 ":" > @@ -248,22 +250,31 @@ void ipe_audit_policy_activation(const struct ipe_policy *const op, > } > > /** > - * ipe_audit_policy_load() - Audit a policy being loaded into the kernel. > - * @p: Supplies a pointer to the policy to audit. > + * ipe_audit_policy_load() - Audit a policy being loaded or failing > + * to load into the kernel. The brief description needs to fit in a single line, if necessary you can create a longer description. See https://origin.kernel.org/doc/html/latest/doc-guide/kernel-doc.html. > + * @p: Supplies a pointer to the policy to audit or an error pointer > + * to audit a failure with the associated error code from PTR_ERR(p). The second line doesn't seem necessary. > */ > void ipe_audit_policy_load(const struct ipe_policy *const p) > { > struct audit_buffer *ab; > + int err = 0; > > ab = audit_log_start(audit_context(), GFP_KERNEL, > AUDIT_IPE_POLICY_LOAD); > 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); > + } else { > + audit_log_format(ab, AUDIT_POLICY_LOAD_FAIL_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), !err, err); > > audit_log_end(ab); > } > diff --git a/security/ipe/fs.c b/security/ipe/fs.c > index 5b6d19fb844a..db18636470bf 100644 > --- a/security/ipe/fs.c > +++ b/security/ipe/fs.c > @@ -133,6 +133,8 @@ static ssize_t getenforce(struct file *f, char __user *data, > * * %-ERANGE - Policy version number overflow > * * %-EINVAL - Policy version parsing error > * * %-EEXIST - Same name policy already deployed > + * * %-ENOKEY - Key used to sign the IPE policy not found in the keyring This line seems to exceed the 80 char line limit, did it pass checkpatch.pl? See https://www.kernel.org/doc/html/latest/process/coding-style.html. It can be more concise like "Policy signing key not found" > + * * %-EKEYREJECTED - IPE signature verification failed The above comment also applies to here. > */ > static ssize_t new_policy(struct file *f, const char __user *data, > size_t len, loff_t *offset) > @@ -141,12 +143,17 @@ 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); > + copy = NULL; > + goto out; > + } > > p = ipe_new_policy(NULL, 0, copy, len); > if (IS_ERR(p)) { > @@ -161,8 +168,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; Nit: we can change the style of returning like the one in update_policy(). > } > diff --git a/security/ipe/policy.c b/security/ipe/policy.c > index b628f696e32b..68a2078d5b6a 100644 > --- a/security/ipe/policy.c > +++ b/security/ipe/policy.c > @@ -84,8 +84,12 @@ 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 was changed while updating This one means "Policy name mismatch". > + * * %-ESTALE - Attempting to update an IPE policy > + * * with an older version This one can also be more concise, how about "Policy version too old" Also the indentation is too much, see https://origin.kernel.org/doc/html/latest/doc-guide/kernel-doc.html. > */ > int ipe_update_policy(struct inode *root, const char *text, size_t textlen, > const char *pkcs7, size_t pkcs7len) > @@ -150,6 +154,9 @@ int ipe_update_policy(struct inode *root, const char *text, size_t textlen, > * * %-ENOMEM - Out of memory (OOM) > * * %-ERANGE - Policy version number overflow > * * %-EINVAL - Policy version parsing error > + * * %-ENOKEY - Key used to sign the IPE policy > + * not found in the keyring > + * * %-EKEYREJECTED - IPE signature verification failed > */ The above doc style/accuracy issue also applies here. > struct ipe_policy *ipe_new_policy(const char *text, size_t textlen, > const char *pkcs7, size_t pkcs7len) > diff --git a/security/ipe/policy_fs.c b/security/ipe/policy_fs.c > index 3bcd8cbd09df..b70d2518b182 100644 > --- a/security/ipe/policy_fs.c > +++ b/security/ipe/policy_fs.c > @@ -12,6 +12,7 @@ > #include "policy.h" > #include "eval.h" > #include "fs.h" > +#include "audit.h" > > #define MAX_VERSION_SIZE ARRAY_SIZE("65535.65535.65535") > > @@ -282,8 +283,14 @@ static ssize_t getactive(struct file *f, char __user *data, > * On success this updates the policy represented by $name, > * in-place. > * > - * Return: Length of buffer written on success. If an error occurs, > - * the function will return the -errno. > + * Return: > + * * Length of buffer written - Success > + * * %-EPERM - Insufficient permission > + * * %-ENOMEM - Out of memory (OOM) > + * * %-ENOENT - Policy was deleted while updating > + * * %-EINVAL - Policy name was changed while updating > + * * %-ESTALE - Attempting to update an IPE policy > + * * with an older version > */ The above doc style/accuracy issue also applies here. -Fan
