Repository: randombit/botan
Pull request #2751 (2021-05-24 23:08:07)

Fix an ECKCDSA bug
Add some more test vectors (taken from https://github.com/ANSSI-FR/libecc) and fix a bug which occured when either the group was not an even multiple of 8 bits, or when the x,y coordinates had any leading zero bytes.


Patch:
diff --git a/src/lib/pubkey/eckcdsa/eckcdsa.cpp b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
index 5398946a8..d50d5864e 100644
--- a/src/lib/pubkey/eckcdsa/eckcdsa.cpp
+++ b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
@@ -57,10 +57,15 @@ class ECKCDSA_Signature_Operation final : public PK_Ops::Signature_with_EMSA
          const BigInt public_point_x = eckcdsa.public_point().get_affine_x();
          const BigInt public_point_y = eckcdsa.public_point().get_affine_y();
 
-         m_prefix.resize(public_point_x.bytes() + public_point_y.bytes());
-         public_point_x.binary_encode(m_prefix.data());
-         public_point_y.binary_encode(&m_prefix[public_point_x.bytes()]);
-         m_prefix.resize(HashFunction::create(hash_for_signature())->hash_block_size()); // use only the "hash input block size" leftmost bits
+         const size_t order_bytes = m_group.get_order_bytes();
+
+         m_prefix.resize(2*order_bytes);
+         BigInt::encode_1363(&m_prefix[0], order_bytes, public_point_x);
+         BigInt::encode_1363(&m_prefix[order_bytes], order_bytes, public_point_y);
+
+         const size_t block_size = HashFunction::create(hash_for_signature())->hash_block_size();
+         // Either truncate or zero-extend to match the hash block size
+         m_prefix.resize(block_size);
          }
 
       secure_vector<uint8_t> raw_sign(const uint8_t msg[], size_t msg_len,
@@ -126,10 +131,15 @@ class ECKCDSA_Verification_Operation final : public PK_Ops::Verification_with_EM
          const BigInt public_point_x = eckcdsa.public_point().get_affine_x();
          const BigInt public_point_y = eckcdsa.public_point().get_affine_y();
 
-         m_prefix.resize(public_point_x.bytes() + public_point_y.bytes());
-         public_point_x.binary_encode(&m_prefix[0]);
-         public_point_y.binary_encode(&m_prefix[public_point_x.bytes()]);
-         m_prefix.resize(HashFunction::create(hash_for_signature())->hash_block_size()); // use only the "hash input block size" leftmost bits
+         const size_t order_bytes = m_group.get_order_bytes();
+
+         m_prefix.resize(2*order_bytes);
+         BigInt::encode_1363(&m_prefix[0], order_bytes, public_point_x);
+         BigInt::encode_1363(&m_prefix[order_bytes], order_bytes, public_point_y);
+
+         const size_t block_size = HashFunction::create(hash_for_signature())->hash_block_size();
+         // Either truncate or zero-extend to match the hash block size
+         m_prefix.resize(block_size);
          }
 
       bool has_prefix() override { return true; }
