Merge branch '3785-openssl-refactoring-17' into 'main'

Refactor OpenSSL ECDSA public and private key export Closes #3785 See merge request isc-projects/bind9!7334
2025-08-30 14:07:59 +00:00 · 2023-01-09 19:32:28 +00:00
parent ddce412489 8b62e7ed99
commit f362cc0bf3
1 changed files with 122 additions and 139 deletions
--- a/lib/dns/opensslecdsa_link.c
+++ b/lib/dns/opensslecdsa_link.c
@@ -46,6 +46,10 @@
 #error "P-384 group is not known (NID_secp384r1)"
 #endif /* ifndef NID_secp384r1 */

+#define MAX_PUBKEY_SIZE DNS_KEY_ECDSA384SIZE
+
+#define MAX_PRIVKEY_SIZE (MAX_PUBKEY_SIZE / 2)
+
 #define DST_RET(a)        \
 	{                 \
 		ret = a;  \
@@ -75,6 +79,18 @@ opensslecdsa_key_alg_to_group_nid(unsigned int key_alg) {
 	}
 }

+static size_t
+opensslecdsa_key_alg_to_publickey_size(unsigned int key_alg) {
+	switch (key_alg) {
+	case DST_ALG_ECDSA256:
+		return (DNS_KEY_ECDSA256SIZE);
+	case DST_ALG_ECDSA384:
+		return (DNS_KEY_ECDSA384SIZE);
+	default:
+		UNREACHABLE();
+	}
+}
+
 /*
 * OpenSSL requires us to set the public key portion, but since our private key
 * file format does not contain it directly, we generate it as needed.
@@ -92,6 +108,17 @@ opensslecdsa_generate_public_key(const EC_GROUP *group, const BIGNUM *privkey) {
 	return (pubkey);
 }

+static int
+BN_bn2bin_fixed(const BIGNUM *bn, unsigned char *buf, int size) {
+	int bytes = size - BN_num_bytes(bn);
+
+	while (bytes-- > 0) {
+		*buf++ = 0;
+	}
+	BN_bn2bin(bn, buf);
+	return (size);
+}
+
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L && OPENSSL_API_LEVEL >= 30000

 static const char *
@@ -177,7 +204,7 @@ opensslecdsa_create_pkey(unsigned int key_alg, bool private,
 	EC_POINT *pubkey = NULL;
 	EC_GROUP *group = NULL;
 	BIGNUM *priv = NULL;
-	unsigned char buf[DNS_KEY_ECDSA384SIZE + 1];
+	unsigned char buf[MAX_PUBKEY_SIZE + 1];

 	bld = OSSL_PARAM_BLD_new();
 	if (bld == NULL) {
@@ -292,6 +319,44 @@ opensslecdsa_validate_pkey_group(unsigned int key_alg, EVP_PKEY *pkey) {
 	return (ISC_R_SUCCESS);
 }

+static bool
+opensslecdsa_extract_public_key(const dst_key_t *key, unsigned char *buf,
+				size_t buflen) {
+	EVP_PKEY *pkey = key->keydata.pkeypair.pub;
+	BIGNUM *x = NULL;
+	BIGNUM *y = NULL;
+	bool ret = false;
+
+	if (EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_EC_PUB_X, &x) != 1) {
+		goto err;
+	}
+	if (EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_EC_PUB_Y, &y) != 1) {
+		goto err;
+	}
+	BN_bn2bin_fixed(x, &buf[0], buflen / 2);
+	BN_bn2bin_fixed(y, &buf[buflen / 2], buflen / 2);
+	ret = true;
+err:
+	BN_clear_free(x);
+	BN_clear_free(y);
+	return (ret);
+}
+
+static bool
+opensslecdsa_extract_private_key(const dst_key_t *key, unsigned char *buf,
+				 size_t buflen) {
+	EVP_PKEY *pkey = key->keydata.pkeypair.priv;
+	BIGNUM *priv = NULL;
+
+	if (EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_PRIV_KEY, &priv) != 1) {
+		return (false);
+	}
+
+	BN_bn2bin_fixed(priv, buf, buflen);
+	BN_clear_free(priv);
+	return (true);
+}
+
 #else

 static isc_result_t
@@ -339,7 +404,7 @@ opensslecdsa_create_pkey(unsigned int key_alg, bool private,
 	EVP_PKEY *pkey = NULL;
 	BIGNUM *privkey = NULL;
 	EC_POINT *pubkey = NULL;
-	unsigned char buf[DNS_KEY_ECDSA384SIZE + 1];
+	unsigned char buf[MAX_PUBKEY_SIZE + 1];
 	int group_nid = opensslecdsa_key_alg_to_group_nid(key_alg);

 	eckey = EC_KEY_new_by_curve_name(group_nid);
@@ -409,6 +474,44 @@ opensslecdsa_validate_pkey_group(unsigned int key_alg, EVP_PKEY *pkey) {
 	return (ISC_R_SUCCESS);
 }

+static bool
+opensslecdsa_extract_public_key(const dst_key_t *key, unsigned char *dst,
+				size_t dstlen) {
+	const EC_KEY *eckey = EVP_PKEY_get0_EC_KEY(key->keydata.pkeypair.pub);
+	const EC_GROUP *group = EC_KEY_get0_group(eckey);
+	const EC_POINT *pub = EC_KEY_get0_public_key(eckey);
+	unsigned char buf[MAX_PUBKEY_SIZE + 1];
+	size_t len;
+
+	len = EC_POINT_point2oct(group, pub, POINT_CONVERSION_UNCOMPRESSED, buf,
+				 sizeof(buf), NULL);
+	if (len != dstlen + 1) {
+		return (false);
+	}
+	memmove(dst, buf + 1, dstlen);
+	return (true);
+}
+
+static bool
+opensslecdsa_extract_private_key(const dst_key_t *key, unsigned char *buf,
+				 size_t buflen) {
+	const EC_KEY *eckey = NULL;
+	const BIGNUM *privkey = NULL;
+
+	eckey = EVP_PKEY_get0_EC_KEY(key->keydata.pkeypair.priv);
+	if (eckey == NULL) {
+		return (false);
+	}
+
+	privkey = EC_KEY_get0_private_key(eckey);
+	if (privkey == NULL) {
+		return (false);
+	}
+
+	BN_bn2bin_fixed(privkey, buf, buflen);
+	return (true);
+}
+
 #endif /* OPENSSL_VERSION_NUMBER >= 0x30000000L && OPENSSL_API_LEVEL >= 30000 \
 	*/

