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 <[email protected]>
> > ---
> > 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 :|
