Clean up PK decryption encoding.

Previously RSA and ElGamal stripped off leading zeros which were then
assumed by the padding decoders. Instead have them produce ciphertexts
with leading zeros. Changes EME_Raw to strip leading zeros to match
existing behavior.

17 files changed, 103 insertions, 86 deletions

diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index 0ce2c0c53..e221b1052 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -726,8 +726,8 @@ class Speed final : public Command
          Botan::PK_Encryptor_EME enc(key, padding, provider);
          Botan::PK_Decryptor_EME dec(key, padding, provider);
 
-         Timer enc_timer(nm, provider, "encrypt");
-         Timer dec_timer(nm, provider, "decrypt");
+         Timer enc_timer(nm, provider, padding + " encrypt");
+         Timer dec_timer(nm, provider, padding + " decrypt");
 
          while(enc_timer.under(msec) || dec_timer.under(msec))
             {
@@ -793,8 +793,8 @@ class Speed final : public Command
          Botan::PK_Signer   sig(key, padding, Botan::IEEE_1363, provider);
          Botan::PK_Verifier ver(key, padding, Botan::IEEE_1363, provider);
 
-         Timer sig_timer(nm, provider, "sign");
-         Timer ver_timer(nm, provider, "verify");
+         Timer sig_timer(nm, provider, padding + " sign");
+         Timer ver_timer(nm, provider, padding + " verify");
 
          while(ver_timer.under(msec) || sig_timer.under(msec))
             {
@@ -855,7 +855,10 @@ class Speed final : public Command
 
             // Using PKCS #1 padding so OpenSSL provider can play along
             bench_pk_enc(*key, nm, provider, "EME-PKCS1-v1_5", msec);
+            bench_pk_enc(*key, nm, provider, "OAEP(SHA-1)", msec);
+
             bench_pk_sig(*key, nm, provider, "EMSA-PKCS1-v1_5(SHA-1)", msec);
+            bench_pk_sig(*key, nm, provider, "PSSR(SHA-256)", msec);
             }
          }
 #endif
diff --git a/src/cli/tls_server.cpp b/src/cli/tls_server.cpp
index 2ccacb1f7..2496f5508 100644
--- a/src/cli/tls_server.cpp
+++ b/src/cli/tls_server.cpp
@@ -117,35 +117,42 @@ class TLS_Server final : public Command
                                       protocol_chooser,
                                       !is_tcp);
 
-            while(!server.is_closed())
+            try
                {
-               uint8_t buf[4*1024] = { 0 };
-               ssize_t got = ::read(fd, buf, sizeof(buf));
-
-               if(got == -1)
+               while(!server.is_closed())
                   {
-                  std::cout << "Error in socket read - " << strerror(errno) << std::endl;
-                  break;
-                  }
+                  uint8_t buf[4*1024] = { 0 };
+                  ssize_t got = ::read(fd, buf, sizeof(buf));
 
-               if(got == 0)
-                  {
-                  std::cout << "EOF on socket" << std::endl;
-                  break;
-                  }
+                  if(got == -1)
+                     {
+                     std::cout << "Error in socket read - " << strerror(errno) << std::endl;
+                     break;
+                     }
+
+                  if(got == 0)
+                     {
+                     std::cout << "EOF on socket" << std::endl;
+                     break;
+                     }
 
-               server.received_data(buf, got);
+                  server.received_data(buf, got);
 
-               while(server.is_active() && !pending_output.empty())
-                  {
-                  std::string output = pending_output.front();
-                  pending_output.pop_front();
-                  server.send(output);
+                  while(server.is_active() && !pending_output.empty())
+                     {
+                     std::string output = pending_output.front();
+                     pending_output.pop_front();
+                     server.send(output);
 
-                  if(output == "quit\n")
-                     server.close();
+                     if(output == "quit\n")
+                        server.close();
+                     }
                   }
                }
+            catch(Botan::Exception& e)
+               {
+               error_output() << "Connection failed: " << e.what() << "\n";
+               }
 
             if(is_tcp)
                ::close(fd);
