Repository: randombit/botan
Pull request #2104 (2019-09-13 09:51:37)

Add a variant of RandomNumberGenerator::random_vec
This avoids the unlock(rng.random_vec(...)) pattern which is pretty wasteful in terms of heap overhead.


Patch:
diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index 0d5bb34cd..8361243dc 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -1644,7 +1644,7 @@ class Speed final : public Command
             // Generate a new random ciphertext to decrypt
             if(ciphertext.empty() || enc_timer->under(msec))
                {
-               plaintext = unlock(rng().random_vec(enc.maximum_input_size()));
+               rng().random_vec(plaintext, enc.maximum_input_size());
                ciphertext = enc_timer->run([&]() { return enc.encrypt(plaintext, rng()); });
                }
 
@@ -1790,7 +1790,7 @@ class Speed final : public Command
                Length here is kind of arbitrary, but 48 bytes fits into a single
                hash block so minimizes hashing overhead versus the PK op itself.
                */
-               message = unlock(rng().random_vec(48));
+               rng().random_vec(message, 48);
 
                signature = sig_timer->run([&]() { return sig.sign_message(message, rng()); });
 
diff --git a/src/lib/base/symkey.cpp b/src/lib/base/symkey.cpp
index 4b853d995..1e1781c67 100644
--- a/src/lib/base/symkey.cpp
+++ b/src/lib/base/symkey.cpp
@@ -18,7 +18,7 @@ namespace Botan {
 OctetString::OctetString(RandomNumberGenerator& rng,
                          size_t len)
    {
-   m_data = rng.random_vec(len);
+   rng.random_vec(m_data, len);
    }
 
 /*
diff --git a/src/lib/passhash/bcrypt/bcrypt.cpp b/src/lib/passhash/bcrypt/bcrypt.cpp
index 29bcc9d1b..1d28ddfb4 100644
--- a/src/lib/passhash/bcrypt/bcrypt.cpp
+++ b/src/lib/passhash/bcrypt/bcrypt.cpp
@@ -146,7 +146,10 @@ std::string generate_bcrypt(const std::string& pass,
 
    if(version != 'a' && version != 'b' && version != 'y')
       throw Invalid_Argument("Unknown bcrypt version '" + std::string(1, version) + "'");
-   return make_bcrypt(pass, unlock(rng.random_vec(16)), work_factor, version);
+
+   std::vector<uint8_t> salt;
+   rng.random_vec(salt, 16);
+   return make_bcrypt(pass, salt, work_factor, version);
    }
 
 bool check_bcrypt(const std::string& pass, const std::string& hash)
diff --git a/src/lib/pubkey/keypair/keypair.cpp b/src/lib/pubkey/keypair/keypair.cpp
index e8d88e99f..d5cd00172 100644
--- a/src/lib/pubkey/keypair/keypair.cpp
+++ b/src/lib/pubkey/keypair/keypair.cpp
@@ -31,8 +31,8 @@ bool encryption_consistency_check(RandomNumberGenerator& rng,
    if(encryptor.maximum_input_size() == 0)
       return true;
 
-   std::vector<uint8_t> plaintext =
-      unlock(rng.random_vec(encryptor.maximum_input_size() - 1));
+   std::vector<uint8_t> plaintext;
+   rng.random_vec(plaintext, encryptor.maximum_input_size() - 1);
 
    std::vector<uint8_t> ciphertext = encryptor.encrypt(plaintext, rng);
    if(ciphertext == plaintext)
diff --git a/src/lib/rng/rng.h b/src/lib/rng/rng.h
index db42c8cf7..d61a35f2a 100644
--- a/src/lib/rng/rng.h
+++ b/src/lib/rng/rng.h
@@ -140,11 +140,18 @@ class BOTAN_PUBLIC_API(2,0) RandomNumberGenerator
       */
       secure_vector<uint8_t> random_vec(size_t bytes)
          {
-         secure_vector<uint8_t> output(bytes);
-         this->randomize(output.data(), output.size());
+         secure_vector<uint8_t> output;
+         random_vec(output, bytes);
          return output;
          }
 
+      template<typename Alloc>
+         void random_vec(std::vector<uint8_t, Alloc>& v, size_t bytes)
+         {
+         v.resize(bytes);
+         this->randomize(v.data(), v.size());
+         }
+
       /**
       * Return a random byte
       * @return random byte
diff --git a/src/lib/tls/msg_client_kex.cpp b/src/lib/tls/msg_client_kex.cpp
index 9cb92447b..39266962b 100644
--- a/src/lib/tls/msg_client_kex.cpp
+++ b/src/lib/tls/msg_client_kex.cpp
@@ -212,7 +212,7 @@ Client_Key_Exchange::Client_Key_Exchange(Handshake_IO& io,
          {
          const Protocol_Version offered_version = state.client_hello()->version();
 
-         m_pre_master = rng.random_vec(48);
+         rng.random_vec(m_pre_master, 48);
          m_pre_master[0] = offered_version.major_version();
          m_pre_master[1] = offered_version.minor_version();
 
@@ -381,15 +381,15 @@ Client_Key_Exchange::Client_Key_Exchange(const std::vector<uint8_t>& contents,
             {
             throw TLS_Exception(Alert::ILLEGAL_PARAMETER, e.what());
             }
-         catch(std::exception &)
+         catch(std::exception&)
             {
             /*
-            * Something failed in the DH computation. To avoid possible
-            * timing attacks, randomize the pre-master output and carry
-            * on, allowing the protocol to fail later in the finished
-            * checks.
+            * Something failed in the DH/ECDH computation. To avoid possible
+            * attacks which are based on triggering and detecting some edge
+            * failure condition, randomize the pre-master output and carry on,
+            * allowing the protocol to fail later in the finished checks.
             */
-            m_pre_master = rng.random_vec(ka_key->public_value().size());
+            rng.random_vec(m_pre_master, ka_key->public_value().size());
             }
 
          reader.assert_done();
