Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr

From: Ard Biesheuvel <ardb@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Stephan Mueller <smueller@chronox.de>
Subject: Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
Date: Fri, 29 May 2020 10:20:27 +0200	[thread overview]
Message-ID: <CAMj1kXE43VvEXyYQF=s5HybhF6Wq23wDTrvTfNV9d4fUVZZ8aw@mail.gmail.com> (raw)
In-Reply-To: <20200529080508.GA2880@gondor.apana.org.au>

On Fri, 29 May 2020 at 10:05, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, May 28, 2020 at 10:33:25AM +0200, Ard Biesheuvel wrote:
> >
> > The reason we return output IVs for CBC is because our generic
> > implementation of CTS can wrap any CBC implementation, and relies on
> > this output IV rather than grabbing it from the ciphertext directly
> > (which may be tricky and is best left up to the CBC code)
>
> No that's not the main reason.  The main user of chaining is
> algif_skcipher.
>

But many implementation do not return an output IV at all. The only
mode that requires it (for the selftests to pass) is CBC.

> > So if you are saying that we should never split up algif_skcipher
> > requests into multiple calls into the underlying skcipher, then I
> > agree with you. Even if the generic CTS seems to work more or less as
> > expected by, e.g., the NIST validation tool, this is unspecified
> > behavior, and definitely broken in various other places.
>
> I was merely suggesting that requests to CTS/XTS shouldn't be
> split up.  Doing it for others would be a serious regression.
>

Given that in these cases, doing so will give incorrect results even
if the input size is a whole multiple of the block size, I agree that
adding this restriction will not break anything that is working
consistently at the moment.

But could you elaborate on the serious regression for other cases? Do
you have anything particular in mind?

> However, having looked at this it would seem that the effort
> in marking CTS/XTS is not that different to just adding support
> to hold the last two blocks of data so that CTS/XTS can support
> chaining.
>

For XTS, we would have to carry some metadata around that tells you
whether the initial encryption of the IV has occurred or not. In the
CTS case, you need two swap the last two blocks of ciphertext at the
very end.

So does that mean some kind of init/update/final model for skcipher? I
can see how that could address these issues (init() would encrypt the
IV for XTS, and final() would do the final block handling for CTS).
Just holding two blocks of data does not seem sufficient to me to
handle these issues.