Re: [PATCH] KEYS: Add ECDH support

Re: [PATCH] KEYS: Add ECDH support

[Eric Biggers]

[+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.

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.

- Eric

Re: [PATCH] KEYS: Add ECDH support

[James Bottomley]

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

Re: [PATCH] KEYS: Add ECDH support

[Eric Biggers]

On Sat, Mar 30, 2024 at 09:09:51AM -0400, 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@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.

First, that blog post never actually said that the "replace ssh-agent with
kernel keyrings" idea was deployed.  It sounds like a proof of concept idea that
someone thought was interesting and decided to blog about.  Upstream OpenSSH has
no support for Linux keyrings.  It seems unlikely it would get added, especially
given the OpenSSH developers' healthy skepticism of using broken Linux-isms.
You're welcome to bring it up on openssh-unix-dev and get their buy-in first.

Second, as mentioned by the blog post, the kernel also does not support private
keys in the default OpenSSH format.  That sort of thing is an example of the
fundamental problem with trying to make the kernel support every cryptographic
protocol and format in existence.  Userspace simply has much more flexibility to
implement whatever it happens to need.

Third, ssh-agent is already a separate process, and like any other process the
kernel enforces isolation of its address space.  The potential loopholes are
ptrace and coredumps, which ssh-agent already disables, except for ptrace by
root which it can't do alone, but the system administrator can do that by
setting the ptrace_scope sysctl to 3 or by using SELinux.

Amusingly, the existing KEYCTL_DH_* APIs, and the KEYCTL_ECDH_* APIs proposed by
this patch, only operate on user keys that the process has READ access to.  This
means that the keys can be trivially extracted by a shell script running in your
user session.  That's *less* secure than using an isolated process...

That's not to mention that merging this will likely add vulnerabilities
reachable by unprivileged users, just as the original KEYCTL_DH_* did.  I had to
fix some of them last time around (e.g. commits 12d41a023efb, bbe240454d86,
3619dec5103d, 1d9ddde12e3c, ccd9888f14a8, 199512b1234f).  I don't really feel
like doing it again. (Wait, was this supposed to *improve* security?)

> > 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.

This patch is about using the kernel to handle a class of algorithm,
Elliptic-Curve Diffie-Hellman.  Which specific algorithm in that class (i.e.
which elliptic curve), who knows.  Just like the existing APIs like this, it's
undocumented which algorithm(s) are actually supported.

- Eric

Re: [PATCH] KEYS: Add ECDH support

[Denis Kenzior]

Hi Eric,

> 
> Amusingly, the existing KEYCTL_DH_* APIs, and the KEYCTL_ECDH_* APIs proposed by
> this patch, only operate on user keys that the process has READ access to.  This
> means that the keys can be trivially extracted by a shell script running in your
> user session.  That's *less* secure than using an isolated process...
> 

I can see this being true for user or session keys, but I don't think this is 
true of process or thread specific keys.  At least I couldn't read any keys out 
of a test app when I tried.

Regards,
-Denis

Re: [PATCH] KEYS: Add ECDH support

[Denis Kenzior]

Hi Eric,

> 
> Amusingly, the existing KEYCTL_DH_* APIs, and the KEYCTL_ECDH_* APIs proposed by
> this patch, only operate on user keys that the process has READ access to.  This
> means that the keys can be trivially extracted by a shell script running in your
> user session.  That's *less* secure than using an isolated process...
> 

I can see this being true for user or session keys, but I don't think this is 
true of process or thread specific keys.  At least I couldn't read any keys out 
of a test app when I tried.

Regards,
-Denis

X-sender: <linux-kernel+bounces-125931-steffen.klassert=secunet.com@vger.kernel.org>
X-Receiver: <steffen.klassert@secunet.com> ORCPT=rfc822;steffen.klassert@secunet.com NOTIFY=NEVER; X-ExtendedProps=BQAVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ==
X-CreatedBy: MSExchange15
X-HeloDomain: b.mx.secunet.com
X-ExtendedProps: BQBjAAoAwapAQuxQ3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAGIACgB2AAAAqIoAAAUABAAUIAEAAAAcAAAAc3RlZmZlbi5rbGFzc2VydEBzZWN1bmV0LmNvbQUABgACAAEFACkAAgABDwAJAAAAQ0lBdWRpdGVkAgABBQACAAcAAQAAAAUAAwAHAAAAAAAFAAUAAgABBQBkAA8AAwAAAEh1Yg==
X-Source: SMTP:Default MBX-DRESDEN-01
X-SourceIPAddress: 62.96.220.37
X-EndOfInjectedXHeaders: 12053
Received: from cas-essen-02.secunet.de (10.53.40.202) by
 mbx-dresden-01.secunet.de (10.53.40.199) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.37; Sun, 31 Mar 2024 04:38:26 +0200
Received: from b.mx.secunet.com (62.96.220.37) by cas-essen-02.secunet.de
 (10.53.40.202) with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37 via Frontend
 Transport; Sun, 31 Mar 2024 04:38:26 +0200
Received: from localhost (localhost [127.0.0.1])
	by b.mx.secunet.com (Postfix) with ESMTP id 2AB2D20322
	for <steffen.klassert@secunet.com>; Sun, 31 Mar 2024 04:38:26 +0200 (CEST)
X-Virus-Scanned: by secunet
X-Spam-Flag: NO
X-Spam-Score: -2.749
X-Spam-Level:
X-Spam-Status: No, score=-2.749 tagged_above=-999 required=2.1
	tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1,
	DKIM_VALID_AU=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.001,
	FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249,
	MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_NONE=-0.0001,
	SPF_HELO_NONE=0.001, SPF_PASS=-0.001]
	autolearn=unavailable autolearn_force=no
