1 files changed, 37 insertions, 33 deletions

diff --git a/src/lib/pubkey/dsa/dsa.cpp b/src/lib/pubkey/dsa/dsa.cpp
index 9a8418d46..f1d412013 100644
--- a/src/lib/pubkey/dsa/dsa.cpp
+++ b/src/lib/pubkey/dsa/dsa.cpp
@@ -47,14 +47,14 @@ DSA_PrivateKey::DSA_PrivateKey(RandomNumberGenerator& rng,
    else
       m_x = x_arg;
 
-   m_y = power_mod(group_g(), m_x, group_p());
+   m_y = m_group.power_g_p(m_x);
    }
 
 DSA_PrivateKey::DSA_PrivateKey(const AlgorithmIdentifier& alg_id,
                                const secure_vector<uint8_t>& key_bits) :
    DL_Scheme_PrivateKey(alg_id, key_bits, DL_Group::ANSI_X9_57)
    {
-   m_y = power_mod(group_g(), m_x, group_p());
+   m_y = m_group.power_g_p(m_x);
    }
 
 /*
@@ -81,9 +81,8 @@ class DSA_Signature_Operation final : public PK_Ops::Signature_with_EMSA
    public:
       DSA_Signature_Operation(const DSA_PrivateKey& dsa, const std::string& emsa) :
          PK_Ops::Signature_with_EMSA(emsa),
-         m_q(dsa.group_q()),
+         m_group(dsa.get_group()),
          m_x(dsa.get_x()),
-         m_powermod_g_p(dsa.group_g(), dsa.group_p()),
          m_mod_q(dsa.group_q())
          {
 #if defined(BOTAN_HAS_RFC6979_GENERATOR)
@@ -91,14 +90,13 @@ class DSA_Signature_Operation final : public PK_Ops::Signature_with_EMSA
 #endif
          }
 
-      size_t max_input_bits() const override { return m_q.bits(); }
+      size_t max_input_bits() const override { return m_group.get_q().bits(); }
 
       secure_vector<uint8_t> raw_sign(const uint8_t msg[], size_t msg_len,
                                    RandomNumberGenerator& rng) override;
    private:
-      const BigInt& m_q;
+      const DL_Group m_group;
       const BigInt& m_x;
-      Fixed_Base_Power_Mod m_powermod_g_p;
       Modular_Reducer m_mod_q;
 #if defined(BOTAN_HAS_RFC6979_GENERATOR)
       std::string m_rfc6979_hash;
@@ -109,36 +107,38 @@ secure_vector<uint8_t>
 DSA_Signature_Operation::raw_sign(const uint8_t msg[], size_t msg_len,
                                   RandomNumberGenerator& rng)
    {
+   const BigInt& q = m_group.get_q();
+
    BigInt i(msg, msg_len);
 
-   while(i >= m_q)
-      i -= m_q;
+   while(i >= q)
+      i -= q;
 
 #if defined(BOTAN_HAS_RFC6979_GENERATOR)
    BOTAN_UNUSED(rng);
-   const BigInt k = generate_rfc6979_nonce(m_x, m_q, i, m_rfc6979_hash);
+   const BigInt k = generate_rfc6979_nonce(m_x, q, i, m_rfc6979_hash);
 #else
-   const BigInt k = BigInt::random_integer(rng, 1, m_q);
+   const BigInt k = BigInt::random_integer(rng, 1, q);
 #endif
 
 #if defined(BOTAN_TARGET_OS_HAS_THREADS)
    auto future_r = std::async(std::launch::async,
-                              [&]() { return m_mod_q.reduce(m_powermod_g_p(k)); });
+                              [&]() { return m_mod_q.reduce(m_group.power_g_p(k)); });
 
-   BigInt s = inverse_mod(k, m_q);
+   BigInt s = inverse_mod(k, q);
    const BigInt r = future_r.get();
 #else
-   BigInt s = inverse_mod(k, m_q);
-   const BigInt r = m_mod_q.reduce(m_powermod_g_p(k));
+   BigInt s = inverse_mod(k, q);
+   const BigInt r = m_mod_q.reduce(m_group.power_g_p(k));
 #endif
 
    s = m_mod_q.multiply(s, mul_add(m_x, r, i));
 
    // With overwhelming probability, a bug rather than actual zero r/s
-   BOTAN_ASSERT(s != 0, "invalid s");
-   BOTAN_ASSERT(r != 0, "invalid r");
+   if(r.is_zero() || s.is_zero())
+      throw Internal_Error("Computed zero r/s during DSA signature");
 
-   return BigInt::encode_fixed_length_int_pair(r, s, m_q.bytes());
+   return BigInt::encode_fixed_length_int_pair(r, s, q.bytes());
    }
 
 /**
@@ -150,52 +150,56 @@ class DSA_Verification_Operation final : public PK_Ops::Verification_with_EMSA
       DSA_Verification_Operation(const DSA_PublicKey& dsa,
                                  const std::string& emsa) :
          PK_Ops::Verification_with_EMSA(emsa),
-         m_q(dsa.group_q()), m_y(dsa.get_y()), m_powermod_g_p{Fixed_Base_Power_Mod(dsa.group_g(), dsa.group_p())},
-         m_powermod_y_p{Fixed_Base_Power_Mod(m_y, dsa.group_p())}, m_mod_p{Modular_Reducer(dsa.group_p())},
-         m_mod_q{Modular_Reducer(dsa.group_q())}
+         m_group(dsa.get_group()),
+         m_y(dsa.get_y()),
+         m_powermod_y_p(m_y, dsa.group_p()),
+         m_mod_q(dsa.group_q())
          {}
 
-      size_t max_input_bits() const override { return m_q.bits(); }
+      size_t max_input_bits() const override { return m_group.get_q().bits(); }
 
       bool with_recovery() const override { return false; }
 
       bool verify(const uint8_t msg[], size_t msg_len,
                   const uint8_t sig[], size_t sig_len) override;
    private:
-      const BigInt& m_q;
+      const DL_Group m_group;
       const BigInt& m_y;
 
-      Fixed_Base_Power_Mod m_powermod_g_p, m_powermod_y_p;
-      Modular_Reducer m_mod_p, m_mod_q;
+      Fixed_Base_Power_Mod m_powermod_y_p;
+      Modular_Reducer m_mod_q;
    };
 
 bool DSA_Verification_Operation::verify(const uint8_t msg[], size_t msg_len,
                                         const uint8_t sig[], size_t sig_len)
    {
-   if(sig_len != 2*m_q.bytes() || msg_len > m_q.bytes())
+   const BigInt& q = m_group.get_q();
+   const size_t q_bytes = q.bytes();
+
+   if(sig_len != 2*q_bytes || msg_len > q_bytes)
       return false;
 
-   BigInt r(sig, m_q.bytes());
-   BigInt s(sig + m_q.bytes(), m_q.bytes());
+   BigInt r(sig, q_bytes);
+   BigInt s(sig + q_bytes, q_bytes);
    BigInt i(msg, msg_len);
 
-   if(r <= 0 || r >= m_q || s <= 0 || s >= m_q)
+   if(r <= 0 || r >= q || s <= 0 || s >= q)
       return false;
 
-   s = inverse_mod(s, m_q);
+   s = inverse_mod(s, q);
 
 #if defined(BOTAN_TARGET_OS_HAS_THREADS)
    auto future_s_i = std::async(std::launch::async,
-      [&]() { return m_powermod_g_p(m_mod_q.multiply(s, i)); });
+                                [&]() { return m_group.power_g_p(m_mod_q.multiply(s, i)); });
 
    BigInt s_r = m_powermod_y_p(m_mod_q.multiply(s, r));
    BigInt s_i = future_s_i.get();
 #else
    BigInt s_r = m_powermod_y_p(m_mod_q.multiply(s, r));
-   BigInt s_i = m_powermod_g_p(m_mod_q.multiply(s, i));
+   BigInt s_i = m_group.power_g_p(m_mod_q.multiply(s, i));
 #endif
 
-   s = m_mod_p.multiply(s_i, s_r);
+   s = m_group.multiply_mod_p(s_i, s_r);
 
    return (m_mod_q.reduce(s) == r);
    }