Don't assume output size in symmetric key encrypt/decrypt functions (#3592)

include/ccf/crypto/symmetric_key.h

@@ -77,7 +77,7 @@ namespace crypto
       std::span<const uint8_t> iv,
       std::span<const uint8_t> plain,
       std::span<const uint8_t> aad,
-      uint8_t* cipher,
+      std::vector<uint8_t>& cipher,
       uint8_t tag[GCM_SIZE_TAG]) const = 0;
 
     // AES-GCM decryption
@@ -86,7 +86,7 @@ namespace crypto
       const uint8_t tag[GCM_SIZE_TAG],
       std::span<const uint8_t> cipher,
       std::span<const uint8_t> aad,
-      uint8_t* plain) const = 0;
+      std::vector<uint8_t>& plain) const = 0;
 
     // Key size in bits
     virtual size_t key_size() const = 0;

src/crypto/openssl/symmetric_key.cpp

@@ -51,10 +51,10 @@ namespace crypto
     std::span<const uint8_t> iv,
     std::span<const uint8_t> plain,
     std::span<const uint8_t> aad,
-    uint8_t* cipher,
+    std::vector<uint8_t>& cipher,
     uint8_t tag[GCM_SIZE_TAG]) const
   {
-    std::vector<uint8_t> cb(plain.size() + GCM_SIZE_TAG);
+    std::vector<uint8_t> cb(plain.size());
     int len = 0;
     Unique_EVP_CIPHER_CTX ctx;
     CHECK1(EVP_EncryptInit_ex(ctx, evp_cipher, NULL, key.data(), NULL));
@@ -68,7 +68,9 @@ namespace crypto
       EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, GCM_SIZE_TAG, &tag[0]));
 
     if (!plain.empty())
-      memcpy(cipher, cb.data(), plain.size());
+    {
+      cipher = std::move(cb);
+    }
   }
 
   bool KeyAesGcm_OpenSSL::decrypt(
@@ -76,9 +78,9 @@ namespace crypto
     const uint8_t tag[GCM_SIZE_TAG],
     std::span<const uint8_t> cipher,
     std::span<const uint8_t> aad,
-    uint8_t* plain) const
+    std::vector<uint8_t>& plain) const
   {
-    std::vector<uint8_t> pb(cipher.size() + GCM_SIZE_TAG);
+    std::vector<uint8_t> pb(cipher.size());
 
     int len = 0;
     Unique_EVP_CIPHER_CTX ctx;
@@ -95,7 +97,9 @@ namespace crypto
     int r = EVP_DecryptFinal_ex(ctx, pb.data() + len, &len) > 0;
 
     if (r == 1 && !cipher.empty())
-      memcpy(plain, pb.data(), cipher.size());
+    {
+      plain = std::move(pb);
+    }
 
     return r == 1;
   }

src/crypto/openssl/symmetric_key.h

@@ -31,7 +31,7 @@ namespace crypto
       std::span<const uint8_t> iv,
       std::span<const uint8_t> plain,
       std::span<const uint8_t> aad,
-      uint8_t* cipher,
+      std::vector<uint8_t>& cipher,
       uint8_t tag[GCM_SIZE_TAG]) const override;
 
     virtual bool decrypt(
@@ -39,7 +39,7 @@ namespace crypto
       const uint8_t tag[GCM_SIZE_TAG],
       std::span<const uint8_t> cipher,
       std::span<const uint8_t> aad,
-      uint8_t* plain) const override;
+      std::vector<uint8_t>& plain) const override;
 
     // @brief RFC 5649 AES key wrap with padding (CKM_AES_KEY_WRAP_PAD)
     // @param plain Plaintext key to wrap

src/crypto/symmetric_key.cpp

@@ -107,10 +107,10 @@ namespace crypto
   {
     check_supported_aes_key_size(key.size() * 8);
 
-    std::vector<uint8_t> r(plaintext.size());
+    std::vector<uint8_t> r;
     std::vector<uint8_t> tag(GCM_SIZE_TAG);
     auto k = make_key_aes_gcm(key);
-    k->encrypt(iv, plaintext, aad, r.data(), tag.data());
+    k->encrypt(iv, plaintext, aad, r, tag.data());
     r.insert(r.end(), tag.begin(), tag.end());
     return r;
   }
@@ -127,14 +127,14 @@ namespace crypto
       throw std::runtime_error("Not enough ciphertext");
 
     size_t ciphertext_length = ciphertext.size() - GCM_SIZE_TAG;
-    std::vector<uint8_t> r(ciphertext_length);
+    std::vector<uint8_t> r;
     auto k = make_key_aes_gcm(key);
     k->decrypt(
       iv,
       ciphertext.data() + ciphertext_length,
       std::span<const uint8_t>(ciphertext.data(), ciphertext_length),
       aad,
-      r.data());
+      r);
     return r;
   }
 }

src/crypto/test/crypto.cpp