Authentication-Results: a.mx.secunet.com (amavisd-new);
	dkim=pass (2048-bit key) header.d=gmail.com
Received: from b.mx.secunet.com ([127.0.0.1])
	by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id Y6JZ0jC6kB3o for <steffen.klassert@secunet.com>;
	Sun, 31 Mar 2024 04:38:25 +0200 (CEST)
Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=147.75.80.249; helo=am.mirrors.kernel.org; envelope-from=linux-kernel+bounces-125931-steffen.klassert=secunet.com@vger.kernel.org; receiver=steffen.klassert@secunet.com 
DKIM-Filter: OpenDKIM Filter v2.11.0 b.mx.secunet.com A615920199
Authentication-Results: b.mx.secunet.com;
	dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XleIRPSp"
Received: from am.mirrors.kernel.org (am.mirrors.kernel.org [147.75.80.249])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by b.mx.secunet.com (Postfix) with ESMTPS id A615920199
	for <steffen.klassert@secunet.com>; Sun, 31 Mar 2024 04:38:25 +0200 (CEST)
Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by am.mirrors.kernel.org (Postfix) with ESMTPS id 584181F218D9
	for <steffen.klassert@secunet.com>; Sun, 31 Mar 2024 02:38:25 +0000 (UTC)
Received: from localhost.localdomain (localhost.localdomain [127.0.0.1])
	by smtp.subspace.kernel.org (Postfix) with ESMTP id EB1C03D6D;
	Sun, 31 Mar 2024 02:38:11 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XleIRPSp"
