RE: Interesting crypto manager behavior

[Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf
> Of Pascal Van Leeuwen
> Sent: Tuesday, September 10, 2019 3:10 PM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-crypto@vger.kernel.org; Eric Biggers <ebiggers@kernel.org>
> Subject: RE: Interesting crypto manager behavior
> 
> 
> 
> > -----Original Message-----
> > From: Herbert Xu <herbert@gondor.apana.org.au>
> > Sent: Tuesday, September 10, 2019 2:59 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: linux-crypto@vger.kernel.org; Eric Biggers <ebiggers@kernel.org>
> > Subject: Re: Interesting crypto manager behavior
> >
> > On Tue, Sep 10, 2019 at 12:50:04PM +0000, Pascal Van Leeuwen wrote:
> > >
> > > I'm allocating a fallback (AEAD) cipher to handle some corner cases
> > > my HW cannot handle, but I noticed that the fallback itself is being
> > > tested when I allocate it (or so it seems) and if the fallback itself
> > > fails on some testvector, it is not replaced by an alternative while
> > > such an alternative should be available. So I have to fail my entire
> > > init because the fallback could not be allocated.
> >
> > This has nothing to do with fallbacks and it's just how template
> > instantiation works.  If the instantiation fails it will not try
> > to construct another one.  The point is that if your algorithm
> > works then it should not fail the instantiation self-test.  And
> > if it does fail then it's a bug in the algorithm, not the API.
> >
> > > i.e. while requesting a fallback for rfc7539(chacha20, poly1305), it
> > > attempts rfc7539(safexcel-chacha20,poly1305-simd), which fails, but
> > > it could still fall back to e.g. rfc7539(chacha20-simd, poly1305-simd),
> > > which should work.
> > >
> > > Actually, I really do not want the fallback to hit another algorithm
> > > of my own driver. Is there some way to prevent that from happening?
> > > (without actually referencing hard cra_driver_name's ...)
> >
> > I think if safexcel-chacha20 causes a failure when used with rfc7539
> > then it should just be fixed, or unexported.
> >
> Actually, the whole situation occurred because I manually added a
> testvector to testmgr.h that even fails with generic, generic :-)
> So no, safexcel-chacha20 does not cause the failure, I was surprised
> by the behavior, expecting it to fallback all the way to generic if
> needed.
> 
> Anyway, this leads me to a follow-up question:
> I'm trying to implement rfc7539esp(chacha20,poly1305) but I cannot
> make sense of what it *should* do.
> From the generic template code, the only difference from the regular
> rfc7539 I could reverse engineer is that the  first 4 bytes of the IV
> move to the end of the key (as "salt").
> However, when I implemented just that, I got mismatches on the appended
> ICV. And, with rfc7539 working just fine and considering the minimal
> differences which are easy to review, I cannot make sense of it.
> 
> So I copied the single rfc7539esp testvector to the regular rfc7539
> vector set and modified it accordingly, i.e. I moved the last 4 bytes
> of the key to the start of IV again.
> But if I do that, the vector even fails on rfc7539esp(generic,generic).
> 
> Upon closer inspection of the vector, I noticed the following:
> It is basically the exact same vector as the last (2nd) regular rfc7539
> vector, with the first 4 bytes of the IV moved to the end of the key
> (OK) BUT also with 8 bytes added to the AAD (which look like it's the IV).
> While the expected ICV is still the same, which is of course impossible
> if you add more AAD data.
> 
> So really, can someone tell me how this rfc7539esp mode is supposed to
> work? And where this is handled in chacha20_poly1305.c as I cannot find
> where it makes that exception ...
> 

Ok, never mind. After confirming that, for ESP, the IV is not actually
authenticated at all, and therefore should be somehow skipped, and after
searching through the code for all instances of "assoclen", I finally
came across the following in the poly_genkey function (~last place you
would for such a thing):

	rctx->assoclen = req->assoclen;

	if (crypto_aead_ivsize(tfm) == 8) {
		if (rctx->assoclen < 8)
			return -EINVAL;
		rctx->assoclen -= 8;
	}

So, as it turns out, for an ivsize of 8, which happens to be the
case for rfc7539esp only, we need to skip the last 8 bytes of AAD.
QED

> > Cheers,
> > --
> > Email: Herbert Xu <herbert@gondor.apana.org.au>
> > Home Page: http://gondor.apana.org.au/~herbert/
> > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> 
> 
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com