@@ -435,10 +435,10 @@ static const vector<uint8_t>& get_raw_key()
 TEST_CASE("ExtendedIv0")
 {
   auto k = crypto::make_key_aes_gcm(get_raw_key());
+
   // setup plain text
-  unsigned char rawP[100];
-  memset(rawP, 'x', sizeof(rawP));
-  const std::span<const uint8_t> p{rawP, sizeof(rawP)};
+  std::vector<uint8_t> plain(100);
+  std::iota(plain.begin(), plain.end(), 0);
 
   // test large IV
   using LargeIVGcmHeader = FixedSizeGcmHeader<1234>;
@@ -451,10 +451,13 @@ TEST_CASE("ExtendedIv0")
     h.set_random_iv();
   }
 
-  k->encrypt(h.get_iv(), p, {}, rawP, h.tag);
+  std::vector<uint8_t> cipher;
+  k->encrypt(h.get_iv(), plain, {}, cipher, h.tag);
 
   auto k2 = crypto::make_key_aes_gcm(get_raw_key());
-  REQUIRE(k2->decrypt(h.get_iv(), h.tag, p, {}, rawP));
+  std::vector<uint8_t> decrypted_plain;
+  REQUIRE(k2->decrypt(h.get_iv(), h.tag, cipher, {}, decrypted_plain));
+  REQUIRE(plain == decrypted_plain);
 }
 
 TEST_CASE("AES Key wrap with padding")

src/indexing/enclave_lfs_access.h

@@ -38,9 +38,8 @@ namespace ccf::indexing
     plaintext = gcm.cipher;
     auto success = true;
 #else
-    plaintext.resize(gcm.cipher.size());
     auto success = encryption_key.decrypt(
-      gcm.hdr.get_iv(), gcm.hdr.tag, gcm.cipher, {}, plaintext.data());
+      gcm.hdr.get_iv(), gcm.hdr.tag, gcm.cipher, {}, plaintext);
 #endif
 
     // Check key prefix in plaintext
@@ -110,7 +109,7 @@ namespace ccf::indexing
       gcm.hdr.set_random_iv();
 
       encryption_key->encrypt(
-        gcm.hdr.get_iv(), contents, {}, gcm.cipher.data(), gcm.hdr.tag);
+        gcm.hdr.get_iv(), contents, {}, gcm.cipher, gcm.hdr.tag);
 
 #ifdef PLAINTEXT_CACHE
       gcm.cipher = contents;

src/kv/encryptor.h

@@ -75,7 +75,6 @@ namespace kv
       EntryType entry_type = EntryType::WriteSet) override
     {
       S hdr;
-      cipher.resize(plain.size());
 
       set_iv(hdr, tx_id, entry_type);
 
@@ -85,8 +84,7 @@ namespace kv
         return false;
       }
 
-      key->encrypt(
-        hdr.get_iv(), plain, additional_data, cipher.data(), hdr.tag);
+      key->encrypt(hdr.get_iv(), plain, additional_data, cipher, hdr.tag);
 
       serialised_header = hdr.serialise();
 
@@ -121,7 +119,6 @@ namespace kv
       S hdr;
       hdr.deserialise(serialised_header);
       term = hdr.get_term();
-      plain.resize(cipher.size());
 
       auto key =
         ledger_secrets->get_encryption_key_for(version, historical_hint);
@@ -130,8 +127,8 @@ namespace kv
         return false;
       }
 
-      auto ret = key->decrypt(
-        hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data());
+      auto ret =
+        key->decrypt(hdr.get_iv(), hdr.tag, cipher, additional_data, plain);
       if (!ret)
       {
         plain.resize(0);

src/node/channels.h

@@ -188,11 +188,11 @@ namespace ccf
     std::array<ChannelSeqno, threading::ThreadMessaging::max_num_threads>
       local_recv_nonce = {{}};
 
-    bool verify_or_decrypt(
+    bool decrypt(
       const GcmHdr& header,
       std::span<const uint8_t> aad,
-      std::span<const uint8_t> cipher = {},
-      std::span<uint8_t> plain = {})
+      std::span<const uint8_t> cipher,
+      std::vector<uint8_t>& plain)
     {
       status.expect(ESTABLISHED);
 
@@ -216,7 +216,7 @@ namespace ccf
       }
 
       CHANNEL_RECV_TRACE(
-        "verify_or_decrypt({} bytes, {} bytes) (nonce={})",
+        "decrypt({} bytes, {} bytes) (nonce={})",
         aad.size(),
         cipher.size(),
         (size_t)recv_nonce.nonce);
@@ -238,8 +238,8 @@ namespace ccf
         return false;
       }
 
-      auto ret = recv_key->decrypt(
-        header.get_iv(), header.tag, cipher, aad, plain.data());
+      auto ret =
+        recv_key->decrypt(header.get_iv(), header.tag, cipher, aad, plain);
       if (ret)
       {
         // Set local recv nonce to received nonce only if verification is
@@ -262,6 +262,12 @@ namespace ccf
       return ret;
     }
 
