Re: [PATCH 1/3] keys: add KEY_SPEC_DM_VERITY_KEYRING
Jarkko Sakkinen <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,dev.linux.lists.dm-devel,dev.linux.lists.fsverity,org.kernel.vger.bpf,org.kernel.vger.keyrings,org.kernel.vger.linux-kernel,org.kernel.vger.linux-security-module |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Sep 14, 2026 at 11:41:42AM -0500, Andrew Halaney wrote:
> The .dm-verity keyring is not linked into any process keyring, so
> request_key() and KEYCTL_SEARCH cannot find it. Userspace has to scrape
> /proc/keys for the serial to use.
>
> That's not great, one could put in a bogus description containing
> a newline to confuse userspace further. For example, thanks to AI
> models, which I run as root but you could show off as any old user:
>
> # DESC=$'.dm-verity: 1\n0badf00d I--Q--- 1 perm 082f0000 0 0 keyring .dm-verity'
> # k=$(echo -n "1234567" | keyctl padd user "$DESC" @s)
> # keyctl setperm "$k" 0x3f010001
>
> # cat /proc/keys | grep dm-verity
> 10402023 I------ 2 perm 082f0000 0 0 keyring .dm-verity: empty
> 12e0bee6 I--Q--- 1 perm 3f010001 0 0 user .dm-verity: 1
> 0badf00d I--Q--- 1 perm 082f0000 0 0 keyring .dm-verity: 7
>
> 0x0badf00d is not a keyring, it is the text inside our user's key
> description. Telling the two apart needs a KEYCTL_DESCRIBE per row
> which is annoying to do. i.e.:
>
> # keyctl rdescribe 0x10402023
> keyring;0;0;082f0000;.dm-verity
> # keyctl rdescribe 0x0badf00d
> keyctl_describe: Required key not available
>
> Let's just give it a well known identifier, i.e.:
>
> # keyctl rdescribe -10
> keyring;0;0;082f0000;.dm-verity
>
> This is exactly what
> commit 264d8fd2794f ("bpf, keys: Add a bpf keyring for program signature validation")
> did, and was the inspiration for this commit minus the spinlock bits
> required due to dm-verity being a module.
This is great for discussion but what we want for the commit message
is just motivation and resolution.
I don't think we need all this just to say that /proc/keys in a racy
query mechanism for production, which is an issue for dm-verity, given
that nothing else is available.
And secondly special keys are meant for implicit keyrings so isn't
that all there's to it?
>
> Suggested-by: Christian Brauner (Amutable) <[email protected]>
> Signed-off-by: Andrew Halaney <[email protected]>
> ---
> drivers/md/dm-verity-verify-sig.c | 3 +++
> include/linux/key.h | 1 +
> include/uapi/linux/keyctl.h | 1 +
> security/keys/process_keys.c | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 38 insertions(+)
>
> diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c
> index b2b55c41e2cb..da509dcdd7e5 100644
> --- a/drivers/md/dm-verity-verify-sig.c
> +++ b/drivers/md/dm-verity-verify-sig.c
> @@ -189,11 +189,14 @@ int __init dm_verity_verify_sig_init(void)
> keyring_restrict(make_key_ref(dm_verity_keyring, true), NULL, NULL))
> panic("dm-verity can't seal keyring\n");
>
> + key_register_dm_verity_keyring(dm_verity_keyring);
> +
> return 0;
> }
>
> void dm_verity_verify_sig_exit(void)
> {
> + key_register_dm_verity_keyring(NULL);
> key_revoke(dm_verity_keyring);
> key_put(dm_verity_keyring);
> }
> diff --git a/include/linux/key.h b/include/linux/key.h
> index bd10fe45819d..ae8d3314fd93 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -441,6 +441,7 @@ extern int keyring_restrict(key_ref_t keyring, const char *type,
> const char *restriction);
>
> extern void key_register_bpf_keyring(struct key *keyring);
> +extern void key_register_dm_verity_keyring(struct key *keyring);
>
> extern struct key *key_lookup(key_serial_t id);
>
> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> index fa85b9760391..75923941f1b3 100644
> --- a/include/uapi/linux/keyctl.h
> +++ b/include/uapi/linux/keyctl.h
> @@ -25,6 +25,7 @@
> #define KEY_SPEC_REQKEY_AUTH_KEY -7 /* - key ID for assumed request_key auth key */
> #define KEY_SPEC_REQUESTOR_KEYRING -8 /* - key ID for request_key() dest keyring */
> #define KEY_SPEC_BPF_KEYRING -9 /* - key ID for the BPF-specific keyring */
> +#define KEY_SPEC_DM_VERITY_KEYRING -10 /* - key ID for the .dm-verity keyring */
>
> /* request-key default keyrings */
> #define KEY_REQKEY_DEFL_NO_CHANGE -1
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index 44358388e395..dba3df41638b 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
> @@ -25,6 +25,10 @@ static DEFINE_MUTEX(key_session_mutex);
> /* BPF keyring reachable through KEY_SPEC_BPF_KEYRING */
> static struct key *bpf_keyring __ro_after_init;
>
> +/* dm-verity keyring reachable through KEY_SPEC_DM_VERITY_KEYRING */
> +static struct key *dm_verity_keyring;
> +static DEFINE_SPINLOCK(dm_verity_keyring_lock);
> +
> /* The root user's tracking struct */
> struct key_user root_key_user = {
> .usage = REFCOUNT_INIT(3),
> @@ -607,6 +611,24 @@ void key_register_bpf_keyring(struct key *keyring)
> bpf_keyring = keyring;
> }
>
> +/**
> + * key_register_dm_verity_keyring - Publish the keyring for KEY_SPEC_DM_VERITY_KEYRING
> + * @keyring: The keyring to publish, or NULL to withdraw it
> + *
> + * Make @keyring reachable by userspace through the KEY_SPEC_DM_VERITY_KEYRING
> + * special key ID, so that provisioning it does not require scraping its
> + * serial out of /proc/keys first. dm-verity can be unloaded as a module
> + * and the keyring deregistered, as such serialize access with
> + * dm_verity_keyring_lock.
> + */
> +void key_register_dm_verity_keyring(struct key *keyring)
> +{
> + spin_lock(&dm_verity_keyring_lock);
> + dm_verity_keyring = keyring;
> + spin_unlock(&dm_verity_keyring_lock);
> +}
> +EXPORT_SYMBOL_GPL(key_register_dm_verity_keyring);
> +
> /*
> * Look up a key ID given us by userspace with a given permissions mask to get
> * the key it refers to.
> @@ -766,6 +788,17 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> key_ref = make_key_ref(key, 0);
> break;
>
> + case KEY_SPEC_DM_VERITY_KEYRING:
> + spin_lock(&dm_verity_keyring_lock);
> + key = dm_verity_keyring;
> + if (key)
> + __key_get(key);
> + spin_unlock(&dm_verity_keyring_lock);
> + if (!key)
> + goto error;
> + key_ref = make_key_ref(key, 0);
> + break;
> +
> default:
> key_ref = ERR_PTR(-EINVAL);
> if (id < 1)
>
> --
> 2.55.0
>
BR, Jarkko