diff mbox series

[v2,3/4] crypto: ecdsa - Fix enc/dec size reported by KEYCTL_PKEY_QUERY

Message ID 3d74d6134f4f87a90ebe0a37cb06c6ec144ceef7.1738521533.git.lukas@wunner.de
State New
Headers show
Series ecdsa KEYCTL_PKEY_QUERY fixes | expand

Commit Message

Lukas Wunner Feb. 2, 2025, 7 p.m. UTC
KEYCTL_PKEY_QUERY system calls for ecdsa keys return the key size as
max_enc_size and max_dec_size, even though such keys cannot be used for
encryption/decryption.  They're exclusively for signature generation or
verification.

Only rsa keys with pkcs1 encoding can also be used for encryption or
decryption.

Return 0 instead for ecdsa keys (as well as ecrdsa keys).

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 crypto/asymmetric_keys/public_key.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Herbert Xu Feb. 10, 2025, 7:54 a.m. UTC | #1
On Sun, Feb 09, 2025 at 12:29:54PM +0100, Lukas Wunner wrote:
>
> One user of this API is the Embedded Linux Library, which in turn
> is used by Intel Wireless Daemon:
> 
> https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/key.c
> https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/src/eap-tls.c

Surely this doesn't use the private key part of the API, does it?

While I intensely dislike the entire API being there, it's only the
private key part that I really want to remove.

Thanks,
Herbert Xu Feb. 16, 2025, 4:19 a.m. UTC | #2
On Mon, Feb 10, 2025 at 07:53:57PM +0100, Lukas Wunner wrote:
>
> > > https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/key.c
> > > https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/src/eap-tls.c
> > 
> > Surely this doesn't use the private key part of the API, does it?
> 
> It does use the private key part:
> 
> It takes advantage of the kernel's Key Retention Service for EAP-TLS,
> which generally uses mutual authentication.  E.g. clients authenticate
> against a wireless hotspot.  Hence it does invoke KEYCTL_PKEY_SIGN and
> KEYCTL_PKEY_ENCRYPT (with private keys, obviously).

Does it really? I grepped the whole iwd git tree and the only
use of private key functionality is to check that it matches
the public key, IOW it encrypts a piece of text and then decrypts
it again to check whether they match.

It doesn't make use of any other private key functionality AFAICS.

Cheers,
Lukas Wunner Feb. 16, 2025, 10:45 a.m. UTC | #3
On Sun, Feb 16, 2025 at 12:19:44PM +0800, Herbert Xu wrote:
> On Mon, Feb 10, 2025 at 07:53:57PM +0100, Lukas Wunner wrote:
> > > > https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/key.c
> > > > https://git.kernel.org/pub/scm/network/wireless/iwd.git/tree/src/eap-tls.c
> > > 
> > > Surely this doesn't use the private key part of the API, does it?
> > 
> > It does use the private key part:
> > 
> > It takes advantage of the kernel's Key Retention Service for EAP-TLS,
> > which generally uses mutual authentication.  E.g. clients authenticate
> > against a wireless hotspot.  Hence it does invoke KEYCTL_PKEY_SIGN and
> > KEYCTL_PKEY_ENCRYPT (with private keys, obviously).
> 
> Does it really? I grepped the whole iwd git tree and the only
> use of private key functionality is to check that it matches
> the public key, IOW it encrypts a piece of text and then decrypts
> it again to check whether they match.
> 
> It doesn't make use of any other private key functionality AFAICS.

__eap_handle_request()                            [iwd src/eap.c]
  eap->method->handle_request()
    eap_tls_common_handle_request()               [iwd src/eap-tls-common.c]
      l_tls_handle_rx()                           [ell ell/tls-record.c]
        tls_handle_ciphertext()
          tls_handle_plaintext()
            tls_handle_message()                  [ell ell/tls.c]
              tls_handle_handshake()
                tls_handle_server_hello_done()
                  tls_send_certificate_verify()
                    tls->pending.cipher_suite->signature->sign
                      tls_rsa_sign()              [ell ell/tls-suites.c]
                        l_key_sign()              [ell ell/key.c]
                          eds_common()
                            kernel_key_eds()
                              syscall(__NR_keyctl, KEYCTL_PKEY_SIGN, ...)

... where tls_handle_server_hello_done() performs client authentication
per RFC 8446 sec 4.6.2:

  "When the client has sent the "post_handshake_auth" extension (see
   Section 4.2.6), a server MAY request client authentication at any
   time after the handshake has completed by sending a
   CertificateRequest message.  The client MUST respond with the
   appropriate Authentication messages (see Section 4.4).  If the client
   chooses to authenticate, it MUST send Certificate, CertificateVerify,
   and Finished."

   https://datatracker.ietf.org/doc/html/rfc8446#section-4.6.2

I think the best option at this point isn't to aim for removal
but to wait for Cloudflare to beat their out-of-tree implementation
(which apparently isn't susceptible to side channel attacks)
into shape so that it can be upstreamed.

Thanks,

Lukas
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index bf165d321440..dd44a966947f 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -188,6 +188,8 @@  static int software_key_query(const struct kernel_pkey_params *params,
 	ptr = pkey_pack_u32(ptr, pkey->paramlen);
 	memcpy(ptr, pkey->params, pkey->paramlen);
 
+	memset(info, 0, sizeof(*info));
+
 	if (issig) {
 		sig = crypto_alloc_sig(alg_name, 0, 0);
 		if (IS_ERR(sig)) {
@@ -211,6 +213,9 @@  static int software_key_query(const struct kernel_pkey_params *params,
 			info->supported_ops |= KEYCTL_SUPPORTS_SIGN;
 
 		if (strcmp(params->encoding, "pkcs1") == 0) {
+			info->max_enc_size = len;
+			info->max_dec_size = len;
+
 			info->supported_ops |= KEYCTL_SUPPORTS_ENCRYPT;
 			if (pkey->key_is_private)
 				info->supported_ops |= KEYCTL_SUPPORTS_DECRYPT;
@@ -232,6 +237,8 @@  static int software_key_query(const struct kernel_pkey_params *params,
 		len = crypto_akcipher_maxsize(tfm);
 		info->max_sig_size = len;
 		info->max_data_size = len;
+		info->max_enc_size = len;
+		info->max_dec_size = len;
 
 		info->supported_ops = KEYCTL_SUPPORTS_ENCRYPT;
 		if (pkey->key_is_private)
@@ -239,8 +246,6 @@  static int software_key_query(const struct kernel_pkey_params *params,
 	}
 
 	info->key_size = len * 8;
-	info->max_enc_size = len;
-	info->max_dec_size = len;
 
 	ret = 0;