From 2086ae46b3cc26e0a81c8f68429b5b8875739d3f Mon Sep 17 00:00:00 2001 From: Cedric Izoard Date: Fri, 29 Oct 2021 11:05:32 +0200 Subject: [PATCH] DPP: Replace dpp_bootstrap_key_der() with crypto_ec_key_get_subject_public_key() As BoringSSL version of i2d_PUBKEY() doesn't respect the POINT_CONVERSION_COMPRESSED flag redefine a specific crypto_ec_key_get_subject_public_key() version for BoringSSL based on dpp_bootstrap_key_der(). The only other user of crypto_ec_key_get_subject_public_key() is SAE-PK for which the public key should also be formatted using compressed format. Signed-off-by: Cedric Izoard --- src/common/dpp.c | 2 - src/common/dpp_crypto.c | 89 +---------------------------------- src/crypto/crypto_openssl.c | 94 +++++++++++++++++++++++++++++++++++++ tests/hwsim/test_dpp.py | 3 +- 4 files changed, 98 insertions(+), 90 deletions(-) diff --git a/src/common/dpp.c b/src/common/dpp.c index 63189c169..1fd074f05 100644 --- a/src/common/dpp.c +++ b/src/common/dpp.c @@ -8,8 +8,6 @@ */ #include "utils/includes.h" -#include -#include #include "utils/common.h" #include "utils/base64.h" diff --git a/src/common/dpp_crypto.c b/src/common/dpp_crypto.c index fa8b240c2..da59730eb 100644 --- a/src/common/dpp_crypto.c +++ b/src/common/dpp_crypto.c @@ -8,10 +8,6 @@ */ #include "utils/includes.h" -#include -#include -#include -#include #include "utils/common.h" #include "utils/base64.h" @@ -295,93 +291,12 @@ struct crypto_ec_key * dpp_set_keypair(const struct dpp_curve_params **curve, } -typedef struct { - /* AlgorithmIdentifier ecPublicKey with optional parameters present - * as an OID identifying the curve */ - X509_ALGOR *alg; - /* Compressed format public key per ANSI X9.63 */ - ASN1_BIT_STRING *pub_key; -} DPP_BOOTSTRAPPING_KEY; - -ASN1_SEQUENCE(DPP_BOOTSTRAPPING_KEY) = { - ASN1_SIMPLE(DPP_BOOTSTRAPPING_KEY, alg, X509_ALGOR), - ASN1_SIMPLE(DPP_BOOTSTRAPPING_KEY, pub_key, ASN1_BIT_STRING) -} ASN1_SEQUENCE_END(DPP_BOOTSTRAPPING_KEY); - -IMPLEMENT_ASN1_FUNCTIONS(DPP_BOOTSTRAPPING_KEY); - - -static struct wpabuf * dpp_bootstrap_key_der(struct crypto_ec_key *key) -{ - unsigned char *der = NULL; - int der_len; - const EC_KEY *eckey; - struct wpabuf *ret = NULL; - size_t len; - const EC_GROUP *group; - const EC_POINT *point; - BN_CTX *ctx; - DPP_BOOTSTRAPPING_KEY *bootstrap = NULL; - int nid; - - ctx = BN_CTX_new(); - eckey = EVP_PKEY_get0_EC_KEY((EVP_PKEY *) key); - if (!ctx || !eckey) - goto fail; - - group = EC_KEY_get0_group(eckey); - point = EC_KEY_get0_public_key(eckey); - if (!group || !point) - goto fail; - nid = EC_GROUP_get_curve_name(group); - - bootstrap = DPP_BOOTSTRAPPING_KEY_new(); - if (!bootstrap || - X509_ALGOR_set0(bootstrap->alg, OBJ_nid2obj(EVP_PKEY_EC), - V_ASN1_OBJECT, (void *) OBJ_nid2obj(nid)) != 1) - goto fail; - - len = EC_POINT_point2oct(group, point, POINT_CONVERSION_COMPRESSED, - NULL, 0, ctx); - if (len == 0) - goto fail; - - der = OPENSSL_malloc(len); - if (!der) - goto fail; - len = EC_POINT_point2oct(group, point, POINT_CONVERSION_COMPRESSED, - der, len, ctx); - - OPENSSL_free(bootstrap->pub_key->data); - bootstrap->pub_key->data = der; - der = NULL; - bootstrap->pub_key->length = len; - /* No unused bits */ - bootstrap->pub_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - bootstrap->pub_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; - - der_len = i2d_DPP_BOOTSTRAPPING_KEY(bootstrap, &der); - if (der_len <= 0) { - wpa_printf(MSG_ERROR, - "DDP: Failed to build DER encoded public key"); - goto fail; - } - - ret = wpabuf_alloc_copy(der, der_len); -fail: - DPP_BOOTSTRAPPING_KEY_free(bootstrap); - OPENSSL_free(der); - BN_CTX_free(ctx); - return ret; -} - - int dpp_bootstrap_key_hash(struct dpp_bootstrap_info *bi) { struct wpabuf *der; int res; - der = dpp_bootstrap_key_der(bi->pubkey); + der = crypto_ec_key_get_subject_public_key(bi->pubkey); if (!der) return -1; wpa_hexdump_buf(MSG_DEBUG, "DPP: Compressed public key (DER)", @@ -416,7 +331,7 @@ int dpp_keygen(struct dpp_bootstrap_info *bi, const char *curve, goto fail; bi->own = 1; - der = dpp_bootstrap_key_der(bi->pubkey); + der = crypto_ec_key_get_subject_public_key(bi->pubkey); if (!der) goto fail; wpa_hexdump_buf(MSG_DEBUG, "DPP: Compressed public key (DER)", diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c index 138ab9aed..98b93d9a7 100644 --- a/src/crypto/crypto_openssl.c +++ b/src/crypto/crypto_openssl.c @@ -2501,12 +2501,105 @@ void crypto_ec_key_deinit(struct crypto_ec_key *key) } +#ifdef OPENSSL_IS_BORINGSSL + +/* BoringSSL version of i2d_PUBKEY() always outputs public EC key using + * uncompressed form so define a custom function to export EC pubkey using + * the compressed format that is explicitly required for some protocols. */ + +#include +#include + +typedef struct { + /* AlgorithmIdentifier ecPublicKey with optional parameters present + * as an OID identifying the curve */ + X509_ALGOR *alg; + /* Compressed format public key per ANSI X9.63 */ + ASN1_BIT_STRING *pub_key; +} EC_COMP_PUBKEY; + +ASN1_SEQUENCE(EC_COMP_PUBKEY) = { + ASN1_SIMPLE(EC_COMP_PUBKEY, alg, X509_ALGOR), + ASN1_SIMPLE(EC_COMP_PUBKEY, pub_key, ASN1_BIT_STRING) +} ASN1_SEQUENCE_END(EC_COMP_PUBKEY); + +IMPLEMENT_ASN1_FUNCTIONS(EC_COMP_PUBKEY); + +#endif /* OPENSSL_IS_BORINGSSL */ + + struct wpabuf * crypto_ec_key_get_subject_public_key(struct crypto_ec_key *key) { +#ifdef OPENSSL_IS_BORINGSSL + unsigned char *der = NULL; + int der_len; + const EC_KEY *eckey; + struct wpabuf *ret = NULL; + size_t len; + const EC_GROUP *group; + const EC_POINT *point; + BN_CTX *ctx; + EC_COMP_PUBKEY *pubkey = NULL; + int nid; + + ctx = BN_CTX_new(); + eckey = EVP_PKEY_get0_EC_KEY((EVP_PKEY *) key); + if (!ctx || !eckey) + goto fail; + + group = EC_KEY_get0_group(eckey); + point = EC_KEY_get0_public_key(eckey); + if (!group || !point) + goto fail; + nid = EC_GROUP_get_curve_name(group); + + pubkey = EC_COMP_PUBKEY_new(); + if (!pubkey || + X509_ALGOR_set0(pubkey->alg, OBJ_nid2obj(EVP_PKEY_EC), + V_ASN1_OBJECT, (void *) OBJ_nid2obj(nid)) != 1) + goto fail; + + len = EC_POINT_point2oct(group, point, POINT_CONVERSION_COMPRESSED, + NULL, 0, ctx); + if (len == 0) + goto fail; + + der = OPENSSL_malloc(len); + if (!der) + goto fail; + len = EC_POINT_point2oct(group, point, POINT_CONVERSION_COMPRESSED, + der, len, ctx); + + OPENSSL_free(pubkey->pub_key->data); + pubkey->pub_key->data = der; + der = NULL; + pubkey->pub_key->length = len; + /* No unused bits */ + pubkey->pub_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); + pubkey->pub_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; + + der_len = i2d_EC_COMP_PUBKEY(pubkey, &der); + if (der_len <= 0) { + wpa_printf(MSG_ERROR, + "BoringSSL: Failed to build DER encoded public key"); + goto fail; + } + + ret = wpabuf_alloc_copy(der, der_len); +fail: + EC_COMP_PUBKEY_free(pubkey); + OPENSSL_free(der); + BN_CTX_free(ctx); + return ret; +#else /* OPENSSL_IS_BORINGSSL */ unsigned char *der = NULL; int der_len; struct wpabuf *buf; + /* For now, all users expect COMPRESSED form */ + EC_KEY_set_conv_form(EVP_PKEY_get0_EC_KEY((EVP_PKEY *) key), + POINT_CONVERSION_COMPRESSED); + der_len = i2d_PUBKEY((EVP_PKEY *) key, &der); if (der_len <= 0) { wpa_printf(MSG_INFO, "OpenSSL: i2d_PUBKEY() failed: %s", @@ -2517,6 +2610,7 @@ struct wpabuf * crypto_ec_key_get_subject_public_key(struct crypto_ec_key *key) buf = wpabuf_alloc_copy(der, der_len); OPENSSL_free(der); return buf; +#endif /* OPENSSL_IS_BORINGSSL */ } diff --git a/tests/hwsim/test_dpp.py b/tests/hwsim/test_dpp.py index 558f882cd..0579b67f3 100644 --- a/tests/hwsim/test_dpp.py +++ b/tests/hwsim/test_dpp.py @@ -214,7 +214,8 @@ def test_dpp_qr_code_keygen_fail(dev, apdev): """DPP QR Code and keygen failure""" check_dpp_capab(dev[0]) - with alloc_fail(dev[0], 1, "dpp_bootstrap_key_der;dpp_keygen"): + with alloc_fail(dev[0], 1, + "crypto_ec_key_get_subject_public_key;dpp_keygen"): if "FAIL" not in dev[0].request("DPP_BOOTSTRAP_GEN type=qrcode"): raise Exception("Failure not reported")