On Thu, Nov 13, 2025 at 07:02:20PM +0530, Arun Menon wrote:
> Adds `virCryptoDecryptDataAESgnutls` and `virCryptoDecryptData`
> as wrapper functions for GnuTLS decryption.
>
> These functions are the inverse of the existing GnuTLS encryption wrappers.
> This commit also includes a corresponding test case to validate data
> decryption.
>
> Signed-off-by: Arun Menon <[email protected]>
> ---
> src/libvirt_private.syms | 1 +
> src/util/vircrypto.c | 130 ++++++++++++++++++++++++++++++++++++++-
> src/util/vircrypto.h | 8 +++
> tests/vircryptotest.c | 65 ++++++++++++++++++++
> 4 files changed, 202 insertions(+), 2 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fb482fff40..fc5fdb00f4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2252,6 +2252,7 @@ virConfWriteMem;
>
>
> # util/vircrypto.h
> +virCryptoDecryptData;
> virCryptoEncryptData;
> virCryptoHashBuf;
> virCryptoHashString;
> diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c
> index 3ce23264ca..e0d2b794a1 100644
> --- a/src/util/vircrypto.c
> +++ b/src/util/vircrypto.c
> @@ -98,7 +98,7 @@ virCryptoHashString(virCryptoHash hash,
> }
>
>
> -/* virCryptoEncryptDataAESgntuls:
> +/* virCryptoEncryptDataAESgnutls:
> *
> * Performs the AES gnutls encryption
> *
> @@ -200,7 +200,7 @@ virCryptoEncryptData(virCryptoCipher algorithm,
> {
> switch (algorithm) {
> case VIR_CRYPTO_CIPHER_AES256CBC:
> - if (enckeylen != 32) {
> + if (enckeylen < 32) {
Why change this ? For AES-256 the key *must* be exactly
32 bytes, no more, no less.
> virReportError(VIR_ERR_INVALID_ARG,
> _("AES256CBC encryption invalid keylen=%1$zu"),
> enckeylen);
> @@ -233,3 +233,129 @@ virCryptoEncryptData(virCryptoCipher algorithm,
> _("algorithm=%1$d is not supported"), algorithm);
> return -1;
> }
> +
> +/* virCryptoDecryptDataAESgnutls:
> + *
> + * Performs the AES gnutls decryption
> + *
> + * Same input as virCryptoDecryptData, except the algorithm is replaced
> + * by the specific gnutls algorithm.
> + *
> + * Decrypts the @data buffer using the @deckey and if available the @iv
> + *
> + * Returns 0 on success with the plaintext being filled. It is the
> + * caller's responsibility to clear and free it. Returns -1 on failure
> + * w/ error set.
> + */
> +static int
> +virCryptoDecryptDataAESgnutls(gnutls_cipher_algorithm_t gnutls_dec_alg,
> + uint8_t *deckey,
> + size_t deckeylen,
> + uint8_t *iv,
> + size_t ivlen,
> + uint8_t *data,
> + size_t datalen,
> + uint8_t **plaintextret,
> + size_t *plaintextlenret)
> +{
> + int rc;
> + size_t i;
> + gnutls_cipher_hd_t handle = NULL;
> + gnutls_datum_t dec_key = { .data = deckey, .size = deckeylen };
> + gnutls_datum_t iv_buf = { .data = iv, .size = ivlen };
> + g_autofree uint8_t *plaintext = NULL;
> + size_t plaintextlen;
> +
> + if ((rc = gnutls_cipher_init(&handle, gnutls_dec_alg,
> + &dec_key, &iv_buf)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to initialize cipher: '%1$s'"),
> + gnutls_strerror(rc));
> + return -1;
> + }
> +
> + plaintext = g_memdup2(data, datalen);
> + plaintextlen = datalen;
> +
> + rc = gnutls_cipher_decrypt(handle, plaintext, plaintextlen);
> + gnutls_cipher_deinit(handle);
> + if (rc < 0) {
> + virSecureErase(plaintext, plaintextlen);
Instead of repeating virSecureErase many times over, create
an 'error:' label and jump to that after the virReportError
call.
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to decrypt the data: '%1$s'"),
> + gnutls_strerror(rc));
> + return -1;
> + }
> + if (plaintextlen == 0) {
> + virSecureErase(plaintext, plaintextlen);
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("decrypted data has zero length"));
> + return -1;
> + }
> + i = plaintext[plaintextlen - 1];
> + if (i > plaintextlen) {
Is that check right ? I thought that padding would be in the
range 0 -> key length. IOW, 0 through 31 inclusive for AES256 ?
> + virSecureErase(plaintext, plaintextlen);
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("decrypted data has invalid padding"));
> + return -1;
> + }
> + *plaintextlenret = plaintextlen - i;
> + *plaintextret = g_steal_pointer(&plaintext);
> + return 0;
> +}
> +int
> +virCryptoDecryptData(virCryptoCipher algorithm,
> + uint8_t *deckey,
> + size_t deckeylen,
> + uint8_t *iv,
> + size_t ivlen,
> + uint8_t *data,
> + size_t datalen,
> + uint8_t **plaintext,
> + size_t *plaintextlen)
> +{
> + switch (algorithm) {
> + case VIR_CRYPTO_CIPHER_AES256CBC:
> + if (deckeylen < 32) {
Again, why < instead of != ?
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("AES256CBC decryption invalid keylen=%1$zu"),
> + deckeylen);
> + return -1;
> + }
> + if (ivlen != 16) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("AES256CBC initialization vector invalid
> len=%1$zu"),
> + ivlen);
> + return -1;
> + }
> + /*
> + * Decrypt the data buffer using a decryption key and
> + * initialization vector via the gnutls_cipher_decrypt API
> + * for GNUTLS_CIPHER_AES_256_CBC.
> + */
> + return virCryptoDecryptDataAESgnutls(GNUTLS_CIPHER_AES_256_CBC,
> + deckey, deckeylen, iv, ivlen,
> + data, datalen,
> + plaintext, plaintextlen);
> + case VIR_CRYPTO_CIPHER_NONE:
> + case VIR_CRYPTO_CIPHER_LAST:
> + break;
> + }
> +
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("algorithm=%1$d is not supported"), algorithm);
> + return -1;
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|