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 37e1a19ac59a57980eb2f0267a0bcde23f9ec664..39e31a7f4257ff4086861c4c84b99433266d4973 100644 (file)
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -39,6 +39,7 @@ struct QCryptoCipherBuiltin {
         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 @@ static int qcrypto_cipher_init_aes(QCryptoCipher *cipher,
         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 @@ static int qcrypto_cipher_init_des_rfb(QCryptoCipher *cipher,
     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 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
 {
     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 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
 {
     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 8cfc5625004370121cfb8eec04ef0098a42d0233..c4f811487aaf43bdbbcc68956533915547a477d9 100644 (file)
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -34,6 +34,11 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg)
     }
 }
 
+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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
                                   Error **errp)
 {
     QCryptoCipher *cipher;
-    gcry_cipher_hd_t handle;
+    QCryptoCipherGcrypt *ctx;
     gcry_error_t err;
     int gcryalg, gcrymode;
 
@@ -87,7 +92,9 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
     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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
          * 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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
 
 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 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
                            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 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
                            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 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
                          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 b01cb1c85785806d333ccd9844a215f7d2507388..7449338d3b657029a9775e5aff89484d5a870719 100644 (file)
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -69,7 +69,7 @@ struct QCryptoCipherNettle {
     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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         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 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         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 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
 {
     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 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
 
     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 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
 {
     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 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
 
     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 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
                          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 1b60c343e7015c060dd966c59753a21b89b3545b..f4946a0af0e7528135da6eee6c0d6dd035965790 100644 (file)
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -313,6 +313,53 @@ static void test_cipher_null_iv(void)
     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 @@ int main(int argc, char **argv)
     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();
 }