Skip to content

Commit

Permalink
crypto: add sanity checking of plaintext/ciphertext length
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@berrange
berrange committed Oct 22, 2015
1 parent eb2a770 commit 3a661f1
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 24 deletions.
15 changes: 15 additions & 0 deletions crypto/cipher-builtin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
61 changes: 45 additions & 16 deletions crypto/cipher-gcrypt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,19 @@ 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,
const uint8_t *key, size_t nkey,
Error **errp)
{
QCryptoCipher *cipher;
gcry_cipher_hd_t handle;
QCryptoCipherGcrypt *ctx;
gcry_error_t err;
int gcryalg, gcrymode;

Expand Down Expand Up @@ -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));
Expand All @@ -100,35 +107,39 @@ 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",
gcry_strerror(err));
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;
}


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);
}

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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));
Expand Down
28 changes: 20 additions & 8 deletions crypto/cipher-nettle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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;
Expand Down Expand Up @@ -184,14 +184,20 @@ 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);
break;

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:
Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -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);
Expand Down
50 changes: 50 additions & 0 deletions tests/test-crypto-cipher.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

0 comments on commit 3a661f1

Please sign in to comment.