Merge pull request #4598 from randombit/jack/group-init-cleanup

Cleanup EC_Group and DL_Group construction

src/lib/misc/srp6/srp6.cpp

@@ -160,42 +160,45 @@ BigInt SRP6_Server_Session::step1(
    const BigInt& v, const DL_Group& group, std::string_view hash_id, size_t b_bits, RandomNumberGenerator& rng) {
    BOTAN_ARG_CHECK(b_bits <= group.p_bits(), "Invalid b_bits");
 
-   m_group = group;
+   BOTAN_STATE_CHECK(!m_group);
+   m_group = std::make_unique<DL_Group>(group);
 
-   const BigInt& g = group.get_g();
-   const BigInt& p = group.get_p();
+   const BigInt& g = m_group->get_g();
+   const BigInt& p = m_group->get_p();
 
    m_v = v;
    m_b = BigInt(rng, b_bits);
    m_hash_id = hash_id;
 
    auto hash_fn = HashFunction::create_or_throw(hash_id);
-   if(8 * hash_fn->output_length() >= group.p_bits()) {
+   if(8 * hash_fn->output_length() >= m_group->p_bits()) {
       throw Invalid_Argument(fmt("Hash function {} too large for SRP6 with this group", hash_fn->name()));
    }
 
-   const BigInt k = hash_seq(*hash_fn, m_group.p_bytes(), p, g);
-   m_B = group.mod_p(v * k + group.power_g_p(m_b, b_bits));
+   const BigInt k = hash_seq(*hash_fn, m_group->p_bytes(), p, g);
+   m_B = m_group->mod_p(v * k + m_group->power_g_p(m_b, b_bits));
 
    return m_B;
 }
 
 SymmetricKey SRP6_Server_Session::step2(const BigInt& A) {
-   if(A <= 0 || A >= m_group.get_p()) {
+   BOTAN_STATE_CHECK(m_group);
+
+   if(A <= 0 || A >= m_group->get_p()) {
       throw Decoding_Error("Invalid SRP parameter from client");
    }
 
    auto hash_fn = HashFunction::create_or_throw(m_hash_id);
-   if(8 * hash_fn->output_length() >= m_group.p_bits()) {
+   if(8 * hash_fn->output_length() >= m_group->p_bits()) {
       throw Invalid_Argument(fmt("Hash function {} too large for SRP6 with this group", hash_fn->name()));
    }
 
-   const BigInt u = hash_seq(*hash_fn, m_group.p_bytes(), A, m_B);
+   const BigInt u = hash_seq(*hash_fn, m_group->p_bytes(), A, m_B);
 
-   const BigInt vup = m_group.power_b_p(m_v, u, m_group.p_bits());
-   const BigInt S = m_group.power_b_p(m_group.multiply_mod_p(A, vup), m_b, m_group.p_bits());
+   const BigInt vup = m_group->power_b_p(m_v, u, m_group->p_bits());
+   const BigInt S = m_group->power_b_p(m_group->multiply_mod_p(A, vup), m_b, m_group->p_bits());
 
-   return SymmetricKey(S.serialize<secure_vector<uint8_t>>(m_group.p_bytes()));
+   return SymmetricKey(S.serialize<secure_vector<uint8_t>>(m_group->p_bytes()));
 }
 
 }  // namespace Botan

src/lib/misc/srp6/srp6.h

@@ -133,7 +133,7 @@ class BOTAN_PUBLIC_API(2, 0) SRP6_Server_Session final {
       SymmetricKey step2(const BigInt& A);
 
    private:
-      DL_Group m_group;
+      std::unique_ptr<DL_Group> m_group;
       std::string m_hash_id;
       BigInt m_B, m_b, m_v, m_S;
 };

src/lib/prov/pkcs11/p11_ecc_key.cpp

@@ -65,22 +65,16 @@ EC_PrivateKeyImportProperties::EC_PrivateKeyImportProperties(const std::vector<u
    add_binary(AttributeType::Value, m_value.serialize());
 }
 
-PKCS11_EC_PrivateKey::PKCS11_EC_PrivateKey(Session& session, ObjectHandle handle) : Object(session, handle) {
-   secure_vector<uint8_t> ec_parameters = get_attribute_value(AttributeType::EcParams);
-   m_domain_params = EC_Group(unlock(ec_parameters));
-}
+PKCS11_EC_PrivateKey::PKCS11_EC_PrivateKey(Session& session, ObjectHandle handle) :
+      Object(session, handle), m_domain_params(get_attribute_value(AttributeType::EcParams)) {}
 
 PKCS11_EC_PrivateKey::PKCS11_EC_PrivateKey(Session& session, const EC_PrivateKeyImportProperties& props) :
-      Object(session, props) {
-   m_domain_params = EC_Group(props.ec_params());
-}
+      Object(session, props), m_domain_params(EC_Group(props.ec_params())) {}
 
 PKCS11_EC_PrivateKey::PKCS11_EC_PrivateKey(Session& session,
                                            const std::vector<uint8_t>& ec_params,
                                            const EC_PrivateKeyGenerationProperties& props) :
-      Object(session) {
-   m_domain_params = EC_Group(ec_params);
-
+      Object(session), m_domain_params(ec_params) {
    EC_PublicKeyGenerationProperties pub_key_props(ec_params);
    pub_key_props.set_verify(true);
    pub_key_props.set_private(false);

src/lib/pubkey/dl_group/dl_group.cpp

@@ -600,8 +600,4 @@ DL_Group::DL_Group(const uint8_t ber[], size_t ber_len, DL_Group_Format format)
    m_data = BER_decode_DL_group(ber, ber_len, format, DL_Group_Source::ExternalSource);
 }
 
-void DL_Group::BER_decode(const std::vector<uint8_t>& ber, DL_Group_Format format) {
-   m_data = BER_decode_DL_group(ber.data(), ber.size(), format, DL_Group_Source::ExternalSource);
-}
-
 }  // namespace Botan

src/lib/pubkey/dl_group/dl_group.h

@@ -52,10 +52,8 @@ class BOTAN_PUBLIC_API(2, 0) DL_Group final {
 
       /**
       * Construct a DL group with uninitialized internal value.
-      * Use this constructor is you wish to set the groups values
-      * from a DER or PEM encoded group.
       */
-      DL_Group() = default;
+      BOTAN_DEPRECATED("Deprecated no replacement") DL_Group() = default;
 
       /**
       * Construct a DL group that is registered in the configuration.
@@ -65,7 +63,7 @@ class BOTAN_PUBLIC_API(2, 0) DL_Group final {
       * deprecated and will be removed in a future major release. Instead
       * use DL_Group::from_PEM or DL_Group::from_name
       */
-      explicit DL_Group(std::string_view name);
+      BOTAN_DEPRECATED("Use DL_Group::from_name or DL_Group::from_PEM") explicit DL_Group(std::string_view name);
 
       /**
       * Construct a DL group that is registered in the configuration.
@@ -361,12 +359,16 @@ class BOTAN_PUBLIC_API(2, 0) DL_Group final {
       *
       * @warning avoid this. Instead use the DL_Group constructor
       */
-      void BER_decode(const std::vector<uint8_t>& ber, DL_Group_Format format);
+      BOTAN_DEPRECATED("Use DL_Group constructor taking BER encoding")
+      void BER_decode(const std::vector<uint8_t>& ber, DL_Group_Format format) {
+         *this = DL_Group(ber, format);
+      }
 
       DL_Group_Source source() const;
 
       /*
       * For internal use only
+      * TODO(Botan4) Underscore prefix this
       */
       static std::shared_ptr<DL_Group_Data> DL_group_info(std::string_view name);
 

src/lib/pubkey/ec_group/ec_group.h

@@ -180,7 +180,7 @@ class BOTAN_PUBLIC_API(2, 0) EC_Group final {
       /**
       * Create an uninitialized EC_Group
       */
-      EC_Group();
+      BOTAN_DEPRECATED("Deprecated no replacement") EC_Group();
 
       ~EC_Group();
 

src/tests/test_dh.cpp

@@ -32,12 +32,13 @@ class Diffie_Hellman_KAT_Tests final : public PK_Key_Agreement_Test {
          const Botan::BigInt g = vars.get_req_bn("G");
          const Botan::BigInt x = vars.get_req_bn("X");
 
-         Botan::DL_Group group;
-         if(q == 0) {
-            group = Botan::DL_Group(p, g);
-         } else {
-            group = Botan::DL_Group(p, q, g);
-         }
+         auto group = [&]() {
+            if(q == 0) {
+               return Botan::DL_Group(p, g);
+            } else {
+               return Botan::DL_Group(p, q, g);
+            }
+         }();
 
          return std::make_unique<Botan::DH_PrivateKey>(group, x);
       }
@@ -48,12 +49,13 @@ class Diffie_Hellman_KAT_Tests final : public PK_Key_Agreement_Test {
          const Botan::BigInt g = vars.get_req_bn("G");
          const Botan::BigInt y = vars.get_req_bn("Y");
 
-         Botan::DL_Group group;
-         if(q == 0) {
-            group = Botan::DL_Group(p, g);
-         } else {
-            group = Botan::DL_Group(p, q, g);
-         }
+         auto group = [&]() {
+            if(q == 0) {
+               return Botan::DL_Group(p, g);
+            } else {
+               return Botan::DL_Group(p, q, g);
+            }
+         }();
 
          Botan::DH_PublicKey key(group, y);
          return key.public_value();