Fix crashes when modes were used unkeyed.

Fix crashes in OCB, GCM and CFB when called without a key being set.

7 files changed, 36 insertions, 26 deletions

diff --git a/src/lib/modes/aead/aead.h b/src/lib/modes/aead/aead.h
index 4d4b60ce1..92957fb24 100644
--- a/src/lib/modes/aead/aead.h
+++ b/src/lib/modes/aead/aead.h
@@ -61,6 +61,13 @@ class BOTAN_PUBLIC_API(2,0) AEAD_Mode : public Cipher_Mode
       virtual void set_associated_data(const uint8_t ad[], size_t ad_len) = 0;
 
       /**
+      * Most AEADs require the key to be set prior to setting the AD
+      * A few allow the AD to be set even before the cipher is keyed.
+      * Such ciphers would return false from this function.
+      */
+      virtual bool associated_data_requires_key() const { return true; }
+
+      /**
       * Set associated data that is not included in the ciphertext but
       * that should be authenticated. Must be called after set_key and
       * before start.
diff --git a/src/lib/modes/aead/ccm/ccm.h b/src/lib/modes/aead/ccm/ccm.h
index b819ac76e..37d3c0c04 100644
--- a/src/lib/modes/aead/ccm/ccm.h
+++ b/src/lib/modes/aead/ccm/ccm.h
@@ -25,6 +25,8 @@ class BOTAN_PUBLIC_API(2,0) CCM_Mode : public AEAD_Mode
 
       void set_associated_data(const uint8_t ad[], size_t ad_len) override;
 
+      bool associated_data_requires_key() const override { return false; }
+
       std::string name() const override;
 
       size_t update_granularity() const override;
diff --git a/src/lib/modes/aead/chacha20poly1305/chacha20poly1305.h b/src/lib/modes/aead/chacha20poly1305/chacha20poly1305.h
index e12cfb115..5f6417333 100644
--- a/src/lib/modes/aead/chacha20poly1305/chacha20poly1305.h
+++ b/src/lib/modes/aead/chacha20poly1305/chacha20poly1305.h
@@ -26,6 +26,8 @@ class BOTAN_PUBLIC_API(2,0) ChaCha20Poly1305_Mode : public AEAD_Mode
    public:
       void set_associated_data(const uint8_t ad[], size_t ad_len) override;
 
+      bool associated_data_requires_key() const override { return false; }
+
       std::string name() const override { return "ChaCha20Poly1305"; }
 
       size_t update_granularity() const override { return 64; }
diff --git a/src/lib/modes/aead/gcm/ghash.cpp b/src/lib/modes/aead/gcm/ghash.cpp
index 4b0a4c5ec..763879ce4 100644
--- a/src/lib/modes/aead/gcm/ghash.cpp
+++ b/src/lib/modes/aead/gcm/ghash.cpp
@@ -49,6 +49,8 @@ void GHASH::gcm_multiply(secure_vector<uint8_t>& x,
                          const uint8_t input[],
                          size_t blocks)
    {
+   verify_key_set(m_HM.size());
+
 #if defined(BOTAN_HAS_GCM_CLMUL)
    if(CPUID::has_clmul())
       {
@@ -248,8 +250,8 @@ secure_vector<uint8_t> GHASH::nonce_hash(const uint8_t nonce[], size_t nonce_len
 
 void GHASH::clear()
    {
-   zeroise(m_H);
-   zeroise(m_HM);
+   zap(m_H);
+   zap(m_HM);
    reset();
    }
 
diff --git a/src/lib/modes/aead/ocb/ocb.cpp b/src/lib/modes/aead/ocb/ocb.cpp
index 317b417b3..35366260d 100644
--- a/src/lib/modes/aead/ocb/ocb.cpp
+++ b/src/lib/modes/aead/ocb/ocb.cpp
@@ -231,7 +231,7 @@ void OCB_Mode::key_schedule(const uint8_t key[], size_t length)
 
 void OCB_Mode::set_associated_data(const uint8_t ad[], size_t ad_len)
    {
-   BOTAN_ASSERT(m_L, "A key was set");
+   verify_key_set(m_L != nullptr);
    m_ad_hash = ocb_hash(*m_L, *m_cipher, ad, ad_len);
    }
 
@@ -331,7 +331,7 @@ void OCB_Mode::start_msg(const uint8_t nonce[], size_t nonce_len)
    if(!valid_nonce_length(nonce_len))
       throw Invalid_IV_Length(name(), nonce_len);
 
-   BOTAN_ASSERT(m_L, "A key was set");
+   verify_key_set(m_L != nullptr);
 
    m_L->init(update_nonce(nonce, nonce_len));
    zeroise(m_checksum);
@@ -340,9 +340,9 @@ void OCB_Mode::start_msg(const uint8_t nonce[], size_t nonce_len)
 
 void OCB_Encryption::encrypt(uint8_t buffer[], size_t blocks)
    {
-   const size_t BS = block_size();
+   verify_key_set(m_L != nullptr);
 
-   BOTAN_ASSERT(m_L, "A key was set");
+   const size_t BS = block_size();
 
    while(blocks)
       {
@@ -370,6 +370,8 @@ size_t OCB_Encryption::process(uint8_t buf[], size_t sz)
 
 void OCB_Encryption::finish(secure_vector<uint8_t>& buffer, size_t offset)
    {
+   verify_key_set(m_L != nullptr);
+
    const size_t BS = block_size();
 
    BOTAN_ASSERT(buffer.size() >= offset, "Offset is sane");
@@ -427,6 +429,8 @@ void OCB_Encryption::finish(secure_vector<uint8_t>& buffer, size_t offset)
 
 void OCB_Decryption::decrypt(uint8_t buffer[], size_t blocks)
    {
+   verify_key_set(m_L != nullptr);
+
    const size_t BS = block_size();
 
    while(blocks)
@@ -455,6 +459,8 @@ size_t OCB_Decryption::process(uint8_t buf[], size_t sz)
 
 void OCB_Decryption::finish(secure_vector<uint8_t>& buffer, size_t offset)
    {
+   verify_key_set(m_L != nullptr);
+
    const size_t BS = block_size();
 
    BOTAN_ASSERT(buffer.size() >= offset, "Offset is sane");
diff --git a/src/lib/modes/cfb/cfb.cpp b/src/lib/modes/cfb/cfb.cpp
index 45f787e7d..1010cf450 100644
--- a/src/lib/modes/cfb/cfb.cpp
+++ b/src/lib/modes/cfb/cfb.cpp
@@ -23,13 +23,14 @@ CFB_Mode::CFB_Mode(BlockCipher* cipher, size_t feedback_bits) :
 void CFB_Mode::clear()
    {
    m_cipher->clear();
+   m_keystream.clear();
    reset();
    }
 
 void CFB_Mode::reset()
    {
    m_state.clear();
-   m_keystream.clear();
+   zeroise(m_keystream);
    }
 
 std::string CFB_Mode::name() const
@@ -73,6 +74,7 @@ bool CFB_Mode::valid_nonce_length(size_t n) const
 void CFB_Mode::key_schedule(const uint8_t key[], size_t length)
    {
    m_cipher->set_key(key, length);
+   m_keystream.resize(m_cipher->block_size());
    }
 
 void CFB_Mode::start_msg(const uint8_t nonce[], size_t nonce_len)
@@ -80,6 +82,8 @@ void CFB_Mode::start_msg(const uint8_t nonce[], size_t nonce_len)
    if(!valid_nonce_length(nonce_len))
       throw Invalid_IV_Length(name(), nonce_len);
 
+   verify_key_set(m_keystream.size());
+
    if(nonce_len == 0)
       {
       if(m_state.empty())
@@ -91,7 +95,6 @@ void CFB_Mode::start_msg(const uint8_t nonce[], size_t nonce_len)
    else
       {
       m_state.assign(nonce, nonce + nonce_len);
-      m_keystream.resize(m_state.size());
       cipher().encrypt(m_state, m_keystream);
       m_keystream_pos = 0;
       }
@@ -113,6 +116,8 @@ void CFB_Mode::shift_register()
 
 size_t CFB_Encryption::process(uint8_t buf[], size_t sz)
    {
+   verify_key_set(m_keystream.size());
+
    const size_t shift = feedback();
 
    size_t left = sz;
@@ -175,6 +180,8 @@ inline void xor_copy(uint8_t buf[], uint8_t key_buf[], size_t len)
 
 size_t CFB_Decryption::process(uint8_t buf[], size_t sz)
    {
+   verify_key_set(m_keystream.size());
+
    const size_t shift = feedback();
 
    size_t left = sz;
diff --git a/src/lib/modes/cipher_mode.h b/src/lib/modes/cipher_mode.h
index f67e737a4..c1cd16fdd 100644
--- a/src/lib/modes/cipher_mode.h
+++ b/src/lib/modes/cipher_mode.h
@@ -9,9 +9,8 @@
 #define BOTAN_CIPHER_MODE_H_
 
 #include <botan/secmem.h>
-#include <botan/key_spec.h>
+#include <botan/sym_algo.h>
 #include <botan/exceptn.h>
-#include <botan/symkey.h>
 #include <string>
 #include <vector>
 
@@ -26,11 +25,9 @@ enum Cipher_Dir : int { ENCRYPTION, DECRYPTION };
 /**
 * Interface for cipher modes
 */
-class BOTAN_PUBLIC_API(2,0) Cipher_Mode
+class BOTAN_PUBLIC_API(2,0) Cipher_Mode : public SymmetricAlgorithm
    {
    public:
-      virtual ~Cipher_Mode() = default;
-
       /**
       * @return list of available providers for this algorithm, empty if not available
       * @param algo_spec algorithm name
@@ -159,14 +156,6 @@ class BOTAN_PUBLIC_API(2,0) Cipher_Mode
       */
       virtual bool valid_nonce_length(size_t nonce_len) const = 0;
 
-      virtual std::string name() const = 0;
-
-      /**
-      * Zeroise all state
-      * See also reset_msg()
-      */
-      virtual void clear() = 0;
-
       /**
       * Resets just the message specific state and allows encrypting again under the existing key
       */
@@ -184,11 +173,6 @@ class BOTAN_PUBLIC_API(2,0) Cipher_Mode
       virtual size_t tag_size() const { return 0; }
 
       /**
-      * @return object describing limits on key size
-      */
-      virtual Key_Length_Specification key_spec() const = 0;
-
-      /**
       * Check whether a given key length is valid for this algorithm.
       * @param length the key length to be checked.
       * @return true if the key length is valid.