diff --git a/src/lib/pk_pad/eme.h b/src/lib/pk_pad/eme.h
index f4c85da70..9c72cb023 100644
--- a/src/lib/pk_pad/eme.h
+++ b/src/lib/pk_pad/eme.h
@@ -65,8 +65,8 @@ class BOTAN_DLL EME
       */
       virtual secure_vector<byte> unpad(byte& valid_mask,
                                         const byte in[],
-                                        size_t in_len,
-                                        size_t key_length) const = 0;
+                                        size_t in_len) const = 0;
+
       /**
       * Encode an input
       * @param in the plaintext
diff --git a/src/lib/pk_pad/eme_oaep/oaep.cpp b/src/lib/pk_pad/eme_oaep/oaep.cpp
index 894368e2d..0ae0d8554 100644
--- a/src/lib/pk_pad/eme_oaep/oaep.cpp
+++ b/src/lib/pk_pad/eme_oaep/oaep.cpp
@@ -61,8 +61,7 @@ secure_vector<byte> OAEP::pad(const byte in[], size_t in_length,
 * OAEP Unpad Operation
 */
 secure_vector<byte> OAEP::unpad(byte& valid_mask,
-                                const byte in[], size_t in_length,
-                                size_t key_length) const
+                                const byte in[], size_t in_length) const
    {
    /*
    Must be careful about error messages here; if an attacker can
@@ -75,15 +74,13 @@ secure_vector<byte> OAEP::unpad(byte& valid_mask,
    Strenzke.
    */
 
-   key_length /= 8;
-
-   // Invalid input: truncate to zero length input, causing later
-   // checks to fail
-   if(in_length > key_length)
-      in_length = 0;
+   if(in[0] == 0)
+      {
+      in += 1;
+      in_length -= 1;
+      }
 
-   secure_vector<byte> input(key_length);
-   buffer_insert(input, key_length - in_length, in, in_length);
+   secure_vector<byte> input(in, in + in_length);
 
    CT::poison(input.data(), input.size());
 
diff --git a/src/lib/pk_pad/eme_oaep/oaep.h b/src/lib/pk_pad/eme_oaep/oaep.h
index dce706613..8b21ea81d 100644
--- a/src/lib/pk_pad/eme_oaep/oaep.h
+++ b/src/lib/pk_pad/eme_oaep/oaep.h
@@ -36,8 +36,7 @@ class BOTAN_DLL OAEP final : public EME
 
       secure_vector<byte> unpad(byte& valid_mask,
                                 const byte in[],
-                                size_t in_len,
-                                size_t key_length) const override;
+                                size_t in_len) const override;
 
       secure_vector<byte> m_Phash;
       std::unique_ptr<HashFunction> m_hash;
diff --git a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
index 4780fe43b..8148b7bc9 100644
--- a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
+++ b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
@@ -1,6 +1,6 @@
 /*
 * PKCS #1 v1.5 Type 2 (encryption) padding
-* (C) 1999-2007,2015 Jack Lloyd
+* (C) 1999-2007,2015,2016 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -27,8 +27,16 @@ secure_vector<byte> EME_PKCS1v15::pad(const byte in[], size_t inlen,
    secure_vector<byte> out(olen);
 
    out[0] = 0x02;
+   rng.randomize(out.data() + 1, (olen - inlen - 2));
+
    for(size_t j = 1; j != olen - inlen - 1; ++j)
-      out[j] = rng.next_nonzero_byte();
+      {
+      if(out[j] == 0)
+         {
+         out[j] = rng.next_nonzero_byte();
+         }
+      }
+
    buffer_insert(out, olen - inlen, in, inlen);
 
    return out;
@@ -38,21 +46,18 @@ secure_vector<byte> EME_PKCS1v15::pad(const byte in[], size_t inlen,
 * PKCS1 Unpad Operation
 */
 secure_vector<byte> EME_PKCS1v15::unpad(byte& valid_mask,
-                                        const byte in[], size_t inlen,
-                                        size_t key_len) const
+                                        const byte in[], size_t inlen) const
    {
-   if(inlen != key_len / 8 || inlen < 10)
-      throw Decoding_Error("PKCS1::unpad");
-
    CT::poison(in, inlen);
 
    byte bad_input_m = 0;
    byte seen_zero_m = 0;
    size_t delim_idx = 0;
 
-   bad_input_m |= ~CT::is_equal<byte>(in[0], 2);
+   bad_input_m |= ~CT::is_equal<byte>(in[0], 0);
+   bad_input_m |= ~CT::is_equal<byte>(in[1], 2);
 
-   for(size_t i = 1; i != inlen; ++i)
+   for(size_t i = 2; i < inlen; ++i)
       {
       const byte is_zero_m = CT::is_zero<byte>(in[i]);
 
@@ -63,12 +68,13 @@ secure_vector<byte> EME_PKCS1v15::unpad(byte& valid_mask,
       }
 
    bad_input_m |= ~seen_zero_m;
+   bad_input_m |= CT::is_less<size_t>(delim_idx, 8);
 
    CT::unpoison(in, inlen);
    CT::unpoison(bad_input_m);
    CT::unpoison(delim_idx);
 
-   secure_vector<byte> output(&in[delim_idx + 1], &in[inlen]);
+   secure_vector<byte> output(&in[delim_idx + 2], &in[inlen]);
    CT::cond_zero_mem(bad_input_m, output.data(), output.size());
    valid_mask = ~bad_input_m;
    return output;
diff --git a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h
index d5f8879d6..006b39997 100644
--- a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h
+++ b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h
@@ -25,8 +25,7 @@ class BOTAN_DLL EME_PKCS1v15 final : public EME
 
       secure_vector<byte> unpad(byte& valid_mask,
                                 const byte in[],
-                                size_t in_len,
-                                size_t key_length) const override;
+                                size_t in_len) const override;
    };
 
 }
