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
