linux-kernel - Re: [PATCH] KEYS: Add ECDH support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D081UQF5758Q.3TO9YN0PEQ0O1@kernel.org>
Date: Sun, 31 Mar 2024 18:44:19 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "James Bottomley" <James.Bottomley@...senPartnership.com>, "Eric
 Biggers" <ebiggers@...nel.org>, "Zhang Yiqun" <zhangyiqun@...tium.com.cn>
Cc: <dhowells@...hat.com>, <corbet@....net>, <keyrings@...r.kernel.org>,
 <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-crypto@...r.kernel.org>
Subject: Re: [PATCH] KEYS: Add ECDH support

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]
> > 
> > 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@...tium.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@...gle.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.

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

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

BR, Jarkko