From 603a58f73e71323294c0d840bbfef4b5d9676e32 Mon Sep 17 00:00:00 2001

Message-Id: <603a58f73e71323294c0d840bbfef4b5d9676e32@dist-git>

From: "Daniel P. Berrange" <berrange@redhat.com>

Date: Wed, 3 May 2017 08:52:13 +0200

Subject: [PATCH] Fix padding of encrypted data

If we are encoding a block of data that is 16 bytes in length,

we cannot leave it as 16 bytes, we must pad it out to the next

block boundary, 32 bytes. Without this padding, the decoder will

incorrectly treat the last byte of plain text as the padding

length, as it can't distinguish padded from non-padded data.

The problem exhibited itself when using a 16 byte passphrase

for a LUKS volume

  $ virsh secret-set-value 55806c7d-8e93-456f-829b-607d8c198367 \

       $(echo -n 1234567812345678 | base64)

  Secret value set

  $ virsh start demo

  error: Failed to start domain demo

  error: internal error: process exited while connecting to monitor: >>>>>>>>>>Len 16

  2017-05-02T10:35:40.016390Z qemu-system-x86_64: -object \

    secret,id=virtio-disk1-luks-secret0,data=SEtNi5vDUeyseMKHwc1c1Q==,\

    keyid=masterKey0,iv=zm7apUB1A6dPcH53VW960Q==,format=base64: \

    Incorrect number of padding bytes (56) found on decrypted data

Notice how the padding '56' corresponds to the ordinal value of

the character '8'.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

(cherry picked from commit 71890992daf37ec78b00b4ce873369421dc99731)

https://bugzilla.redhat.com/show_bug.cgi?id=1447297

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/util/vircrypto.c | 10 ++++++++--

 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c

index 8748e1c4e..48b04fc8c 100644

--- a/src/util/vircrypto.c

+++ b/src/util/vircrypto.c

@@ -152,8 +152,14 @@ virCryptoEncryptDataAESgnutls(gnutls_cipher_algorithm_t gnutls_enc_alg,

     uint8_t *ciphertext;

     size_t ciphertextlen;

-    /* Allocate a padded buffer, copy in the data */

-    ciphertextlen = VIR_ROUND_UP(datalen, 16);

+    /* Allocate a padded buffer, copy in the data.

+     *

+     * NB, we must *always* have at least 1 byte of

+     * padding - we can't skip it on multiples of

+     * 16, otherwise decoder can't distinguish padded

+     * data from non-padded data. Hence datalen + 1

+     */

+    ciphertextlen = VIR_ROUND_UP(datalen + 1, 16);

     if (VIR_ALLOC_N(ciphertext, ciphertextlen) < 0)

         return -1;

     memcpy(ciphertext, data, datalen);

-- 

2.12.2