Move KDF handling to ECDH_compute_key()

In OpenSSL e2285d87, the KDF handling was moved from the compute_key()
method into the public API. A consequence of this change is that the
ECDH_compute_key() API no longer returns -1 for some errors. Existing
checks for <= 0 are safe as are those checking for the exact length as
return value, which is all what the ecosystem seems to be doing.

ok jsing

diff --git a/lib/libcrypto/ec/ec.h b/lib/libcrypto/ec/ec.h
index 686f018..85951f0 100644 (file)
--- a/lib/libcrypto/ec/ec.h
+++ b/lib/libcrypto/ec/ec.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec.h,v 1.44 2023/07/28 09:25:12 tb Exp $ */
+/* $OpenBSD: ec.h,v 1.45 2023/07/28 09:28:37 tb Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -368,8 +368,8 @@ void EC_KEY_METHOD_set_init(EC_KEY_METHOD *meth,
 void EC_KEY_METHOD_set_keygen(EC_KEY_METHOD *meth,
     int (*keygen)(EC_KEY *key));
 void EC_KEY_METHOD_set_compute_key(EC_KEY_METHOD *meth,
-    int (*ckey)(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-       void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen)));
+    int (*ckey)(unsigned char **out, size_t *out_len, const EC_POINT *pub_key,
+        const EC_KEY *ecdh));
 void EC_KEY_METHOD_set_sign(EC_KEY_METHOD *meth,
     int (*sign)(int type, const unsigned char *digest, int digest_len,
        unsigned char *signature, unsigned int *signature_len,
@@ -392,8 +392,8 @@ void EC_KEY_METHOD_get_init(const EC_KEY_METHOD *meth,
 void EC_KEY_METHOD_get_keygen(const EC_KEY_METHOD *meth,
     int (**pkeygen)(EC_KEY *key));
 void EC_KEY_METHOD_get_compute_key(const EC_KEY_METHOD *meth,
-    int (**pck)(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-       void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen)));
+    int (**pck)(unsigned char **out, size_t *out_len, const EC_POINT *pub_key,
+        const EC_KEY *ecdh));
 void EC_KEY_METHOD_get_sign(const EC_KEY_METHOD *meth,
     int (**psign)(int type, const unsigned char *digest, int digest_len,
         unsigned char *signature, unsigned int *signature_len,
@@ -684,6 +684,7 @@ void ERR_load_EC_strings(void);
 #define EC_R_INVALID_FORM                               104
 #define EC_R_INVALID_GROUP_ORDER                        122
 #define EC_R_INVALID_KEY                                165
+#define EC_R_INVALID_OUTPUT_LENGTH                      171
 #define EC_R_INVALID_PEER_KEY                           152
 #define EC_R_INVALID_PENTANOMIAL_BASIS                  132
 #define EC_R_INVALID_PRIVATE_KEY                        123

diff --git a/lib/libcrypto/ec/ec_err.c b/lib/libcrypto/ec/ec_err.c
index d797b93..9f2253d 100644 (file)
--- a/lib/libcrypto/ec/ec_err.c
+++ b/lib/libcrypto/ec/ec_err.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_err.c,v 1.17 2023/07/07 13:54:45 beck Exp $ */
+/* $OpenBSD: ec_err.c,v 1.18 2023/07/28 09:28:37 tb Exp $ */
 /* ====================================================================
  * Copyright (c) 1999-2011 The OpenSSL Project.  All rights reserved.
  *
@@ -98,6 +98,7 @@ static ERR_STRING_DATA EC_str_reasons[] =
        {ERR_REASON(EC_R_INVALID_FORM), "invalid form"},
        {ERR_REASON(EC_R_INVALID_GROUP_ORDER), "invalid group order"},
        {ERR_REASON(EC_R_INVALID_KEY), "invalid key"},
+       {ERR_REASON(EC_R_INVALID_OUTPUT_LENGTH), "invalid output length"},
        {ERR_REASON(EC_R_INVALID_PEER_KEY), "invalid peer key"},
        {ERR_REASON(EC_R_INVALID_PENTANOMIAL_BASIS), "invalid pentanomial basis"},
        {ERR_REASON(EC_R_INVALID_PRIVATE_KEY), "invalid private key"},

diff --git a/lib/libcrypto/ec/ec_kmeth.c b/lib/libcrypto/ec/ec_kmeth.c
index 3e997f8..38aca00 100644 (file)
--- a/lib/libcrypto/ec/ec_kmeth.c
+++ b/lib/libcrypto/ec/ec_kmeth.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ec_kmeth.c,v 1.11 2023/07/07 13:54:45 beck Exp $      */
+/*     $OpenBSD: ec_kmeth.c,v 1.12 2023/07/28 09:28:37 tb Exp $        */
 /*
  * Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
@@ -238,8 +238,8 @@ LCRYPTO_ALIAS(EC_KEY_METHOD_set_keygen);
 
 void
 EC_KEY_METHOD_set_compute_key(EC_KEY_METHOD *meth,
-   int (*ckey)(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-   void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen)))
+    int (*ckey)(unsigned char **out, size_t *out_len, const EC_POINT *pub_key,
+        const EC_KEY *ecdh))
 {
        meth->compute_key = ckey;
 }
@@ -310,8 +310,8 @@ LCRYPTO_ALIAS(EC_KEY_METHOD_get_keygen);
 
 void
 EC_KEY_METHOD_get_compute_key(const EC_KEY_METHOD *meth,
-    int (**pck)(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-    void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen)))
+    int (**pck)(unsigned char **out, size_t *out_len, const EC_POINT *pub_key,
+        const EC_KEY *ecdh))
 {
        if (pck != NULL)
                *pck = meth->compute_key;

diff --git a/lib/libcrypto/ec/ec_local.h b/lib/libcrypto/ec/ec_local.h
index 7a1f908..8153d4a 100644 (file)
--- a/lib/libcrypto/ec/ec_local.h
+++ b/lib/libcrypto/ec/ec_local.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_local.h,v 1.24 2023/07/05 08:39:40 tb Exp $ */
+/* $OpenBSD: ec_local.h,v 1.25 2023/07/28 09:28:37 tb Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -323,8 +323,8 @@ struct ec_key_method_st {
        int (*set_private)(EC_KEY *key, const BIGNUM *priv_key);
        int (*set_public)(EC_KEY *key, const EC_POINT *pub_key);
        int (*keygen)(EC_KEY *key);
-       int (*compute_key)(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-           void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen));
+       int (*compute_key)(unsigned char **out, size_t *out_len,
+           const EC_POINT *pub_key, const EC_KEY *ecdh);
        int (*sign)(int type, const unsigned char *dgst, int dlen, unsigned char
            *sig, unsigned int *siglen, const BIGNUM *kinv,
            const BIGNUM *r, EC_KEY *eckey);
@@ -342,8 +342,8 @@ struct ec_key_method_st {
 #define EC_KEY_METHOD_DYNAMIC   1
 
 int ec_key_gen(EC_KEY *eckey);
-int ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-    void *(*KDF) (const void *in, size_t inlen, void *out, size_t *outlen));
+int ecdh_compute_key(unsigned char **out, size_t *out_len,
+    const EC_POINT *pub_key, const EC_KEY *ecdh);
 int ecdsa_verify(int type, const unsigned char *dgst, int dgst_len,
     const unsigned char *sigbuf, int sig_len, EC_KEY *eckey);
 int ecdsa_verify_sig(const unsigned char *dgst, int dgst_len,

diff --git a/lib/libcrypto/ecdh/ecdh.c b/lib/libcrypto/ecdh/ecdh.c
index 6ab4ff8..034bd84 100644 (file)
--- a/lib/libcrypto/ecdh/ecdh.c
+++ b/lib/libcrypto/ecdh/ecdh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecdh.c,v 1.6 2023/07/24 17:08:53 tb Exp $ */
+/* $OpenBSD: ecdh.c,v 1.7 2023/07/28 09:28:37 tb Exp $ */
 /* ====================================================================
  * Copyright 2002 Sun Microsystems, Inc. ALL RIGHTS RESERVED.
  *
@@ -145,10 +145,9 @@ ecdh_KDF_X9_63(unsigned char *out, size_t outlen, const unsigned char *Z,
 /*
  * Based on the ECKAS-DH1 and ECSVDP-DH primitives in the IEEE 1363 standard.
  */
