[PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM

Jamin Lin posted 15 patches 1 month, 4 weeks ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Kane Chen <kane_chen@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
Posted by Jamin Lin 1 month, 4 weeks ago
Map QCRYPTO_CIPHER_MODE_GCM to GCRY_CIPHER_MODE_GCM and advertise it in
qcrypto_cipher_supports() for 128-bit block ciphers. Add a GCM driver
whose setiv accepts the (typically 96-bit) nonce, whose encrypt/decrypt
do not require block-aligned lengths, and which implements setaad via
gcry_cipher_authenticate() and gettag via gcry_cipher_gettag().

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 crypto/cipher-gcrypt.c.inc | 101 +++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
index 12eb9ddb5a..fce09a3c77 100644
--- a/crypto/cipher-gcrypt.c.inc
+++ b/crypto/cipher-gcrypt.c.inc
@@ -65,6 +65,8 @@ static int qcrypto_cipher_mode_to_gcry_mode(QCryptoCipherMode mode)
         return GCRY_CIPHER_MODE_CBC;
     case QCRYPTO_CIPHER_MODE_CTR:
         return GCRY_CIPHER_MODE_CTR;
+    case QCRYPTO_CIPHER_MODE_GCM:
+        return GCRY_CIPHER_MODE_GCM;
     default:
         return GCRY_CIPHER_MODE_NONE;
     }
@@ -104,6 +106,10 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgo alg,
     case QCRYPTO_CIPHER_MODE_XTS:
     case QCRYPTO_CIPHER_MODE_CTR:
         return true;
+    case QCRYPTO_CIPHER_MODE_GCM:
+        /* GCM requires a 128-bit block cipher. */
+        return gcry_cipher_get_algo_blklen(
+                   qcrypto_cipher_alg_to_gcry_alg(alg)) == 16;
     default:
         return false;
     }
@@ -228,6 +234,99 @@ static const struct QCryptoCipherDriver qcrypto_gcrypt_ctr_driver = {
     .cipher_free = qcrypto_gcrypt_ctx_free,
 };
 