+    bool verify(const GcmHdr& header, std::span<const uint8_t> aad)
+    {
+      std::vector<uint8_t> empty_plaintext;
+      return decrypt(header, aad, {}, empty_plaintext);
+    }
+
     void send_key_exchange_init()
     {
       std::vector<uint8_t> payload;
@@ -866,10 +872,9 @@ namespace ccf
       const auto nonce_n = nonce.get_val();
       gcm_hdr.set_iv((const uint8_t*)&nonce_n, sizeof(nonce_n));
 
-      std::vector<uint8_t> cipher(plain.size());
+      std::vector<uint8_t> cipher;
       assert(send_key);
-      send_key->encrypt(
-        gcm_hdr.get_iv(), plain, aad, cipher.data(), gcm_hdr.tag);
+      send_key->encrypt(gcm_hdr.get_iv(), plain, aad, cipher, gcm_hdr.tag);
 
       const auto gcm_hdr_serialised = gcm_hdr.serialise();
 
@@ -908,7 +913,7 @@ namespace ccf
       GcmHdr hdr;
       hdr.deserialise(data, size);
 
-      if (!verify_or_decrypt(hdr, aad))
+      if (!verify(hdr, aad))
       {
         CHANNEL_RECV_FAIL("Failed to verify node");
         return false;
@@ -942,7 +947,7 @@ namespace ccf
       hdr.deserialise(data_, size_);
       size -= hdr.serialised_size();
 
-      if (!verify_or_decrypt(hdr, std::span<const uint8_t>(data, size)))
+      if (!verify(hdr, std::span<const uint8_t>(data, size)))
       {
         CHANNEL_RECV_FAIL("Failed to verify node message with payload");
         return false;
@@ -969,9 +974,8 @@ namespace ccf
       GcmHdr hdr;
       hdr.deserialise(data, size);
 
-      std::vector<uint8_t> plain(size);
-      if (!verify_or_decrypt(
-            hdr, aad, std::span<const uint8_t>(data, size), plain))
+      std::vector<uint8_t> plain;
+      if (!decrypt(hdr, aad, std::span<const uint8_t>(data, size), plain))
       {
         CHANNEL_RECV_FAIL("Failed to decrypt node message");
         return std::nullopt;

src/node/ledger_secret.h

@@ -85,14 +85,14 @@ namespace ccf
   {
     crypto::GcmCipher encrypted_ls;
     encrypted_ls.deserialise(encrypted_previous_secret_raw);
-    std::vector<uint8_t> decrypted_ls_raw(encrypted_ls.cipher.size());
+    std::vector<uint8_t> decrypted_ls_raw;
 
     if (!ledger_secret->key->decrypt(
           encrypted_ls.hdr.get_iv(),
           encrypted_ls.hdr.tag,
           encrypted_ls.cipher,
           {},
-          decrypted_ls_raw.data()))
+          decrypted_ls_raw))
     {
       throw std::logic_error("Decryption of previous ledger secret failed");
     }

src/node/share_manager.h

@@ -64,7 +64,7 @@ namespace ccf
                                    // key is never re-used for encryption
         ledger_secret->raw_key,
         {},
-        encrypted_ls.cipher.data(),
+        encrypted_ls.cipher,
         encrypted_ls.hdr.tag);
 
       has_wrapped = true;
@@ -77,14 +77,14 @@ namespace ccf
     {
       crypto::GcmCipher encrypted_ls;
       encrypted_ls.deserialise(wrapped_latest_ledger_secret);
-      std::vector<uint8_t> decrypted_ls(encrypted_ls.cipher.size());
+      std::vector<uint8_t> decrypted_ls;
 
       if (!crypto::make_key_aes_gcm(data)->decrypt(
             encrypted_ls.hdr.get_iv(),
             encrypted_ls.hdr.tag,
             encrypted_ls.cipher,
             {},
-            decrypted_ls.data()))
+            decrypted_ls))
       {
         throw std::logic_error("Unwrapping latest ledger secret failed");
       }
@@ -201,7 +201,7 @@ namespace ccf
           encrypted_previous_ls.hdr.get_iv(),
           previous_ledger_secret->second->raw_key,
           {},
-          encrypted_previous_ls.cipher.data(),
+          encrypted_previous_ls.cipher,
           encrypted_previous_ls.hdr.tag);
 
         encrypted_previous_secret = encrypted_previous_ls.serialise();
@@ -231,7 +231,7 @@ namespace ccf
         encrypted_submitted_share.hdr.get_iv(),
         submitted_share,
         {},
-        encrypted_submitted_share.cipher.data(),
+        encrypted_submitted_share.cipher,
         encrypted_submitted_share.hdr.tag);
 
       return encrypted_submitted_share.serialise();
@@ -243,14 +243,14 @@ namespace ccf
     {
       crypto::GcmCipher encrypted_share;
       encrypted_share.deserialise(encrypted_submitted_share);
-      std::vector<uint8_t> decrypted_share(encrypted_share.cipher.size());
+      std::vector<uint8_t> decrypted_share;
 
       current_ledger_secret->key->decrypt(
         encrypted_share.hdr.get_iv(),
         encrypted_share.hdr.tag,
         encrypted_share.cipher,
         {},
-        decrypted_share.data());
+        decrypted_share);
 
       return decrypted_share;
     }