linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/1] ipe: add errno field to IPE policy load auditing
@ 2025-03-13 21:51 Jasjiv Singh
  2025-03-13 21:51 ` [PATCH v5 1/1] " Jasjiv Singh
  0 siblings, 1 reply; 5+ messages in thread
From: Jasjiv Singh @ 2025-03-13 21:51 UTC (permalink / raw)
  To: corbet, jmorris, serge, eparis, paul
  Cc: linux-doc, linux-security-module, audit, linux-kernel,
	Jasjiv Singh

Hello,

When deployment of a new IPE policy fails, there is no audit trail.
The failure is written to stderr, but not to the system log. So,
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.

Previous Postings
-----------------
v4: https://lore.kernel.org/linux-security-module/1741385035-22090-2-git-send-email-jasjivsingh@linux.microsoft.com/
v3: https://lore.kernel.org/linux-security-module/1740784265-19829-1-git-send-email-jasjivsingh@linux.microsoft.com/
v2: https://lore.kernel.org/linux-security-module/1740696377-3986-1-git-send-email-jasjivsingh@linux.microsoft.com/
v1: https://lore.kernel.org/linux-security-module/1739569319-22015-1-git-send-email-jasjivsingh@linux.microsoft.com/

Changelog
---------

v5:
* changed audit log format from AUDIT_POLICY_LOAD_FAIL_FMT to
  AUDIT_POLICY_LOAD_NULL_FMT.
* added success case in IPE policy errno documentation.
* removed unnecessary errno documentation in new_policy(),
  update_policy(), ipe_new_policy() and ipe_update_policy(). 
* merged success and failed case together in new_policy() for easy
  maintenance.

v4:
* added a seperate errno table to IPE AUDIT_IPE_POLICY_LOAD documentation.
* fixed error code handling that happens when memdup_user_nul is called
  in new_policy() and update_policy().
* added additional errno documentation to new_policy(), update_policy(),
  ipe_new_policy() and ipe_update_policy().
* added ENOKEY and EKEYREJECTED to IPE errno table documentation.

v3:
* used ERR_PTR(rc) directly rather than assigning to struct ipe_policy.
* removed unnecessary var from update_policy().
* removed unnecessary error handling from update_policy().

v2:
* added additional IPE audit log information to commit to show the errno case.
* changed log format from AUDIT_POLICY_LOAD_NULL_FMT to
  AUDIT_POLICY_LOAD_FAIL_FMT.
* removed unnecessary res var from ipe_audit_policy_load().
* handled security fs failure case in new_policy() and update_policy().
* handled insufficent failure case in new_policy() and update_policy().

Jasjiv Singh (1):
  ipe: add errno field to IPE policy load auditing

 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(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing
  2025-03-13 21:51 [PATCH v5 0/1] ipe: add errno field to IPE policy load auditing Jasjiv Singh
@ 2025-03-13 21:51 ` Jasjiv Singh
  2025-03-17 20:59   ` Fan Wu
  0 siblings, 1 reply; 5+ messages in thread
From: Jasjiv Singh @ 2025-03-13 21:51 UTC (permalink / raw)
  To: corbet, jmorris, serge, eparis, paul
  Cc: linux-doc, linux-security-module, audit, linux-kernel,
	Jasjiv Singh

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@linux.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/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index f93a467db628..dc7088451f9d 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              | Success                                                |
++----------------+--------------------------------------------------------+
+| -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  | Policy signature verification failed                   |
++----------------+--------------------------------------------------------+
+| -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..9668ecc5acd5 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=?"
 #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,29 @@ 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 loading event.
+ * @p: Supplies a pointer to the policy to audit or an error pointer.
  */
 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_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), !err, err);
 
 	audit_log_end(ab);
 }
diff --git a/security/ipe/fs.c b/security/ipe/fs.c
index 5b6d19fb844a..f40e50bfd2e7 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			- Policy signing key not found
+ * * %-EKEYREJECTED		- Policy signature verification failed
  */
 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)) {
@@ -158,12 +165,14 @@ static ssize_t new_policy(struct file *f, const char __user *data,
 	if (rc)
 		goto out;
 
-	ipe_audit_policy_load(p);
-
 out:
-	if (rc < 0)
-		ipe_free_policy(p);
 	kfree(copy);
+	if (rc < 0) {
+		ipe_free_policy(p);
+		ipe_audit_policy_load(ERR_PTR(rc));
+	} else {
+		ipe_audit_policy_load(p);
+	}
 	return (rc < 0) ? rc : len;
 }
 
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
  */
 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..8fa69506b8d5 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,13 @@ 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 mismatch
+ * * %-ESTALE				- Policy version too old
  */
 static ssize_t update_policy(struct file *f, const char __user *data,
 			     size_t len, loff_t *offset)
@@ -292,21 +298,29 @@ static ssize_t update_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(data, len);
-	if (IS_ERR(copy))
-		return PTR_ERR(copy);
+	if (IS_ERR(copy)) {
+		rc = PTR_ERR(copy);
+		copy = NULL;
+		goto out;
+	}
 
 	root = d_inode(f->f_path.dentry->d_parent);
 	inode_lock(root);
 	rc = ipe_update_policy(root, NULL, 0, copy, len);
 	inode_unlock(root);
 
+out:
 	kfree(copy);
-	if (rc)
+	if (rc) {
+		ipe_audit_policy_load(ERR_PTR(rc));
 		return rc;
+	}
 
 	return len;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing
  2025-03-13 21:51 ` [PATCH v5 1/1] " Jasjiv Singh
