Fix off by one in PKCS #1 v1.5 decryption decoding

When the code was changed in b8966d0f89e, the offset was not changed,
so it would reject ciphertexts with exactly 8 bytes of random padding
(the required minimum).

Found by pkcs1 fuzzer which also had problems due to not having been
updated at the same time.

Add a test suite for decoding of PK decryption padding to cover the
problem cases.

8 files changed, 156 insertions, 20 deletions

diff --git a/src/extra_tests/fuzzers/GNUmakefile b/src/extra_tests/fuzzers/GNUmakefile
index 3ebe64be7..aa45eb040 100644
--- a/src/extra_tests/fuzzers/GNUmakefile
+++ b/src/extra_tests/fuzzers/GNUmakefile
@@ -40,11 +40,11 @@ dirs:
 
 afl-build:
 	../../../configure.py $(CFG_FLAGS) --with-build-dir=afl-build --cc=$(AFL_CXX_TYPE) --cc-bin=$(AFL_CXX)
-	make -f afl-build/Makefile afl-build/libbotan-1.11.a -j8
+	make -j$(nproc) -f afl-build/Makefile afl-build/libbotan-1.11.a
 
 llvm-build:
 	../../../configure.py $(CFG_FLAGS) --with-build-dir=llvm-build --cc=clang --cc-bin=$(CLANG_CXX) --cc-abi-flags="$(CLANG_SAN_FLAGS)"
-	make -f llvm-build/Makefile llvm-build/libbotan-1.11.a -j8
+	make -j$(nproc) -f llvm-build/Makefile llvm-build/libbotan-1.11.a
 
 # libFuzzer default is max_len 64 this sets 140 but allows override via args=
 run_llvm_%: bin/llvm_fuzz_%
diff --git a/src/extra_tests/fuzzers/jigs/pkcs1.cpp b/src/extra_tests/fuzzers/jigs/pkcs1.cpp
index 889308f0e..8a16d17e5 100644
--- a/src/extra_tests/fuzzers/jigs/pkcs1.cpp
+++ b/src/extra_tests/fuzzers/jigs/pkcs1.cpp
@@ -13,14 +13,14 @@ secure_vector<byte> simple_pkcs1_unpad(const byte in[], size_t len)
    if(len < 10)
       throw Botan::Decoding_Error("bad len");
 
-   if(in[0] != 2)
-      throw Botan::Decoding_Error("bad field");
+   if(in[0] != 0 || in[1] != 2)
+      throw Botan::Decoding_Error("bad header field");
 
-   for(size_t i = 1; i < len; ++i)
+   for(size_t i = 2; i < len; ++i)
       {
       if(in[i] == 0)
          {
-         if(i < 9)
+         if(i < 10) // at least 8 padding bytes required
             throw Botan::Decoding_Error("insufficient padding bytes");
          return secure_vector<byte>(in + i + 1, in + len);
          }
@@ -42,9 +42,9 @@ void fuzz(const uint8_t in[], size_t len)
       secure_vector<byte> decoded = ((EME*)&pkcs1)->unpad(valid_mask, in, len);
 
       if(valid_mask == 0)
-         lib_rejected = false;
-      else if(valid_mask == 0xFF)
          lib_rejected = true;
+      else if(valid_mask == 0xFF)
+         lib_rejected = false;
       else
          abort();
       }
@@ -54,15 +54,24 @@ void fuzz(const uint8_t in[], size_t len)
       {
       ref_result = simple_pkcs1_unpad(in, len);
       }
-   catch(Botan::Decoding_Error&) { ref_rejected = true; }
+   catch(Botan::Decoding_Error& e) { ref_rejected = true; /*printf("%s\n", e.what());*/ }
 
-   FUZZER_ASSERT_EQUAL(lib_rejected, ref_rejected);
+   if(lib_rejected == ref_rejected)
+      {
+      return; // ok, they agree
+      }
 
