linux-kernel - Re: [PATCH v4 10/12] RISC-V: crypto: add Zvkned accelerated AES encryption implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 21 Jul 2023 16:23:38 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Heiko Stuebner <heiko@...ech.de>, palmer@...belt.com,
        paul.walmsley@...ive.com, aou@...s.berkeley.edu,
        herbert@...dor.apana.org.au, davem@...emloft.net,
        conor.dooley@...rochip.com, linux-riscv@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        christoph.muellner@...ll.eu,
        Heiko Stuebner <heiko.stuebner@...ll.eu>
Subject: Re: [PATCH v4 10/12] RISC-V: crypto: add Zvkned accelerated AES
 encryption implementation

On Fri, 21 Jul 2023 at 13:39, Ard Biesheuvel <ardb@...nel.org> wrote:
>
> On Fri, 21 Jul 2023 at 07:40, Eric Biggers <ebiggers@...nel.org> wrote:
> >
> > On Tue, Jul 11, 2023 at 05:37:41PM +0200, Heiko Stuebner wrote:
> ...
> > > +static int riscv64_aes_setkey_zvkned(struct crypto_tfm *tfm, const u8 *key,
> > > +                      unsigned int keylen)
> > > +{
> > > +     struct riscv_aes_ctx *ctx = crypto_tfm_ctx(tfm);
> > > +     int ret;
> > > +
> > > +     ctx->keylen = keylen;
> > > +
> > > +     if (keylen == 16 || keylen == 32) {
> > > +             kernel_rvv_begin();
> > > +             ret = rv64i_zvkned_set_encrypt_key(key, keylen * 8, &ctx->enc_key);
> > > +             if (ret != 1) {
> > > +                     kernel_rvv_end();
> > > +                     return -EINVAL;
> > > +             }
> > > +
> > > +             ret = rv64i_zvkned_set_decrypt_key(key, keylen * 8, &ctx->dec_key);
>
> The asm suggests that the encryption and decryption key schedules are
> the same, and the decryption algorithm does not implement the
> Equivalent Inverse Cipher, but simply iterates over they key schedule
> in reverse order. This makes much more sense for instruction based
> AES, so it doesn't surprise me but it does mean you can just drop this
> part, and pass enc_key everywhere.
>
> > > +             kernel_rvv_end();
> > > +             if (ret != 1)
> > > +                     return -EINVAL;
> > > +     }
> > > +
> > > +     ret = crypto_cipher_setkey(ctx->fallback, key, keylen);
> > > +
> > > +     return ret ? -EINVAL : 0;
> > > +}
> >
> > It's a bit annoying that RISC-V doesn't support AES-192, though also not
> > particularly surprising, seeing as AES-192 is almost never used.  (Intel's Key
> > Locker, for example, is another recent CPU feature that doesn't support
> > AES-192.)  IMO the issue here is really with the kernel crypto API -- it should
> > treat AES-128, AES-192, and AES-256 as separate algorithms so that
> > implementations aren't forced to support all three key sizes...
> >
>
> Why is this a fundamental limitation? AES-192 uses the same AES block
> size and round structure, the only difference is the number of rounds
> and how the round keys are calculated.
>
> Creating the key schedule should never be performance critical, so if
> the lack of AES-192 support is due to a limitation in the key schedule
> generation instructions, I'd suggest to avoid those if possible and
> just use the generic library code to derive the key schedule. If that
> works, I'm pretty sure AES-192 support is just a matter of
> implementing a 12-round variant modeled after the existing 10/14 round
> ones.

This seems to work:
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=riscv-crypto

Feel free to incorporate/squash any of those changes into your series.