Question on loading trusted key with keyctl command

Question on loading trusted key with keyctl command

[Sughosh Ganu]

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

-sughosh

[1] - I enable trusted and encrypted keys above the said commit

Re: Question on loading trusted key with keyctl command

[Mimi Zohar]

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
> 
> -sughosh
> 
> [1] - I enable trusted and encrypted keys above the said commit

Try deleting/unlinking the existing kmk key before re-loading it.

-- 
thanks,

Mimi

Re: Question on loading trusted key with keyctl command

[James Bottomley]

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.

James

Re: Question on loading trusted key with keyctl command

[Sughosh Ganu]

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.

-sughosh

Re: Question on loading trusted key with keyctl command

[James Bottomley]

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;
 	}

Re: Question on loading trusted key with keyctl command

[Mimi Zohar]

On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote:
> 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.

Re-loading an existing key was previously permitted.  Obviously this
changed at some point.   Any "fixes" should point out when it changed.

-- 
thanks,

Mimi

Re: Question on loading trusted key with keyctl command

[James Bottomley]

On Tue, 2022-12-20 at 08:54 -0500, Mimi Zohar wrote:
> On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote:
> > On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote:
[...]
> > > 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.
> 
> Re-loading an existing key was previously permitted.  Obviously this
> changed at some point.   Any "fixes" should point out when it
> changed.

Git history doesn't think so.  It thinks when you added trusted keys
with d00a1c72f7f4661212299e6cb132dfa58030bcdb the update path already
had the -EINVAL return, so reload has always failed this way unless we
were doing a reseal update.  We could certainly permit overwriting an
existing key with load, but that would be a more extensive change.

James

Re: Question on loading trusted key with keyctl command

[Mimi Zohar]

On Tue, 2022-12-20 at 09:03 -0500, James Bottomley wrote:
> On Tue, 2022-12-20 at 08:54 -0500, Mimi Zohar wrote:
> > On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote:
> > > On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote:
> [...]
> > > > 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.
> > 
> > Re-loading an existing key was previously permitted.  Obviously this
> > changed at some point.   Any "fixes" should point out when it
> > changed.
> 
> Git history doesn't think so.  It thinks when you    added trusted keys
> with d00a1c72f7f4661212299e6cb132dfa58030bcdb the update path already
> had the -EINVAL return, so reload has always failed this way unless we
> were doing a reseal update.  We could certainly permit overwriting an
> existing key with load, but that would be a more extensive change.

Replacing existing keys/keyrings was part of the infrastructure.  I
don't think this change has anything to do with trusted type keys.  
The ability of replacing keys/keyrings was one of the main reasons for
trusted keyrings (dot prefixed keyrings).

-- 
thanks,

Mimi

Re: Question on loading trusted key with keyctl command

[James Bottomley]

On Tue, 2022-12-20 at 09:13 -0500, Mimi Zohar wrote:
> On Tue, 2022-12-20 at 09:03 -0500, James Bottomley wrote:
> > On Tue, 2022-12-20 at 08:54 -0500, Mimi Zohar wrote:
> > > On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote:
> > > > On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote:
> > [...]
> > > > > 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.
> > > 
> > > Re-loading an existing key was previously permitted.  Obviously
> > > this changed at some point.   Any "fixes" should point out when
> > > it changed.
> > 
> > Git history doesn't think so.  It thinks when you    added trusted
> > keys with d00a1c72f7f4661212299e6cb132dfa58030bcdb the update path
> > already had the -EINVAL return, so reload has always failed this
> > way unless we were doing a reseal update.  We could certainly
> > permit overwriting an existing key with load, but that would be a
> > more extensive change.
> 
> Replacing existing keys/keyrings was part of the infrastructure.  I
> don't think this change has anything to do with trusted type keys.  
> The ability of replacing keys/keyrings was one of the main reasons
> for trusted keyrings (dot prefixed keyrings).

Keys can only be replaced by calling the ->update() helper for the key
type.  If that doesn't exist keyctl add will return -EEXIST (behaviour
for at least the last 12 years).  Most key type update routines do
unconditionally update, so the error they return is the same error they
would have returned for an add of a non existent key (EINVAL if the
payload is too large, for instance).  The trusted keys ->update()
helper (trusted_update()) only allows update if the trusted operation
is update, so they've always failed a load with EINVAL going back to
the original commit I quoted.  At no time that I can find has there
ever been a modification to this supporting updating trusted keys with
anything other than an update trusted operation.  So they've supported
changing the sealing parameters (PCR values) but not changing the
payload.  This contrasts with user keys where add of a new payload on
an existing key changes the payload.

James

Re: Question on loading trusted key with keyctl command

[Mimi Zohar]

On Tue, 2022-12-20 at 09:23 -0500, James Bottomley wrote:
> On Tue, 2022-12-20 at 09:13 -0500, Mimi Zohar wrote:
> > On Tue, 2022-12-20 at 09:03 -0500, James Bottomley wrote:
> > > On Tue, 2022-12-20 at 08:54 -0500, Mimi Zohar wrote:
> > > > On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote:
> > > > > On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote:
> > > [...]
> > > > > > 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.
> > > > 
> > > > Re-loading an existing key was previously permitted.  Obviously
> > > > this changed at some point.   Any "fixes" should point out when
> > > > it changed.
> > > 
> > > Git history doesn't think so.  It thinks when you    added trusted
> > > keys with d00a1c72f7f4661212299e6cb132dfa58030bcdb the update path
> > > already had the -EINVAL return, so reload has always failed this
> > > way unless we were doing a reseal update.  We could certainly
> > > permit overwriting an existing key with load, but that would be a
> > > more extensive change.
> > 
> > Replacing existing keys/keyrings was part of the infrastructure.  I
> > don't think this change has anything to do with trusted type keys.  
> > The ability of replacing keys/keyrings was one of the main reasons
> > for trusted keyrings (dot prefixed keyrings).
> 
> Keys can only be replaced by calling the ->update() helper for the key
> type.  If that doesn't exist keyctl add will return -EEXIST (behaviour
> for at least the last 12 years).  Most key type update routines do
> unconditionally update, so the error they return is the same error they
> would have returned for an add of a non existent key (EINVAL if the
> payload is too large, for instance).  The trusted keys ->update()
> helper (trusted_update()) only allows update if the trusted operation
> is update, so they've always failed a load with EINVAL going back to
> the original commit I quoted.  At no time that I can find has there
> ever been a modification to this supporting updating trusted keys with
> anything other than an update trusted operation.  So they've supported
> changing the sealing parameters (PCR values) but not changing the
> payload.  This contrasts with user keys where add of a new payload on
> an existing key changes the payload.

Yes, my mistake.  With your change, it's now returning "add_key: File
exists".

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
-- 
thanks,

Mimi