@@ -178,7 +188,10 @@ bool ECKCDSA_Verification_Operation::verify(const uint8_t msg[], size_t,
 
    const PointGFp q = m_gy_mul.multi_exp(w, s);
    if(q.is_zero())
+      {
       return false;
+      }
+
    const BigInt q_x = q.get_affine_x();
    secure_vector<uint8_t> c(q_x.bytes());
    q_x.binary_encode(c.data());


Test patch:
diff --git a/src/tests/data/pubkey/eckcdsa.vec b/src/tests/data/pubkey/eckcdsa.vec
index d9bd659e2..49d979d07 100644
--- a/src/tests/data/pubkey/eckcdsa.vec
+++ b/src/tests/data/pubkey/eckcdsa.vec
@@ -4,26 +4,68 @@
 # http://www.tta.or.kr/include/Download.jsp?filename=stnfile/TTAK.KO-12.0015_R2.pdf
 
 Group = secp224r1
+Hash = SHA-224
 X = 0x562A6F64E162FFCB51CD4707774AE36681B6CEF205FE5D43912956A2
-
 Msg = 5468697320697320612073616D706C65206D65737361676520666F722045432D4B4344534120696D706C656D656E746174696F6E2076616C69646174696F6E2E
-Hash = SHA-224
 Nonce = 76A0AFC18646D1B620A079FB223865A7BCB447F3C03A35D878EA4CDA
 Signature = EEA58C91E0CDCEB5799B00D2412D928FDD23122A1C2BDF43C2F8DAFAAEBAB53C7A44A8B22F35FDB9DE265F23B89F65A69A8B7BD4061911A6
 
 Group = secp256r1
-X = 0x9051A275AA4D98439EDDED13FA1C6CBBCCE775D8CC9433DEE69C59848B3594DF
-
 Hash = SHA-256
+X = 0x9051A275AA4D98439EDDED13FA1C6CBBCCE775D8CC9433DEE69C59848B3594DF
+Msg = 5468697320697320612073616D706C65206D65737361676520666F722045432D4B4344534120696D706C656D656E746174696F6E2076616C69646174696F6E2E
 Nonce = 71B88F398916DA9C90F555F1B5732B7DC636B49C638150BAC11BF05CFE16596A
 Signature = 0EDDF680601266EE1DA83E55A6D9445FC781DAEB14C765E7E5D0CDBAF1F14A689B333457661C7CF741BDDBC0835553DFBB37EE74F53DB699E0A17780C7B6F1D0
 
 # Taken from ISO/IEC 14888-3:2006, with corrections from ISO/IEC 14888-3:2006/Cor.2:2009
 
 Group = secp192r1
+Hash = SHA-1
 X = 0x444811A323E03C28A34CD859EE2FF1A34D1AAF3CB0B5603B
-
 Msg = 616263
-Hash = SHA-1
 Nonce = 4B19A0725424CD3310B02D8C8416C98D64C618BFE935597D
-Signature = 3CA29800D425FCAA51CCB209B4ED5D6C352108223143B2EA5A0E8644CE8F768A6FA4D193C726AD08019788E5
\ No newline at end of file
+Signature = 3CA29800D425FCAA51CCB209B4ED5D6C352108223143B2EA5A0E8644CE8F768A6FA4D193C726AD08019788E5
+
+# From https://github.com/ANSSI-FR/libecc
+
+Group = secp384r1
+Hash = SHA-384
+X = 0x4fdbf2e74e56814e4c2f4087909ce9c4731bff061a788273841bf568ce63213e470404ea3eb678c500ba3792f24a7a51
+Msg = 5468697320697320612073616D706C65206D65737361676520666F722045432D4B4344534120696D706C656D656E746174696F6E2076616C69646174696F6E2E
+Nonce = 08162ef824d2d5114e0861f393b46fb602a495a2ca177f47da4e6f2d23a185c5b88f1b7e2cba8d773716093af5979a7e
+Signature = d037264744e38a38caccdc8fcb08d8cb52187d383cc0b629f62771d93f5afcd3c144c153ef06d99ac9205881de03d171f1ffc143b94e44dcecd161fdc5270a32713e5938fb35600eddd00c6389fb33b03e77d2f7694a04085af98531dfe6f173
+
+Group = secp521r1
+Hash = SHA-512
+X = 0x0165e1f1f2e7ae6b502a194ef03ad050e3a5ae31fcb4f70d58f5c6df2866717f4409e778024de52befc96db9fbba21306d125a27cfcdce9100097963bf0c40e696dc
+Msg = 5468697320697320612073616D706C65206D65737361676520666F722045432D4B4344534120696D706C656D656E746174696F6E2076616C69646174696F6E2E
+Nonce = 01b6faf45fb86f579c4b54aec7a7f2956e4d9394b362820aa53f1b6c073bbec0243314db11b62021355020e805ad8e6d0ea27c04359ad3f844d65bbb73b791d59a1c
+Signature = 9052bbc4dfe880fb25cba7a747a0d0e90e10635be1c203c05e6d1c7d3d071ce05ad7d51c38e6a57a85580179b0322e66dc957fe6d3f3a30a0537fbf0d124b44901364343dfd8526415e02579d5e111ee5a8e50f341903d5f28a078ac1782fe7dbff7d8cc3f5117765937d492c36a00b30b70b1dbd39534022aa6507afde615beecba
+
+Group = brainpool256r1
+Hash = SHA-256
+X = 0xa1456788204e69baf38488cae77763cadee5d0d91aa0e5e60d19b00d3a67ed48
+Msg = 627261696E706F6F6C503235367231
+Nonce = 098a5b7cfa317b796af446c40e3eb5287903422c56624978d902c59b7a92fe9b
+Signature = ecc82e85669005af077c67debaea8ebb332614e661efc7ffd1f452e1c44223cd2f53b09ddf97a82242552965e1a209278be14dde16fd54bc16f1f09d28e7fd11
+
+Group = brainpool384r1
+Hash = SHA-384
+X = 0x21241d9952cb236721cd02d4d5e0fdbe22d7ab69afc1221216c609960514ec3be65f5273dd069fdf637ed95c6ce54f08
+Msg = 627261696E706F6F6C503338347231
+Nonce = 070f21eca6ef53d8817533e89cb6b74fb82f4c2300808cab199e6ffe96e61d29227d76433c3433c34d185a8f0121385b
+Signature = e49a764eeb131ea1ba0cefa5d679a1f9635b9341a6b13c40bbd55c3820e509fa158c5a1af1b929268be106beb208c4c93ba436a68e1ee92597dd125e6efbe52818474175db2f47a5d0d02bd7c08143f1c3e9bf2ff14c1846ffbcdf399388596b
+
+Group = brainpool512r1
+Hash = SHA-512
+X = 0xa9eb22d41a8aa10ddfecd31f93c8af9a387ff529b981eb8e3a73be9070519950c28d81ce3184796718bfe439178c38e9a81099f7d2607cbf53a712c8b8b335cb
+Msg = 627261696E706F6F6C503531327231
+Nonce = 463d92923fe337d8e5fc5abe76f63d9eec9aaf57487defe1fae1765d7ef72f5d1064e1be65685c0840597e9879751bdac842cfbbb08e5c428d1ca6e7634b37ea
+Signature = c00d74be190345e3d97390fccfa1271d5e8d8ffb153149f6a7c569a878637bf5cd0278f92bce8c644cd3d425c9501434fedb1ec8f6556079ce51f8a4cc2ea9e345d8cf978c1558bd11babe4197e106190d6058535ebdf40006bdf22b112c877ee44ca2dcf90afe2104bdf6d8b907aa4c75bd2e5565a3ca8faeb1107e8539e21f
+
+Group = frp256v1
+Hash = SHA-256
+X = 0x9051A275AA4D98439EDDED13FA1C6CBBCCE775D8CC9433DEE69C59848B3594DF
+Msg = 616263
+Nonce = 71B88F398916DA9C90F555F1B5732B7DC636B49C638150BAC11BF05CFE16596A
+Signature = 3758d027181116e92baf70bf164864349e75603e89a4c6c1bdb801522dca20312d206f3e705e5efc32d43cb0e1f7b2c70cc2b02693cb69b729b7975c67d6a3e6
diff --git a/src/tests/test_eckcdsa.cpp b/src/tests/test_eckcdsa.cpp
index fe2b14b21..71d2f5544 100644
--- a/src/tests/test_eckcdsa.cpp
+++ b/src/tests/test_eckcdsa.cpp
@@ -28,11 +28,6 @@ class ECKCDSA_Signature_KAT_Tests final : public PK_Signature_Generation_Test
               "pubkey/eckcdsa.vec",
               "Group,X,Hash,Msg,Nonce,Signature") {}
 
-      bool clear_between_callbacks() const override
-         {
-         return false;
-         }
-
       std::unique_ptr<Botan::Private_Key> load_private_key(const VarMap& vars) override
          {
          const std::string group_id = vars.get_req_str("Group");
diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
index 103c70ce0..d82f354a4 100644
--- a/src/tests/test_pubkey.cpp
+++ b/src/tests/test_pubkey.cpp
@@ -103,7 +103,13 @@ PK_Signature_Generation_Test::run_one_test(const std::string& pad_hdr, const Var
    const std::vector<uint8_t> signature = vars.get_req_bin("Signature");
    const std::string padding = choose_padding(vars, pad_hdr);
 
-   Test::Result result(algo_name() + "/" + padding + " signature generation");
+   std::ostringstream test_name;
+   test_name << algo_name();
+   if(vars.has_key("Group"))
+      test_name << "-" << vars.get_req_str("Group");
+   test_name << "/" << padding << " signature generation";
+
+   Test::Result result(test_name.str());
 
    std::unique_ptr<Botan::Private_Key> privkey;
    try