Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221007152105.3057788-1-ardb@kernel.org> <3e395ad2-48f6-87f6-9042-a3ca21c0baba@amd.com>
In-Reply-To: <3e395ad2-48f6-87f6-9042-a3ca21c0baba@amd.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Mon, 10 Oct 2022 10:15:51 +0200
Message-ID: <CAMj1kXGNXYvS9_-RwzzO-VeZN8+ZErhxGiCbfbBA5thJ84RxYA@mail.gmail.com>
Subject: Re: [PATCH] crypto: gcm - Provide minimal library implementation
To:     "Nikunj A. Dadhania" <nikunj@amd.com>
Cc:     linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au,
        ebiggers@kernel.org, keescook@chromium.org, jason@zx2c4.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 10 Oct 2022 at 09:27, Nikunj A. Dadhania <nikunj@amd.com> wrote:
>
> Hi Ard,
>
> On 07/10/22 20:51, Ard Biesheuvel wrote:
> > Implement a minimal library version of GCM based on the existing library
> > implementations of AES and multiplication in GF(2^128). Using these
> > primitives, GCM can be implemented in a straight-forward manner.
> >
> > GCM has a couple of sharp edges, i.e., the amount of input data
> > processed with the same initialization vector (IV) should be capped to
> > protect the counter from 32-bit rollover (or carry), and the size of the
> > authentication tag should be fixed for a given key. [0]
> >
> > The former concern is addressed trivially, given that the function call
> > API uses 32-bit signed types for the input lengths. It is still up to
> > the caller to avoid IV reuse in general, but this is not something we
> > can police at the implementation level.
> >
> > As for the latter concern, let's make the authentication tag size part
> > of the key schedule, and only permit it to be configured as part of the
> > key expansion routine.
> >
> > Note that table based AES implementations are susceptible to known
> > plaintext timing attacks on the encryption key. The AES library already
> > attempts to mitigate this to some extent, but given that the counter
> > mode encryption used by GCM operates exclusively on known plaintext by
> > construction (the IV and therefore the initial counter value are known
> > to an attacker), let's take some extra care to mitigate this, by calling
> > the AES library with interrupts disabled.
> >
> > [0] https://nvlpubs.nist.gov/nistpubs/legacy/sp/nistspecialpublication800-38d.pdf
> >
> > Cc: "Nikunj A. Dadhania" <nikunj@amd.com>
> > Link: https://lore.kernel.org/all/c6fb9b25-a4b6-2e4a-2dd1-63adda055a49@amd.com/
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > +/**
> > + * gcm_decrypt - Perform GCM decryption on a block of data
> > + * @ctx:     The GCM key schedule
> > + * @dst:     Pointer to the plaintext output buffer
> > + * @src:     Pointer the ciphertext (may equal @dst for decryption in place)
> > + * @crypt_len:       The size in bytes of the plaintext and ciphertext.
> > + * @assoc:   Pointer to the associated data,
> > + * @assoc_len:       The size in bytes of the associated data
> > + * @iv:              The initialization vector (IV) to use for this block of data
> > + *           (must be 12 bytes in size as per the GCM spec recommendation)
> > + * @authtag: The address of the buffer in memory where the authentication
> > + *           tag is stored.
> > + *
> > + * Returns 0 on success, or -EBADMSG if the ciphertext failed authentication.
> > + * On failure, no plaintext will be returned.
> > + */
> > +int __must_check gcm_decrypt(const struct gcm_ctx *ctx, u8 *dst, const u8 *src,
> > +                          int crypt_len, const u8 *assoc, int assoc_len,
> > +                          const u8 iv[GCM_AES_IV_SIZE], const u8 *authtag)
> > +{
> > +     u8 tagbuf[AES_BLOCK_SIZE];
> > +     __be32 ctr[4];
> > +
> > +     memcpy(ctr, iv, GCM_AES_IV_SIZE);
> > +
> > +     gcm_mac(ctx, src, crypt_len, assoc, assoc_len, ctr, tagbuf);
> > +     if (crypto_memneq(authtag, tagbuf, ctx->authsize)) {
> > +             memzero_explicit(tagbuf, sizeof(tagbuf));
> > +             return -EBADMSG;
> > +     }
>
> The gcm_mac computation seems to be broken in this version. When I receive the encrypted
> packet back from the security processor the authtag does not match. Will debug further
> and report back.
>

Sorry to hear that. If you find out what's wrong, can you please
provide a test vector that reproduces it so we can add it to the list?