+/*
+ * GCM is an AEAD stream mode: the IV/nonce need not match the block size,
+ * the message length need not be a multiple of the block size, associated
+ * data is fed with gcry_cipher_authenticate() and the authentication tag is
+ * read back with gcry_cipher_gettag().
+ */
+static int qcrypto_gcrypt_gcm_setiv(QCryptoCipher *cipher,
+                                    const uint8_t *iv, size_t niv,
+                                    Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    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));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_gcm_setaad(QCryptoCipher *cipher,
+                                     const uint8_t *aad, size_t len,
+                                     Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    err = gcry_cipher_authenticate(ctx->handle, aad, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot set AAD: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_gcm_encrypt(QCryptoCipher *cipher, const void *in,
+                                      void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    err = gcry_cipher_encrypt(ctx->handle, out, len, in, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot encrypt data: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_gcm_decrypt(QCryptoCipher *cipher, const void *in,
+                                      void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    err = gcry_cipher_decrypt(ctx->handle, out, len, in, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot decrypt data: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_gcm_gettag(QCryptoCipher *cipher,
+                                     uint8_t *tag, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    err = gcry_cipher_gettag(ctx->handle, tag, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot get tag: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static const struct QCryptoCipherDriver qcrypto_gcrypt_gcm_driver = {
+    .cipher_encrypt = qcrypto_gcrypt_gcm_encrypt,
+    .cipher_decrypt = qcrypto_gcrypt_gcm_decrypt,
+    .cipher_setiv = qcrypto_gcrypt_gcm_setiv,
+    .cipher_setaad = qcrypto_gcrypt_gcm_setaad,
+    .cipher_gettag = qcrypto_gcrypt_gcm_gettag,
+    .cipher_free = qcrypto_gcrypt_ctx_free,
+};
+
 static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
                                              QCryptoCipherMode mode,
                                              const uint8_t *key,
@@ -259,6 +358,8 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
 
     if (mode == QCRYPTO_CIPHER_MODE_CTR) {
         drv = &qcrypto_gcrypt_ctr_driver;
+    } else if (mode == QCRYPTO_CIPHER_MODE_GCM) {
+        drv = &qcrypto_gcrypt_gcm_driver;
     } else {
         drv = &qcrypto_gcrypt_driver;
     }
-- 
2.43.0
Re: [PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
Posted by Daniel P. Berrangé 1 month, 4 weeks ago
On Tue, Jul 14, 2026 at 07:29:14AM +0000, Jamin Lin wrote:
> Map QCRYPTO_CIPHER_MODE_GCM to GCRY_CIPHER_MODE_GCM and advertise it in
> qcrypto_cipher_supports() for 128-bit block ciphers. Add a GCM driver
> whose setiv accepts the (typically 96-bit) nonce, whose encrypt/decrypt
> do not require block-aligned lengths, and which implements setaad via
> gcry_cipher_authenticate() and gettag via gcry_cipher_gettag().
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  crypto/cipher-gcrypt.c.inc | 101 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
> index 12eb9ddb5a..fce09a3c77 100644
> --- a/crypto/cipher-gcrypt.c.inc
> +++ b/crypto/cipher-gcrypt.c.inc
> @@ -65,6 +65,8 @@ static int qcrypto_cipher_mode_to_gcry_mode(QCryptoCipherMode mode)
>          return GCRY_CIPHER_MODE_CBC;
>      case QCRYPTO_CIPHER_MODE_CTR:
>          return GCRY_CIPHER_MODE_CTR;
> +    case QCRYPTO_CIPHER_MODE_GCM:
> +        return GCRY_CIPHER_MODE_GCM;
>      default:
>          return GCRY_CIPHER_MODE_NONE;
>      }
> @@ -104,6 +106,10 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgo alg,
>      case QCRYPTO_CIPHER_MODE_XTS:
>      case QCRYPTO_CIPHER_MODE_CTR:
>          return true;
> +    case QCRYPTO_CIPHER_MODE_GCM:
> +        /* GCM requires a 128-bit block cipher. */
> +        return gcry_cipher_get_algo_blklen(
> +                   qcrypto_cipher_alg_to_gcry_alg(alg)) == 16;
>      default:
>          return false;
>      }
> @@ -228,6 +234,99 @@ static const struct QCryptoCipherDriver qcrypto_gcrypt_ctr_driver = {
>      .cipher_free = qcrypto_gcrypt_ctx_free,
>  };
>  
> +/*
> + * GCM is an AEAD stream mode: the IV/nonce need not match the block size,
> + * the message length need not be a multiple of the block size, associated
> + * data is fed with gcry_cipher_authenticate() and the authentication tag is
> + * read back with gcry_cipher_gettag().
> + */
> +static int qcrypto_gcrypt_gcm_setiv(QCryptoCipher *cipher,
> +                                    const uint8_t *iv, size_t niv,
> +                                    Error **errp)
> +{
> +    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
> +    gcry_error_t err;
> +
> +    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));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
>
> +static int qcrypto_gcrypt_gcm_encrypt(QCryptoCipher *cipher, const void *in,
> +                                      void *out, size_t len, Error **errp)
> +{
> +    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
> +    gcry_error_t err;
> +
> +    err = gcry_cipher_encrypt(ctx->handle, out, len, in, len);
> +    if (err != 0) {
> +        error_setg(errp, "Cannot encrypt data: %s", gcry_strerror(err));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int qcrypto_gcrypt_gcm_decrypt(QCryptoCipher *cipher, const void *in,
> +                                      void *out, size_t len, Error **errp)
> +{
> +    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
> +    gcry_error_t err;
> +
> +    err = gcry_cipher_decrypt(ctx->handle, out, len, in, len);
> +    if (err != 0) {
> +        error_setg(errp, "Cannot decrypt data: %s", gcry_strerror(err));
> +        return -1;
> +    }
> +
> +    return 0;
> +}

Is there a reason you can't use the common qcrypto_gcrypt_setiv,
qcrypto_gcrypt_decrypt and qcrypto_gcrypt_encrypt methods ?

> +
> +static int qcrypto_gcrypt_gcm_gettag(QCryptoCipher *cipher,
> +                                     uint8_t *tag, size_t len, Error **errp)
> +{
> +    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
> +    gcry_error_t err;
> +
> +    err = gcry_cipher_gettag(ctx->handle, tag, len);
> +    if (err != 0) {
> +        error_setg(errp, "Cannot get tag: %s", gcry_strerror(err));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static const struct QCryptoCipherDriver qcrypto_gcrypt_gcm_driver = {
> +    .cipher_encrypt = qcrypto_gcrypt_gcm_encrypt,
> +    .cipher_decrypt = qcrypto_gcrypt_gcm_decrypt,
> +    .cipher_setiv = qcrypto_gcrypt_gcm_setiv,
> +    .cipher_setaad = qcrypto_gcrypt_gcm_setaad,
> +    .cipher_gettag = qcrypto_gcrypt_gcm_gettag,
> +    .cipher_free = qcrypto_gcrypt_ctx_free,
> +};
> +
>  static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
>                                               QCryptoCipherMode mode,
>                                               const uint8_t *key,
> @@ -259,6 +358,8 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
>  
>      if (mode == QCRYPTO_CIPHER_MODE_CTR) {
>          drv = &qcrypto_gcrypt_ctr_driver;
> +    } else if (mode == QCRYPTO_CIPHER_MODE_GCM) {
> +        drv = &qcrypto_gcrypt_gcm_driver;
>      } else {
>          drv = &qcrypto_gcrypt_driver;
>      }
> -- 
> 2.43.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
RE: [PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
Posted by Jamin Lin 1 month, 4 weeks ago
Hi Daniel

> Subject: Re: [PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
> 
> On Tue, Jul 14, 2026 at 07:29:14AM +0000, Jamin Lin wrote:
> > Map QCRYPTO_CIPHER_MODE_GCM to GCRY_CIPHER_MODE_GCM and
> advertise it
> > in
> > qcrypto_cipher_supports() for 128-bit block ciphers. Add a GCM driver
> > whose setiv accepts the (typically 96-bit) nonce, whose
> > encrypt/decrypt do not require block-aligned lengths, and which
> > implements setaad via
> > gcry_cipher_authenticate() and gettag via gcry_cipher_gettag().
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  crypto/cipher-gcrypt.c.inc | 101
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> >
> > diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
> > index 12eb9ddb5a..fce09a3c77 100644
> > --- a/crypto/cipher-gcrypt.c.inc
> > +++ b/crypto/cipher-gcrypt.c.inc
> > @@ -65,6 +65,8 @@ static int
> qcrypto_cipher_mode_to_gcry_mode(QCryptoCipherMode mode)
> >          return GCRY_CIPHER_MODE_CBC;
> >      case QCRYPTO_CIPHER_MODE_CTR:
> >          return GCRY_CIPHER_MODE_CTR;
> > +    case QCRYPTO_CIPHER_MODE_GCM:
> > +        return GCRY_CIPHER_MODE_GCM;
> >      default:
> >          return GCRY_CIPHER_MODE_NONE;
> >      }
> > @@ -104,6 +106,10 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgo
> alg,
> >      case QCRYPTO_CIPHER_MODE_XTS:
> >      case QCRYPTO_CIPHER_MODE_CTR:
> >          return true;
> > +    case QCRYPTO_CIPHER_MODE_GCM:
> > +        /* GCM requires a 128-bit block cipher. */
> > +        return gcry_cipher_get_algo_blklen(
> > +                   qcrypto_cipher_alg_to_gcry_alg(alg)) == 16;
> >      default:
> >          return false;
> >      }
> > @@ -228,6 +234,99 @@ static const struct QCryptoCipherDriver
> qcrypto_gcrypt_ctr_driver = {
> >      .cipher_free = qcrypto_gcrypt_ctx_free,  };
> >
> > +/*
> > + * GCM is an AEAD stream mode: the IV/nonce need not match the block
> > +size,
> > + * the message length need not be a multiple of the block size,
> > +associated
> > + * data is fed with gcry_cipher_authenticate() and the authentication
> > +tag is
> > + * read back with gcry_cipher_gettag().
> > + */
> > +static int qcrypto_gcrypt_gcm_setiv(QCryptoCipher *cipher,
> > +                                    const uint8_t *iv, size_t niv,
> > +                                    Error **errp) {
> > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> QCryptoCipherGcrypt, base);
> > +    gcry_error_t err;
> > +
> > +    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));
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> >
> > +static int qcrypto_gcrypt_gcm_encrypt(QCryptoCipher *cipher, const void
> *in,
> > +                                      void *out, size_t len, Error
> > +**errp) {
> > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> QCryptoCipherGcrypt, base);
> > +    gcry_error_t err;
> > +
> > +    err = gcry_cipher_encrypt(ctx->handle, out, len, in, len);
> > +    if (err != 0) {
> > +        error_setg(errp, "Cannot encrypt data: %s", gcry_strerror(err));
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int qcrypto_gcrypt_gcm_decrypt(QCryptoCipher *cipher, const void
> *in,
> > +                                      void *out, size_t len, Error
> > +**errp) {
> > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> QCryptoCipherGcrypt, base);
> > +    gcry_error_t err;
> > +
> > +    err = gcry_cipher_decrypt(ctx->handle, out, len, in, len);
> > +    if (err != 0) {
> > +        error_setg(errp, "Cannot decrypt data: %s", gcry_strerror(err));
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> 
> Is there a reason you can't use the common qcrypto_gcrypt_setiv,
> qcrypto_gcrypt_decrypt and qcrypto_gcrypt_encrypt methods ?
> 

Thanks for the review and suggestion.

The GCM path differs from the common methods in two input checks that GCM legitimately violates:

  - qcrypto_gcrypt_setiv() rejects niv != blocksize, but GCM uses a
    96-bit (12-byte) nonce.
  - qcrypto_gcrypt_encrypt()/decrypt() reject a length that is not a
    multiple of the block size, but GCM must accept arbitrary lengths
    (e.g. the 60-byte NIST test vectors).

So reusing the common methods means relaxing those two checks for GCM,
e.g.:
  /* qcrypto_gcrypt_setiv() */
  if (cipher->mode != QCRYPTO_CIPHER_MODE_GCM && niv != ctx->blocksize) {
      error_setg(errp, "Expected IV size %zu not %zu", ctx->blocksize, niv);
      return -1;
  }
  /* qcrypto_gcrypt_encrypt() / _decrypt() */
  if (cipher->mode != QCRYPTO_CIPHER_MODE_GCM &&
      (len & (ctx->blocksize - 1))) {
      error_setg(errp, "Length %zu must be a multiple of block size %zu",
                 len, ctx->blocksize);
      return -1;
  }

I kept them separate to avoid adding GCM special-cases to the shared
block-cipher path, but I'm happy to switch the GCM driver to the common
qcrypto_gcrypt_{setiv,encrypt,decrypt} (dropping the GCM-specific ones and
keeping only the AEAD setaad/gettag hooks)

If you prefer. I'll make that change in v2 if that's the direction you'd like.

Thanks,
Jamin

> > +static int qcrypto_gcrypt_gcm_gettag(QCryptoCipher *cipher,
> > +                                     uint8_t *tag, size_t len, Error
> > +**errp) {
> > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> QCryptoCipherGcrypt, base);
> > +    gcry_error_t err;
> > +
> > +    err = gcry_cipher_gettag(ctx->handle, tag, len);
> > +    if (err != 0) {
> > +        error_setg(errp, "Cannot get tag: %s", gcry_strerror(err));
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static const struct QCryptoCipherDriver qcrypto_gcrypt_gcm_driver = {
> > +    .cipher_encrypt = qcrypto_gcrypt_gcm_encrypt,
> > +    .cipher_decrypt = qcrypto_gcrypt_gcm_decrypt,
> > +    .cipher_setiv = qcrypto_gcrypt_gcm_setiv,
> > +    .cipher_setaad = qcrypto_gcrypt_gcm_setaad,
> > +    .cipher_gettag = qcrypto_gcrypt_gcm_gettag,
> > +    .cipher_free = qcrypto_gcrypt_ctx_free, };
> > +
> >  static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
> >
> QCryptoCipherMode mode,
> >                                               const uint8_t *key,
> @@
> > -259,6 +358,8 @@ static QCryptoCipher
> > *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
> >
> >      if (mode == QCRYPTO_CIPHER_MODE_CTR) {
> >          drv = &qcrypto_gcrypt_ctr_driver;
> > +    } else if (mode == QCRYPTO_CIPHER_MODE_GCM) {
> > +        drv = &qcrypto_gcrypt_gcm_driver;
> >      } else {
> >          drv = &qcrypto_gcrypt_driver;
> >      }
> > --
> > 2.43.0
> >
> >
> 
> With regards,
> Daniel
> --
> |: https://berrange.com       ~~
> https://hachyderm.io/@berrange :|
> |: https://libvirt.org          ~~          https://entangle-photo.org :|
> |: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
Posted by Daniel P. Berrangé 1 month, 4 weeks ago
On Tue, Jul 14, 2026 at 08:57:24AM +0000, Jamin Lin wrote:
> Hi Daniel
> 
> > Subject: Re: [PATCH v1 09/15] crypto/cipher-gcrypt: Implement AES-GCM
> > 
> > On Tue, Jul 14, 2026 at 07:29:14AM +0000, Jamin Lin wrote:
> > > Map QCRYPTO_CIPHER_MODE_GCM to GCRY_CIPHER_MODE_GCM and
> > advertise it
> > > in
> > > qcrypto_cipher_supports() for 128-bit block ciphers. Add a GCM driver
> > > whose setiv accepts the (typically 96-bit) nonce, whose
> > > encrypt/decrypt do not require block-aligned lengths, and which
> > > implements setaad via
> > > gcry_cipher_authenticate() and gettag via gcry_cipher_gettag().
> > >
> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > > ---
> > >  crypto/cipher-gcrypt.c.inc | 101
> > > +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 101 insertions(+)
> > >
> > > diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
> > > index 12eb9ddb5a..fce09a3c77 100644
> > > --- a/crypto/cipher-gcrypt.c.inc
> > > +++ b/crypto/cipher-gcrypt.c.inc
> > > @@ -65,6 +65,8 @@ static int
> > qcrypto_cipher_mode_to_gcry_mode(QCryptoCipherMode mode)
> > >          return GCRY_CIPHER_MODE_CBC;
> > >      case QCRYPTO_CIPHER_MODE_CTR:
> > >          return GCRY_CIPHER_MODE_CTR;
> > > +    case QCRYPTO_CIPHER_MODE_GCM:
> > > +        return GCRY_CIPHER_MODE_GCM;
> > >      default:
> > >          return GCRY_CIPHER_MODE_NONE;
> > >      }
> > > @@ -104,6 +106,10 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgo
> > alg,
> > >      case QCRYPTO_CIPHER_MODE_XTS:
> > >      case QCRYPTO_CIPHER_MODE_CTR:
> > >          return true;
> > > +    case QCRYPTO_CIPHER_MODE_GCM:
> > > +        /* GCM requires a 128-bit block cipher. */
> > > +        return gcry_cipher_get_algo_blklen(
> > > +                   qcrypto_cipher_alg_to_gcry_alg(alg)) == 16;
> > >      default:
> > >          return false;
> > >      }
> > > @@ -228,6 +234,99 @@ static const struct QCryptoCipherDriver
> > qcrypto_gcrypt_ctr_driver = {
> > >      .cipher_free = qcrypto_gcrypt_ctx_free,  };
> > >
> > > +/*
> > > + * GCM is an AEAD stream mode: the IV/nonce need not match the block
> > > +size,
> > > + * the message length need not be a multiple of the block size,
> > > +associated
> > > + * data is fed with gcry_cipher_authenticate() and the authentication
> > > +tag is
> > > + * read back with gcry_cipher_gettag().
> > > + */
> > > +static int qcrypto_gcrypt_gcm_setiv(QCryptoCipher *cipher,
> > > +                                    const uint8_t *iv, size_t niv,
> > > +                                    Error **errp) {
> > > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> > QCryptoCipherGcrypt, base);
> > > +    gcry_error_t err;
> > > +
> > > +    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));
> > > +        return -1;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > >
> > > +static int qcrypto_gcrypt_gcm_encrypt(QCryptoCipher *cipher, const void
> > *in,
> > > +                                      void *out, size_t len, Error
> > > +**errp) {
> > > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> > QCryptoCipherGcrypt, base);
> > > +    gcry_error_t err;
> > > +
> > > +    err = gcry_cipher_encrypt(ctx->handle, out, len, in, len);
> > > +    if (err != 0) {
> > > +        error_setg(errp, "Cannot encrypt data: %s", gcry_strerror(err));
> > > +        return -1;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +static int qcrypto_gcrypt_gcm_decrypt(QCryptoCipher *cipher, const void
> > *in,
> > > +                                      void *out, size_t len, Error
> > > +**errp) {
> > > +    QCryptoCipherGcrypt *ctx = container_of(cipher,
> > QCryptoCipherGcrypt, base);
> > > +    gcry_error_t err;
> > > +
> > > +    err = gcry_cipher_decrypt(ctx->handle, out, len, in, len);
> > > +    if (err != 0) {
> > > +        error_setg(errp, "Cannot decrypt data: %s", gcry_strerror(err));
> > > +        return -1;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > 
> > Is there a reason you can't use the common qcrypto_gcrypt_setiv,
> > qcrypto_gcrypt_decrypt and qcrypto_gcrypt_encrypt methods ?
> > 
> 
> Thanks for the review and suggestion.
> 
> The GCM path differs from the common methods in two input checks that GCM legitimately violates:
> 
>   - qcrypto_gcrypt_setiv() rejects niv != blocksize, but GCM uses a
>     96-bit (12-byte) nonce.
>   - qcrypto_gcrypt_encrypt()/decrypt() reject a length that is not a
>     multiple of the block size, but GCM must accept arbitrary lengths
>     (e.g. the 60-byte NIST test vectors).
> 
> So reusing the common methods means relaxing those two checks for GCM,
> e.g.:
>   /* qcrypto_gcrypt_setiv() */
>   if (cipher->mode != QCRYPTO_CIPHER_MODE_GCM && niv != ctx->blocksize) {
>       error_setg(errp, "Expected IV size %zu not %zu", ctx->blocksize, niv);
>       return -1;
>   }
>   /* qcrypto_gcrypt_encrypt() / _decrypt() */
>   if (cipher->mode != QCRYPTO_CIPHER_MODE_GCM &&
>       (len & (ctx->blocksize - 1))) {
>       error_setg(errp, "Length %zu must be a multiple of block size %zu",
>                  len, ctx->blocksize);
>       return -1;
>   }
> 
> I kept them separate to avoid adding GCM special-cases to the shared
> block-cipher path, but I'm happy to switch the GCM driver to the common
> qcrypto_gcrypt_{setiv,encrypt,decrypt} (dropping the GCM-specific ones and
> keeping only the AEAD setaad/gettag hooks)
> 
> If you prefer. I'll make that change in v2 if that's the direction you'd like.

No, leave it as it is.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>




With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|