Received: from mail-oo1-f45.google.com (mail-oo1-f45.google.com [209.85.161.45])
	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54A6C181;
	Sun, 31 Mar 2024 02:38:08 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.45
ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1711852689; cv=none; b=gXNEolHo55cI9s9E0fe7uOOSm88Jz7dwj3ls8ge3nw8RDM4vYnsK3QkV/TYCu8HKWXSxelrGFg26OaTa0ta2xAeaumLm+bNicuklMkDBxzgMakTmXxNf8xfV/uZLU1lr3i868qhdgUvfJgx0ptM9DjM8hr8IuQzNZ6hDb2tE66w=
ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1711852689; c=relaxed/simple;
	bh=SNeN2CYj+6bo6yYIrP8F5A4iIl0/Q9yGn+qoKYRUWH4=;
	h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:
	 In-Reply-To:Content-Type; b=WX0BCLiJLkXYSF23cXoAUUoCaN3U++73B96a084d5eByR6abt19vx+RRgPeFHn/FNkK/J6TmDIJzyF4IYk3FZTEjQ9I/pyxKjnmYqJBhqHBxbDk+/e+NGJ90rlOfa4MF1hGhvlAequCF3PKJT9TuvWJc3UIpKmFlHj11ZC0GCMk=
ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=XleIRPSp; arc=none smtp.client-ip=209.85.161.45
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com
Received: by mail-oo1-f45.google.com with SMTP id 006d021491bc7-5a47cecb98bso2073304eaf.0;
        Sat, 30 Mar 2024 19:38:08 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20230601; t=1711852687; x=1712457487; darn=vger.kernel.org;
        h=content-transfer-encoding:in-reply-to:from:references:cc:to
         :content-language:subject:user-agent:mime-version:date:message-id
         :from:to:cc:subject:date:message-id:reply-to;
        bh=lUs6NaIvupBIrN3kNgIHykr6WEWtZD3EhPX18G9uddY=;
        b=XleIRPSp55P9VHB7a2r/titnJwBaAjVmwFFWncW/trJnpln7+XtSjSvi9uqMgHENno
         mXoHhat/Z/Iu/etVc504MD8mbcqjpCdo92CyUAjqoOvDmqxWOTlUEoKSZpMXMU1tjGDE
         XbpXwWhrrBDGTCSBhMimQlOAAiFIgIn6MMASG45+bZdtNZH3XVRJ5bVJUjjXsZsqVSuD
         EAr0yLfv7Xw4ek1Nrgh1EsDej1shKOAN+fHmpmCt2k67uc0kQqgZTsLvlgNdkJABYumM
         NqMAc3CT/Ikjfg8Q4m9fVK1ahVo7HmKBBnIuDdfrFTz3L4Mf85eUXTvDWIq6NjILU5Zk
         EE5A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20230601; t=1711852687; x=1712457487;
        h=content-transfer-encoding:in-reply-to:from:references:cc:to
         :content-language:subject:user-agent:mime-version:date:message-id
         :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
        bh=lUs6NaIvupBIrN3kNgIHykr6WEWtZD3EhPX18G9uddY=;
        b=dR8iI2wu/bgmWoKgd3fQy1Qs3x8Gg8zaFnPHbg0FZVEZ0RXNeEaOyPNYJ1aIfjy24D
         15h1F4+67W51lrSYAej/3JXzlWZZfwN+sEoBc+2m+UdBjvHf18AI150uR/j7E1kwLdbX
         sGYqzl76u5sQvTr0S681UIXwuJI8SbyuckQSRFHBqVNDfJVH0TIeYJflzO0R6FpSMC8R
         9zz21UCZ3xMyhIWChGyQmc7Wt48iBBORmO2pxhHMcl7c4qiPRydQGd654U39D+gB3weL
         ELldBLvIApWZmFYYutf7AN/jmQk6rSAo74Gm8P8UkZUIxFx+1qy0xVLs34o8roNuo1WX
         +QIg==
X-Forwarded-Encrypted: i=1; AJvYcCVwL3cU3nCsvXYDTwS66oYsIOZe9xNTNPxXhx+b5LI0hF4hupv8P5wIUYO2JPUDl0WepDijhDJPYBg1N560PbnJ5RAa4R88i26Vu9VypwbBZecB5aqbsTeOFXgu4wuUSU6yA7yyNW3bsx+drdJwvoi1WZf5gLyATZ+18fbURnSBI4TAocIuILlIqVomkqoToJcDzLA9S5fTrbiTCkqMZeE=
X-Gm-Message-State: AOJu0YyW7TJsviPHgdYwWIVD+v3Bv1LiX1phxqUeZ9O5THjJ2TKywZ9M
	IdQhsJIEEtX7xf5p5m/dh/a51J+VTrkHVZa0tY90NWObJeeGoGmG
X-Google-Smtp-Source: AGHT+IG8y/mw6Sg+NJ68AiBnOVhIGZhncP4yQFjwCnn6QvLTTTIKr8wBQM2lppgPQLGZ1h9+K7oVJA==
X-Received: by 2002:a05:6820:260e:b0:5a5:639a:2fb8 with SMTP id cy14-20020a056820260e00b005a5639a2fb8mr6074835oob.4.1711852687375;
        Sat, 30 Mar 2024 19:38:07 -0700 (PDT)
