Merge GH #1218 Change SM2 ciphertext encoding to match most recent standard

6 files changed, 75 insertions, 35 deletions

diff --git a/src/lib/pubkey/sm2/sm2_enc.cpp b/src/lib/pubkey/sm2/sm2_enc.cpp
index aca31941d..2d44faacb 100644
--- a/src/lib/pubkey/sm2/sm2_enc.cpp
+++ b/src/lib/pubkey/sm2/sm2_enc.cpp
@@ -8,13 +8,15 @@
 #include <botan/sm2_enc.h>
 #include <botan/pk_ops.h>
 #include <botan/keypair.h>
+#include <botan/der_enc.h>
+#include <botan/ber_dec.h>
 #include <botan/kdf.h>
 #include <botan/hash.h>
 
 namespace Botan {
 
 bool SM2_Encryption_PrivateKey::check_key(RandomNumberGenerator& rng,
-                                         bool strong) const
+                                          bool strong) const
    {
    if(!public_point().on_the_curve())
       return false;
@@ -64,9 +66,6 @@ class SM2_Encryption_Operation : public PK_Ops::Encryption
          std::unique_ptr<HashFunction> hash = HashFunction::create_or_throw(m_kdf_hash);
          std::unique_ptr<KDF> kdf = KDF::create_or_throw("KDF2(" + m_kdf_hash + ")");
 
-         secure_vector<uint8_t> ciphertext;
-         ciphertext.reserve(1 + m_p_bytes*2 + msg_len + hash->output_length());
-
          const BigInt k = BigInt::random_integer(rng, 1, m_order);
 
          const PointGFp C1 = m_base_point.blinded_multiply(k, rng);
@@ -102,13 +101,14 @@ class SM2_Encryption_Operation : public PK_Ops::Encryption
          std::vector<uint8_t> C3(hash->output_length());
          hash->final(C3.data());
 
-         ciphertext.push_back(0x04);
-         ciphertext += x1_bytes;
-         ciphertext += y1_bytes;
-         ciphertext += C3;
-         ciphertext += masked_msg;
-
-         return ciphertext;
+         return DER_Encoder()
+            .start_cons(SEQUENCE)
+            .encode(x1)
+            .encode(y1)
+            .encode(C3, OCTET_STRING)
+            .encode(masked_msg, OCTET_STRING)
+            .end_cons()
+            .get_contents();
          }
 
    private:
@@ -137,7 +137,7 @@ class SM2_Decryption_Operation : public PK_Ops::Decryption
          const BigInt& cofactor = m_key.domain().get_cofactor();
          const size_t p_bytes = m_key.domain().get_curve().get_p().bytes();
 
-         valid_mask = 0;
+         valid_mask = 0x00;
 
          std::unique_ptr<HashFunction> hash = HashFunction::create_or_throw(m_kdf_hash);
          std::unique_ptr<KDF> kdf = KDF::create_or_throw("KDF2(" + m_kdf_hash + ")");
@@ -148,15 +148,21 @@ class SM2_Decryption_Operation : public PK_Ops::Decryption
             return secure_vector<uint8_t>();
             }
 
-         if(ciphertext[0] != 0x04)
-            {
-            return secure_vector<uint8_t>();
-            }
+         BigInt x1, y1;
+         secure_vector<uint8_t> C3, masked_msg;
 
-         const size_t msg_len = ciphertext_len - (1 + p_bytes*2 + hash->output_length());
+         BER_Decoder(ciphertext, ciphertext_len)
+            .start_cons(SEQUENCE)
+            .decode(x1)
+            .decode(y1)
+            .decode(C3, OCTET_STRING)
+            .decode(masked_msg, OCTET_STRING)
+            .end_cons()
+            .verify_end();
 
-         const PointGFp C1 = OS2ECP(ciphertext, 1 + p_bytes*2, m_key.domain().get_curve());
-         // OS2ECP verifies C1 is on the curve
+         const PointGFp C1(m_key.domain().get_curve(), x1, y1);
+         if(!C1.on_the_curve())
+            return secure_vector<uint8_t>();
 
          Blinded_Point_Multiply C1_mul(C1, m_key.domain().get_order());
 
@@ -180,21 +186,20 @@ class SM2_Decryption_Operation : public PK_Ops::Decryption
          kdf_input += y2_bytes;
 
          const secure_vector<uint8_t> kdf_output =
-            kdf->derive_key(msg_len, kdf_input.data(), kdf_input.size());
+            kdf->derive_key(masked_msg.size(), kdf_input.data(), kdf_input.size());
 
-         secure_vector<uint8_t> msg(msg_len);
-         xor_buf(msg.data(), ciphertext + (1+p_bytes*2+hash->output_length()), kdf_output.data(), msg_len);
+         xor_buf(masked_msg.data(), kdf_output.data(), kdf_output.size());
 
          hash->update(x2_bytes);
