Repository: randombit/botan
Pull request #2186 (2019-10-28 10:52:16)

Deprecate DER_Encoder::get_contents_unlocked
It's better to use the version taking the vector in the constructor as otherwise we store to locked memory then copy out at the end.

Convert all library uses.


Patch:
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,


Test patch:
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