From: W.C.A. Wijngaards <wouter@nlnetlabs.nl>
Date: Tue, 18 Aug 2015 08:06:41 +0000 (+0200)
Subject: Fix ECDSA signature generation, do not omit leading zeroes.
X-Git-Tag: release-1.7.0-rc1~98
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1139fdc7f6d78cc9a93e46d3defcd05d15c45af0;p=thirdparty%2Fldns.git

Fix ECDSA signature generation, do not omit leading zeroes.
---

diff --git a/Changelog b/Changelog
index 3392329e..5d4bf83a 100644
--- a/Changelog
+++ b/Changelog
@@ -53,6 +53,7 @@ TBD
 	* Spelling fixes.  Thanks Andreas Schulze
 	* Hyphen used as minus in manpages.  Thanks Andreas Schulze.
 	* RFC7553 RR Type URI is supported by default.
+	* Fix ECDSA signature generation, do not omit leading zeroes.
 
 1.6.17	2014-01-10
 	* Fix ldns_dnssec_zone_new_frm_fp_l to allow the last parsed line of a
diff --git a/dnssec.c b/dnssec.c
index a41a9f63..2471e86a 100644
--- a/dnssec.c
+++ b/dnssec.c
@@ -1806,7 +1806,8 @@ ldns_convert_dsa_rrsig_rdf2asn1(ldns_buffer *target_buffer,
 #ifdef USE_ECDSA
 #ifndef S_SPLINT_S
 ldns_rdf *
-ldns_convert_ecdsa_rrsig_asn12rdf(const ldns_buffer *sig, const long sig_len)
+ldns_convert_ecdsa_rrsig_asn1len2rdf(const ldns_buffer *sig,
+	const long sig_len, int num_bytes)
 {
         ECDSA_SIG* ecdsa_sig;
 	unsigned char *data = (unsigned char*)ldns_buffer_begin(sig);
@@ -1815,16 +1816,22 @@ ldns_convert_ecdsa_rrsig_asn12rdf(const ldns_buffer *sig, const long sig_len)
         if(!ecdsa_sig) return NULL;
 
         /* "r | s". */
-        data = LDNS_XMALLOC(unsigned char,
-                BN_num_bytes(ecdsa_sig->r) + BN_num_bytes(ecdsa_sig->s));
+        if(BN_num_bytes(ecdsa_sig->r) > num_bytes ||
+		BN_num_bytes(ecdsa_sig->s) > num_bytes) {
+                ECDSA_SIG_free(ecdsa_sig);
+		return NULL; /* numbers too big for passed curve size */
+	}
+        data = LDNS_XMALLOC(unsigned char, num_bytes*2);
         if(!data) {
                 ECDSA_SIG_free(ecdsa_sig);
                 return NULL;
         }
-        BN_bn2bin(ecdsa_sig->r, data);
-        BN_bn2bin(ecdsa_sig->s, data+BN_num_bytes(ecdsa_sig->r));
-	rdf = ldns_rdf_new(LDNS_RDF_TYPE_B64, (size_t)(
-		BN_num_bytes(ecdsa_sig->r) + BN_num_bytes(ecdsa_sig->s)), data);
+	/* write the bignums (in big-endian) a little offset if the BN code
+	 * wants to write a shorter number of bytes, with zeroes prefixed */
+	memset(data, 0, num_bytes*2);
+        BN_bn2bin(ecdsa_sig->r, data+num_bytes-BN_num_bytes(ecdsa_sig->r));
+        BN_bn2bin(ecdsa_sig->s, data+num_bytes*2-BN_num_bytes(ecdsa_sig->s));
+	rdf = ldns_rdf_new(LDNS_RDF_TYPE_B64, (size_t)(num_bytes*2), data);
         ECDSA_SIG_free(ecdsa_sig);
         return rdf;
 }
diff --git a/dnssec_sign.c b/dnssec_sign.c
index 4af882a2..4f605461 100644
--- a/dnssec_sign.c
+++ b/dnssec_sign.c
@@ -367,6 +367,7 @@ ldns_sign_public_dsa(ldns_buffer *to_sign, DSA *key)
 
 #ifdef USE_ECDSA
 #ifndef S_SPLINT_S
+/** returns the number of bytes per signature-component (i.e. bits/8), or 0. */
 static int
 ldns_pkey_is_ecdsa(EVP_PKEY* pkey)
 {
@@ -380,11 +381,13 @@ ldns_pkey_is_ecdsa(EVP_PKEY* pkey)
                 EC_KEY_free(ec);
                 return 0;
         }
-        if(EC_GROUP_get_curve_name(g) == NID_secp224r1 ||
-                EC_GROUP_get_curve_name(g) == NID_X9_62_prime256v1 ||
-                EC_GROUP_get_curve_name(g) == NID_secp384r1) {
+        if(EC_GROUP_get_curve_name(g) == NID_X9_62_prime256v1) {
                 EC_KEY_free(ec);
-                return 1;
+                return 32; /* 256/8 */
+	}
+        if(EC_GROUP_get_curve_name(g) == NID_secp384r1) {
+                EC_KEY_free(ec);
+                return 48; /* 384/8 */
         }
         /* downref the eckey, the original is still inside the pkey */
         EC_KEY_free(ec);
@@ -448,7 +451,8 @@ ldns_sign_public_evp(ldns_buffer *to_sign,
 #ifdef USE_ECDSA
         } else if(EVP_PKEY_type(key->type) == EVP_PKEY_EC &&
                 ldns_pkey_is_ecdsa(key)) {
-                sigdata_rdf = ldns_convert_ecdsa_rrsig_asn12rdf(b64sig, siglen);
+                sigdata_rdf = ldns_convert_ecdsa_rrsig_asn1len2rdf(
+			b64sig, siglen, ldns_pkey_is_ecdsa(key));
 #endif
 	} else {
 		/* ok output for other types is the same */
diff --git a/ldns/dnssec.h b/ldns/dnssec.h
index f4cdafbe..18b1a3e9 100644
--- a/ldns/dnssec.h
+++ b/ldns/dnssec.h
@@ -511,13 +511,19 @@ ldns_convert_dsa_rrsig_rdf2asn1(ldns_buffer *target_buffer,
  * Converts the ECDSA signature from ASN1 representation (as 
  * used by OpenSSL) to raw signature data as used in DNS
  * This routine is only present if ldns is compiled with ecdsa support.
+ * The older ldns_convert_ecdsa_rrsig_asn12rdf routine could not (always)
+ * construct a valid rdf because it did not have the num_bytes parameter.
+ * The num_bytes parameter is 32 for p256 and 48 for p384 (bits/8).
  *
  * \param[in] sig The signature in ASN1 format
  * \param[in] sig_len The length of the signature
+ * \param[in] num_bytes number of bytes for values in the curve, the curve
+ * 		size divided by 8.
  * \return a new rdf with the signature
  */
 ldns_rdf *
-ldns_convert_ecdsa_rrsig_asn12rdf(const ldns_buffer *sig, const long sig_len);
+ldns_convert_ecdsa_rrsig_asn1len2rdf(const ldns_buffer *sig,
+	const long sig_len, int num_bytes);
 
 /**
  * Converts the RRSIG signature RDF (from DNS) to a buffer with the