On Sun, 16 Jun 2019 at 21:43, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > 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) > Ah yes, I see what you mean. 0x10000000f % 0x10000000f == 0 not 15 (after truncation). So yes, that is definitely a bug. > 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. > Highly unlikely. But also highly irrelevant :-) > > > 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. > I'd be fine with dropping this driver. I am just trying to get rid of (ab)use of the crypto cipher interface, so either we change it or we drop it.
