security: fix AES IV length check in transform::BlockCipher
AES in CBC mode requires the IV length to be equal to the
block size (which is always 128 bits), not to the key size.
Change-Id: I7b8479f84317ddd2cf758271cc50c1af6c936780
diff --git a/src/security/transform/block-cipher.cpp b/src/security/transform/block-cipher.cpp
index b1bb091..678a161 100644
--- a/src/security/transform/block-cipher.cpp
+++ b/src/security/transform/block-cipher.cpp
@@ -31,7 +31,7 @@
class BlockCipher::Impl
{
public:
- Impl()
+ Impl() noexcept
: m_cipher(BIO_new(BIO_f_cipher()))
, m_sink(BIO_new(BIO_s_mem()))
{
@@ -80,7 +80,7 @@
int wLen = BIO_write(m_impl->m_cipher, data, dataLen);
- if (wLen <= 0) { // fail to write data
+ if (wLen <= 0) { // failed to write data
if (!BIO_should_retry(m_impl->m_cipher)) {
// we haven't written everything but some error happens, and we cannot retry
BOOST_THROW_EXCEPTION(Error(getIndex(), "Failed to accept more input"));
@@ -89,7 +89,7 @@
}
else { // update number of bytes written
fillOutputBuffer();
- return wLen;
+ return static_cast<size_t>(wLen);
}
}
@@ -110,35 +110,30 @@
void
BlockCipher::fillOutputBuffer()
{
- int nRead = BIO_pending(m_impl->m_sink);
- if (nRead <= 0)
+ int nPending = BIO_pending(m_impl->m_sink);
+ if (nPending <= 0)
return;
// there is something to read from BIO
- auto buffer = make_unique<OBuffer>(nRead);
- int rLen = BIO_read(m_impl->m_sink, buffer->data(), nRead);
- if (rLen < 0)
+ auto buffer = make_unique<OBuffer>(nPending);
+ int nRead = BIO_read(m_impl->m_sink, buffer->data(), nPending);
+ if (nRead < 0)
return;
- if (rLen < nRead)
- buffer->erase(buffer->begin() + rLen, buffer->end());
+ buffer->erase(buffer->begin() + nRead, buffer->end());
setOutputBuffer(std::move(buffer));
}
bool
BlockCipher::isConverterEmpty() const
{
- return (BIO_pending(m_impl->m_sink) <= 0);
+ return BIO_pending(m_impl->m_sink) <= 0;
}
void
BlockCipher::initializeAesCbc(const uint8_t* key, size_t keyLen,
- const uint8_t* iv, size_t ivLen,
- CipherOperator op)
+ const uint8_t* iv, size_t ivLen, CipherOperator op)
{
- if (keyLen != ivLen)
- BOOST_THROW_EXCEPTION(Error(getIndex(), "Key length must be the same as IV length"));
-
const EVP_CIPHER* cipherType = nullptr;
switch (keyLen) {
case 16:
@@ -151,8 +146,13 @@
cipherType = EVP_aes_256_cbc();
break;
default:
- BOOST_THROW_EXCEPTION(Error(getIndex(), "Key length is not supported"));
+ BOOST_THROW_EXCEPTION(Error(getIndex(), "Unsupported key length " + to_string(keyLen)));
}
+
+ size_t requiredIvLen = static_cast<size_t>(EVP_CIPHER_iv_length(cipherType));
+ if (ivLen != requiredIvLen)
+ BOOST_THROW_EXCEPTION(Error(getIndex(), "IV length must be " + to_string(requiredIvLen)));
+
BIO_set_cipher(m_impl->m_cipher, cipherType, key, iv, static_cast<int>(op));
}
diff --git a/src/security/transform/block-cipher.hpp b/src/security/transform/block-cipher.hpp
index ee56682..feb7827 100644
--- a/src/security/transform/block-cipher.hpp
+++ b/src/security/transform/block-cipher.hpp
@@ -32,7 +32,7 @@
/**
* @brief The module to encrypt data using block cipher.
*
- * The padding scheme of the block cipher is set to the default padding scheme of OpenSSl,
+ * The padding scheme of the block cipher is set to the OpenSSL default,
* which is PKCS padding.
*/
class BlockCipher : public Transform
@@ -41,12 +41,12 @@
/**
* @brief Create a block cipher
*
- * @param algo The block cipher algorithm (e.g., EncryptMode::AES_CBC).
- * @param op The operation that the cipher needs to perform, e.g., CipherOperator::ENCRYPT or CipherOperator::DECRYPT
- * @param key The pointer to the key.
- * @param keyLen The size of the key.
- * @param iv The pointer to the initial vector.
- * @param ivLen The length of the initial vector.
+ * @param algo The block cipher algorithm to use.
+ * @param op Whether to encrypt or decrypt.
+ * @param key Pointer to the key.
+ * @param keyLen Size of the key.
+ * @param iv Pointer to the initialization vector.
+ * @param ivLen Length of the initialization vector.
*/
BlockCipher(BlockCipherAlgorithm algo, CipherOperator op,
const uint8_t* key, size_t keyLen,
@@ -90,8 +90,7 @@
private:
void
initializeAesCbc(const uint8_t* key, size_t keyLen,
- const uint8_t* iv, size_t ivLen,
- CipherOperator op);
+ const uint8_t* iv, size_t ivLen, CipherOperator op);
private:
class Impl;
diff --git a/tests/unit-tests/security/transform/block-cipher.t.cpp b/tests/unit-tests/security/transform/block-cipher.t.cpp
index 6a6b722..0803b44 100644
--- a/tests/unit-tests/security/transform/block-cipher.t.cpp
+++ b/tests/unit-tests/security/transform/block-cipher.t.cpp
@@ -88,7 +88,12 @@
// invalid key length
const uint8_t badKey[] = {0x00, 0x01, 0x02, 0x03};
BOOST_CHECK_THROW(BlockCipher(BlockCipherAlgorithm::AES_CBC, CipherOperator::ENCRYPT,
- badKey, sizeof(badKey), badKey, sizeof(badKey)), Error);
+ badKey, sizeof(badKey), iv, sizeof(iv)), Error);
+
+ // wrong iv length
+ const uint8_t badIv[] = {0x00, 0x01, 0x02, 0x03};
+ BOOST_CHECK_THROW(BlockCipher(BlockCipherAlgorithm::AES_CBC, CipherOperator::ENCRYPT,
+ key, sizeof(key), badIv, sizeof(badIv)), Error);
}
BOOST_AUTO_TEST_CASE(InvalidAlgorithm)