linux-kernel - Re: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YWcyYBuNppjrVOe2@gmail.com>
Date:   Wed, 13 Oct 2021 12:24:16 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     deven.desai@...ux.microsoft.com
Cc:     corbet@....net, axboe@...nel.dk, agk@...hat.com,
        snitzer@...hat.com, tytso@....edu, paul@...l-moore.com,
        eparis@...hat.com, jmorris@...ei.org, serge@...lyn.com,
        jannh@...gle.com, dm-devel@...hat.com, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
        linux-fscrypt@...r.kernel.org, linux-audit@...hat.com,
        linux-security-module@...r.kernel.org
Subject: Re: [RFC PATCH v7 12/16] fsverity|security: add security hooks to
 fsverity digest and signature

On Wed, Oct 13, 2021 at 12:06:31PM -0700, deven.desai@...ux.microsoft.com wrote:
> From: Fan Wu <wufan@...ux.microsoft.com>
> 
> Add security_inode_setsecurity to fsverity signature verification.
> This can let LSMs save the signature data and digest hashes provided
> by fsverity.

Can you elaborate on why LSMs need this information?

> 
> Also changes the implementaion inside the hook function to let
> multiple LSMs can add hooks.

Please split fs/verity/ changes and security/ changes into separate patches, if
possible.

> 
> Signed-off-by: Fan Wu <wufan@...ux.microsoft.com>
> Signed-off-by: Deven Bowers <deven.desai@...ux.microsoft.com>

> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
>  		fsverity_err(inode, "Error %d computing file digest", err);
>  		goto out;
>  	}
> +
> +	err = security_inode_setsecurity((struct inode *)inode,

If a non-const inode is needed, please propagate that into the callers rather
than randomly casting away the const.

> +					 FS_VERITY_DIGEST_SEC_NAME,
> +					 vi->file_digest,
> +					 vi->tree_params.hash_alg->digest_size,
> +					 0);

The digest isn't meaningful without knowing the hash algorithm it uses.
It's available here, but you aren't passing it to this function.

> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
>  
>  	pr_debug("Valid signature for file digest %s:%*phN\n",
>  		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
> -	return 0;
> +	return security_inode_setsecurity((struct inode *)inode,

Likewise, please don't cast away const.

> +					FS_VERITY_SIGNATURE_SEC_NAME,
> +					signature, sig_size, 0);

This is only for fs-verity built-in signatures which aren't the only way to do
signatures with fs-verity.  Are you sure this is what you're looking for?  Can
you elaborate on your use case for fs-verity built-in signatures, and what the
LSM hook will do with them?

- Eric