Re: [RFC PATCH 3/7] crypto: hctr2 - Add HCTR2 support

From: Eric Biggers <ebiggers@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Nathan Huckleberry <nhuck@google.com>,
	linux-crypto@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	linux-arm-kernel@lists.infradead.org,
	Paul Crowley <paulcrowley@google.com>,
	Sami Tolvanen <samitolvanen@google.com>
Subject: Re: [RFC PATCH 3/7] crypto: hctr2 - Add HCTR2 support
Date: Wed, 26 Jan 2022 21:36:09 -0800	[thread overview]
Message-ID: <YfIvSXgnUMoPglCt@sol.localdomain> (raw)
In-Reply-To: <YfIrr8MagPw9scLr@gondor.apana.org.au>

On Thu, Jan 27, 2022 at 04:20:47PM +1100, Herbert Xu wrote:
> On Wed, Jan 26, 2022 at 09:08:38PM -0800, Eric Biggers wrote:
> >
> > The optional parameters mentioned in the comment above don't appear to be
> > implemented.  Also, the syntax described is ambiguous.  I think you meant for
> > there to be only one set of square brackets?
> > 
> > xctr(blockcipher_name) should be xctr_name, since it would have to be a driver /
> > implementation name, and those don't necessarily include parentheses like the
> > algorithm names do.
> > 
> > Did you consider not allowing the single block cipher implementation to be
> > overridden?  The single block cipher is a minor part compared to xctr.  This
> > would simplify the "full" syntax slighty, as then it would be
> > "hctr2(xctr_name, polyval_name)" instead of
> > "hctr2(blockcipher_name, xctr_name, polyval_name)".
> > 
> > I suppose it does make sense to take the single block cipher parameter, given
> > that it is used.  But you'll need to make sure to make hctr2_create() enforce
> > that the same block cipher is used in both parameters.
> 
> For the single block cipher parameter, another option is to derive
> it from the xctr_name.  That is, once you have the skcipher for
> xctr_name, you extract its cra_name, assuming that it must be of
> the form xctr(%s) then you just strip away the xctr() and get the
> single block cipher name that way.

That's what I had in mind with "hctr2(xctr_name, polyval_name)".

> 
> The purpose of having the parameter explicitly is so that the
> instantiatied algorithm is automatically torn down when the
> underlying algorithm is replaced with a better version.
> 
> This is in general unnecessary if you're simply using the
> single block cipher to generate setup material.
> 

Well, the single block cipher isn't used just in ->setkey(), but also in
->encrypt() and ->decrypt() (not for the bulk of the data, but for 1 block).
So allowing its implementation to be specified might make sense.

Alternatively the single block cipher could be emulated with xctr which the
template already has, similar to how the cts template only uses cbc, but I think
that would be pretty messy here.

- Eric

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel