From: Alexander Wetzel <alexander@wetzel-home.de>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [RFC PATCH v2 2/2] mac80211: Add support for Extended Key ID
Date: Thu, 6 Dec 2018 17:27:03 +0100 [thread overview]
Message-ID: <cb55102c-456c-3aea-44c2-cd9d0ebc3c3e@wetzel-home.de> (raw)
In-Reply-To: <61c9ed7fc16517991cda3c9d930b53c24306234e.camel@sipsolutions.net>
>
>>>> - /* PTK only using key ID 0 needs special handling on rekey */
>>>> - if (new_key && sta && ptk0rekey) {
>>>> + /* PTK rekey without Extended Key ID needs special handling */
>>>> + if (new_key && pairwise && sta &&
>>>> + !test_sta_flag(sta, WLAN_STA_EXT_KEY_ID)) {
>>>> local = old_key->local;
>>>> sdata = old_key->sdata;
>>>
>>> This seems wrong, even if you have ext key ID support and everything,
>>> but you do 0 -> 0 rekeying, then you still need all the special handling
>>> (in fact also then if you go 1->1!). So it seems you'd instead want to
>>> see if you're going from a TX key to a TX key with the same key ID, and
>>> then you don't need this flag at all.
>>>
>>
>> The intention for Extended Key ID is, to have a comparable short time
>> frame where both key IDs can be used. When replacing e.g. key ID 0 again
>> it should be idle for a long time. I guess if someone starts re-keying
>> in 1s intervals it may become an issue, but then anyone re-keying that
>> often can't be helped...
>
> Sure. But ... not sure how that's related?
>
>> With Extended Key IDs it's impossible to directly switch from a TX key
>> with one key ID to another one with the same id.
>>
>> 1) Association
>> 2) key ID 0 installed RX only
>> 3) key Id 0 set_tx
>> 4) rekey timeout passes
>> 5) key ID 1 installed RX only
>> 6) key ID 1 set_tx (also making key ID 0 RX only)
>> 7) rekey timeout passes
>> 8) key ID 0 replaced with new RX only key
>> 9) key ID 0 set_tx
>> 10) rekey timeout passes
>> ...
>>
>> So nobody will use the key being replaced, we don't have to protect
>> against PN poisoning.
>
> Exactly.
>
>> And when a driver supports Extended Key ID we
>> don't care about if the driver is able to rekey PTK0 correctly.
>
> Strictly speaking, that's false, since you don't know if wpa_s actually
> used it, and the peer STA allowed it.
>
> It's also not what you implemented, you implemented checking if
> NL80211_KEY_RX_ONLY was ever used.
>
> However, what I'm trying to say is that I'm not sure this makes sense?
>
> It seems to me it would be safer, and easier (no station flag), to just
> check
>
> if ("we're replacing the current TX key")
>
> and trigger the workarounds in that case. No?
>
> Yes, parts of the issue also manifest themselves on the RX side, but if
> you're not replacing the current key then you were using extended key ID
> support?
>
Ah, now I get it:-)
Will try that out also.
>>>> +++ b/net/mac80211/sta_info.c
>>>> @@ -350,6 +350,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
>>>> sta->sta.max_rx_aggregation_subframes =
>>>> local->hw.max_rx_aggregation_subframes;
>>>>
>>>> + sta->ptk_idx = NUM_DEFAULT_KEYS - 1;
>>>
>>> That makes no sense? Why should it be 3? That's invalid anyway?
>>
>> Yes, that's the whole reason for that change:-) Setting it to 2 would
>> also be fine, as long as it's not 0 or 1.
>
> Hmm, ok. So that probably wants a big comment saying that it relies on
> key idx 2/3 being invalid. I'm not sure I like the NUM_DEFAULT_KEYS-1,
> better perhaps to do something like
>
> /* comment saying why */
> BUILD_BUG_ON(ARRAY_SIZE(sta->ptks) > 2);
> sta->ptk_idx = 2;
>
> or so?
>
>> ieee80211_tx_h_select_key starts encrypting packets as soon as
>> sta->ptk[tx->sta->ptk_idx] is not null.
>
> Right, so I guess this makes sense.
>
> johannes
>Thank you very much for all the helpful tips and suggestions!
Alexander
next prev parent reply other threads:[~2018-12-06 21:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-11 11:02 [RFC PATCH v2 0/2] Extended Key ID support for linux Alexander Wetzel
2018-11-11 11:02 ` [RFC PATCH v2 1/2] nl80211/cfg80211: Add support for Extended Key ID Alexander Wetzel
2018-12-05 14:51 ` Johannes Berg
2018-12-05 20:54 ` Alexander Wetzel
2018-12-06 7:25 ` Johannes Berg
2018-12-06 16:21 ` Alexander Wetzel
2018-11-11 11:02 ` [RFC PATCH v2 2/2] mac80211: " Alexander Wetzel
2018-12-05 14:58 ` Johannes Berg
2018-12-05 21:58 ` Alexander Wetzel
2018-12-06 7:32 ` Johannes Berg
2018-12-06 16:27 ` Alexander Wetzel [this message]
2018-12-05 14:42 ` [RFC PATCH v2 0/2] Extended Key ID support for linux Johannes Berg
2018-12-05 19:06 ` Alexander Wetzel
2018-12-07 10:01 ` Jouni Malinen
2018-12-08 13:58 ` Alexander Wetzel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cb55102c-456c-3aea-44c2-cd9d0ebc3c3e@wetzel-home.de \
--to=alexander@wetzel-home.de \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).