Merge GH #2186 Deprecate DER_Encoder::get_contents_unlocked

9 files changed, 71 insertions, 93 deletions

diff --git a/src/lib/asn1/der_enc.h b/src/lib/asn1/der_enc.h
index 135a70d07..fac11ebf2 100644
--- a/src/lib/asn1/der_enc.h
+++ b/src/lib/asn1/der_enc.h
@@ -52,7 +52,14 @@ class BOTAN_PUBLIC_API(2,0) DER_Encoder final
 
       secure_vector<uint8_t> get_contents();
 
-      std::vector<uint8_t> get_contents_unlocked();
+      /**
+      * Return the encoded contents as a std::vector
+      *
+      * If using this function, instead pass a std::vector to the
+      * contructor of DER_Encoder where the output will be placed. This
+      * avoids several unecessary copies.
+      */
+      std::vector<uint8_t> BOTAN_DEPRECATED("Use DER_Encoder(vector) instead") get_contents_unlocked();
 
       DER_Encoder& start_cons(ASN1_Tag type_tag,
                               ASN1_Tag class_tag = UNIVERSAL);
diff --git a/src/lib/prov/pkcs11/p11_ecdh.cpp b/src/lib/prov/pkcs11/p11_ecdh.cpp
index f6e27e513..32904aef6 100644
--- a/src/lib/prov/pkcs11/p11_ecdh.cpp
+++ b/src/lib/prov/pkcs11/p11_ecdh.cpp
@@ -55,7 +55,7 @@ class PKCS11_ECDH_KA_Operation final : public PK_Ops::Key_Agreement
          std::vector<uint8_t> der_encoded_other_key;
          if(m_key.point_encoding() == PublicPointEncoding::Der)
             {
-            der_encoded_other_key = DER_Encoder().encode(other_key, other_key_len, OCTET_STRING).get_contents_unlocked();
+            DER_Encoder(der_encoded_other_key).encode(other_key, other_key_len, OCTET_STRING);
             m_mechanism.set_ecdh_other_key(der_encoded_other_key.data(), der_encoded_other_key.size());
             }
          else
diff --git a/src/lib/prov/pkcs11/p11_x509.h b/src/lib/prov/pkcs11/p11_x509.h
index ed084e9c1..d3eafbe35 100644
--- a/src/lib/prov/pkcs11/p11_x509.h
+++ b/src/lib/prov/pkcs11/p11_x509.h
@@ -31,6 +31,10 @@ class BOTAN_PUBLIC_API(2,0) X509_CertificateProperties final : public Certificat
       */
       X509_CertificateProperties(const std::vector<uint8_t>& subject, const std::vector<uint8_t>& value);
 
