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>
Move the logic from qemuDomainGenerateRandomKey into this new
function, altering the comments, variable names, and error messages
to keep things more generic.
NB: Although perhaps more reasonable to add soemthing to virrandom.c.
The virrandom.c was included in the setuid_rpc_client, so I chose
placement in vircrypto.
Introduce virCryptoHaveCipher and virCryptoEncryptData to handle
performing encryption.
virCryptoHaveCipher:
Boolean function to determine whether the requested cipher algorithm
is available. It's expected this API will be called prior to
virCryptoEncryptdata. It will return true/false.
virCryptoEncryptData:
Based on the requested cipher type, call the specific encryption
API to encrypt the data.
Currently the only algorithm support is the AES 256 CBC encryption.
Adjust tests for the API's
While running vircryptotest, it was found that valgrind pointed out the
following error:
==27453== Invalid write of size 1
==27453== at 0x4C7D7C9: virCryptoHashString (vircrypto.c:76)
==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41)
==27453== by 0x402A11: virtTestRun (testutils.c:199)
==27453== by 0x401AD5: mymain (vircryptotest.c:76)
==27453== by 0x40318D: virtTestMain (testutils.c:782)
==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27453== Address 0x51f0541 is 0 bytes after a block of size 65 alloc'd
==27453== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27453== by 0x4C69F2E: virAllocN (viralloc.c:189)
==27453== by 0x4C7D76B: virCryptoHashString (vircrypto.c:69)
==27453== by 0x401C4E: testCryptoHash (vircryptotest.c:41)
==27453== by 0x402A11: virtTestRun (testutils.c:199)
==27453== by 0x401AD5: mymain (vircryptotest.c:76)
==27453== by 0x40318D: virtTestMain (testutils.c:782)
==27453== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27453==
...and many more. Two observations: hashstrlen was already set
to include the trailing NUL byte (so writing to hashstrlen as
the array offset was indeed writing one byte beyond bounds), and
VIR_ALLOC_N already guarantees zero-initialization (so we already
have a trailing NUL without needing to explicitly write one).
Signed-off-by: Eric Blake <eblake@redhat.com>
GNULIB provides APIs for calculating md5 and sha256 hashes,
but these APIs only return you raw byte arrays. Most users
in libvirt want the hash in printable string format. Add
some helper APIs in util/vircrypto.{c,h} for doing this.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
