Fix leading zero bytes in DSA, ECDSA, ECGDSA and ECKCDSA signatures

8 files changed, 57 insertions, 23 deletions

diff --git a/src/lib/math/bigint/big_code.cpp b/src/lib/math/bigint/big_code.cpp
index 299fdc246..c8687715d 100644
--- a/src/lib/math/bigint/big_code.cpp
+++ b/src/lib/math/bigint/big_code.cpp
@@ -98,6 +98,17 @@ void BigInt::encode_1363(byte output[], size_t bytes, const BigInt& n)
    }
 
 /*
+* Encode two BigInt, with leading 0s if needed, and concatenate
+*/
+secure_vector<byte> BigInt::encode_fixed_length_int_pair(const BigInt& n1, const BigInt& n2, size_t bytes)
+   {
+   secure_vector<byte> output(2 * bytes);
+   BigInt::encode_1363(output.data(), bytes, n1);
+   BigInt::encode_1363(output.data() + bytes, bytes, n2);
+   return output;
+   }
+
+/*
 * Decode a BigInt
 */
 BigInt BigInt::decode(const byte buf[], size_t length, Base base)
diff --git a/src/lib/math/bigint/bigint.h b/src/lib/math/bigint/bigint.h
index 2963ba35d..a61bee39c 100644
--- a/src/lib/math/bigint/bigint.h
+++ b/src/lib/math/bigint/bigint.h
@@ -566,6 +566,15 @@ class BOTAN_DLL BigInt
 
      static void encode_1363(byte out[], size_t bytes, const BigInt& n);
 
+     /**
+     * Encode two BigInt to a byte array according to IEEE 1363
+     * @param n1 the first BigInt to encode
+     * @param n2 the second BigInt to encode
+     * @param bytes the length of the encoding of each single BigInt
+     * @result a secure_vector<byte> containing the concatenation of the two encoded BigInt
+     */
+     static secure_vector<byte> encode_fixed_length_int_pair(const BigInt& n1, const BigInt& n2, size_t bytes);
+
    private:
       secure_vector<word> m_reg;
       Sign m_signedness = Positive;
