Re: [PATCH] wireless: airo: switch to skcipher interface

[Eric Biggers <ebiggers@kernel.org>]

On Sun, Jun 16, 2019 at 09:03:58PM +0200, Ard Biesheuvel wrote:
> >
> > Otherwise this patch looks correct to me.
> >
> > The actual crypto in this driver, on the other hand, looks very outdated and
> > broken.  Apparently it's implementing some Cisco proprietary extension to WEP
> > that uses a universal hashing based MAC, where the hash key is generated from
> > AES-CTR.  But the MAC is only 32 bits, and the universal hash (MMH) is
> > implemented incorrectly: there's an off-by-one error in emmh32_final() in the
> > code that is supposed to be an optimized version of 'sum % ((1ULL << 32) + 15)'.
> >
> 
> I stared at that code for a bit, and I don't see the problem.
> 

I'm fairly certain that the line:

	if (utmp > 0x10000000fLL)

is supposed to be:

	if (utmp >= 0x10000000fLL)

Since it's doing mod 0x10000000f.  It's supposed to be an optimized
implementation of 'val = (u32)(context->accum % 0x10000000f)' where 0x10000000f
is the prime number 2^32 + 15.  It's meant to be the MMH algorithm: Section 3.2
of https://link.springer.com/content/pdf/10.1007/BFb0052345.pdf.  But there are
values of 'accum' where it gives the wrong result, e.g. 14137323879880455377.

Possibly this is a bug in the Cisco MIC protocol itself so can't be fixed.

> > Do we know whether anyone is actually using this, or is this just another old
> > driver that's sitting around unused?
> >
> 
> Excellent question. I take it this is pre-802.11b hardware, and so
> even the OpenWRT people are unlikely to still be using this.

- Eric