diff mbox series

[08/21] dpp: use crypto_ec_key_get_subject_public_key when possible

Message ID 20210628162538.21067-9-cedric.izoard@ceva-dsp.com
State Superseded
Headers show
Series DPP: Remove direct dependency on OpenSSL | expand

Commit Message

Cedric Izoard June 28, 2021, 4:25 p.m. UTC 
From: Cedric Izoard <cedric.izoard@laposte.net>

Unless I miss something I don't see any difference between the locally
defined ASN.1 sequence DPP_BOOTSTRAPPING_KEY and SubjectPublicKeyInfo
so use the latter one instead.

Signed-off-by: Cedric Izoard <cedric.izoard@ceva-dsp.com>
---
 src/common/dpp.c            | 22 +++++-----
 src/common/dpp_crypto.c     | 88 +------------------------------------
 src/crypto/crypto_openssl.c |  4 ++
 tests/hwsim/test_dpp.py     |  2 +-
 4 files changed, 17 insertions(+), 99 deletions(-)

Comments

Jouni Malinen Oct. 26, 2021, 5 p.m. UTC | #1 
On Mon, Jun 28, 2021 at 06:25:25PM +0200, Cedric Izoard wrote:
> Unless I miss something I don't see any difference between the locally
> defined ASN.1 sequence DPP_BOOTSTRAPPING_KEY and SubjectPublicKeyInfo
> so use the latter one instead.

That special construction routine was added to work around an issue wuth
BoringSSL. See
https://w1.fi/cgit/hostap/commit/?id=746c1792ac285a67431cfa5e0c3b10ed3b806b13
for more details. Unless this version described here is known to work
with BoringSSL, I'm not keen on applying this patch since I don't want
to risk breaking DPP functionality on Android.
diff mbox series

Patch

diff --git a/src/common/dpp.c b/src/common/dpp.c
index 8fa662bf4..2f0f9552d 100644
--- a/src/common/dpp.c
+++ b/src/common/dpp.c
@@ -2412,29 +2412,27 @@  fail:
 
 static void dpp_copy_csign(struct dpp_config_obj *conf, struct crypto_ec_key *csign)
 {
-	unsigned char *der = NULL;
-	int der_len;
+	struct wpabuf *c_sign_key;
 
-	der_len = i2d_PUBKEY((EVP_PKEY *)csign, &der);
-	if (der_len <= 0)
+	c_sign_key = crypto_ec_key_get_subject_public_key(csign);
+	if (!c_sign_key)
 		return;
+
 	wpabuf_free(conf->c_sign_key);
-	conf->c_sign_key = wpabuf_alloc_copy(der, der_len);
-	OPENSSL_free(der);
+	conf->c_sign_key = c_sign_key;
 }
 
 
 static void dpp_copy_ppkey(struct dpp_config_obj *conf, struct crypto_ec_key *ppkey)
 {
-	unsigned char *der = NULL;
-	int der_len;
+	struct wpabuf *pp_key;
 
-	der_len = i2d_PUBKEY((EVP_PKEY *)ppkey, &der);
-	if (der_len <= 0)
+	pp_key = crypto_ec_key_get_subject_public_key(ppkey);
+	if (!pp_key)
 		return;
+
 	wpabuf_free(conf->pp_key);
-	conf->pp_key = wpabuf_alloc_copy(der, der_len);
-	OPENSSL_free(der);
+	conf->pp_key = pp_key;
 }
 
 
diff --git a/src/common/dpp_crypto.c b/src/common/dpp_crypto.c
index e274ee95f..e4f0f817b 100644
--- a/src/common/dpp_crypto.c
+++ b/src/common/dpp_crypto.c
@@ -10,8 +10,6 @@ 
 #include "utils/includes.h"
 #include <openssl/opensslv.h>
 #include <openssl/err.h>
-#include <openssl/asn1.h>
-#include <openssl/asn1t.h>
 #include <openssl/pem.h>
 
 #include "utils/common.h"
@@ -452,94 +450,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;
-	dpp_debug_print_point("DPP: bootstrap public key", group, point);
-	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)",
@@ -574,7 +490,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 c29a6e3a1..275ec6252 100644
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -2400,6 +2400,10 @@  struct wpabuf * crypto_ec_key_get_subject_public_key(struct crypto_ec_key *key)
 	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",
diff --git a/tests/hwsim/test_dpp.py b/tests/hwsim/test_dpp.py
index 50827b816..c3b8ca4ea 100644
--- a/tests/hwsim/test_dpp.py
+++ b/tests/hwsim/test_dpp.py
@@ -214,7 +214,7 @@  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")