diff --git a/src/lib/pk_pad/eme_raw/eme_raw.cpp b/src/lib/pk_pad/eme_raw/eme_raw.cpp
index 5c5dd6e40..84fd6f545 100644
--- a/src/lib/pk_pad/eme_raw/eme_raw.cpp
+++ b/src/lib/pk_pad/eme_raw/eme_raw.cpp
@@ -1,29 +1,27 @@
 /*
-* (C) 2015 Jack Lloyd
+* (C) 2015,2016 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
 
-#include <botan/internal/bit_ops.h>
 #include <botan/eme_raw.h>
+#include <botan/internal/bit_ops.h>
+#include <botan/internal/ct_utils.h>
 
 namespace Botan {
 
 secure_vector<byte> EME_Raw::pad(const byte in[], size_t in_length,
-                                 size_t key_bits,
+                                 size_t,
                                  RandomNumberGenerator&) const
    {
-   if(in_length > 0 && (8*(in_length - 1) + high_bit(in[0]) > key_bits))
-      throw Invalid_Argument("EME_Raw: Input is too large");
    return secure_vector<byte>(in, in + in_length);
    }
 
 secure_vector<byte> EME_Raw::unpad(byte& valid_mask,
-                                   const byte in[], size_t in_length,
-                                   size_t) const
+                                   const byte in[], size_t in_length) const
    {
    valid_mask = 0xFF;
-   return secure_vector<byte>(in, in + in_length);
+   return CT::strip_leading_zeros(in, in_length);
    }
 
 size_t EME_Raw::maximum_input_size(size_t keybits) const
diff --git a/src/lib/pk_pad/eme_raw/eme_raw.h b/src/lib/pk_pad/eme_raw/eme_raw.h
index 60d23323c..fa30c684e 100644
--- a/src/lib/pk_pad/eme_raw/eme_raw.h
+++ b/src/lib/pk_pad/eme_raw/eme_raw.h
@@ -23,8 +23,7 @@ class BOTAN_DLL EME_Raw final : public EME
 
       secure_vector<byte> unpad(byte& valid_mask,
                                 const byte in[],
-                                size_t in_len,
-                                size_t key_length) const override;
+                                size_t in_len) const override;
    };
 
 }
diff --git a/src/lib/prov/openssl/openssl_ec.cpp b/src/lib/prov/openssl/openssl_ec.cpp
index 74d8f744a..4378833ec 100644
--- a/src/lib/prov/openssl/openssl_ec.cpp
+++ b/src/lib/prov/openssl/openssl_ec.cpp
@@ -5,7 +5,6 @@
 * Botan is released under the Simplified BSD License (see license.txt)
 */
 
-#include <iostream>
 #include <botan/internal/openssl.h>
 
 #if defined(BOTAN_HAS_ECC_PUBLIC_KEY_CRYPTO)
diff --git a/src/lib/prov/openssl/openssl_rsa.cpp b/src/lib/prov/openssl/openssl_rsa.cpp
index e3c0c0fec..ed8f2b0fd 100644
--- a/src/lib/prov/openssl/openssl_rsa.cpp
+++ b/src/lib/prov/openssl/openssl_rsa.cpp
@@ -140,13 +140,21 @@ class OpenSSL_RSA_Decryption_Operation : public PK_Ops::Decryption
 
       size_t max_input_bits() const override { return ::BN_num_bits(m_openssl_rsa->n) - 1; }
 