diff --git a/src/lib/pubkey/dsa/dsa.cpp b/src/lib/pubkey/dsa/dsa.cpp
index 6effb81dd..399756b1a 100644
--- a/src/lib/pubkey/dsa/dsa.cpp
+++ b/src/lib/pubkey/dsa/dsa.cpp
@@ -133,10 +133,7 @@ DSA_Signature_Operation::raw_sign(const byte msg[], size_t msg_len,
    BOTAN_ASSERT(s != 0, "invalid s");
    BOTAN_ASSERT(r != 0, "invalid r");
 
-   secure_vector<byte> output(2*m_q.bytes());
-   r.binary_encode(&output[output.size() / 2 - r.bytes()]);
-   s.binary_encode(&output[output.size() - s.bytes()]);
-   return output;
+   return BigInt::encode_fixed_length_int_pair(r, s, m_q.bytes());
    }
 
 /**
diff --git a/src/lib/pubkey/ecdsa/ecdsa.cpp b/src/lib/pubkey/ecdsa/ecdsa.cpp
index 6fe7ce319..264a36963 100644
--- a/src/lib/pubkey/ecdsa/ecdsa.cpp
+++ b/src/lib/pubkey/ecdsa/ecdsa.cpp
@@ -86,10 +86,7 @@ ECDSA_Signature_Operation::raw_sign(const byte msg[], size_t msg_len,
    BOTAN_ASSERT(s != 0, "invalid s");
    BOTAN_ASSERT(r != 0, "invalid r");
 
-   secure_vector<byte> output(2*m_order.bytes());
-   r.binary_encode(&output[output.size() / 2 - r.bytes()]);
-   s.binary_encode(&output[output.size() - s.bytes()]);
-   return output;
+   return BigInt::encode_fixed_length_int_pair(r, s, m_order.bytes());
    }
 
 /**
diff --git a/src/lib/pubkey/ecgdsa/ecgdsa.cpp b/src/lib/pubkey/ecgdsa/ecgdsa.cpp
index 3e14aa8f4..30ea32817 100644
--- a/src/lib/pubkey/ecgdsa/ecgdsa.cpp
+++ b/src/lib/pubkey/ecgdsa/ecgdsa.cpp
@@ -73,10 +73,7 @@ ECGDSA_Signature_Operation::raw_sign(const byte msg[], size_t msg_len,
    BOTAN_ASSERT(s != 0, "invalid s");
    BOTAN_ASSERT(r != 0, "invalid r");
 
-   secure_vector<byte> output(2*m_order.bytes());
-   r.binary_encode(&output[output.size() / 2 - r.bytes()]);
-   s.binary_encode(&output[output.size() - s.bytes()]);
-   return output;
+   return BigInt::encode_fixed_length_int_pair(r, s, m_order.bytes());
    }
 
 /**
diff --git a/src/lib/pubkey/eckcdsa/eckcdsa.cpp b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
index 83439332e..5ca89675c 100644
--- a/src/lib/pubkey/eckcdsa/eckcdsa.cpp
+++ b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
@@ -100,10 +100,9 @@ ECKCDSA_Signature_Operation::raw_sign(const byte msg[], size_t,
    const BigInt s = m_mod_order.multiply(m_x, k - w);
    BOTAN_ASSERT(s != 0, "invalid s");
 
-   secure_vector<byte> signature(r.bytes() + s.bytes());
-   r.binary_encode(signature.data());
-   s.binary_encode(&signature[r.bytes()]);
-   return signature;
+   secure_vector<byte> output = BigInt::encode_1363(r, c.size());
+   output += BigInt::encode_1363(s, m_mod_order.get_modulus().bytes());
+   return output;
    }
 
 /**
diff --git a/src/tests/test_bigint.cpp b/src/tests/test_bigint.cpp
index 6f3d603db..cee7b5b8b 100644
--- a/src/tests/test_bigint.cpp
+++ b/src/tests/test_bigint.cpp
@@ -30,6 +30,7 @@ class BigInt_Unit_Tests : public Test
 
          results.push_back(test_bigint_sizes());
          results.push_back(test_random_integer());
+         results.push_back(test_encode());
 
          return results;
          }
@@ -143,6 +144,32 @@ class BigInt_Unit_Tests : public Test
 
          return result;
          }
+
+      Test::Result test_encode()
+         {
+         Test::Result result("BigInt encoding functions");
+
+         const BigInt n1(0xffff);
+         const BigInt n2(1023);
+
+         Botan::secure_vector<byte> encoded_n1 = BigInt::encode_1363(n1, 256);
+         Botan::secure_vector<byte> encoded_n2 = BigInt::encode_1363(n2, 256);
+         Botan::secure_vector<byte> expected = encoded_n1;
+         expected += encoded_n2;
+
+         Botan::secure_vector<byte> encoded_n1_n2 = BigInt::encode_fixed_length_int_pair(n1, n2, 256);
+         result.test_eq("encode_fixed_length_int_pair", encoded_n1_n2, expected);
+
+         for (size_t i = 0; i < 256 - n1.bytes(); ++i)
+            {
+               if ( encoded_n1[i] != 0 )
+                  {
+                  result.test_failure("encode_1363", "no zero byte");
+                  }
+            }
+
+         return result;
+         }
    };
 
 BOTAN_REGISTER_TEST("bigint_unit", BigInt_Unit_Tests);
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index 8bbad5028..5110c7d23 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -481,20 +481,17 @@ class X509_Cert_Unit_Tests : public Test
          std::vector<Test::Result> results;
          const std::vector<std::string> sig_algos { "RSA", "DSA", "ECDSA", "ECGDSA", "ECKCDSA" };
          Test::Result cert_result("X509 Unit");
+         Test::Result usage_result("X509 Usage");
+
          for(const auto& algo : sig_algos)
             {
             cert_result.merge(test_x509_cert(algo));
+            usage_result.merge(test_usage(algo));
             }
 
          results.push_back(cert_result);
-         results.push_back(test_x509_dates());
-
-         Test::Result usage_result("X509 Usage");
-         for(const auto& algo : sig_algos)
-            {
-            usage_result.merge(test_usage(algo));
-            }
          results.push_back(usage_result);
+         results.push_back(test_x509_dates());
 
          return results;
          }