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

5 files changed, 73 insertions, 47 deletions

diff --git a/src/lib/libcrypto/ec/ec.h b/src/lib/libcrypto/ec/ec.h
index 686f018a9d..85951f0b97 100644
--- a/src/lib/libcrypto/ec/ec.h
+++ b/src/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/src/lib/libcrypto/ec/ec_err.c b/src/lib/libcrypto/ec/ec_err.c
index d797b937c2..9f2253dddd 100644
--- a/src/lib/libcrypto/ec/ec_err.c
+++ b/src/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/src/lib/libcrypto/ec/ec_kmeth.c b/src/lib/libcrypto/ec/ec_kmeth.c
index 3e997f8a5e..38aca0028e 100644
--- a/src/lib/libcrypto/ec/ec_kmeth.c
+++ b/src/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/src/lib/libcrypto/ec/ec_local.h b/src/lib/libcrypto/ec/ec_local.h
index 7a1f90886d..8153d4a96a 100644
--- a/src/lib/libcrypto/ec/ec_local.h
+++ b/src/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/src/lib/libcrypto/ecdh/ecdh.c b/src/lib/libcrypto/ecdh/ecdh.c
index 6ab4ff8382..034bd84a49 100644
--- a/src/lib/libcrypto/ecdh/ecdh.c
+++ b/src/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);