diff --git a/src/lib/tls/sessions_sql/tls_session_manager_sql.cpp b/src/lib/tls/sessions_sql/tls_session_manager_sql.cpp
index 1959db266..09cfbc4e9 100644
--- a/src/lib/tls/sessions_sql/tls_session_manager_sql.cpp
+++ b/src/lib/tls/sessions_sql/tls_session_manager_sql.cpp
@@ -80,7 +80,8 @@ Session_Manager_SQL::Session_Manager_SQL(std::shared_ptr<SQL_Database> db,
 
       // new database case
 
-      std::vector<uint8_t> salt = unlock(rng.random_vec(16));
+      std::vector<uint8_t> salt;
+      rng.random_vec(salt, 16);
       size_t iterations = 0;
 
       secure_vector<uint8_t> x = pbkdf->pbkdf_timed(32 + 2,
diff --git a/src/lib/tls/tls_session_manager_memory.cpp b/src/lib/tls/tls_session_manager_memory.cpp
index 5768723d2..600eb440a 100644
--- a/src/lib/tls/tls_session_manager_memory.cpp
+++ b/src/lib/tls/tls_session_manager_memory.cpp
@@ -101,7 +101,7 @@ size_t Session_Manager_In_Memory::remove_all()
    const size_t removed = m_sessions.size();
    m_info_sessions.clear();
    m_sessions.clear();
-   m_session_key = m_rng.random_vec(32);
+   m_rng.random_vec(m_session_key, 32);
    return removed;
    }
 


Test patch:
diff --git a/src/tests/test_package_transform.cpp b/src/tests/test_package_transform.cpp
index f09251e1d..d72c3bbcb 100644
--- a/src/tests/test_package_transform.cpp
+++ b/src/tests/test_package_transform.cpp
@@ -30,7 +30,8 @@ class Package_Transform_Tests final : public Test
 
          for(size_t input_len = 2; input_len != 256; ++input_len)
             {
-            std::vector<uint8_t> input = unlock(Test::rng().random_vec(input_len));
+            std::vector<uint8_t> input;
+            Test::rng().random_vec(input, input_len);
             std::vector<uint8_t> output(input.size() + cipher->block_size());
 
             // aont_package owns/deletes the passed cipher object, kind of a bogus API
diff --git a/src/tests/test_rng.h b/src/tests/test_rng.h
index 01fe89c92..080603a23 100644
--- a/src/tests/test_rng.h
+++ b/src/tests/test_rng.h
@@ -69,6 +69,13 @@ class Fixed_Output_RNG : public Botan::RandomNumberGenerator
          m_buf.insert(m_buf.end(), in.begin(), in.end());
          }
 
+      Fixed_Output_RNG(RandomNumberGenerator& rng, size_t len)
+         {
+         std::vector<uint8_t> output;
+         rng.random_vec(output, len);
+         m_buf.insert(m_buf.end(), output.begin(), output.end());
+         }
+
       Fixed_Output_RNG() = default;
    protected:
       uint8_t random()
diff --git a/src/tests/test_rsa.cpp b/src/tests/test_rsa.cpp
index 3ac5d4733..725e3c313 100644
--- a/src/tests/test_rsa.cpp
+++ b/src/tests/test_rsa.cpp
@@ -284,7 +284,7 @@ class RSA_Blinding_Tests final : public Test
          */
          const size_t rng_bytes = rsa.get_n().bytes() + (2*8*BOTAN_BLINDING_REINIT_INTERVAL);
 
-         Botan_Tests::Fixed_Output_RNG fixed_rng(Botan::unlock(Test::rng().random_vec(rng_bytes)));
+         Botan_Tests::Fixed_Output_RNG fixed_rng(Test::rng(), rng_bytes);
          Botan::PK_Decryptor_EME decryptor(rsa, fixed_rng, "Raw", "base");
 
          for(size_t i = 1; i <= BOTAN_BLINDING_REINIT_INTERVAL ; ++i)
diff --git a/src/tests/test_srp6.cpp b/src/tests/test_srp6.cpp
index a25d857a2..8c84709cd 100644
--- a/src/tests/test_srp6.cpp
+++ b/src/tests/test_srp6.cpp
@@ -103,7 +103,8 @@ class SRP6_RT_Tests final : public Test
          const std::string group_id = "modp/srp/1024";
          const std::string hash_id = "SHA-256";
 
-         const std::vector<uint8_t> salt = unlock(Test::rng().random_vec(16));
+         std::vector<uint8_t> salt;
+         Test::rng().random_vec(salt, 16);
 
          const Botan::BigInt verifier = Botan::generate_srp6_verifier(username, password, salt, group_id, hash_id);