crypto: add sanity checking of plaintext/ciphertext length
When encrypting/decrypting data, the plaintext/ciphertext
buffers are required to be a multiple of the cipher block
size. If this is not done, nettle will abort and gcrypt
will report an error. To get consistent behaviour add
explicit checks upfront for the buffer sizes.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index 37e1a19..39e31a7 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -39,6 +39,7 @@
QCryptoCipherBuiltinAES aes;
QCryptoCipherBuiltinDESRFB desrfb;
} state;
+ size_t blocksize;
void (*free)(QCryptoCipher *cipher);
int (*setiv)(QCryptoCipher *cipher,
const uint8_t *iv, size_t niv,
@@ -181,6 +182,7 @@
goto error;
}
+ ctxt->blocksize = AES_BLOCK_SIZE;
ctxt->free = qcrypto_cipher_free_aes;
ctxt->setiv = qcrypto_cipher_setiv_aes;
ctxt->encrypt = qcrypto_cipher_encrypt_aes;
@@ -282,6 +284,7 @@
memcpy(ctxt->state.desrfb.key, key, nkey);
ctxt->state.desrfb.nkey = nkey;
+ ctxt->blocksize = 8;
ctxt->free = qcrypto_cipher_free_des_rfb;
ctxt->setiv = qcrypto_cipher_setiv_des_rfb;
ctxt->encrypt = qcrypto_cipher_encrypt_des_rfb;
@@ -370,6 +373,12 @@
{
QCryptoCipherBuiltin *ctxt = cipher->opaque;
+ if (len % ctxt->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctxt->blocksize);
+ return -1;
+ }
+
return ctxt->encrypt(cipher, in, out, len, errp);
}
@@ -382,6 +391,12 @@
{
QCryptoCipherBuiltin *ctxt = cipher->opaque;
+ if (len % ctxt->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctxt->blocksize);
+ return -1;
+ }
+
return ctxt->decrypt(cipher, in, out, len, errp);
}
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 8cfc562..c4f8114 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -34,6 +34,11 @@
}
}
+typedef struct QCryptoCipherGcrypt QCryptoCipherGcrypt;
+struct QCryptoCipherGcrypt {
+ gcry_cipher_hd_t handle;
+ size_t blocksize;
+};
QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode,
@@ -41,7 +46,7 @@
Error **errp)
{
QCryptoCipher *cipher;
- gcry_cipher_hd_t handle;
+ QCryptoCipherGcrypt *ctx;
gcry_error_t err;
int gcryalg, gcrymode;
@@ -87,7 +92,9 @@
cipher->alg = alg;
cipher->mode = mode;
- err = gcry_cipher_open(&handle, gcryalg, gcrymode, 0);
+ ctx = g_new0(QCryptoCipherGcrypt, 1);
+
+ err = gcry_cipher_open(&ctx->handle, gcryalg, gcrymode, 0);
if (err != 0) {
error_setg(errp, "Cannot initialize cipher: %s",
gcry_strerror(err));
@@ -100,10 +107,12 @@
* bizarre RFB variant of DES :-)
*/
uint8_t *rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
- err = gcry_cipher_setkey(handle, rfbkey, nkey);
+ err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);
g_free(rfbkey);
+ ctx->blocksize = 8;
} else {
- err = gcry_cipher_setkey(handle, key, nkey);
+ err = gcry_cipher_setkey(ctx->handle, key, nkey);
+ ctx->blocksize = 16;
}
if (err != 0) {
error_setg(errp, "Cannot set key: %s",
@@ -111,11 +120,12 @@
goto error;
}
- cipher->opaque = handle;
+ cipher->opaque = ctx;
return cipher;
error:
- gcry_cipher_close(handle);
+ gcry_cipher_close(ctx->handle);
+ g_free(ctx);
g_free(cipher);
return NULL;
}
@@ -123,12 +133,13 @@
void qcrypto_cipher_free(QCryptoCipher *cipher)
{
- gcry_cipher_hd_t handle;
+ QCryptoCipherGcrypt *ctx;
if (!cipher) {
return;
}
- handle = cipher->opaque;
- gcry_cipher_close(handle);
+ ctx = cipher->opaque;
+ gcry_cipher_close(ctx->handle);
+ g_free(ctx);
g_free(cipher);
}
@@ -139,10 +150,16 @@
size_t len,
Error **errp)
{
- gcry_cipher_hd_t handle = cipher->opaque;
+ QCryptoCipherGcrypt *ctx = cipher->opaque;
gcry_error_t err;
- err = gcry_cipher_encrypt(handle,
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
+ err = gcry_cipher_encrypt(ctx->handle,
out, len,
in, len);
if (err != 0) {
@@ -161,10 +178,16 @@
size_t len,
Error **errp)
{
- gcry_cipher_hd_t handle = cipher->opaque;
+ QCryptoCipherGcrypt *ctx = cipher->opaque;
gcry_error_t err;
- err = gcry_cipher_decrypt(handle,
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
+ err = gcry_cipher_decrypt(ctx->handle,
out, len,
in, len);
if (err != 0) {
@@ -180,11 +203,17 @@
const uint8_t *iv, size_t niv,
Error **errp)
{
- gcry_cipher_hd_t handle = cipher->opaque;
+ QCryptoCipherGcrypt *ctx = cipher->opaque;
gcry_error_t err;
- gcry_cipher_reset(handle);
- err = gcry_cipher_setiv(handle, iv, niv);
+ if (niv != ctx->blocksize) {
+ error_setg(errp, "Expected IV size %zu not %zu",
+ ctx->blocksize, niv);
+ return -1;
+ }
+
+ gcry_cipher_reset(ctx->handle);
+ err = gcry_cipher_setiv(ctx->handle, iv, niv);
if (err != 0) {
error_setg(errp, "Cannot set IV: %s",
gcry_strerror(err));
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index b01cb1c..7449338 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -69,7 +69,7 @@
nettle_cipher_func *alg_encrypt;
nettle_cipher_func *alg_decrypt;
uint8_t *iv;
- size_t niv;
+ size_t blocksize;
};
bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
@@ -125,7 +125,7 @@
ctx->alg_encrypt = des_encrypt_wrapper;
ctx->alg_decrypt = des_decrypt_wrapper;
- ctx->niv = DES_BLOCK_SIZE;
+ ctx->blocksize = DES_BLOCK_SIZE;
break;
case QCRYPTO_CIPHER_ALG_AES_128:
@@ -140,14 +140,14 @@
ctx->alg_encrypt = aes_encrypt_wrapper;
ctx->alg_decrypt = aes_decrypt_wrapper;
- ctx->niv = AES_BLOCK_SIZE;
+ ctx->blocksize = AES_BLOCK_SIZE;
break;
default:
error_setg(errp, "Unsupported cipher algorithm %d", alg);
goto error;
}
- ctx->iv = g_new0(uint8_t, ctx->niv);
+ ctx->iv = g_new0(uint8_t, ctx->blocksize);
cipher->opaque = ctx;
return cipher;
@@ -184,6 +184,12 @@
{
QCryptoCipherNettle *ctx = cipher->opaque;
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
switch (cipher->mode) {
case QCRYPTO_CIPHER_MODE_ECB:
ctx->alg_encrypt(ctx->ctx_encrypt, len, out, in);
@@ -191,7 +197,7 @@
case QCRYPTO_CIPHER_MODE_CBC:
cbc_encrypt(ctx->ctx_encrypt, ctx->alg_encrypt,
- ctx->niv, ctx->iv,
+ ctx->blocksize, ctx->iv,
len, out, in);
break;
default:
@@ -211,6 +217,12 @@
{
QCryptoCipherNettle *ctx = cipher->opaque;
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
switch (cipher->mode) {
case QCRYPTO_CIPHER_MODE_ECB:
ctx->alg_decrypt(ctx->ctx_decrypt ? ctx->ctx_decrypt : ctx->ctx_encrypt,
@@ -219,7 +231,7 @@
case QCRYPTO_CIPHER_MODE_CBC:
cbc_decrypt(ctx->ctx_decrypt ? ctx->ctx_decrypt : ctx->ctx_encrypt,
- ctx->alg_decrypt, ctx->niv, ctx->iv,
+ ctx->alg_decrypt, ctx->blocksize, ctx->iv,
len, out, in);
break;
default:
@@ -235,9 +247,9 @@
Error **errp)
{
QCryptoCipherNettle *ctx = cipher->opaque;
- if (niv != ctx->niv) {
+ if (niv != ctx->blocksize) {
error_setg(errp, "Expected IV size %zu not %zu",
- ctx->niv, niv);
+ ctx->blocksize, niv);
return -1;
}
memcpy(ctx->iv, iv, niv);
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 1b60c34..f4946a0 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -313,6 +313,53 @@
qcrypto_cipher_free(cipher);
}
+static void test_cipher_short_plaintext(void)
+{
+ Error *err = NULL;
+ QCryptoCipher *cipher;
+ uint8_t key[32] = { 0 };
+ uint8_t plaintext1[20] = { 0 };
+ uint8_t ciphertext1[20] = { 0 };
+ uint8_t plaintext2[40] = { 0 };
+ uint8_t ciphertext2[40] = { 0 };
+ int ret;
+
+ cipher = qcrypto_cipher_new(
+ QCRYPTO_CIPHER_ALG_AES_256,
+ QCRYPTO_CIPHER_MODE_CBC,
+ key, sizeof(key),
+ &error_abort);
+ g_assert(cipher != NULL);
+
+ /* Should report an error as plaintext is shorter
+ * than block size
+ */
+ ret = qcrypto_cipher_encrypt(cipher,
+ plaintext1,
+ ciphertext1,
+ sizeof(plaintext1),
+ &err);
+ g_assert(ret == -1);
+ g_assert(err != NULL);
+
+ error_free(err);
+ err = NULL;
+
+ /* Should report an error as plaintext is larger than
+ * block size, but not a multiple of block size
+ */
+ ret = qcrypto_cipher_encrypt(cipher,
+ plaintext2,
+ ciphertext2,
+ sizeof(plaintext2),
+ &err);
+ g_assert(ret == -1);
+ g_assert(err != NULL);
+
+ error_free(err);
+ qcrypto_cipher_free(cipher);
+}
+
int main(int argc, char **argv)
{
size_t i;
@@ -328,5 +375,8 @@
g_test_add_func("/crypto/cipher/null-iv",
test_cipher_null_iv);
+ g_test_add_func("/crypto/cipher/short-plaintext",
+ test_cipher_short_plaintext);
+
return g_test_run();
}