@@ -501,17 +604,6 @@ err:
 	return (ret);
 }

-static int
-BN_bn2bin_fixed(const BIGNUM *bn, unsigned char *buf, int size) {
-	int bytes = size - BN_num_bytes(bn);
-
-	while (bytes-- > 0) {
-		*buf++ = 0;
-	}
-	BN_bn2bin(bn, buf);
-	return (size);
-}
-
 static isc_result_t
 opensslecdsa_sign(dst_context_t *dctx, isc_buffer_t *sig) {
 	isc_result_t ret;
@@ -694,90 +786,25 @@ opensslecdsa_destroy(dst_key_t *key) {
 static isc_result_t
 opensslecdsa_todns(const dst_key_t *key, isc_buffer_t *data) {
 	isc_result_t ret;
-	EVP_PKEY *pkey;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	EC_KEY *eckey = NULL;
-	int len;
-	unsigned char *cp;
-#else
-	int status;
-	BIGNUM *x = NULL;
-	BIGNUM *y = NULL;
-	size_t keysize = 0;
-	size_t len = 0;
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
 	isc_region_t r;
-	unsigned char buf[DNS_KEY_ECDSA384SIZE + 1];
+	size_t keysize;

+	REQUIRE(opensslecdsa_valid_key_alg(key->key_alg));
 	REQUIRE(key->keydata.pkeypair.pub != NULL);

-	pkey = key->keydata.pkeypair.pub;
-
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	eckey = EVP_PKEY_get1_EC_KEY(pkey);
-	if (eckey == NULL) {
-		DST_RET(dst__openssl_toresult(ISC_R_FAILURE));
-	}
-	len = i2o_ECPublicKey(eckey, NULL);
-
-	/* skip form */
-	len--;
-#else
-	if (key->key_alg == DST_ALG_ECDSA256) {
-		keysize = DNS_KEY_ECDSA256SIZE;
-	} else if (key->key_alg == DST_ALG_ECDSA384) {
-		keysize = DNS_KEY_ECDSA384SIZE;
-	} else {
-		DST_RET(ISC_R_NOTIMPLEMENTED);
-	}
-
-	len = keysize;
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
-
+	keysize = opensslecdsa_key_alg_to_publickey_size(key->key_alg);
 	isc_buffer_availableregion(data, &r);
-	if (r.length < (unsigned int)len) {
+	if (r.length < keysize) {
 		DST_RET(ISC_R_NOSPACE);
 	}
+	if (!opensslecdsa_extract_public_key(key, r.base, keysize)) {
+		DST_RET(DST_R_OPENSSLFAILURE);
+	}

-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	cp = buf;
-	if (!i2o_ECPublicKey(eckey, &cp)) {
-		DST_RET(dst__openssl_toresult(ISC_R_FAILURE));
-	}
-	memmove(r.base, buf + 1, len);
-#else
-	status = EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_EC_PUB_X, &x);
-	if (status != 1 || x == NULL) {
-		DST_RET(dst__openssl_toresult2("EVP_PKEY_get_bn_param",
-					       DST_R_OPENSSLFAILURE));
-	}
-	status = EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_EC_PUB_Y, &y);
-	if (status != 1 || y == NULL) {
-		DST_RET(dst__openssl_toresult2("EVP_PKEY_get_bn_param",
-					       DST_R_OPENSSLFAILURE));
-	}
-	BN_bn2bin_fixed(x, &buf[0], keysize / 2);
-	BN_bn2bin_fixed(y, &buf[keysize / 2], keysize / 2);
-	memmove(r.base, buf, len);
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
-
-	isc_buffer_add(data, len);
+	isc_buffer_add(data, keysize);
 	ret = ISC_R_SUCCESS;

 err:
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	if (eckey != NULL) {
-		EC_KEY_free(eckey);
-	}
-#else
-	if (x != NULL) {
-		BN_clear_free(x);
-	}
-	if (y != NULL) {
-		BN_clear_free(y);
-	}
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
-
 	return (ret);
 }

@@ -786,16 +813,10 @@ opensslecdsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
 	isc_result_t ret;
 	EVP_PKEY *pkey = NULL;
 	isc_region_t r;
-
 	size_t len;

 	REQUIRE(opensslecdsa_valid_key_alg(key->key_alg));
-
-	if (key->key_alg == DST_ALG_ECDSA256) {
-		len = DNS_KEY_ECDSA256SIZE;
-	} else {
-		len = DNS_KEY_ECDSA384SIZE;
-	}
+	len = opensslecdsa_key_alg_to_publickey_size(key->key_alg);

 	isc_buffer_remainingregion(data, &r);
 	if (r.length == 0) {
@@ -822,16 +843,9 @@ err:
 static isc_result_t
 opensslecdsa_tofile(const dst_key_t *key, const char *directory) {
 	isc_result_t ret;
-	EVP_PKEY *pkey;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	EC_KEY *eckey = NULL;
-	const BIGNUM *privkey = NULL;
-#else
-	int status;
-	BIGNUM *privkey = NULL;
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
 	dst_private_t priv;
-	unsigned char *buf = NULL;
+	unsigned char buf[MAX_PRIVKEY_SIZE];
+	size_t keylen = 0;
 	unsigned short i;

 	if (key->keydata.pkeypair.pub == NULL) {
@@ -847,34 +861,15 @@ opensslecdsa_tofile(const dst_key_t *key, const char *directory) {
 		DST_RET(DST_R_NULLKEY);
 	}

-	pkey = key->keydata.pkeypair.priv;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	eckey = EVP_PKEY_get1_EC_KEY(pkey);
-	if (eckey == NULL) {
-		DST_RET(dst__openssl_toresult2("EVP_PKEY_get1_EC_KEY",
-					       DST_R_OPENSSLFAILURE));
+	keylen = opensslecdsa_key_alg_to_publickey_size(key->key_alg) / 2;
+	INSIST(keylen <= sizeof(buf));
+	if (!opensslecdsa_extract_private_key(key, buf, keylen)) {
+		DST_RET(DST_R_OPENSSLFAILURE);
 	}
-	privkey = EC_KEY_get0_private_key(eckey);
-	if (privkey == NULL) {
-		DST_RET(dst__openssl_toresult2("EC_KEY_get0_private_key",
-					       DST_R_OPENSSLFAILURE));
-	}
-#else
-	status = EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_PRIV_KEY,
-				       &privkey);
-	if (status != 1 || privkey == NULL) {
-		DST_RET(dst__openssl_toresult2("EVP_PKEY_get_bn_param",
-					       DST_R_OPENSSLFAILURE));
-	}
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
-
-	buf = isc_mem_get(key->mctx, BN_num_bytes(privkey));

 	i = 0;
-
 	priv.elements[i].tag = TAG_ECDSA_PRIVATEKEY;
-	priv.elements[i].length = BN_num_bytes(privkey);
-	BN_bn2bin(privkey, buf);
+	priv.elements[i].length = keylen;
 	priv.elements[i].data = buf;
 	i++;

@@ -898,19 +893,7 @@ opensslecdsa_tofile(const dst_key_t *key, const char *directory) {
 	ret = dst__privstruct_writefile(key, &priv, directory);

 err:
-	if (buf != NULL && privkey != NULL) {
-		isc_mem_put(key->mctx, buf, BN_num_bytes(privkey));
-	}
-#if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000
-	if (eckey != NULL) {
-		EC_KEY_free(eckey);
-	}
-#else
-	if (privkey != NULL) {
-		BN_clear_free(privkey);
-	}
-#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */
-
+	isc_safe_memwipe(buf, keylen);
 	return (ret);
 }