@ 2025-03-17 20:59   ` Fan Wu
  2025-03-17 21:03     ` Paul Moore
  0 siblings, 1 reply; 5+ messages in thread
From: Fan Wu @ 2025-03-17 20:59 UTC (permalink / raw)
  To: Jasjiv Singh
  Cc: corbet, jmorris, serge, eparis, paul, linux-doc,
	linux-security-module, audit, linux-kernel

On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh
<jasjivsingh@linux.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@linux.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.

-Fan

.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing
  2025-03-17 20:59   ` Fan Wu
@ 2025-03-17 21:03     ` Paul Moore
  2025-03-18 22:50       ` Fan Wu
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Moore @ 2025-03-17 21:03 UTC (permalink / raw)
  To: Fan Wu
  Cc: Jasjiv Singh, corbet, jmorris, serge, eparis, linux-doc,
	linux-security-module, audit, linux-kernel

On Mon, Mar 17, 2025 at 4:59 PM Fan Wu <wufan@kernel.org> wrote:
> On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh
> <jasjivsingh@linux.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@linux.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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/1] ipe: add errno field to IPE policy load auditing
  2025-03-17 21:03     ` Paul Moore
@ 2025-03-18 22:50       ` Fan Wu
  0 siblings, 0 replies; 5+ messages in thread
From: Fan Wu @ 2025-03-18 22:50 UTC (permalink / raw)
  To: Paul Moore
  Cc: Fan Wu, Jasjiv Singh, corbet, jmorris, serge, eparis, linux-doc,
	linux-security-module, audit, linux-kernel

On Mon, Mar 17, 2025 at 2:04 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Mar 17, 2025 at 4:59 PM Fan Wu <wufan@kernel.org> wrote:
> > On Thu, Mar 13, 2025 at 2:51 PM Jasjiv Singh
> > <jasjivsingh@linux.microsoft.com> wrote:
> > >
...
> >
> > 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

This patch is in ipe/next now.

-Fan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-03-18 22:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 21:51 [PATCH v5 0/1] ipe: add errno field to IPE policy load auditing Jasjiv Singh
2025-03-13 21:51 ` [PATCH v5 1/1] " Jasjiv Singh
2025-03-17 20:59   ` Fan Wu
2025-03-17 21:03     ` Paul Moore
2025-03-18 22:50       ` Fan Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).