Re: [PATCH] crypto: caam - Remove broken arc4 support

From: Ard Biesheuvel <ardb@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "Horia Geantă" <horia.geanta@nxp.com>,
	"Aymen Sghaier" <aymen.sghaier@nxp.com>,
	"Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
	"Iuliana Prodan" <iuliana.prodan@nxp.com>
Subject: Re: [PATCH] crypto: caam - Remove broken arc4 support
Date: Thu, 2 Jul 2020 10:12:24 +0200	[thread overview]
Message-ID: <CAMj1kXEgGOihAm_cYjs2_FaTn2KiSh-g3VroTm6Z=SZdG=Jr7g@mail.gmail.com> (raw)
In-Reply-To: <20200702080045.GA4472@gondor.apana.org.au>

On Thu, 2 Jul 2020 at 09:56, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Thu, Jul 02, 2020 at 09:51:29AM +0200, Ard Biesheuvel wrote:
> >
> > I'll wait for the code to be posted (please put me on cc), but my
>
> Sure I will.
>
> > suspicion is that carrying opaque state like that is going to bite us
> > down the road.
>
> Well it's only going to be arc4 at first, where it's definitely
> an improvement over modifying the tfm context in encrypt/decrypt.
>

I agree that the current approach is flawed, but starting multiple
requests with the same state essentially comes down to IV reuse in a
stream cipher, which will cause it to fail catastrophically.
(ciphertext1 ^ ciphertext2 == plaintext1 ^ plaintext2)

I wonder if we should simply try to get rid of arc4 in the crypto API,
as you suggested. There are a couple of WEP implementations that could
be switched over to the library interface, and the KerberosV
implementation of RC4-HMAC(md5) was added for Windows 2000
compatibility based on RFC 4757 [0], which was deprecated by RFC 8429
[1], since Windows Domain Controllers running Windows Server 2008r2 or
later can use newer algorithms.

[0] https://tools.ietf.org/html/rfc4757
[1] https://tools.ietf.org/html/rfc8429

>
> On Thu, Jul 02, 2020 at 05:56:16PM +1000, Herbert Xu wrote:
> >
> > For XTS I haven't decided whether to go this way or not.  If it
> > does work out though we could even extend it to AEAD.
>
> But there is clearly a need for this functionality, and it's
> not just af_alg.  Have a look at net/sunrpc/auth_gss/gss_krb5_crypto.c,
> it has three versions of the same crypto code (arc4, cts, and
> everything else), in order to deal with continuing requests just
> like algif_skcipher.
>
> Perhaps at the end of this we could distill it down to just one.
>

I agree that there is a gap here.

Perhaps we can decouple ARC4 from the other cases? ARC4 is too quirky
and irrelevant to model this on top of, I think.