Received-SPF: pass (google.com: domain of linux-kernel+bounces-157973-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 24 Apr 2024 20:36:23 -0700
From: Eric Biggers <ebiggers@kernel.org>
To: Fan Wu <wufan@linux.microsoft.com>
Cc: corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
	serge@hallyn.com, tytso@mit.edu, axboe@kernel.dk, agk@redhat.com,
	snitzer@kernel.org, eparis@redhat.com, paul@paul-moore.com,
	linux-doc@vger.kernel.org, linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, fsverity@lists.linux.dev,
	linux-block@vger.kernel.org, dm-devel@lists.linux.dev,
	audit@vger.kernel.org, linux-kernel@vger.kernel.org,
	Deven Bowers <deven.desai@linux.microsoft.com>
Subject: Re: [PATCH v17 16/21] fsverity: expose verified fsverity built-in
 signatures to LSMs
Message-ID: <20240425033623.GA1401@sol.localdomain>
References: <1712969764-31039-1-git-send-email-wufan@linux.microsoft.com>
 <1712969764-31039-17-git-send-email-wufan@linux.microsoft.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1712969764-31039-17-git-send-email-wufan@linux.microsoft.com>

On Fri, Apr 12, 2024 at 05:55:59PM -0700, Fan Wu wrote:
> For instance, a policy could be established to permit the execution of all
> files with verified built-in fsverity signatures while restricting kernel
> module loading from specified fsverity files via fsverity digets.

"digets" => "digests"

> The introduction of a security_inode_setintegrity() hook call within
> fsverity's workflow ensures that the verified built-in signature of a file
> is exposed to LSMs, This enables LSMs to recognize and label fsverity files

"LSMs, This" => "LSMs. This"

> +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> +static int fsverity_inode_setintegrity(struct inode *inode,
> +				       const struct fsverity_descriptor *desc)
> +{
> +	return security_inode_setintegrity(inode,
> +					   LSM_INT_FSVERITY_BUILTINSIG_VALID,
> +					   desc->signature,
> +					   le32_to_cpu(desc->sig_size));
> +}
> +#else
> +static inline int fsverity_inode_setintegrity(struct inode *inode,
> +					      const struct fsverity_descriptor *desc)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_FS_VERITY_BUILTIN_SIGNATURES */
[...]
> @@ -241,6 +259,10 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
> 		}
> 	}
> 
> +	err = fsverity_inode_setintegrity(inode, desc);
> +	if (err)
> +		goto fail;
> +

Wouldn't it be much simpler to put the LSM call in fsverity_verify_signature()?
Then no #ifdef would be needed, and there would be no weird cases where the LSM
gets passed LSM_INT_FSVERITY_BUILTINSIG_VALID with an empty signature.

> diff --git a/fs/verity/signature.c b/fs/verity/signature.c
> index 90c07573dd77..fd60e9704e78 100644
> --- a/fs/verity/signature.c
> +++ b/fs/verity/signature.c
> @@ -41,7 +41,11 @@ static struct key *fsverity_keyring;
>   * @sig_size: size of signature in bytes, or 0 if no signature
>   *
>   * If the file includes a signature of its fs-verity file digest, verify it
> - * against the certificates in the fs-verity keyring.
> + * against the certificates in the fs-verity keyring. Note that signatures
> + * are verified regardless of the state of the 'fsverity_require_signatures'
> + * variable and the LSM subsystem relies on this behavior to help enforce
> + * file integrity policies. Please discuss changes with the LSM list
> + * (thank you!).
>   *
>   * Return: 0 on success (signature valid or not required); -errno on failure
>   */

.. and it would also make the above easier to understand if the LSM call were
to happen right in fsverity_verify_signature().

- Eric