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
