Fix ECDSA signature generation, do not omit leading zeroes.

diff --git a/Changelog b/Changelog
index 3392329e1cb33ce9e9cec9966a9ec45b2ad8ef01..5d4bf83abefd239fd785cd43c4eea8b76011ee41 100644 (file)
--- 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 a41a9f633c4e437eddbd86291631d620200c8893..2471e86ac06abd5b164477b8234ba123909c52b1 100644 (file)
--- 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 4af882a2845f9e383d5823471568a7cfb6579b4c..4f605461d32bd4cbb23011c857e2ce47a16d3ad7 100644 (file)
--- 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 f4cdafbe9deb71b455ac976fc5da6639be85d3d9..18b1a3e9cfa0712578d23fac2399aa1ba3d28d8f 100644 (file)
--- 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