-/* XXX - KDF handling moved to ECDH_compute_key().  See OpenSSL e2285d87. */
 int
-ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh,
-    void *(*KDF)(const void *in, size_t inlen, void *out, size_t *outlen))
+ecdh_compute_key(unsigned char **out, size_t *out_len, const EC_POINT *pub_key,
+    const EC_KEY *ecdh)
 {
        BN_CTX *ctx;
        BIGNUM *x;
@@ -157,13 +156,10 @@ ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh
        EC_POINT *point = NULL;
        unsigned char *buf = NULL;
        int buflen;
-       int ret = -1;
+       int ret = 0;
 
-       if (outlen > INT_MAX) {
-               /* Sort of, anyway. */
-               ECerror(ERR_R_MALLOC_FAILURE);
-               return -1;
-       }
+       *out = NULL;
+       *out_len = 0;
 
        if ((ctx = BN_CTX_new()) == NULL)
                goto err;
@@ -203,11 +199,6 @@ ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh
                ECerror(ERR_R_INTERNAL_ERROR);
                goto err;
        }
-       if (KDF == NULL && outlen < buflen) {
-               /* The resulting key would be truncated. */
-               ECerror(EC_R_KEY_TRUNCATION);
-               goto err;
-       }
        if ((buf = malloc(buflen)) == NULL) {
                ECerror(ERR_R_MALLOC_FAILURE);
                goto err;
@@ -217,19 +208,12 @@ ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh
                goto err;
        }
 
