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

1 files changed, 55 insertions, 31 deletions

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);