Received: from [192.168.1.22] (070-114-247-242.res.spectrum.com. [70.114.247.242])
        by smtp.googlemail.com with ESMTPSA id bf14-20020a056820174e00b005a4bcb155basm1611035oob.23.2024.03.30.19.38.06
        (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
        Sat, 30 Mar 2024 19:38:07 -0700 (PDT)
Message-ID: <aae9bc89-ca34-400f-9c5e-44be1df2befa@gmail.com>
Date: Sat, 30 Mar 2024 21:38:05 -0500
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] KEYS: Add ECDH support
Content-Language: en-US
To: Eric Biggers <ebiggers@kernel.org>,
 James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Zhang Yiqun <zhangyiqun@phytium.com.cn>, 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
References: <20240330065506.3146-1-zhangyiqun@phytium.com.cn>
 <20240330070436.GA2116@sol.localdomain>
 <087bbfcf95c9014ee8f87d482773244f0833b892.camel@HansenPartnership.com>
 <20240331004844.GA104623@sol.localdomain>
From: Denis Kenzior <denkenz@gmail.com>
In-Reply-To: <20240331004844.GA104623@sol.localdomain>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Return-Path: linux-kernel+bounces-125931-steffen.klassert=secunet.com@vger.kernel.org
X-MS-Exchange-Organization-OriginalArrivalTime: 31 Mar 2024 02:38:26.2603
 (UTC)
X-MS-Exchange-Organization-Network-Message-Id: 25f48256-d726-4dcd-c568-08dc512ba45a
X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.37
X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.202
X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-02.secunet.de
X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=mbx-dresden-01.secunet.de:TOTAL-HUB=0.407|SMR=0.357(SMRDE=0.050|SMRC=0.306(SMRCL=0.100|X-SMRCR=0.306))|CAT=0.049(CATOS=0.011
 (CATSM=0.011(CATSM-Malware
 Agent=0.011))|CATRESL=0.023(CATRESLP2R=0.017)|CATORES=0.012
 (CATRS=0.012(CATRS-Index Routing Agent=0.011)));2024-03-31T02:38:26.699Z
X-MS-Exchange-Forest-ArrivalHubServer: mbx-dresden-01.secunet.de
X-MS-Exchange-Organization-AuthSource: cas-essen-02.secunet.de
X-MS-Exchange-Organization-AuthAs: Anonymous
X-MS-Exchange-Organization-FromEntityHeader: Internet
X-MS-Exchange-Organization-OriginalSize: 9469
X-MS-Exchange-Organization-HygienePolicy: Standard
X-MS-Exchange-Organization-MessageLatency: SRV=cas-essen-02.secunet.de:TOTAL-FE=0.032|SMR=0.022(SMRPI=0.020(SMRPI-FrontendProxyAgent=0.019))|SMS=0.009
X-MS-Exchange-Organization-AVStamp-Enterprise: 1.0
X-MS-Exchange-Organization-Recipient-Limit-Verified: True
X-MS-Exchange-Organization-TotalRecipientCount: 1
X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b
X-MS-Exchange-Forest-RulesExecuted: mbx-dresden-01
X-MS-Exchange-Organization-RulesExecuted: mbx-dresden-01
X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAfgBAAAPAAADH4sIAAAAAAAEAE1SXWvbMBRVE38k7jLYP7
 hvheDlJwxCE2jYHkbpy56KYt/Uoq5kJHmr/9N+5I6kzBQuRvfcc849
 Ev6bPSg6WtXUm2pTfUPR/m10Sr/0U02+Y+J35Tx6+n78df/04/nw8L
 yl/c+Tq0nqNlKuk+P9PKPBmsE4buk8BU/fKWDSN11NRvcTmYGt9IyG
 RseWXnlyYEkfDaFu2DnqpKPH4/5Asom9NzuiJ3gFzzeW+oMmOjRS05
 nJW/VbyR57+N1b2fgYhCS5jvueXGPV4MmOWoeLKU2TGW3wjFkcVimj
 4yrp7xxteyBb4M1oOWwMoYMSB+VML4P/NfNut4vvuKlOMY1jTrc/c1
 B4OzJdjE2bzLwsxq/pPHo6UWv0XbiU0q9JGy4cleYyvw20vrMsW3ID
 N+qimuiB1HtPPUsXnBoz9m0wi0Spp/RMZvSbCl6SPIMnh4H+dKwhwM
 txuwvxH/lF2tbhv/h6YB0SbCohFmKZic9rcbsU2UqsixtRiXIh8kwU
 QFBAPokyC5WDsBAFDnlsS7GK8jwVyEDwxbm4mqzyyE8gvuCvRYXC4U
 YsMAXzv88qelZFOAQ8Liowwt5SfAE/RUr4bAtCGSVok2eaLj5M8YUh
 fLIQbAkCmLGds+Up0oxEq6oUt/k/l0NX+VQDAAABDs4BUmV0cmlldm
 VyT3BlcmF0b3IsMTAsMTtSZXRyaWV2ZXJPcGVyYXRvciwxMSwwO1Bv
 c3REb2NQYXJzZXJPcGVyYXRvciwxMCwwO1Bvc3REb2NQYXJzZXJPcG
 VyYXRvciwxMSwwO1Bvc3RXb3JkQnJlYWtlckRpYWdub3N0aWNPcGVy
 YXRvciwxMCwwO1Bvc3RXb3JkQnJlYWtlckRpYWdub3N0aWNPcGVyYX
 RvciwxMSwwO1RyYW5zcG9ydFdyaXRlclByb2R1Y2VyLDIwLDY=