+      X509_CertificateProperties(const X509_Certificate& cert) :
+         X509_CertificateProperties(cert.raw_subject_dn(), cert.BER_encode())
+         {}
+
       /// @param id key identifier for public/private key pair
       inline void set_id(const std::vector<uint8_t>& id)
          {
diff --git a/src/lib/prov/tpm/tpm.cpp b/src/lib/prov/tpm/tpm.cpp
index c77981e55..dec2316b1 100644
--- a/src/lib/prov/tpm/tpm.cpp
+++ b/src/lib/prov/tpm/tpm.cpp
@@ -352,12 +352,13 @@ AlgorithmIdentifier TPM_PrivateKey::algorithm_identifier() const
 
 std::vector<uint8_t> TPM_PrivateKey::public_key_bits() const
    {
-   return DER_Encoder()
+   std::vector<uint8_t> bits;
+   DER_Encoder(bits)
       .start_cons(SEQUENCE)
         .encode(get_n())
         .encode(get_e())
-      .end_cons()
-      .get_contents_unlocked();
+      .end_cons();
+   return bits;
    }
 
 secure_vector<uint8_t> TPM_PrivateKey::private_key_bits() const
diff --git a/src/lib/pubkey/pbes2/pbes2.cpp b/src/lib/pubkey/pbes2/pbes2.cpp
index d68bf184b..66c621644 100644
--- a/src/lib/pubkey/pbes2/pbes2.cpp
+++ b/src/lib/pubkey/pbes2/pbes2.cpp
@@ -239,16 +239,14 @@ pbes2_encrypt_shared(const secure_vector<uint8_t>& key_bits,
    secure_vector<uint8_t> ctext = key_bits;
    enc->finish(ctext);
 
-   std::vector<uint8_t> pbes2_params;
+   std::vector<uint8_t> encoded_iv;
+   DER_Encoder(encoded_iv).encode(iv, OCTET_STRING);
 
+   std::vector<uint8_t> pbes2_params;
    DER_Encoder(pbes2_params)
       .start_cons(SEQUENCE)
       .encode(kdf_algo)
-      .encode(
-         AlgorithmIdentifier(cipher,
-            DER_Encoder().encode(iv, OCTET_STRING).get_contents_unlocked()
-            )
-         )
+      .encode(AlgorithmIdentifier(cipher, encoded_iv))
       .end_cons();
 
    AlgorithmIdentifier id(OID::from_string("PBE-PKCS5v20"), pbes2_params);
@@ -256,7 +254,6 @@ pbes2_encrypt_shared(const secure_vector<uint8_t>& key_bits,
    return std::make_pair(id, unlock(ctext));
    }
 
-
 }
 
 std::pair<AlgorithmIdentifier, std::vector<uint8_t>>
diff --git a/src/lib/x509/pkcs10.cpp b/src/lib/x509/pkcs10.cpp
index 5e40cb4c3..d35e8994a 100644
--- a/src/lib/x509/pkcs10.cpp
+++ b/src/lib/x509/pkcs10.cpp
@@ -80,26 +80,17 @@ PKCS10_Request PKCS10_Request::create(const Private_Key& key,
 
    if(challenge.empty() == false)
       {
-      ASN1_String challenge_str(challenge, DIRECTORY_STRING);
-
-      tbs_req.encode(
-         Attribute("PKCS9.ChallengePassword",
-                   DER_Encoder().encode(challenge_str).get_contents_unlocked()
-            )
-         );
+      std::vector<uint8_t> value;
+      DER_Encoder(value).encode(ASN1_String(challenge, DIRECTORY_STRING));
+      tbs_req.encode(Attribute("PKCS9.ChallengePassword", value));
       }
 
-   tbs_req.encode(
-      Attribute("PKCS9.ExtensionRequest",
-                DER_Encoder()
-                   .start_cons(SEQUENCE)
-                      .encode(extensions)
-                   .end_cons()
-               .get_contents_unlocked()
-         )
-      )
-      .end_explicit()
-      .end_cons();
+   std::vector<uint8_t> extension_req;
+   DER_Encoder(extension_req).start_cons(SEQUENCE).encode(extensions).end_cons();
+   tbs_req.encode(Attribute("PKCS9.ExtensionRequest", extension_req));
+
+   // end the start_explicit above
+   tbs_req.end_explicit().end_cons();
 
    const std::vector<uint8_t> req =
       X509_Object::make_signed(signer.get(), rng, sig_algo,
diff --git a/src/tests/test_hash_id.cpp b/src/tests/test_hash_id.cpp
index c82139bc3..47507914e 100644
--- a/src/tests/test_hash_id.cpp
+++ b/src/tests/test_hash_id.cpp
@@ -55,9 +55,9 @@ class PKCS_HashID_Test final : public Test
                const Botan::AlgorithmIdentifier alg(oid, Botan::AlgorithmIdentifier::USE_NULL_PARAM);
                const std::vector<uint8_t> dummy_hash(hash_len);
 
-               Botan::DER_Encoder der;
+               std::vector<uint8_t> bits;
+               Botan::DER_Encoder der(bits);
                der.start_cons(Botan::SEQUENCE).encode(alg).encode(dummy_hash, Botan::OCTET_STRING).end_cons();
-               const std::vector<uint8_t> bits = der.get_contents_unlocked();
 
                result.test_eq("Dummy hash is expected size", bits.size() - pkcs_id.size(), dummy_hash.size());
 
diff --git a/src/tests/test_pkcs11_high_level.cpp b/src/tests/test_pkcs11_high_level.cpp
index 994dfbbd3..37e56a0a0 100644
--- a/src/tests/test_pkcs11_high_level.cpp
+++ b/src/tests/test_pkcs11_high_level.cpp
@@ -489,27 +489,37 @@ Test::Result test_attribute_container()
    return result;
    }
 
-#if defined(BOTAN_HAS_ASN1)
-Test::Result test_create_destroy_data_object()
+DataObjectProperties make_test_object(const std::string& label)
    {
-   Test::Result result("Object create/delete data object");
-
-   TestSession test_session(true);
-
    std::string value_string("test data");
    secure_vector<uint8_t> value(value_string.begin(), value_string.end());
 
    std::size_t id = 1337;
-   std::string label = "Botan test data object";
    std::string application = "Botan test application";
+
+   std::vector<uint8_t> encoded_id;
+   DER_Encoder(encoded_id).encode(id);
+
    DataObjectProperties data_obj_props;
    data_obj_props.set_application(application);
    data_obj_props.set_label(label);
    data_obj_props.set_value(value);
    data_obj_props.set_token(true);
    data_obj_props.set_modifiable(true);
-   data_obj_props.set_object_id(DER_Encoder().encode(id).get_contents_unlocked());
+   data_obj_props.set_object_id(encoded_id);
+
+   return data_obj_props;
+   }
 
+#if defined(BOTAN_HAS_ASN1)
+Test::Result test_create_destroy_data_object()
+   {
+   Test::Result result("Object create/delete data object");
+
+   TestSession test_session(true);
+
+   const std::string label = "Botan test data object";
+   auto data_obj_props = make_test_object(label);
    Object data_obj(test_session.session(), data_obj_props);
    result.test_success("Data object creation was successful");
 
@@ -526,19 +536,8 @@ Test::Result test_get_set_attribute_values()
    TestSession test_session(true);
 
    // create object
-   std::string value_string("test data");
-   secure_vector<uint8_t> value(value_string.begin(), value_string.end());
-
-   std::size_t id = 1337;
-   std::string label = "Botan test data object";
-   std::string application = "Botan test application";
-   DataObjectProperties data_obj_props;
-   data_obj_props.set_application(application);
-   data_obj_props.set_label(label);
-   data_obj_props.set_value(value);
-   data_obj_props.set_token(true);
-   data_obj_props.set_modifiable(true);
-   data_obj_props.set_object_id(DER_Encoder().encode(id).get_contents_unlocked());
+   const std::string label = "Botan test data object";
+   auto data_obj_props = make_test_object(label);
    Object data_obj(test_session.session(), data_obj_props);
 
    // get attribute
@@ -567,19 +566,8 @@ Test::Result test_object_finder()
    TestSession test_session(true);
 
    // create object
-   std::string value_string("test data");
-   secure_vector<uint8_t> value(value_string.begin(), value_string.end());
-
-   std::size_t id = 1337;
-   std::string label = "Botan test data object";
-   std::string application = "Botan test application";
-   DataObjectProperties data_obj_props;
-   data_obj_props.set_application(application);
-   data_obj_props.set_label(label);
-   data_obj_props.set_value(value);
-   data_obj_props.set_token(true);
-   data_obj_props.set_modifiable(true);
-   data_obj_props.set_object_id(DER_Encoder().encode(id).get_contents_unlocked());
+   const std::string label = "Botan test data object";
+   auto data_obj_props = make_test_object(label);
    Object data_obj(test_session.session(), data_obj_props);
 
    // search created object
@@ -610,19 +598,8 @@ Test::Result test_object_copy()
    TestSession test_session(true);
 
    // create object
-   std::string value_string("test data");
-   secure_vector<uint8_t> value(value_string.begin(), value_string.end());
-
-   std::size_t id = 1337;
-   std::string label = "Botan test data object";
-   std::string application = "Botan test application";
-   DataObjectProperties data_obj_props;
-   data_obj_props.set_application(application);
-   data_obj_props.set_label(label);
-   data_obj_props.set_value(value);
-   data_obj_props.set_token(true);
-   data_obj_props.set_modifiable(true);
-   data_obj_props.set_object_id(DER_Encoder().encode(id).get_contents_unlocked());
+   const std::string label = "Botan test data object";
+   auto data_obj_props = make_test_object(label);
    Object data_obj(test_session.session(), data_obj_props);
 
    // copy created object
@@ -993,6 +970,13 @@ Test::Result test_ecdsa_privkey_export()
    return result;
    }
 
+std::vector<uint8_t> encode_ec_point_in_octet_str(const Botan::PointGFp& point)
+   {
+   std::vector<uint8_t> enc;
+   DER_Encoder(enc).encode(point.encode(PointGFp::UNCOMPRESSED), OCTET_STRING);
+   return enc;
+   }
+
 Test::Result test_ecdsa_pubkey_import()
    {
    Test::Result result("PKCS11 import ECDSA public key");
@@ -1003,9 +987,7 @@ Test::Result test_ecdsa_pubkey_import()
    ECDSA_PrivateKey priv_key(Test::rng(), EC_Group("secp256r1"));
    priv_key.set_parameter_encoding(EC_Group_Encoding::EC_DOMPAR_ENC_OID);
 
-   const std::vector<uint8_t> enc_point = DER_Encoder().encode(
-      priv_key.public_point().encode(PointGFp::UNCOMPRESSED), OCTET_STRING).
-      get_contents_unlocked();
+   const auto enc_point = encode_ec_point_in_octet_str(priv_key.public_point());
 
    // import to card
    EC_PublicKeyImportProperties props(priv_key.DER_domain(), enc_point);
@@ -1034,9 +1016,7 @@ Test::Result test_ecdsa_pubkey_export()
    ECDSA_PrivateKey priv_key(Test::rng(), EC_Group("secp256r1"));
    priv_key.set_parameter_encoding(EC_Group_Encoding::EC_DOMPAR_ENC_OID);
 
-   const std::vector<uint8_t> enc_point = DER_Encoder().encode(
-      priv_key.public_point().encode(PointGFp::UNCOMPRESSED), OCTET_STRING).
-      get_contents_unlocked();
+   const auto enc_point = encode_ec_point_in_octet_str(priv_key.public_point());
 
    // import to card
    EC_PublicKeyImportProperties props(priv_key.DER_domain(), enc_point);
@@ -1270,9 +1250,7 @@ Test::Result test_ecdh_pubkey_import()
    ECDH_PrivateKey priv_key(Test::rng(), EC_Group("secp256r1"));
    priv_key.set_parameter_encoding(EC_Group_Encoding::EC_DOMPAR_ENC_OID);
 
-   const std::vector<uint8_t> enc_point = DER_Encoder().encode(
-      priv_key.public_point().encode(PointGFp::UNCOMPRESSED), OCTET_STRING).
-      get_contents_unlocked();
+   const auto enc_point = encode_ec_point_in_octet_str(priv_key.public_point());
 
    // import to card
    EC_PublicKeyImportProperties props(priv_key.DER_domain(), enc_point);
@@ -1301,9 +1279,7 @@ Test::Result test_ecdh_pubkey_export()
    ECDH_PrivateKey priv_key(Test::rng(), EC_Group("secp256r1"));
    priv_key.set_parameter_encoding(EC_Group_Encoding::EC_DOMPAR_ENC_OID);
 
-   const std::vector<uint8_t> enc_point = DER_Encoder().encode(
-      priv_key.public_point().encode(PointGFp::UNCOMPRESSED), OCTET_STRING).
-      get_contents_unlocked();
+   const auto enc_point = encode_ec_point_in_octet_str(priv_key.public_point());
 
    // import to card
    EC_PublicKeyImportProperties props(priv_key.DER_domain(), enc_point);
@@ -1610,7 +1586,7 @@ Test::Result test_x509_import()
    TestSession test_session(true);
 
    X509_Certificate root(Test::data_file("x509/nist/test01/end.crt"));
-   X509_CertificateProperties props(DER_Encoder().encode(root.subject_dn()).get_contents_unlocked(), root.BER_encode());
+   X509_CertificateProperties props(root);
    props.set_label("Botan PKCS#11 test certificate");
    props.set_private(false);
    props.set_token(true);
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index d180e8ffc..eaae35842 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -1356,7 +1356,9 @@ class String_Extension final : public Botan::Certificate_Extension
 
       std::vector<uint8_t> encode_inner() const override
          {
-         return Botan::DER_Encoder().encode(Botan::ASN1_String(m_contents, Botan::UTF8_STRING)).get_contents_unlocked();
+         std::vector<uint8_t> bits;
+         Botan::DER_Encoder(bits).encode(Botan::ASN1_String(m_contents, Botan::UTF8_STRING));
+         return bits;
          }
 
       void decode_inner(const std::vector<uint8_t>& in) override