Merge GH #1666 Enforce salt length in PSS signatures

4 files changed, 95 insertions, 37 deletions

diff --git a/src/lib/pk_pad/emsa.cpp b/src/lib/pk_pad/emsa.cpp
index 52ef268af..eaae898f3 100644
--- a/src/lib/pk_pad/emsa.cpp
+++ b/src/lib/pk_pad/emsa.cpp
@@ -81,25 +81,45 @@ EMSA* get_emsa(const std::string& algo_spec)
 #endif
 
 #if defined(BOTAN_HAS_EMSA_PSSR)
-   if(req.algo_name() == "PSSR" ||
-      req.algo_name() == "EMSA-PSS" ||
-      req.algo_name() == "PSS-MGF1" ||
-      req.algo_name() == "EMSA4" ||
+   if(req.algo_name() == "PSS_Raw" ||
       req.algo_name() == "PSSR_Raw")
       {
-      if(req.arg_count_between(1, 3))
+      if(req.arg_count_between(1, 3) && req.arg(1, "MGF1") == "MGF1")
          {
-         if(req.arg(1, "MGF1") != "MGF1")
-            return nullptr; // not supported
-
          if(auto h = HashFunction::create(req.arg(0)))
             {
-            const size_t salt_size = req.arg_as_integer(2, h->output_length());
-
-            if(req.algo_name() == "PSSR_Raw")
+            if(req.arg_count() == 3)
+               {
+               const size_t salt_size = req.arg_as_integer(2, 0);
                return new PSSR_Raw(h.release(), salt_size);
+               }
             else
+               {
+               return new PSSR_Raw(h.release());
+               }
+            }
+         }
+      }
+   
+   if(req.algo_name() == "PSS" ||
+      req.algo_name() == "PSSR" ||
+      req.algo_name() == "EMSA-PSS" ||
+      req.algo_name() == "PSS-MGF1" ||
+      req.algo_name() == "EMSA4")
+      {
+      if(req.arg_count_between(1, 3) && req.arg(1, "MGF1") == "MGF1")
+         {
+         if(auto h = HashFunction::create(req.arg(0)))
+            {
+            if(req.arg_count() == 3)
+               {
+               const size_t salt_size = req.arg_as_integer(2, 0);
                return new PSSR(h.release(), salt_size);
+               }
+            else
+               {
+               return new PSSR(h.release());
+               }
             }
          }
       }
diff --git a/src/lib/pk_pad/emsa_pssr/pssr.cpp b/src/lib/pk_pad/emsa_pssr/pssr.cpp
index 97b229be3..25c0a191f 100644
--- a/src/lib/pk_pad/emsa_pssr/pssr.cpp
+++ b/src/lib/pk_pad/emsa_pssr/pssr.cpp
@@ -55,9 +55,10 @@ secure_vector<uint8_t> pss_encode(HashFunction& hash,
    }
 
 bool pss_verify(HashFunction& hash,
-                const secure_vector<uint8_t>& const_coded,
-                const secure_vector<uint8_t>& raw,
-                size_t key_bits)
+                const secure_vector<uint8_t>& pss_repr,
+                const secure_vector<uint8_t>& message_hash,
+                size_t key_bits,
+                size_t* out_salt_size)
    {
    const size_t HASH_SIZE = hash.output_length();
    const size_t KEY_BYTES = (key_bits + 7) / 8;
@@ -65,16 +66,16 @@ bool pss_verify(HashFunction& hash,
    if(key_bits < 8*HASH_SIZE + 9)
       return false;
 
-   if(raw.size() != HASH_SIZE)
+   if(message_hash.size() != HASH_SIZE)
       return false;
 
-   if(const_coded.size() > KEY_BYTES || const_coded.size() <= 1)
+   if(pss_repr.size() > KEY_BYTES || pss_repr.size() <= 1)
       return false;
 
-   if(const_coded[const_coded.size()-1] != 0xBC)
+   if(pss_repr[pss_repr.size()-1] != 0xBC)
       return false;
 
-   secure_vector<uint8_t> coded = const_coded;
+   secure_vector<uint8_t> coded = pss_repr;
    if(coded.size() < KEY_BYTES)
       {
       secure_vector<uint8_t> temp(KEY_BYTES);
@@ -110,23 +111,32 @@ bool pss_verify(HashFunction& hash,
 
    for(size_t j = 0; j != 8; ++j)
       hash.update(0);
-   hash.update(raw);
+   hash.update(message_hash);
    hash.update(&DB[salt_offset], salt_size);
 
-   secure_vector<uint8_t> H2 = hash.final();
+   const secure_vector<uint8_t> H2 = hash.final();
 
-   return constant_time_compare(H, H2.data(), HASH_SIZE);
+   const bool ok = constant_time_compare(H, H2.data(), HASH_SIZE);
+
+   if(out_salt_size && ok)
+      *out_salt_size = salt_size;
+
+   return ok;
    }
 
 }
 
 PSSR::PSSR(HashFunction* h) :
-   m_hash(h), m_SALT_SIZE(m_hash->output_length())
+   m_hash(h),
+   m_salt_size(m_hash->output_length()),
+   m_required_salt_len(false)
    {
    }
 
 PSSR::PSSR(HashFunction* h, size_t salt_size) :
-   m_hash(h), m_SALT_SIZE(salt_size)
+   m_hash(h),
+   m_salt_size(salt_size),
+   m_required_salt_len(true)
    {
    }
 
@@ -150,7 +160,7 @@ secure_vector<uint8_t> PSSR::encoding_of(const secure_vector<uint8_t>& msg,
                                          size_t output_bits,
                                          RandomNumberGenerator& rng)
    {
-   secure_vector<uint8_t> salt = rng.random_vec(m_SALT_SIZE);
+   const secure_vector<uint8_t> salt = rng.random_vec(m_salt_size);
    return pss_encode(*m_hash, msg, salt, output_bits);
    }
 
@@ -161,12 +171,23 @@ bool PSSR::verify(const secure_vector<uint8_t>& coded,
                   const secure_vector<uint8_t>& raw,
                   size_t key_bits)
    {
-   return pss_verify(*m_hash, coded, raw, key_bits);
+   size_t salt_size = 0;
+   const bool ok = pss_verify(*m_hash, coded, raw, key_bits, &salt_size);
+
+   if(m_required_salt_len && salt_size != m_salt_size)
+      return false;
+
+   return ok;
+   }
+
+EMSA* PSSR::clone()
+   {
+   return new PSSR(m_hash->clone(), m_salt_size);
    }
 
 std::string PSSR::name() const
    {
-   return "EMSA4(" + m_hash->name() + ",MGF1," + std::to_string(m_SALT_SIZE) + ")";
+   return "EMSA4(" + m_hash->name() + ",MGF1," + std::to_string(m_salt_size) + ")";
    }
 
 AlgorithmIdentifier PSSR::config_for_x509(const Private_Key& key,
@@ -193,7 +214,7 @@ AlgorithmIdentifier PSSR::config_for_x509(const Private_Key& key,
       .start_cons(SEQUENCE)
       .start_cons(ASN1_Tag(0), CONTEXT_SPECIFIC).encode(hash_id).end_cons()
       .start_cons(ASN1_Tag(1), CONTEXT_SPECIFIC).encode(mgf_id).end_cons()
-      .start_cons(ASN1_Tag(2), CONTEXT_SPECIFIC).encode(m_SALT_SIZE).end_cons()
+      .start_cons(ASN1_Tag(2), CONTEXT_SPECIFIC).encode(m_salt_size).end_cons()
       .start_cons(ASN1_Tag(3), CONTEXT_SPECIFIC).encode(size_t(1)).end_cons() // trailer field
       .end_cons();
 
@@ -201,12 +222,16 @@ AlgorithmIdentifier PSSR::config_for_x509(const Private_Key& key,
    }
 
 PSSR_Raw::PSSR_Raw(HashFunction* h) :
-   m_hash(h), m_SALT_SIZE(m_hash->output_length())
+   m_hash(h),
+   m_salt_size(m_hash->output_length()),
+   m_required_salt_len(false)
    {
    }
 
 PSSR_Raw::PSSR_Raw(HashFunction* h, size_t salt_size) :
-   m_hash(h), m_SALT_SIZE(salt_size)
+   m_hash(h),
+   m_salt_size(salt_size),
+   m_required_salt_len(true)
    {
    }
 
@@ -236,7 +261,7 @@ secure_vector<uint8_t> PSSR_Raw::encoding_of(const secure_vector<uint8_t>& msg,
                                              size_t output_bits,
                                              RandomNumberGenerator& rng)
    {
-   secure_vector<uint8_t> salt = rng.random_vec(m_SALT_SIZE);
+   secure_vector<uint8_t> salt = rng.random_vec(m_salt_size);
    return pss_encode(*m_hash, msg, salt, output_bits);
    }
 
@@ -247,12 +272,23 @@ bool PSSR_Raw::verify(const secure_vector<uint8_t>& coded,
                       const secure_vector<uint8_t>& raw,
                       size_t key_bits)
    {
-   return pss_verify(*m_hash, coded, raw, key_bits);
+   size_t salt_size = 0;
+   const bool ok = pss_verify(*m_hash, coded, raw, key_bits, &salt_size);
+
+   if(m_required_salt_len && salt_size != m_salt_size)
+      return false;
+
+   return ok;
+   }
+
+EMSA* PSSR_Raw::clone()
+   {
+   return new PSSR_Raw(m_hash->clone(), m_salt_size);
    }
 
 std::string PSSR_Raw::name() const
    {
-   return "PSSR_Raw(" + m_hash->name() + ",MGF1," + std::to_string(m_SALT_SIZE) + ")";
+   return "PSSR_Raw(" + m_hash->name() + ",MGF1," + std::to_string(m_salt_size) + ")";
    }
 
 }
diff --git a/src/lib/pk_pad/emsa_pssr/pssr.h b/src/lib/pk_pad/emsa_pssr/pssr.h
index 3b902dd6a..fa0ae1228 100644
--- a/src/lib/pk_pad/emsa_pssr/pssr.h
+++ b/src/lib/pk_pad/emsa_pssr/pssr.h
@@ -31,7 +31,7 @@ class BOTAN_PUBLIC_API(2,0) PSSR final : public EMSA
       */
       PSSR(HashFunction* hash, size_t salt_size);
 
-      EMSA* clone() override { return new PSSR(m_hash->clone(), m_SALT_SIZE); }
+      EMSA* clone() override;
 
       std::string name() const override;
 
@@ -51,7 +51,8 @@ class BOTAN_PUBLIC_API(2,0) PSSR final : public EMSA
                   size_t key_bits) override;
 
       std::unique_ptr<HashFunction> m_hash;
-      size_t m_SALT_SIZE;
+      size_t m_salt_size;
+      bool m_required_salt_len;
    };
 
 /**
@@ -73,7 +74,7 @@ class BOTAN_PUBLIC_API(2,3) PSSR_Raw final : public EMSA
       */
       PSSR_Raw(HashFunction* hash, size_t salt_size);
 
-      EMSA* clone() override { return new PSSR_Raw(m_hash->clone(), m_SALT_SIZE); }
+      EMSA* clone() override;
 
       std::string name() const override;
    private:
@@ -90,8 +91,9 @@ class BOTAN_PUBLIC_API(2,3) PSSR_Raw final : public EMSA
                   size_t key_bits) override;
 
       std::unique_ptr<HashFunction> m_hash;
-      size_t m_SALT_SIZE;
       secure_vector<uint8_t> m_msg;
+      size_t m_salt_size;
+      bool m_required_salt_len;
    };
 
 }
diff --git a/src/tests/data/x509/pss_certs/expected.txt b/src/tests/data/x509/pss_certs/expected.txt
index 96192d4d6..633eb946d 100644
--- a/src/tests/data/x509/pss_certs/expected.txt
+++ b/src/tests/data/x509/pss_certs/expected.txt
@@ -95,7 +95,7 @@
 95:1
 96:1
 97:Certificate signature has invalid parameters
-98:Verified
+98:Signature error
 99:Signature error
 100:Verified
 101:Verified