-       if (KDF != NULL) {
-               if (KDF(buf, buflen, out, &outlen) == NULL) {
-                       ECerror(EC_R_KDF_FAILED);
-                       goto err;
-               }
-       } else {
-               memset(out, 0, outlen);
-               if (outlen > buflen)
-                       outlen = buflen;
-               memcpy(out, buf, outlen);
-       }
+       *out = buf;
+       *out_len = buflen;
+       buf = NULL;
+
+       ret = 1;
 
-       ret = outlen;
  err:
        EC_POINT_free(point);
        BN_CTX_end(ctx);
@@ -240,15 +224,55 @@ ecdh_compute_key(void *out, size_t outlen, const EC_POINT *pub_key, EC_KEY *ecdh
 }
 
 int
-ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
+ECDH_compute_key(void *out, size_t out_len, const EC_POINT *pub_key,
     EC_KEY *eckey,
-    void *(*KDF)(const void *in, size_t inlen, void *out, size_t *outlen))
+    void *(*KDF)(const void *in, size_t inlen, void *out, size_t *out_len))
 {
+       unsigned char *secret = NULL;
+       size_t secret_len = 0;
+       int ret = 0;
+
        if (eckey->meth->compute_key == NULL) {
                ECerror(EC_R_NOT_IMPLEMENTED);
-               return 0;
+               goto err;
+       }
+
+       if (out_len > INT_MAX) {
+               ECerror(EC_R_INVALID_OUTPUT_LENGTH);
+               goto err;
+       }
+
+       if (!eckey->meth->compute_key(&secret, &secret_len, pub_key, eckey))
+               goto err;
+
+       if (KDF != NULL) {
+               if (KDF(secret, secret_len, out, &out_len) == NULL) {
+                       ECerror(EC_R_KDF_FAILED);
+                       goto err;
+               }
+       } else {
+               memset(out, 0, out_len);
+               if (out_len < secret_len) {
+                       /* The resulting key would be truncated. */
+                       ECerror(EC_R_KEY_TRUNCATION);
+                       goto err;
+               }
+               if (out_len > secret_len)
+                       out_len = secret_len;
+               memcpy(out, secret, out_len);
        }
-       return eckey->meth->compute_key(out, outlen, pub_key, eckey, KDF);
+
+       if (out_len > INT_MAX) {
+               ECerror(EC_R_INVALID_OUTPUT_LENGTH);
+               goto err;
+       }
+
+       ret = out_len;
+
+ err:
+       freezero(secret, secret_len);
+
+       return ret;
 }
 LCRYPTO_ALIAS(ECDH_compute_key);