-   if(lib_result != ref_result)
+   // otherwise: incorrect result, log info and crash
+   if(lib_rejected == true && ref_rejected == false)
+      {
+      std::cerr << "Library rejected input accepted by ref\n";
+      std::cerr << "Ref decoded " << hex_encode(ref_result) << "\n";
+      }
+   else if(ref_rejected == true && lib_rejected == false)
       {
-      std::cerr << hex_encode(lib_result) << " != ref \n"
-                << hex_encode(ref_result) << std::endl;
-      abort();
+      std::cerr << "Library accepted input reject by ref\n";
+      std::cerr << "Lib decoded " << hex_encode(lib_result) << "\n";
       }
 
+   abort();
    }
diff --git a/src/extra_tests/fuzzers/jigs/tls_client_hello.cpp b/src/extra_tests/fuzzers/jigs/tls_client_hello.cpp
index 5705dca91..33b6f941a 100644
--- a/src/extra_tests/fuzzers/jigs/tls_client_hello.cpp
+++ b/src/extra_tests/fuzzers/jigs/tls_client_hello.cpp
@@ -12,10 +12,6 @@ void fuzz(const uint8_t in[], size_t len)
       {
       std::vector<uint8_t> v(in, in + len);
       Botan::TLS::Client_Hello ch(v);
-
-      printf("%s\n", ch.version().to_string().c_str());
-      if(ch.version() == Botan::TLS::Protocol_Version::TLS_V12)
-         abort();
       }
-   catch(Botan::Exception& e) {printf("%s\n", e.what()); }
+   catch(Botan::Exception& e) {}
    }
diff --git a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
index b14808da0..2b5ee4ba0 100644
--- a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
+++ b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp
@@ -69,7 +69,7 @@ secure_vector<byte> EME_PKCS1v15::unpad(byte& valid_mask,
 
       delim_idx += CT::select<byte>(~seen_zero_m, 1, 0);
 
-      bad_input_m |= is_zero_m & CT::expand_mask<byte>(i < 9);
+      bad_input_m |= is_zero_m & CT::expand_mask<byte>(i < 10);
       seen_zero_m |= is_zero_m;
       }
 
