Received-SPF: pass (google.com: domain of linux-crypto+bounces-3169-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Sun, 31 Mar 2024 18:44:19 +0300
Message-Id: <D081UQF5758Q.3TO9YN0PEQ0O1@kernel.org>
To: "James Bottomley" <James.Bottomley@HansenPartnership.com>, "Eric
 Biggers" <ebiggers@kernel.org>, "Zhang Yiqun" <zhangyiqun@phytium.com.cn>
Cc: <dhowells@redhat.com>, <corbet@lwn.net>, <keyrings@vger.kernel.org>,
 <linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
 <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH] KEYS: Add ECDH support
From: "Jarkko Sakkinen" <jarkko@kernel.org>
References: <20240330065506.3146-1-zhangyiqun@phytium.com.cn>
 <20240330070436.GA2116@sol.localdomain>
 <087bbfcf95c9014ee8f87d482773244f0833b892.camel@HansenPartnership.com>
In-Reply-To: <087bbfcf95c9014ee8f87d482773244f0833b892.camel@HansenPartnership.com>

On Sat Mar 30, 2024 at 3:09 PM EET, James Bottomley wrote:
> On Sat, 2024-03-30 at 00:04 -0700, Eric Biggers wrote:
> > [+Cc linux-crypto]
> >=20
> > On Sat, Mar 30, 2024 at 02:55:06PM +0800, Zhang Yiqun wrote:
> > > This patch is to introduce ECDH into keyctl syscall for
> > > userspace usage, containing public key generation and
> > > shared secret computation.
> > >=20
> > > It is mainly based on dh code, so it has the same condition
> > > to the input which only user keys is supported. The output
> > > result is storing into the buffer with the provided length.
> > >=20
> > > Signed-off-by: Zhang Yiqun <zhangyiqun@phytium.com.cn>
> > > ---
> > > =C2=A0Documentation/security/keys/core.rst |=C2=A0 62 ++++++
> > > =C2=A0include/linux/compat.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0 4 +
> > > =C2=A0include/uapi/linux/keyctl.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 |=C2=A0 11 +
> > > =C2=A0security/keys/Kconfig=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 12 +
> > > =C2=A0security/keys/Makefile=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0 2 +
> > > =C2=A0security/keys/compat_ecdh.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 |=C2=A0 50 +++++
> > > =C2=A0security/keys/ecdh.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 318
> > > +++++++++++++++++++++++++++
> > > =C2=A0security/keys/internal.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 44 ++++
> > > =C2=A0security/keys/keyctl.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 10 +
> > > =C2=A09 files changed, 513 insertions(+)
> > > =C2=A0create mode 100644 security/keys/compat_ecdh.c
> > > =C2=A0create mode 100644 security/keys/ecdh.c
> >=20
> > Nacked-by: Eric Biggers <ebiggers@google.com>
> >=20
> > The existing KEYCTL_PKEY_*, KEYCTL_DH_COMPUTE, and AF_ALG are causing
> > enough problems.=C2=A0 We do not need any more UAPIs like this.=C2=A0 T=
hey are
> > hard to maintain, break often, not properly documented, increase the
> > kernel's attack surface, and what they do is better done in
> > userspace.
>
> Actually that's not entirely true.  There is a use case for keys which
> is where you'd like to harden unwrapped key handling and don't have the
> ability to use a device.  The kernel provides a harder exfiltration
> environment than user space, so there is a use case for getting the
> kernel to handle operations on unwrapped keys for the protection it
> affords the crytpographic key material.
>
> For instance there are people who use the kernel keyring to replace
> ssh-agent and thus *reduce* the attack surface they have for storing
> ssh keys:
>
> https://blog.cloudflare.com/the-linux-kernel-key-retention-service-and-wh=
y-you-should-use-it-in-your-next-application/
>
> The same thing could be done with gpg keys or the gnome keyring.

Eric has a correct standing given that the commit message does not have
motivation part at all.=20

With a description of the problem that this patch is supposed to solve
this would be more meaningful to review.

BR, Jarkko