Fix private key encoding in i2d_ECPrivateKey()

The private key is a random integer between 1 and order - 1. As such it
requires at most as many bytes as the order to encode. SEC 1, Section C.4
is very explicit about padding it to this length:

  The component privateKey is the private key defined to be the octet
  string of length [ceil(log_2 n/8)] (where n is the order of the curve)
  obtained from the unsigned integer via the encoding of Section 2.3.7.

Fix this by generalizing a similar fix for field elements.

ok jsing

1 files changed, 40 insertions, 31 deletions

diff --git a/lib/libcrypto/ec/ec_asn1.c b/lib/libcrypto/ec/ec_asn1.c
index d6af3bdec51..45806174d3f 100644
--- a/lib/libcrypto/ec/ec_asn1.c
+++ b/lib/libcrypto/ec/ec_asn1.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_asn1.c,v 1.92 2024/10/28 18:03:34 tb Exp $ */
+/* $OpenBSD: ec_asn1.c,v 1.93 2024/10/28 18:15:53 tb Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project.
  */
@@ -606,16 +606,12 @@ ec_asn1_group2fieldid(const EC_GROUP *group, X9_62_FIELDID *field)
 }
 
 static int
-ec_asn1_encode_field_element(const EC_GROUP *group, const BIGNUM *bn,
+ec_asn1_encode_bn(const EC_GROUP *group, const BIGNUM *bn, int len,
     ASN1_OCTET_STRING *os)
 {
 	unsigned char *buf;
-	int len;
 	int ret = 0;
 
-	/* Zero-pad field element per SEC 1, section 2.3.5. */
-	len = (EC_GROUP_get_degree(group) + 7) / 8;
-
 	/* One extra byte for historic NUL termination of ASN1_STRINGs. */
 	if ((buf = calloc(1, len + 1)) == NULL)
 		goto err;
@@ -636,6 +632,32 @@ ec_asn1_encode_field_element(const EC_GROUP *group, const BIGNUM *bn,
 }
 
 static int
+ec_asn1_encode_field_element(const EC_GROUP *group, const BIGNUM *bn,
+    ASN1_OCTET_STRING *os)
+{
+	int len;
+
+	/* Zero-pad field element to byte length of p per SEC 1, 2.3.5. */
+	len = (EC_GROUP_get_degree(group) + 7) / 8;
+	return ec_asn1_encode_bn(group, bn, len, os);
+}
+
+static int
+ec_asn1_encode_private_key(const EC_GROUP *group, const BIGNUM *bn,
+    ASN1_OCTET_STRING *os)
+{
+	const BIGNUM *order;
+
+	if ((order = EC_GROUP_get0_order(group)) == NULL) {
+		ECerror(EC_R_INVALID_GROUP_ORDER);
+		return 0;
+	}
+
+	/* Zero-pad private key to byte length of order per SEC 1, C.4. */
+	return ec_asn1_encode_bn(group, bn, BN_num_bytes(order), os);
+}
+
+static int
 ec_asn1_group2curve(const EC_GROUP *group, X9_62_CURVE *curve)
 {
 	BIGNUM *a = NULL, *b = NULL;
@@ -1227,7 +1249,7 @@ i2d_ECPrivateKey(EC_KEY *ec_key, unsigned char **out)
 {
 	int ret = 0, ok = 0;
 	unsigned char *buffer = NULL;
-	size_t buf_len = 0, tmp_len;
+	size_t buf_len = 0;
 	EC_PRIVATEKEY *ec_privatekey = NULL;
 
 	if (ec_key == NULL || ec_key->group == NULL || ec_key->priv_key == NULL ||
@@ -1241,20 +1263,9 @@ i2d_ECPrivateKey(EC_KEY *ec_key, unsigned char **out)
 	}
 	ec_privatekey->version = ec_key->version;
 
-	buf_len = (size_t) BN_num_bytes(ec_key->priv_key);
-	buffer = malloc(buf_len);
-	if (buffer == NULL) {
-		ECerror(ERR_R_MALLOC_FAILURE);
-		goto err;
-	}
-	if (!BN_bn2bin(ec_key->priv_key, buffer)) {
-		ECerror(ERR_R_BN_LIB);
-		goto err;
-	}
-	if (!ASN1_STRING_set(ec_privatekey->privateKey, buffer, buf_len)) {
-		ECerror(ERR_R_ASN1_LIB);
+	if (!ec_asn1_encode_private_key(ec_key->group, ec_key->priv_key,
+	    ec_privatekey->privateKey))
 		goto err;
-	}
 	if (!(ec_key->enc_flag & EC_PKEY_NO_PARAMETERS)) {
 		ECPKPARAMETERS *parameters;
 
@@ -1270,17 +1281,15 @@ i2d_ECPrivateKey(EC_KEY *ec_key, unsigned char **out)
 			ECerror(ERR_R_MALLOC_FAILURE);
 			goto err;
 		}
-		tmp_len = EC_POINT_point2oct(ec_key->group, ec_key->pub_key,
-		    ec_key->conv_form, NULL, 0, NULL);
-
-		if (tmp_len > buf_len) {
-			unsigned char *tmp_buffer = realloc(buffer, tmp_len);
-			if (!tmp_buffer) {
-				ECerror(ERR_R_MALLOC_FAILURE);
-				goto err;
-			}
-			buffer = tmp_buffer;
-			buf_len = tmp_len;
+		if ((buf_len = EC_POINT_point2oct(ec_key->group, ec_key->pub_key,
+		    ec_key->conv_form, NULL, 0, NULL)) == 0) {
+			ECerror(ERR_R_EC_LIB);
+			goto err;
+		}
+
+		if ((buffer = calloc(1, buf_len)) == NULL) {
+			ECerror(ERR_R_MALLOC_FAILURE);
+			goto err;
 		}
 		if (!EC_POINT_point2oct(ec_key->group, ec_key->pub_key,
 			ec_key->conv_form, buffer, buf_len, NULL)) {