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

[Jose Ignacio Tornos Martinez <jtornosm@redhat.com>]

Hi Johannes,

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

Ok, understood, set_rekey_offload() is not needed with WoWLAN
disabled, that was an oversight.

> But again, what are you actually trying to achieve?

What I would like and what we need is: data encryption stays in
software (FIPS-compliant), WiFi connectivity works, and we accept
that some management frame protection is not fully FIPS-validated.

> 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).

Analyzing it again with your comment, you are right about A-MSDUs,
if there is no HW crypto then big A-MSDUs are not possible anyway,
so restoring A-MSDU sizes was wrong.

> 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.

And you are right about IGTK, it has no relation to data protection
so it could be offloaded, and I should not have kept it in software
while sending PTK/GTK to firmware. That was indeed the opposite.

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

That is what I would expect, but in v1 I only re-enabled MFP_CAPABLE
without passing any keys to firmware and got connection but no data
traffic. Looking at the code, mac80211 SW crypto plumbing works
correctly when drv_set_key() does not install keys in hardware,
and the TX path sets IWL_TX_FLAGS_ENCRYPT_DIS for pre-encrypted
frames. So the host side should handle it. My suspicion is that
firmware changes its behavior when MFP is negotiated, expecting
keys to be installed, and drops encrypted data frames it cannot
decrypt instead of forwarding them to the host. The SEC_ENC_ERR
messages I saw in v1 were likely from management frames, not data.
Any guidance on this would help me take the right approach for v3.

> 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.

Fair point. These dependencies were not documented in the original
disabling commits, so I treated everything as a single block to
revert. Could you clarify which features depend on MFP, which on
HW crypto, and which on other reasons? That would help me get
the separation right.

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

I just reverted the necessary parts (to me) to the previous working
state. I can narrow it once I understand the correct separation.

> Which is the opposite of what you actually did.

Understood. The patches gave data keys to firmware (defeating FIPS
for data) while keeping management keys in software. The correct
approach should be the reverse: keep PTK/GTK in software for
FIPS-compliant data encryption, and offload IGTK if needed since
it has no data relation. Happy to rework this properly for v3.

> 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.

Spelled out: data encryption (PTK/GTK) must stay in software
(FIPS-compliant). We accept that firmware-autonomous management
frames (like AddBA) are not FIPS-protected. We accept reduced
uplink throughput from missing TX aggregation. We do not need
big A-MSDUs or WoWLAN. The goal is a working WPA3-SAE connection
with FIPS-compliant data path.

Also, is the general approach of an opt-in boot parameter
(fips_exception) to keep the default behavior unchanged
acceptable, or would you prefer a different mechanism?

Thanks

Best regards
Jose Ignacio