Re: Question on loading trusted key with keyctl command

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote:
> On Mon, 19 Dec 2022 at 18:20, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > 
> > On Mon, 2022-12-19 at 15:50 +0530, Sughosh Ganu wrote:
> > > hi,
> > > I am trying to enable the evm hmac solution on my qemu arm64 virt
> > > platform running Debian. I am using the swtpm 2.0 implementation
> > > for
> > > the TPM trusted source. Before I get into trying out the evm hmac
> > > solution on the target system, I wanted to check creating the
> > > trusted
> > > and encrypted keys.  Other details on my set up are as follows
> > > 
> > > Distro - Debian 11
> > > TPM - swtpm
> > > Linux kernel - Linux version 6.1.0-13032, commit 77856d911a8c [1]
> > > keyctl --version
> > > keyctl from keyutils-1.6.1 (Built 2020-02-10)
> > > 
> > > When trying to follow the steps highlighted in the
> > > Documentation/security/keys/trusted-encrypted.rst, I can generate
> > > the
> > > trusted key. However, when I try to load the trusted key using
> > > the
> > > command shown in the document, it throws an error. Has there been
> > > a
> > > change in the code, or am I missing some step when trying to load
> > > the
> > > trusted key?
> > > 
> > > Steps that I am following (after having created the SRK).
> > > 
> > > # keyctl add trusted kmk "new 32 keyhandle=0x81000001" @u
> > > # keyctl show
> > > Session Keyring
> > >  442944693 --alswrv      0     0  keyring: _ses
> > >  925986946 --alswrv      0 65534   \_ keyring: _uid.0
> > >  401286062 --alswrv      0     0       \_ trusted: kmk
> > > # keyctl pipe 401286062 > kmk.blob
> > > # keyctl add trusted kmk "load `cat kmk.blob`
> > > keyhandle=0x81000001"
> > > @u
> > > add_key: Invalid argument
> > 
> > kmk is your invalid argument ... you already have a key there. 
> > Either
> > unlink %trusted:kmk or add the new key at kmk1.
> 
> I was able to load the key after clearing the keyring. Thanks James
> and Mimi for your pointers.

Actually, I think this is a bug in trusted keys.  Add on existing key
is supposed to go through the update path.  If the path doesn't exist
it returns -EEXIST.  Trusted keys have an update path but they return -
EINVAL if the trusted key command is anything but update (which is used
to reseal a key).  Obviously this is incorrect and the code should be
returning -EEXIST for a key we refuse to update to match every other
key type.

This should be the fix.

James

---

diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index c6fc50d67214..8dcd84137035 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -254,7 +254,7 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
 	datablob[datalen] = '\0';
 	ret = datablob_parse(&datablob, new_p);
 	if (ret != Opt_update) {
-		ret = -EINVAL;
+		ret = -EEXIST;
 		kfree_sensitive(new_p);
 		goto out;
 	}