Received-SPF: pass (google.com: domain of linux-crypto+bounces-3122-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <087bbfcf95c9014ee8f87d482773244f0833b892.camel@HansenPartnership.com>
Subject: Re: [PATCH] KEYS: Add ECDH support
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Eric Biggers <ebiggers@kernel.org>, Zhang Yiqun
 <zhangyiqun@phytium.com.cn>
Cc: dhowells@redhat.com, jarkko@kernel.org, corbet@lwn.net, 
	keyrings@vger.kernel.org, linux-doc@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org
Date: Sat, 30 Mar 2024 09:09:51 -0400
In-Reply-To: <20240330070436.GA2116@sol.localdomain>
References: <20240330065506.3146-1-zhangyiqun@phytium.com.cn>
	 <20240330070436.GA2116@sol.localdomain>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

On Sat, 2024-03-30 at 00:04 -0700, Eric Biggers wrote:
> [+Cc linux-crypto]
> 
> 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.
> > 
> > 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.
> > 
> > Signed-off-by: Zhang Yiqun <zhangyiqun@phytium.com.cn>
> > ---
> >  Documentation/security/keys/core.rst |  62 ++++++
> >  include/linux/compat.h               |   4 +
> >  include/uapi/linux/keyctl.h          |  11 +
> >  security/keys/Kconfig                |  12 +
> >  security/keys/Makefile               |   2 +
> >  security/keys/compat_ecdh.c          |  50 +++++
> >  security/keys/ecdh.c                 | 318
> > +++++++++++++++++++++++++++
> >  security/keys/internal.h             |  44 ++++
> >  security/keys/keyctl.c               |  10 +
> >  9 files changed, 513 insertions(+)
> >  create mode 100644 security/keys/compat_ecdh.c
> >  create mode 100644 security/keys/ecdh.c
> 
> Nacked-by: Eric Biggers <ebiggers@google.com>
> 
> The existing KEYCTL_PKEY_*, KEYCTL_DH_COMPUTE, and AF_ALG are causing
> enough problems.  We do not need any more UAPIs like this.  They 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-why-you-should-use-it-in-your-next-application/

The same thing could be done with gpg keys or the gnome keyring.

> Please refer to the recent thread
> https://lore.kernel.org/linux-crypto/CZSHRUIJ4RKL.34T4EASV5DNJM@matfyz.cz/T/#u
> where these issues were discussed in detail.

This thread was talking about using the kernel for handling the
algorithms themselves (which is probably best done in userspace) and
didn't address using the kernel to harden the key protection
environment.

James