-      secure_vector<byte> decrypt(const byte msg[], size_t msg_len) override
+      secure_vector<byte> decrypt(byte& valid_mask,
+                                  const byte msg[], size_t msg_len) override
          {
          secure_vector<byte> buf(::RSA_size(m_openssl_rsa.get()));
          int rc = ::RSA_private_decrypt(msg_len, msg, buf.data(), m_openssl_rsa.get(), m_padding);
          if(rc < 0 || static_cast<size_t>(rc) > buf.size())
-            throw OpenSSL_Error("RSA_private_decrypt");
-         buf.resize(rc);
+            {
+            valid_mask = 0;
+            buf.resize(0);
+            }
+         else
+            {
+            valid_mask = 0xFF;
+            buf.resize(rc);
+            }
 
          if(m_padding == RSA_NO_PADDING)
             {
diff --git a/src/lib/pubkey/elgamal/elgamal.cpp b/src/lib/pubkey/elgamal/elgamal.cpp
index 10dc195a8..37dfe89cf 100644
--- a/src/lib/pubkey/elgamal/elgamal.cpp
+++ b/src/lib/pubkey/elgamal/elgamal.cpp
@@ -174,7 +174,7 @@ ElGamal_Decryption_Operation::raw_decrypt(const byte msg[], size_t msg_len)
 
    BigInt r = m_mod_p.multiply(b, inverse_mod(m_powermod_x_p(a), p));
 
-   return BigInt::encode_locked(m_blinder.unblind(r));
+   return BigInt::encode_1363(m_blinder.unblind(r), p_bytes);
    }
 
 BOTAN_REGISTER_PK_ENCRYPTION_OP("ElGamal", ElGamal_Encryption_Operation);
diff --git a/src/lib/pubkey/pk_ops.cpp b/src/lib/pubkey/pk_ops.cpp
index 37c31777d..654b68255 100644
--- a/src/lib/pubkey/pk_ops.cpp
+++ b/src/lib/pubkey/pk_ops.cpp
@@ -31,12 +31,7 @@ secure_vector<byte> PK_Ops::Encryption_with_EME::encrypt(const byte msg[], size_
                                                          RandomNumberGenerator& rng)
    {
    const size_t max_raw = max_raw_input_bits();
-
    const std::vector<byte> encoded = unlock(m_eme->encode(msg, msg_len, max_raw, rng));
-
-   if(8*(encoded.size() - 1) + high_bit(encoded[0]) > max_raw)
-      throw Exception("Input is too large to encrypt with this key");
-
    return raw_encrypt(encoded.data(), encoded.size(), rng);
    }
 
@@ -60,7 +55,7 @@ PK_Ops::Decryption_with_EME::decrypt(byte& valid_mask,
                                      size_t ciphertext_len)
    {
    const secure_vector<byte> raw = raw_decrypt(ciphertext, ciphertext_len);
-   return m_eme->unpad(valid_mask, raw.data(), raw.size(), max_raw_input_bits());
+   return m_eme->unpad(valid_mask, raw.data(), raw.size());
    }
 
 PK_Ops::Key_Agreement_with_KDF::Key_Agreement_with_KDF(const std::string& kdf)
diff --git a/src/lib/pubkey/pubkey.cpp b/src/lib/pubkey/pubkey.cpp
index 86665ed93..c0485fec8 100644
--- a/src/lib/pubkey/pubkey.cpp
+++ b/src/lib/pubkey/pubkey.cpp
@@ -53,7 +53,6 @@ PK_Decryptor::decrypt_or_random(const byte in[],
                                 size_t required_contents_length) const
    {
    const secure_vector<byte> fake_pms = rng.random_vec(expected_pt_len);
-   //secure_vector<byte> decoded(expected_pt_len);
 
    CT::poison(in, length);
 
@@ -62,7 +61,6 @@ PK_Decryptor::decrypt_or_random(const byte in[],
 
    valid_mask &= CT::is_equal(decoded.size(), expected_pt_len);
 
-   // fixme
    decoded.resize(expected_pt_len);
 
    for(size_t i = 0; i != required_contents_length; ++i)
diff --git a/src/lib/pubkey/rsa/rsa.cpp b/src/lib/pubkey/rsa/rsa.cpp
index eb9fc2892..e12586014 100644
--- a/src/lib/pubkey/rsa/rsa.cpp
+++ b/src/lib/pubkey/rsa/rsa.cpp
@@ -157,7 +157,7 @@ class RSA_Decryption_Operation : public PK_Ops::Decryption_with_EME,
          const BigInt x = blinded_private_op(m);
          const BigInt c = m_powermod_e_n(x);
          BOTAN_ASSERT(m == c, "RSA decrypt consistency check");
-         return BigInt::encode_locked(x);
+         return BigInt::encode_1363(x, m_n.bytes());
          }
    };
 
diff --git a/src/lib/utils/ct_utils.h b/src/lib/utils/ct_utils.h
index ec055374a..5a1d03d4f 100644
--- a/src/lib/utils/ct_utils.h
+++ b/src/lib/utils/ct_utils.h
@@ -177,20 +177,24 @@ inline T min(T a, T b)
    return select(expand_top_bit(b), b, a);
    }
 