X-MS-Exchange-Forest-IndexAgent: 1 725
X-MS-Exchange-Forest-EmailMessageHash: 759BD60A
X-MS-Exchange-Forest-Language: en
X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent

Hi Eric,

> 
> Amusingly, the existing KEYCTL_DH_* APIs, and the KEYCTL_ECDH_* APIs proposed by
> this patch, only operate on user keys that the process has READ access to.  This
> means that the keys can be trivially extracted by a shell script running in your
> user session.  That's *less* secure than using an isolated process...
> 

I can see this being true for user or session keys, but I don't think this is 
true of process or thread specific keys.  At least I couldn't read any keys out 
of a test app when I tried.

Regards,
-Denis

Re: [PATCH] KEYS: Add ECDH support

[James Bottomley]

On Sat, 2024-03-30 at 17:48 -0700, Eric Biggers wrote:
> On Sat, Mar 30, 2024 at 09:09:51AM -0400, James Bottomley wrote:
[...]
> > 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.
> 
> First, that blog post never actually said that the "replace ssh-agent
> with kernel keyrings" idea was deployed.  It sounds like a proof of
> concept idea that someone thought was interesting and decided to blog
> about.  Upstream OpenSSH has no support for Linux keyrings.

The openssh community is incredibly resistant to out of house
innovation.  It has no support for engine or provider keys, for TPM
keys, or for that systemd start patch xz just exploited ...

>   It seems unlikely it would get added, especially given the OpenSSH
> developers' healthy skepticism of using broken Linux-isms.
> You're welcome to bring it up on openssh-unix-dev and get their buy-
> in first.

I also didn't say just openssh.  You picked the one you apparently know
hardly ever accepts anyone else's ideas.  I don't disagree that finding
implementors is reasonable ... I just wouldn't pick openssh as the
first upstream target.

> Second, as mentioned by the blog post, the kernel also does not
> support private keys in the default OpenSSH format.  That sort of
> thing is an example of the fundamental problem with trying to make
> the kernel support every cryptographic protocol and format in
> existence.  Userspace simply has much more flexibility to implement
> whatever it happens to need.

That's a complete red herring.  You don't need the kernel keyrings to
support every format, you just need a user space converter to import to
the kernel keyring format.  Every device or token that can replace key
handling has their own internal format and they all come with importers
that do conversion.

> Third, ssh-agent is already a separate process, and like any other
> process the kernel enforces isolation of its address space.  The
> potential loopholes are ptrace and coredumps, which ssh-agent already
> disables, except for ptrace by root which it can't do alone, but the
> system administrator can do that by setting the ptrace_scope sysctl
> to 3 or by using SELinux.

Well, a) this doesn't survive privilege escalation and b) I don't think
many people would buy into the notion that we should remove security
functions from the kernel and give them to userspace daemons because
it's safer.

James

Re: [PATCH] KEYS: Add ECDH support

[Jarkko Sakkinen]

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@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.

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