On 05/20/2016 08:56 AM, Peter Krempa wrote:
> On Thu, May 19, 2016 at 16:29:01 -0400, John Ferlan wrote:
>> 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
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> configure.ac | 1 +
>> src/libvirt_private.syms | 2 +
>> src/util/vircrypto.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++-
>> src/util/vircrypto.h | 20 ++++-
>> tests/vircryptotest.c | 100 +++++++++++++++++++++++-
>> 5 files changed, 312 insertions(+), 3 deletions(-)
>
> [...]
>
>> diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c
>> index 39a479a..b8f5554 100644
>> --- a/src/util/vircrypto.c
>> +++ b/src/util/vircrypto.c
>
> [...]
>
>> @@ -76,3 +85,184 @@ virCryptoHashString(virCryptoHash hash,
>>
>> return 0;
>> }
>> +
>> +
>> +/* virCryptoHaveCipher:
>> + * @algorithm: Specific cipher algorithm desired
>> + *
>> + * Expected to be called prior to virCryptoEncryptData in order
>> + * to determine whether the requested encryption option is available,
>> + * so that "other" alternatives can be taken if the algorithm is
>> + * not available.
>> + *
>> + * Returns true if we can support the encryption.
>> + */
>> +bool
>> +virCryptoHaveCipher(virCryptoCipher algorithm)
>> +{
>> + switch (algorithm) {
>> +
>> + case VIR_CRYPTO_CIPHER_AES256CBC:
>> +#ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
>> + return true;
>> +#else
>> + return false;
>> +#endif
>> +
>> + case VIR_CRYPTO_CIPHER_NONE:
>> + case VIR_CRYPTO_CIPHER_LAST:
>> + break;
>> + };
>> +
>> + return false;
>> +}
>> +
>> +
>> +#ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
>> +/* virCryptoEncryptDataAESgntuls:
>> + *
>> + * Performs the AES gnutls encryption
>> + *
>> + * Same input as virCryptoEncryptData, except the algoritm is replaced
>> + * by the specific gnutls algorithm.
>> + *
>> + * Encrypts the @data buffer using the @enckey and if available the @iv
>> + *
>> + * Returns 0 on success with the ciphertext being filled. It is the
>> + * caller's responsibility to clear and free it. Returns -1 on failure
>> + * w/ error set.
>> + */
>> +static int
>> +virCryptoEncryptDataAESgnutls(gnutls_cipher_algorithm_t gnutls_enc_alg,
>> + uint8_t *enckey,
>> + size_t enckeylen,
>> + uint8_t *iv,
>> + size_t ivlen,
>> + uint8_t *data,
>> + size_t datalen,
>> + uint8_t **ciphertextret,
>> + size_t *ciphertextlenret)
>> +{
>> + int rc;
>> + size_t i;
>> + gnutls_cipher_hd_t handle = NULL;
>> + gnutls_datum_t enc_key;
>> + gnutls_datum_t iv_buf;
>> + uint8_t *ciphertext;
>> + size_t ciphertextlen;
>> +
>> + /* Allocate a padded buffer, copy in the data */
>> + ciphertextlen = VIR_ROUND_UP(datalen, 16);
>> + if (VIR_ALLOC_N(ciphertext, ciphertextlen) < 0)
>> + return -1;
>> + memcpy(ciphertext, data, datalen);
>> +
>> + /* Fill in the padding of the buffer with the size of the padding
>> + * which is required for decryption. */
>> + for (i = datalen; i < ciphertextlen; i++)
>> + ciphertext[i] = ciphertextlen - datalen;
>> +
>> + /* Initialize the gnutls cipher */
>> + enc_key.size = enckeylen;
>> + enc_key.data = enckey;
>> + if (iv) {
>> + iv_buf.size = ivlen;
>> + iv_buf.data = iv;
>> + }
>> + if ((rc = gnutls_cipher_init(&handle, gnutls_enc_alg,
>> + &enc_key, &iv_buf)) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("failed to initialize cipher: '%s'"),
>> + gnutls_strerror(rc));
>> + goto error;
>> + }
>> +
>> + /* Encrypt the data and free the memory for cipher operations */
>> + rc = gnutls_cipher_encrypt(handle, ciphertext, ciphertextlen);
>> + gnutls_cipher_deinit(handle);
memset(&enc_key, 0, sizeof(gnutls_datum_t));
memset(&iv_key, 0, sizeof(gnutls_datum_t));
>> + if (rc < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("failed to encrypt the data: '%s'"),
>> + gnutls_strerror(rc));
>> + goto error;
>> + }
>> +
>> + *ciphertextret = ciphertext;
>> + *ciphertextlenret = ciphertextlen;
>> + return 0;
>> +
>> + error:
>> + VIR_DISPOSE_N(ciphertext, ciphertextlen);
>> + return -1;
>
> In both cases you should clear the stack'd copy of the encryption key.
>
>> +}
>> +#endif
>> +
>> +
>> +/* virCryptoEncryptData:
>> + * @algorithm: algoritm desired for encryption
>> + * @enckey: encryption key
>> + * @enckeylen: encription key length
>> + * @iv: initialization vector
>> + * @ivlen: length of initialization vector
>> + * @data: data to encrypt
>> + * @datalen: length of data
>> + * @ciphertext: stream of bytes allocated to store ciphertext
>> + * @ciphertextlen: size of the stream of bytes
>> + *
>> + * If available, attempt and return the requested encryption type
>> + * using the parameters passed.
>> + *
>> + * Returns 0 on success, -1 on failure with error set
>> + */
>> +int
>> +virCryptoEncryptData(virCryptoCipher algorithm,
>> + uint8_t *enckey,
>> + size_t enckeylen,
>> + uint8_t *iv,
>> + size_t ivlen,
>> + uint8_t *data,
>> + size_t datalen,
>> + uint8_t **ciphertext,
>> + size_t *ciphertextlen)
>> +{
>> + switch (algorithm) {
>> + case VIR_CRYPTO_CIPHER_AES256CBC:
>> + if (enckeylen != 32) {
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("AES256CBC encryption invalid keylen=%zu"),
>> + enckeylen);
>> + return -1;
>> + }
>> +
>> + if (ivlen != 16) {
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("AES256CBC initialization vector invalid len=%zu"),
>> + ivlen);
>> + return -1;
>> + }
>> +
>> +#ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
>> + /*
>> + * Encrypt the data buffer using an encryption key and
>> + * initialization vector via the gnutls_cipher_encrypt API
>> + * for GNUTLS_CIPHER_AES_256_CBC.
>> + */
>> + return virCryptoEncryptDataAESgnutls(GNUTLS_CIPHER_AES_256_CBC,
>> + enckey, enckeylen, iv, ivlen,
>> + data, datalen,
>> + ciphertext, ciphertextlen);
>> +#else
>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> + _("no encryption algorithm exists"));
>
> That is quite a strong message. I'd say that the chosen algorithm is not
> supported.
>
OK - I'll replace with a break; to fall into message below
>> + return -1;
>> +#endif
>> +
>> + case VIR_CRYPTO_CIPHER_NONE:
>> + case VIR_CRYPTO_CIPHER_LAST:
>> + break;
>> + }
>> +
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("algorithm=%d is not supported"), algorithm);
>
> e.g. as you do here.
>
> [...]
>
>> --- a/src/util/vircrypto.h
>> +++ b/src/util/vircrypto.h
>
> [...]
>
>> @@ -37,4 +45,14 @@ virCryptoHashString(virCryptoHash hash,
>> ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>> ATTRIBUTE_RETURN_CHECK;
>>
>> +bool virCryptoHaveCipher(virCryptoCipher algorithm);
>> +
>> +int virCryptoEncryptData(virCryptoCipher algorithm,
>> + uint8_t *enckey, size_t enckeylen,
>> + uint8_t *iv, size_t ivlen,
>> + uint8_t *data, size_t datalen,
>> + uint8_t **ciphertext, size_t *ciphertextlen)
>> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(6)
>> + ATTRIBUTE_NONNULL(8) ATTRIBUTE_RETURN_CHECK;
>
> ciphertext and ciphertextlen must be nonnull too.
>
OK - ciphertext is (8)... added (9) for ciphertextlen
>> +
>> #endif /* __VIR_CRYPTO_H__ */
>> diff --git a/tests/vircryptotest.c b/tests/vircryptotest.c
>> index bfc47db..1d719d9 100644
>> --- a/tests/vircryptotest.c
>> +++ b/tests/vircryptotest.c
>
> [...]
>
>> @@ -56,10 +58,82 @@ testCryptoHash(const void *opaque)
>> }
>>
>>
>> +struct testCryptoEncryptData {
>> + virCryptoCipher algorithm;
>> + uint8_t *input;
>> + size_t inputlen;
>> + uint8_t *ciphertext;
>> + size_t ciphertextlen;
>> +};
>> +
>> +static int
>> +testCryptoEncrypt(const void *opaque)
>> +{
>> + const struct testCryptoEncryptData *data = opaque;
>> + size_t i;
>> + uint8_t *enckey = NULL;
>> + size_t enckeylen = 32;
>> + uint8_t *iv = NULL;
>> + size_t ivlen = 16;
>> + uint8_t *ciphertext = NULL;
>> + size_t ciphertextlen = 0;
>> + int ret = -1;
>> +
>> + if (!virCryptoHaveCipher(data->algorithm)) {
>> + fprintf(stderr, "Invalid cipher algorithm=%d\n", data->algorithm);
Would adding "return EXIT_AM_SKIP;" be acceptable?
>> + return -1;
>> + }
>> +
>> + if (VIR_ALLOC_N(enckey, enckeylen) < 0 ||
>> + VIR_ALLOC_N(iv, ivlen) < 0)
>> + goto cleanup;
>> +
>> + if (virRandomBytes(enckey, enckeylen) < 0 ||
>> + virRandomBytes(iv, ivlen) < 0)
>> + goto cleanup;
>> +
>> + if (virCryptoEncryptData(data->algorithm, enckey, enckeylen, iv, ivlen,
>> + data->input, data->inputlen,
>> + &ciphertext, &ciphertextlen) < 0)
>> + goto cleanup;
>> +
>> + if (data->ciphertextlen != ciphertextlen) {
>> + fprintf(stderr, "Expected ciphertextlen(%zu) doesn't match (%zu)\n",
>> + data->ciphertextlen, ciphertextlen);
>> + goto cleanup;
>> + }
>> +
>> + if (memcmp(data->ciphertext, ciphertext, ciphertextlen)) {
>> + fprintf(stderr, "Expected ciphertext doesn't match\n");
>> + for (i = 0; i < ciphertextlen; i++) {
>> + if (data->ciphertext[i] != ciphertext[i]) {
>> + fprintf(stderr, "expected[%zu]=%x ciphertext[%zu]=%x\n",
>> + i, data->ciphertext[i],
>> + i, ciphertext[i]);
>
> Last time I was pointing out that it doesn't make much sense to output
> the difference whether encoded to base64 or not.
>
> [...]
>
I used it mainly for "testing", but OK I'll remove it.
>> @@ -84,7 +158,31 @@ mymain(void)
>> VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_MD5, "The quick brown fox", "a2004f37730b9445670a738fa0fc9ee5");
>> VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_SHA256, "The quick brown fox", "5cac4f980fedc3d3f1f99b4be3472c9b30d56523e632d151237ec9309048bda9");
>
>
>>
>> +#undef VIR_CRYPTO_HASH
>> +
>> +#define VIR_CRYPTO_ENCRYPT(a, n, i, il, c, cl) \
>> + do { \
>> + struct testCryptoEncryptData data = { \
>> + .algorithm = a, \
>> + .input = i, \
>> + .inputlen = il, \
>> + .ciphertext = c, \
>> + .ciphertextlen = cl, \
>> + }; \
>> + if (virtTestRun("Encrypt " n, testCryptoEncrypt, &data) < 0) \
>> + ret = -1; \
>> + } while (0)
>> +
>> + memset(&secretdata, 0, 8);
>> + memcpy(&secretdata, "letmein", 7);
>> +
>> + VIR_CRYPTO_ENCRYPT(VIR_CRYPTO_CIPHER_AES256CBC, "aes265cbc",
>> + secretdata, 7, expected_ciphertext, 16);
>> +
>
> This test will fail on builds without gnutls.
<sigh>
Thanks
John
>
> ACK with the nits fixed.
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list