-template<typename T, typename Alloc>
-std::vector<T, Alloc> strip_leading_zeros(const std::vector<T, Alloc>& input)
+inline secure_vector<uint8_t> strip_leading_zeros(const uint8_t in[], size_t length)
    {
    size_t leading_zeros = 0;
 
    uint8_t only_zeros = 0xFF;
 
-   for(size_t i = 0; i != input.size(); ++i)
+   for(size_t i = 0; i != length; ++i)
       {
-      only_zeros &= CT::is_zero(input[i]);
+      only_zeros &= CT::is_zero(in[i]);
       leading_zeros += CT::select<uint8_t>(only_zeros, 1, 0);
       }
 
-   return secure_vector<byte>(input.begin() + leading_zeros, input.end());
+   return secure_vector<byte>(in + leading_zeros, in + length);
+   }
+
+inline secure_vector<byte> strip_leading_zeros(const secure_vector<uint8_t>& in)
+   {
+   return strip_leading_zeros(in.data(), in.size());
    }
 
 }
diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
index 025628636..4d795ae1d 100644
--- a/src/tests/test_pubkey.cpp
+++ b/src/tests/test_pubkey.cpp
@@ -37,7 +37,7 @@ void check_invalid_signatures(Test::Result& result,
 
    std::vector<uint8_t> bad_sig = signature;
 
-   for(size_t i = 0; i <= Test::soak_level(); ++i)
+   for(size_t i = 0; i < Test::soak_level(); ++i)
       {
       while(bad_sig == signature)
          bad_sig = Test::mutate_vec(bad_sig, true);
@@ -59,7 +59,7 @@ void check_invalid_ciphertexts(Test::Result& result,
 
    size_t ciphertext_accepted = 0, ciphertext_rejected = 0;
 
-   for(size_t i = 0; i <= Test::soak_level(); ++i)
+   for(size_t i = 0; i < Test::soak_level(); ++i)
       {
       while(bad_ctext == ciphertext)
          bad_ctext = Test::mutate_vec(bad_ctext, true);
@@ -222,7 +222,8 @@ PK_Encryption_Decryption_Test::run_one_test(const std::string&, const VarMap& va
 
       if(enc_provider == "base")
          {
-         result.test_eq("generated ciphertext matches KAT", generated_ciphertext, ciphertext);
+         result.test_eq(enc_provider, "generated ciphertext matches KAT",
+                        generated_ciphertext, ciphertext);
          }
 
       for(auto&& dec_provider : possible_pk_providers())
@@ -239,10 +240,14 @@ PK_Encryption_Decryption_Test::run_one_test(const std::string&, const VarMap& va
             continue;
             }
 
-         result.test_eq("decryption of KAT", decryptor->decrypt(ciphertext), plaintext);
+         result.test_eq(dec_provider, "decryption of KAT", decryptor->decrypt(ciphertext), plaintext);
          check_invalid_ciphertexts(result, *decryptor, plaintext, ciphertext);
-         result.test_eq("decryption of generated ciphertext",
-                        decryptor->decrypt(generated_ciphertext), plaintext);
+
+         if(generated_ciphertext != ciphertext)
+            {
+            result.test_eq(dec_provider, "decryption of generated ciphertext",
+                           decryptor->decrypt(generated_ciphertext), plaintext);
+            }
          }
       }