From e9f8f81a821e19673eb8c9b6877403815ed74696 Mon Sep 17 00:00:00 2001 From: Cedric Izoard Date: Fri, 29 Oct 2021 11:05:31 +0200 Subject: [PATCH] DPP: Use ECDH from crypto.h Use crypto.h API to implement ECDH in DPP. This needs a new initialization function in crypto.h to initialize an ECDH with a given EC key. Using crypto_ecdh_set_peerkey() to generate the ECDH secret in an intermediate and dynamically allocated buffer removed the need for the DPP-specific workaround for inconsistent length returned by EVP_PKEY_derive() since that crypto_ecdh_set_peerkey() implementation already had functionality for covering the changing secret_len value from commit d001fe31ab0a ("OpenSSL: Handle EVP_PKEY_derive() secret_len changes for ECDH"). Signed-off-by: Cedric Izoard --- src/common/dpp_crypto.c | 77 +++++++++---------------------------- src/crypto/crypto.h | 36 +++++++++++------ src/crypto/crypto_openssl.c | 26 +++++++++++++ 3 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/common/dpp_crypto.c b/src/common/dpp_crypto.c index ceb621a0b..fa8b240c2 100644 --- a/src/common/dpp_crypto.c +++ b/src/common/dpp_crypto.c @@ -566,84 +566,45 @@ int dpp_derive_bk_ke(struct dpp_authentication *auth) int dpp_ecdh(struct crypto_ec_key *own, struct crypto_ec_key *peer, u8 *secret, size_t *secret_len) { - EVP_PKEY_CTX *ctx; + struct crypto_ecdh *ecdh; + struct wpabuf *peer_pub, *secret_buf = NULL; int ret = -1; - ERR_clear_error(); *secret_len = 0; - ctx = EVP_PKEY_CTX_new((EVP_PKEY *) own, NULL); - if (!ctx) { - wpa_printf(MSG_ERROR, "DPP: EVP_PKEY_CTX_new failed: %s", - ERR_error_string(ERR_get_error(), NULL)); + ecdh = crypto_ecdh_init2(crypto_ec_key_group(own), own); + if (!ecdh) { + wpa_printf(MSG_ERROR, "DPP: crypto_ecdh_init2() failed"); return -1; } - if (EVP_PKEY_derive_init(ctx) != 1) { - wpa_printf(MSG_ERROR, "DPP: EVP_PKEY_derive_init failed: %s", - ERR_error_string(ERR_get_error(), NULL)); - goto fail; - } - - if (EVP_PKEY_derive_set_peer(ctx, (EVP_PKEY *) peer) != 1) { + peer_pub = crypto_ec_key_get_pubkey_point(peer, 0); + if (!peer_pub) { wpa_printf(MSG_ERROR, - "DPP: EVP_PKEY_derive_set_peet failed: %s", - ERR_error_string(ERR_get_error(), NULL)); + "DPP: crypto_ec_key_get_pubkey_point() failed"); goto fail; } - if (EVP_PKEY_derive(ctx, NULL, secret_len) != 1) { - wpa_printf(MSG_ERROR, "DPP: EVP_PKEY_derive(NULL) failed: %s", - ERR_error_string(ERR_get_error(), NULL)); + secret_buf = crypto_ecdh_set_peerkey(ecdh, 1, wpabuf_head(peer_pub), + wpabuf_len(peer_pub)); + if (!secret_buf) { + wpa_printf(MSG_ERROR, "DPP: crypto_ecdh_set_peerkey() failed"); goto fail; } - if (*secret_len > DPP_MAX_SHARED_SECRET_LEN) { - u8 buf[200]; - int level = *secret_len > 200 ? MSG_ERROR : MSG_DEBUG; - - /* It looks like OpenSSL can return unexpectedly large buffer - * need for shared secret from EVP_PKEY_derive(NULL) in some - * cases. For example, group 19 has shown cases where secret_len - * is set to 72 even though the actual length ends up being - * updated to 32 when EVP_PKEY_derive() is called with a buffer - * for the value. Work around this by trying to fetch the value - * and continue if it is within supported range even when the - * initial buffer need is claimed to be larger. */ - wpa_printf(level, - "DPP: Unexpected secret_len=%d from EVP_PKEY_derive()", - (int) *secret_len); - if (*secret_len > 200) - goto fail; - if (EVP_PKEY_derive(ctx, buf, secret_len) != 1) { - wpa_printf(MSG_ERROR, "DPP: EVP_PKEY_derive failed: %s", - ERR_error_string(ERR_get_error(), NULL)); - goto fail; - } - if (*secret_len > DPP_MAX_SHARED_SECRET_LEN) { - wpa_printf(MSG_ERROR, - "DPP: Unexpected secret_len=%d from EVP_PKEY_derive()", - (int) *secret_len); - goto fail; - } - wpa_hexdump_key(MSG_DEBUG, "DPP: Unexpected secret_len change", - buf, *secret_len); - os_memcpy(secret, buf, *secret_len); - forced_memzero(buf, sizeof(buf)); - goto done; - } - - if (EVP_PKEY_derive(ctx, secret, secret_len) != 1) { - wpa_printf(MSG_ERROR, "DPP: EVP_PKEY_derive failed: %s", - ERR_error_string(ERR_get_error(), NULL)); + if (wpabuf_len(secret_buf) > DPP_MAX_SHARED_SECRET_LEN) { + wpa_printf(MSG_ERROR, "DPP: ECDH secret longer than expected"); goto fail; } -done: + *secret_len = wpabuf_len(secret_buf); + os_memcpy(secret, wpabuf_head(secret_buf), wpabuf_len(secret_buf)); ret = 0; fail: - EVP_PKEY_CTX_free(ctx); + wpabuf_clear_free(secret_buf); + wpabuf_free(peer_pub); + crypto_ecdh_deinit(ecdh); return ret; } diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index a59a81262..eb600699d 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -945,7 +945,15 @@ void crypto_ec_point_debug_print(const struct crypto_ec *e, const char *title); /** - * struct crypto_ecdh - Elliptic curve Diffie–Hellman context + * struct crypto_ec_key - Elliptic curve key pair + * + * Internal data structure for EC key pair. The contents is specific to the used + * crypto library. + */ +struct crypto_ec_key; + +/** + * struct crypto_ecdh - Elliptic Curve Diffie–Hellman context * * Internal data structure for ECDH. The contents is specific to the used * crypto library. @@ -956,13 +964,25 @@ struct crypto_ecdh; * crypto_ecdh_init - Initialize elliptic curve Diffie–Hellman context * @group: Identifying number for the ECC group (IANA "Group Description" * attribute registry for RFC 2409) + * This function generates an ephemeral key pair. * Returns: Pointer to ECDH context or %NULL on failure */ struct crypto_ecdh * crypto_ecdh_init(int group); +/** + * crypto_ecdh_init2 - Initialize elliptic curve Diffie–Hellman context with a + * given EC key + * @group: Identifying number for the ECC group (IANA "Group Description" + * attribute registry for RFC 2409) + * @own_key: Our own EC Key + * Returns: Pointer to ECDH context or %NULL on failure + */ +struct crypto_ecdh * crypto_ecdh_init2(int group, + struct crypto_ec_key *own_key); + /** * crypto_ecdh_get_pubkey - Retrieve public key from ECDH context - * @ecdh: ECDH context from crypto_ecdh_init() + * @ecdh: ECDH context from crypto_ecdh_init() or crypto_ecdh_init2() * @inc_y: Whether public key should include y coordinate (explicit form) * or not (compressed form) * Returns: Binary data f the public key or %NULL on failure @@ -971,7 +991,7 @@ struct wpabuf * crypto_ecdh_get_pubkey(struct crypto_ecdh *ecdh, int inc_y); /** * crypto_ecdh_set_peerkey - Compute ECDH secret - * @ecdh: ECDH context from crypto_ecdh_init() + * @ecdh: ECDH context from crypto_ecdh_init() or crypto_ecdh_init2() * @inc_y: Whether peer's public key includes y coordinate (explicit form) * or not (compressed form) * @key: Binary data of the peer's public key @@ -983,7 +1003,7 @@ struct wpabuf * crypto_ecdh_set_peerkey(struct crypto_ecdh *ecdh, int inc_y, /** * crypto_ecdh_deinit - Free ECDH context - * @ecdh: ECDH context from crypto_ecdh_init() + * @ecdh: ECDH context from crypto_ecdh_init() or crypto_ecdh_init2() */ void crypto_ecdh_deinit(struct crypto_ecdh *ecdh); @@ -994,14 +1014,6 @@ void crypto_ecdh_deinit(struct crypto_ecdh *ecdh); */ size_t crypto_ecdh_prime_len(struct crypto_ecdh *ecdh); -/** - * struct crypto_ec_key - Elliptic curve key pair - * - * Internal data structure for EC key pair. The contents is specific to the used - * crypto library. - */ -struct crypto_ec_key; - /** * crypto_ec_key_parse_priv - Initialize EC key pair from ECPrivateKey ASN.1 * @der: DER encoding of ASN.1 ECPrivateKey diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c index c198748de..138ab9aed 100644 --- a/src/crypto/crypto_openssl.c +++ b/src/crypto/crypto_openssl.c @@ -2085,6 +2085,32 @@ fail: } +struct crypto_ecdh * crypto_ecdh_init2(int group, struct crypto_ec_key *own_key) +{ + struct crypto_ecdh *ecdh; + + ecdh = os_zalloc(sizeof(*ecdh)); + if (!ecdh) + goto fail; + + ecdh->ec = crypto_ec_init(group); + if (!ecdh->ec) + goto fail; + + ecdh->pkey = EVP_PKEY_new(); + if (!ecdh->pkey || + EVP_PKEY_assign_EC_KEY(ecdh->pkey, + EVP_PKEY_get1_EC_KEY((EVP_PKEY *) own_key)) + != 1) + goto fail; + + return ecdh; +fail: + crypto_ecdh_deinit(ecdh); + return NULL; +} + + struct wpabuf * crypto_ecdh_get_pubkey(struct crypto_ecdh *ecdh, int inc_y) { struct wpabuf *buf = NULL;