-         hash->update(msg);
+         hash->update(masked_msg);
          hash->update(y2_bytes);
          secure_vector<uint8_t> u = hash->final();
 
-         if(constant_time_compare(u.data(), ciphertext + (1+p_bytes*2), hash->output_length()) == false)
+         if(constant_time_compare(u.data(), C3.data(), hash->output_length()) == false)
             return secure_vector<uint8_t>();
 
          valid_mask = 0xFF;
-         return msg;
+         return masked_msg;
          }
    private:
       const SM2_Encryption_PrivateKey& m_key;
diff --git a/src/lib/pubkey/sm2/sm2_enc.h b/src/lib/pubkey/sm2/sm2_enc.h
index 69a9cd953..cc8d2cacb 100644
--- a/src/lib/pubkey/sm2/sm2_enc.h
+++ b/src/lib/pubkey/sm2/sm2_enc.h
@@ -54,8 +54,8 @@ class BOTAN_PUBLIC_API(2,2) SM2_Encryption_PublicKey : public virtual EC_PublicK
 /**
 * This class represents a private key used for SM2 encryption
 */
-class BOTAN_PUBLIC_API(2,2) SM2_Encryption_PrivateKey : public SM2_Encryption_PublicKey,
-                                            public EC_PrivateKey
+class BOTAN_PUBLIC_API(2,2) SM2_Encryption_PrivateKey final :
+   public SM2_Encryption_PublicKey, public EC_PrivateKey
    {
    public:
 
diff --git a/src/tests/data/pubkey/sm2_enc.vec b/src/tests/data/pubkey/sm2_enc.vec
index 120eab583..0fd3b3e34 100644
--- a/src/tests/data/pubkey/sm2_enc.vec
+++ b/src/tests/data/pubkey/sm2_enc.vec
@@ -1,5 +1,5 @@
-# Example from draft-shen-sm2-ecdsa-02
-# Corrected to use (C1||C3||C2) - the draft is wrong!
+
+# The standard SM2 test input, ASN.1 from GmSSL
 
 P = 0x8542D69E4C044F18E8B92435BF6FF7DE457283915C45517D722EDB8B08F1DFC3
 A = 0x787968B4FA32C3FD2417842E73BBFEFF2F3C848B6831D7E0EC65228B3937E498
@@ -9,7 +9,29 @@ yG = 0x0680512BCBB42C07D47349D2153B70C4E5D7FDFCBFA36EA1A85841B9E46E09A2
 Order = 0x8542D69E4C044F18E8B92435BF6FF7DD297720630485628D5AE74EE7C32E79B7
 Cofactor = 1
 
-Msg = 656E6372797074696F6E207374616E64617264
 x = 0x1649AB77A00637BD5E2EFE283FBF353534AA7F7CB89463F208DDBC2920BB0DA0
+
+Msg = 656E6372797074696F6E207374616E64617264
 Nonce = 4C62EEFD6ECFC2B95B92FD6C3D9575148AFA17425546D49018E5388D49DD7B4F
-Ciphertext = 04245C26FB68B1DDDDB12C4B6BF9F2B6D5FE60A383B0D18D1C4144ABF17F6252E776CB9264C2A7E88E52B19903FDC47378F605E36811F5C07423A24B84400F01B89C3D7360C30156FAB7C80A0276712DA9D8094A634B766D3A285E07480653426D650053A89B41C418B0C3AAD00D886C00286467
+Ciphertext = 307B0220245C26FB68B1DDDDB12C4B6BF9F2B6D5FE60A383B0D18D1C4144ABF17F6252E7022076CB9264C2A7E88E52B19903FDC47378F605E36811F5C07423A24B84400F01B804209C3D7360C30156FAB7C80A0276712DA9D8094A634B766D3A285E07480653426D0413650053A89B41C418B0C3AAD00D886C00286467
+
+# Same test as above but using SHA-256, generated by GmSSL
+
+Hash = SHA-256
+Ciphertext = 307B0220245C26FB68B1DDDDB12C4B6BF9F2B6D5FE60A383B0D18D1C4144ABF17F6252E7022076CB9264C2A7E88E52B19903FDC47378F605E36811F5C07423A24B84400F01B80420BE89139D07853100EFA763F60CBE30099EA3DF7F8F364F9D10A5E988E3C5AAFC0413229E6C9AEE2BB92CAD649FE2C035689785DA33
+
+
+# Random tests by GmSSL
+Hash = SM3
+
+Msg = 61207465737420696E707574
+Nonce = 04B4EC222E5F984A397837C481FC5A81A19B542D694E8BBF2E58E0FB19E92F0E
+Ciphertext = 3074022039EA73E877679ED0BBAD126B9A61F882DEDC329FAAF5529D5B3EE167B95322E502203A36529C0BF5525501FA9F18F18F1571EBDE47951519B4E04F214560363C35990420FE3D9CB71BA55F7F173911E2FF4CD5239392B91291254C33970E6ABF097D5FE1040CADF4FA9A4E56FCAAD1AD9C97
+
+Msg = 61207465737420696E707574
+Nonce = 34B4EC222E5F984A397837C481FC5A81A19B542D694E8BBF2E58E0FB19E92F0E
+Ciphertext = 307402201DFD5BCA0C187B656EBD41628F6B26A406C2874E659D371258CA60365F644C5A0220239CA0E0E482BD9B3C444EFE4618324D9217DCCBBDF70B477EEF93756A85C276042023FBAD7B932F7321F24737E553BE536534D3B0ADF0D77DC631E50D90AD872A07040CD2F70E71D686471CF0F0CD49
+
+Msg = 79657420616E6F74686572207465737420696E70757420666F7220796F75
+Nonce = 218F4211FB45868ADD749992E8C899B28DEF9AFEAE49B19BE0657A481D06D36B
+Ciphertext = 30818602206838137EABA46E36D2E89556A623EB6C790635138CF7DC50B0B4932C5CBFA892022051532CB347883709856962B2762B047B6EA4597C957016AB1F24D4DCD55148340420DD1DC1C83FBFA04B831B61D39BA2F3B60C9443DCC82E172913F598A7357DCA53041E394D660717179357030E5282DD4E7DCE144D6C168ED42A1AD093E406AC0D
diff --git a/src/tests/test_ffi.cpp b/src/tests/test_ffi.cpp
index 2d63bfa69..bc3e72b09 100644
--- a/src/tests/test_ffi.cpp
+++ b/src/tests/test_ffi.cpp
@@ -1532,8 +1532,8 @@ class FFI_Unit_Tests : public Test
          result.test_eq(namebuf, namebuf, "SM2_Enc");
 
          std::vector<uint8_t> message(32);
-         // Assumes 256-bit params:
-         std::vector<uint8_t> ciphertext(1 + 32*2 + message.size() + 32);
+
+         std::vector<uint8_t> ciphertext(4096);
          TEST_FFI_OK(botan_rng_get, (rng, message.data(), message.size()));
 
          botan_pk_op_encrypt_t enc;
@@ -1542,11 +1542,12 @@ class FFI_Unit_Tests : public Test
             size_t ctext_len = ciphertext.size();
             TEST_FFI_OK(botan_pk_op_encrypt, (enc, rng, ciphertext.data(), &ctext_len,
                                               message.data(), message.size()));
+            ciphertext.resize(ctext_len);
 
             botan_pk_op_decrypt_t dec;
             TEST_FFI_OK(botan_pk_op_decrypt_create, (&dec, loaded_privkey, "", 0));
 
-            std::vector<uint8_t> recovered(ciphertext.size());
+            std::vector<uint8_t> recovered(message.size());
             size_t recovered_len = recovered.size();
 
             TEST_FFI_OK(botan_pk_op_decrypt,
diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
index bbaa444f0..a7e58b10f 100644
--- a/src/tests/test_pubkey.cpp
+++ b/src/tests/test_pubkey.cpp
@@ -273,7 +273,17 @@ PK_Encryption_Decryption_Test::run_one_test(const std::string& pad_hdr, const Va
          continue;
          }
 
-      result.test_eq(dec_provider, "decryption of KAT", decryptor->decrypt(ciphertext), plaintext);
+      Botan::secure_vector<uint8_t> decrypted;
+      try
+         {
+         decrypted = decryptor->decrypt(ciphertext);
+         }
+      catch(Botan::Exception& e)
+         {
+         result.test_failure("Failed to decrypt KAT ciphertext", e.what());
+         }
+
+      result.test_eq(dec_provider, "decryption of KAT", decrypted, plaintext);
       check_invalid_ciphertexts(result, *decryptor, plaintext, ciphertext);
       }
 
diff --git a/src/tests/test_sm2.cpp b/src/tests/test_sm2.cpp
index 0497ccedd..0816a5f76 100644
--- a/src/tests/test_sm2.cpp
+++ b/src/tests/test_sm2.cpp
@@ -78,6 +78,8 @@ class SM2_Encryption_KAT_Tests : public PK_Encryption_Decryption_Test
          return get_opt_str(vars, "Hash", "SM3");
          }
 
+      bool clear_between_callbacks() const override { return false; }
+
       Botan::RandomNumberGenerator* test_rng(const std::vector<uint8_t>& nonce) const override
          {
          return new Fixed_Output_Position_RNG(nonce, 1);