Re: [PATCH v2 0/5] wifi: add opt-in FIPS exception for iwlwifi

[Johannes Berg <johannes@sipsolutions.net>]

Hi Jose,

> Based on the discussion around v1, I tried to prepare a more complete
> proposal addressing the concerns raised there (AddBA, CSA, robust
> action frames). I understand I may have gone too broad and missed
> some aspects of how encryption offload works internally, without
> access to firmware documentation it is difficult to get everything
> right.

I don't think that's necessarily it.

> Just to clarify a few points, WoWLAN was intentionally kept disabled
> in the patches (both mvm and mld paths), precisely because it requires
> all traffic via firmware crypto during suspend.

Yes, it does remain disabled, but then (for example) why allow
set_rekey_offload() to go through - it's clearly not necessary.

> IGTK handling was not
> changed either, as mac80211 already handles it in software. A-MSDUs
> are not directly MFP-related, fair point, but they were part of the
> same commit being reverted, because patch 2 and patch 3 are just
> reverting some parts of the FIPS disabling to try to get it working
> again under the exception.

But again, what are you actually trying to achieve?

If you want big A-MSDUs, then you need hardware crypto, which you allow
now, which would seem to _completely_ defeat the purpose of FIPS here
(to protect data, if you read it charitably vs. just saying it's for
paper-pushing).

OTOH, why does IGTK matter so much to you? That particular key has no
relation to data protection, so arguably *that* could still be
offloaded. But you're doing it all precisely the other way around.

Hence - again - my request for you spelling out what you're actually
trying to achieve, rather than throwing random code at me.

> Patch 2: In v1, I only re-enabled MFP_CAPABLE without passing keys
> to firmware, the result was connection but no data traffic. That is
> why I added key delivery in v2. Is it possible to keep software
> crypto for data while having a functional connection, or does the
> firmware require keys installed to pass frames? This would help me
> understand the correct approach.

Well, it does work without MFP, so fundamentally the firmware can pass
data without having keys?

> Patch 3: I can narrow this to the minimum needed. Some features
> (EHT, 6GHz) depend on MFP so I included them, but I agree it
> could be more selective.

That's not the point - the point is that there are different
dependencies and you didn't think about _why_ something is disabled. Not
all of this is all related to MFP.

> Patch 4: The intent was host-side crypto for management frames via
> SW_MGMT_TX (same mechanism as ath9k, rtw89, etc.), but I understand
> this depends on how patch 2 is resolved.

But again, the way you did that doesn't make any sense.

> Ultimately, I just want to fix a problem affecting our customers who
> are interested in keeping WiFi working, accepting that some lower-level
> management frames are not FIPS-compliant while the important part for
> them, data encryption, is.

Which is the opposite of what you actually did.

> I don't want to bother you with more patches without guidance. I can
> try to engage Intel on this, but any help, recommendation on the right
> approach to try to solve this in some way would be appreciated.

I really do think it starts with spelling out what you (or your
customers) actually want, and what trade-offs wrt. FIPS you're willing
to accept.

johannes