From defc2a86a244a5e5c77516ac11e73e9d20c7d35f Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 28 Jul 2023 09:28:37 +0000 Subject: [PATCH] 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 --- lib/libcrypto/ec/ec.h | 11 ++--- lib/libcrypto/ec/ec_err.c | 3 +- lib/libcrypto/ec/ec_kmeth.c | 10 ++--- lib/libcrypto/ec/ec_local.h | 10 ++--- lib/libcrypto/ecdh/ecdh.c | 86 ++++++++++++++++++++++++------------- 5 files changed, 73 insertions(+), 47 deletions(-) diff --git a/lib/libcrypto/ec/ec.h b/lib/libcrypto/ec/ec.h index 686f018a9d1..85951f0b97f 100644 --- 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 d797b937c27..9f2253dddd9 100644 --- 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 3e997f8a5e5..38aca0028e4 100644 --- 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 7a1f90886d1..8153d4a96a7 100644 --- 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 6ab4ff83825..034bd84a49a 100644 --- 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); -- 2.20.1