Fix the ordering of the GOST 34.10 signature values. Add a test

derived from a DNSSEC RFC. Bug reported by Bert Hubert to the
mailing list. According to Bert, this ordering is compatible with
the version included in OpenSSL.

Also, benchmark GOST 34.10 using the GOST 34.11 hash since that
is always what it is used with.

4 files changed, 22 insertions, 20 deletions

diff --git a/checks/pk_bench.cpp b/checks/pk_bench.cpp
index 3597257a4..348cb7ff1 100644
--- a/checks/pk_bench.cpp
+++ b/checks/pk_bench.cpp
@@ -154,7 +154,7 @@ void benchmark_sig_ver(PK_Verifier& ver, PK_Signer& sig,
       if(verify_timer.seconds() < seconds)
          {
          verify_timer.start();
-         bool verified = ver.verify_message(message, signature);
+         const bool verified = ver.verify_message(message, signature);
          verify_timer.stop();
 
          if(!verified)
@@ -165,10 +165,10 @@ void benchmark_sig_ver(PK_Verifier& ver, PK_Signer& sig,
             sig_random = rng.random_vec(signature.size());
 
             verify_timer.start();
-            bool verified2 = ver.verify_message(message, sig_random);
+            const bool verified_bad = ver.verify_message(message, sig_random);
             verify_timer.stop();
 
-            if(verified2)
+            if(verified_bad)
                std::cerr << "Signature verification failure (bad sig OK)\n";
             }
          }
@@ -356,16 +356,9 @@ void benchmark_gost_3410(RandomNumberGenerator& rng,
       {
       EC_Domain_Params params(OIDS::lookup(ec_domains[j]));
 
-      size_t pbits = params.get_curve().get_p().bits();
-
-      size_t hashbits = pbits;
-
-      if(hashbits <= 192)
-         hashbits = 160;
-      if(hashbits == 521)
-         hashbits = 512;
+      const size_t pbits = params.get_curve().get_p().bits();
 
-      const std::string padding = "EMSA1(SHA-" + to_string(hashbits) + ")";
+      const std::string padding = "EMSA1(GOST-34.11)";
 
       Timer keygen_timer("keygen");
       Timer verify_timer(padding + " verify");
diff --git a/checks/pk_valid.dat b/checks/pk_valid.dat
index 3220ba29e..bce2b0e04 100644
--- a/checks/pk_valid.dat
+++ b/checks/pk_valid.dat
@@ -4256,16 +4256,24 @@ gost_256A:\
 864048EA2675E8FD8DB1FEDFC7DD40E3CF3A319EE3130E0BE9FDF994B625\
 BC1885F271:\
 :\
-AA3CB0563295A3E281BA368DF8471DE0A4150B3CFCEA575D8A9CC9779035EC36\
-FE406F383A54127453AED406FA9A3B610B28F89FC918C07A5A75289E97B3A991
+FE406F383A54127453AED406FA9A3B610B28F89FC918C07A5A75289E97B3A991\
+AA3CB0563295A3E281BA368DF8471DE0A4150B3CFCEA575D8A9CC9779035EC36
 
 gost_256A:\
 04BFE0BA366BE575E45C5BBA339C51ACD75D517008A9D3169E3CCEA6EF08\
 046DA74312382D835BEEA1C561A75AFCAFDA0F75A4E5D9787F9DB2870A03\
 2AC1D90465:\
 :\
-B7AB61F33E0B70166C355963BB80B8F6DF54F7F6A43872295CD42B6ACF7DF678\
-F3AFCBE1398DDC01F0A9E4B45397F3ACD8F343399BD2805FB6293E9CB871123A
+F3AFCBE1398DDC01F0A9E4B45397F3ACD8F343399BD2805FB6293E9CB871123A\
+B7AB61F33E0B70166C355963BB80B8F6DF54F7F6A43872295CD42B6ACF7DF678
+
+# From RFC 5933 (via Bert Hubert)
+gost_256A:\
+03773DC3F032886D56439A9F17490B680570043F757252C1F60819D6C30DBF1469:\
+00010C0300000E1070DBD880386D4380E954076578616D706C65036E657400\
+03777777076578616D706C65036E6574000001000100000E100004C0000201:\
+66ED09C0A6C97E22CB4E66BCA61D2082FDF6924F3A717C43B531B2D43FEE76DB\
+B0F490A7901B009CCDF87252EBE1790A9AB1A6A444DBACA3E264AF21D18B5E83
 
 # NR Format: p:q:g:y:x:message:k:output
 [NR/EMSA1(SHA-1)]
diff --git a/doc/log.txt b/doc/log.txt
index b8b6ad53c..280495556 100644
--- a/doc/log.txt
+++ b/doc/log.txt
@@ -1,6 +1,7 @@
 
 * 1.9.13-dev, ????-??-??
  - Update Keccak to the round 3 variant
+ - Fix ordering in GOST 34.10 signatures to match DNSSEC specifications
 
 * 1.9.12, 2010-12-13
  - Add the Keccak hash function
diff --git a/src/pubkey/gost_3410/gost_3410.cpp b/src/pubkey/gost_3410/gost_3410.cpp
index 61693e01f..fa72d0673 100644
--- a/src/pubkey/gost_3410/gost_3410.cpp
+++ b/src/pubkey/gost_3410/gost_3410.cpp
@@ -130,8 +130,8 @@ GOST_3410_Signature_Operation::sign(const byte msg[], size_t msg_len,
       throw Invalid_State("GOST 34.10: r == 0 || s == 0");
 
    SecureVector<byte> output(2*order.bytes());
-   r.binary_encode(&output[output.size() / 2 - r.bytes()]);
-   s.binary_encode(&output[output.size() - s.bytes()]);
+   s.binary_encode(&output[output.size() / 2 - s.bytes()]);
+   r.binary_encode(&output[output.size() - r.bytes()]);
    return output;
    }
 
@@ -150,8 +150,8 @@ bool GOST_3410_Verification_Operation::verify(const byte msg[], size_t msg_len,
 
    BigInt e = decode_le(msg, msg_len);
 
-   BigInt r(sig, sig_len / 2);
-   BigInt s(sig + sig_len / 2, sig_len / 2);
+   BigInt s(sig, sig_len / 2);
+   BigInt r(sig + sig_len / 2, sig_len / 2);
 
    if(r < 0 || r >= order || s < 0 || s >= order)
       return false;