Received-SPF: pass (google.com: domain of linux-kernel+bounces-179581-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240503221634.44274-1-ignat@cloudflare.com> <20240503221634.44274-3-ignat@cloudflare.com>
 <D19QW70177QG.2YC9XL0FT7VME@kernel.org>
In-Reply-To: <D19QW70177QG.2YC9XL0FT7VME@kernel.org>
From: Ignat Korchagin <ignat@cloudflare.com>
Date: Wed, 15 May 2024 08:26:23 +0100
Message-ID: <CALrw=nFsjiGTZDN3vYCN+GC9R3eQ3ix6_u2x0xo3jcpHqWjs3A@mail.gmail.com>
Subject: Re: [RFC PATCH 2/2] KEYS: implement derived keys
To: Jarkko Sakkinen <jarkko@kernel.org>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>, Mimi Zohar <zohar@linux.ibm.com>, 
	David Howells <dhowells@redhat.com>, Paul Moore <paul@paul-moore.com>, 
	James Morris <jmorris@namei.org>, serge@hallyn.com, linux-integrity@vger.kernel.org, 
	keyrings@vger.kernel.org, linux-kernel@vger.kernel.org, 
	kernel-team@cloudflare.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 15, 2024 at 12:10=E2=80=AFAM Jarkko Sakkinen <jarkko@kernel.org=
> wrote:
>
> On Sat May 4, 2024 at 1:16 AM EEST, Ignat Korchagin wrote:
> > Derived keys are similar to user keys, but their payload is derived fro=
m the
> > primary TPM seed and some metadata of the requesting process. This way =
every
>
> What is exactly "some metadata"?
>
> > application can get a unique secret/key, which is cryptographically bou=
nd to
>
> What is "cryptographically bound". Please go straight to the point and
> cut out *all* white paper'ish phrases. We do not need it and will make
> painful to backtrack this commit once in the mainline.
>
> > the TPM without the need to provide the key material externally (unlike=
 trusted
> > keys). Also, the whole key derivation process is deterministic, so as l=
ong as
>
> Why trusted keys is inside braces. It is not important for the point
> you are trying to make here?
>
> > the TPM is available, applications can always recover their keys, which=
 may
> > allow for easier key management on stateless systems.
>
> Please drop "stateless system" unless you provide a rigid definition
> what it is. I have no idea what you mean by it. Probably not that
> important, right?
>
> >
> > In this implementation the following factors will be used as a key deri=
vation
> > factor:
> >   * requested key length
> >   * requesting process effective user id
> >   * either the application executable path or the application integrity
> >     metadata (if available)
>
> NAK for path for any possible key derivation. They are racy and
> and ambiguous.

Can you elaborate here? What kind of a problem you see specifically?
(This is exactly what I want to get from this discussion)

> This should have been in the beginning instead of "some data". What
> other implementations exist. For me "this implementation" implies
> that this one competing alternative to multiple implementations
> of the same thing.

With "this implementation" I meant the current RFC patchset because I
do expect there would be more iterations and just wanted to solicit
comments.

> I do not like this science/white paper style at all. Just express
> short, open code everything right at start when you need and cut
> extras like "stateless system" unless you can provide exact, sound
> and unambiguous definiton of it.
>
> Just want to underline how this really needs a complete rewrite with
> clear and concise explanation :-) This won't ever work.

Understood.

> >
> > Key length is used so requests for keys with different sizes result in =
keys
> > with different cryptographic material.
>
> What is "key length"? Please refer the exact attribute.
>
> >
> > User id is mixed, so different users get different keys even when execu=
ting the
>
> First of all it would be more clear to just s/User id/UID/
>
> And make obvious whether we are talking about ruid or euid and how
> this interacts with GIDs.
>
> I'll look at the code change next round if the commit message starts
> making any sense.
>
> BR, Jarkko
>