diff --git a/src/tests/data/pk_pad_eme/pkcs1.vec b/src/tests/data/pk_pad_eme/pkcs1.vec
new file mode 100644
index 000000000..48b732d95
--- /dev/null
+++ b/src/tests/data/pk_pad_eme/pkcs1.vec
@@ -0,0 +1,46 @@
+[PKCS1v15]
+RawCiphertext =
+ValidInput = false
+
+RawCiphertext = 00
+ValidInput = false
+
+RawCiphertext = 0000
+ValidInput = false
+
+RawCiphertext = FF
+ValidInput = false
+
+RawCiphertext = FF02
+ValidInput = false
+
+RawCiphertext = 0002DEDE24212121DEDEDE5EDEDEDEDE0A5EDE00000000DEDEDE010000000000
+Plaintext = 000000DEDEDE010000000000
+ValidInput = true
+
+RawCiphertext = 022C2C4018181818181818181818183A18181818181818180000002C022C00010A2C2C2C2C2C022C
+ValidInput = false
+
+RawCiphertext = 00022C2C4018181818181818181818183A18181818181818180000002C022C00010A2C2C2C2C2C022C
+Plaintext = 00002C022C00010A2C2C2C2C2C022C
+ValidInput = true
+
+RawCiphertext = 0002FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF010100000021FFFFFFFFFFFFBC
+Plaintext = 000021FFFFFFFFFFFFBC
+ValidInput = true
+
+RawCiphertext = 0002F9CCFFFFCCCCCCCCCCCCCCCC4E0000CCFFFFCCCCCCCCCCCCCCCCCCCCCCCCCC06
+Plaintext = 00CCFFFFCCCCCCCCCCCCCCCCCCCCCCCCCC06
+ValidInput = true
+
+RawCiphertext = 000253FFC43B5253FF0A53DE0000FD
+Plaintext = 00FD
+ValidInput = true
+
+RawCiphertext = 0002FFFF06FFFFFFFFFF00000000000000000000000000000000000000000000000000000000FF0A
+Plaintext = 000000000000000000000000000000000000000000000000000000FF0A
+ValidInput = true
+
+# Padding only 7 bytes
+RawCiphertext = 0002FFFFFFFFFFFFFF000113131313131388
+ValidInput = false
diff --git a/src/tests/test_pk_pad.cpp b/src/tests/test_pk_pad.cpp
new file mode 100644
index 000000000..79448e63f
--- /dev/null
+++ b/src/tests/test_pk_pad.cpp
@@ -0,0 +1,69 @@
+/*
+* (C) 2016 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#include "tests.h"
+
+#if defined(BOTAN_HAS_PK_PADDING)
+  #include <botan/emsa.h>
+  #include <botan/eme.h>
+#endif
+
+namespace Botan_Tests {
+
+#if defined(BOTAN_HAS_PK_PADDING)
+
+class EME_Decoding_Tests : public Text_Based_Test
+   {
+   public:
+      EME_Decoding_Tests() :
+         Text_Based_Test("pk_pad_eme",
+                         std::vector<std::string>{"RawCiphertext","ValidInput"},
+                         std::vector<std::string>{"Plaintext"}) {}
+
+      Test::Result run_one_test(const std::string& algo, const VarMap& vars) override
+         {
+         Test::Result result(algo + " Decoding");
+
+         std::unique_ptr<Botan::EME> eme(Botan::get_eme(algo));
+
+         if(eme == nullptr)
+            {
+            result.note_missing(algo);
+            return result;
+            }
+
+         const std::vector<uint8_t> ciphertext = get_req_bin(vars, "RawCiphertext");
+         const std::vector<uint8_t> plaintext = get_opt_bin(vars, "Plaintext");
+         const bool is_valid = get_req_bool(vars, "ValidInput");
+
+         if(is_valid == false)
+            result.test_eq("Plaintext value is empty for invalid EME inputs", plaintext.size(), 0);
+
+         uint8_t valid_mask = 0;
+         Botan::secure_vector<byte> decoded =
+            eme->unpad(valid_mask, ciphertext.data(), ciphertext.size());
+
+         result.confirm("EME valid_mask has expected value", valid_mask == 0x00 || valid_mask == 0xFF);
+         result.test_eq("EME decoding valid/invalid matches", valid_mask == 0xFF, is_valid);
+
+         if(is_valid && valid_mask == 0xFF)
+            {
+            result.test_eq("EME decoded plaintext correct", decoded, plaintext);
+            }
+
+         // TODO: also test that encoding is accepted
+
+         return result;
+         }
+   };
+
+BOTAN_REGISTER_TEST("pk_pad_eme", EME_Decoding_Tests);
+
+#endif
+
+}
+
+
diff --git a/src/tests/tests.cpp b/src/tests/tests.cpp
index f922c99a6..b78b7a2e2 100644
--- a/src/tests/tests.cpp
+++ b/src/tests/tests.cpp
@@ -602,6 +602,20 @@ std::string Text_Based_Test::get_opt_str(const VarMap& vars,
    return i->second;
    }
 
+bool Text_Based_Test::get_req_bool(const VarMap& vars, const std::string& key) const
+   {
+   auto i = vars.find(key);
+   if(i == vars.end())
+      throw Test_Error("Test missing variable " + key);
+
+   if(i->second == "true")
+      return true;
+   else if(i->second == "false")
+      return false;
+   else
+      throw Test_Error("Invalid boolean for key '" + key + "' value '" + i->second + "'");
+   }
+
 size_t Text_Based_Test::get_req_sz(const VarMap& vars, const std::string& key) const
    {
    auto i = vars.find(key);
diff --git a/src/tests/tests.h b/src/tests/tests.h
index 733e948ea..32aaad0fb 100644
--- a/src/tests/tests.h
+++ b/src/tests/tests.h
@@ -430,6 +430,8 @@ class Text_Based_Test : public Test
 
       virtual std::vector<Test::Result> run_final_tests() { return std::vector<Test::Result>(); }
 
+      bool get_req_bool(const VarMap& vars, const std::string& key) const;
+
       std::vector<uint8_t> get_req_bin(const VarMap& vars, const std::string& key) const;
       std::vector<uint8_t> get_opt_bin(const VarMap& vars, const std::string& key) const;