Re: [PATCH v3 2/2] ipe: also reject policy updates with the same version

[Fan Wu <wufan@linux.microsoft.com>]

On 9/25/2024 2:01 PM, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> Currently IPE accepts an update that has the same version as the policy
> being updated, but it doesn't make it a no-op nor it checks that the
> old and new policyes are the same. So it is possible to change the
> content of a policy, without changing its version. This is very
> confusing from userspace when managing policies.
> Instead change the update logic to reject updates that have the same
> version with ESTALE, as that is much clearer and intuitive behaviour.
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
> v2: also update documentation
> v3: fix typo in documentation
> 
>   Documentation/admin-guide/LSM/ipe.rst | 2 +-
>   security/ipe/policy.c                 | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
> index 47323494d119..f93a467db628 100644
> --- a/Documentation/admin-guide/LSM/ipe.rst
> +++ b/Documentation/admin-guide/LSM/ipe.rst
> @@ -269,7 +269,7 @@ in the kernel. This file is write-only and accepts a PKCS#7 signed
>   policy. Two checks will always be performed on this policy: First, the
>   ``policy_names`` must match with the updated version and the existing
>   version. Second the updated policy must have a policy version greater than
> -or equal to the currently-running version. This is to prevent rollback attacks.
> +the currently-running version. This is to prevent rollback attacks.
>   
>   The ``delete`` file is used to remove a policy that is no longer needed.
>   This file is write-only and accepts a value of ``1`` to delete the policy.
> diff --git a/security/ipe/policy.c b/security/ipe/policy.c
> index 3a0069c6d5af..45f7d6a0ed23 100644
> --- a/security/ipe/policy.c
> +++ b/security/ipe/policy.c
> @@ -106,7 +106,7 @@ int ipe_update_policy(struct inode *root, const char *text, size_t textlen,
>   		goto err;
>   	}
>   
> -	if (ver_to_u64(old) > ver_to_u64(new)) {
> +	if (ver_to_u64(old) >= ver_to_u64(new)) {
>   		rc = -ESTALE;
>   		goto err;
>   	}

Acked-by: Fan Wu <wufan@linux.microsoft.com>

I will pull them along with the policy signing keyring change once I get 
things ready.

-Fan