| Message ID | 20200224091437.15212-10-ilan.peer@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Preparations for Pre association Security Negotiation(PASN) Support | expand |
On Mon, Feb 24, 2020 at 11:14:34AM +0200, Ilan Peer wrote: > PASN authentication requires that group management cipher suite > would be set to 00-0F-AC:7 in the RSN IE, so allow this value > when parsing and validating the RSN IE. Can you please point me to the location in P802.11az/D2.0 that describes this? > diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c > @@ -1369,7 +1369,8 @@ int wpa_parse_wpa_ie_rsn(const u8 *rsn_ie, size_t rsn_ie_len, > > if (left >= 4) { > data->mgmt_group_cipher = rsn_selector_to_bitfield(pos); > - if (!wpa_cipher_valid_mgmt_group(data->mgmt_group_cipher)) { > + if (data->mgmt_group_cipher != WPA_CIPHER_GTK_NOT_USED && > + !wpa_cipher_valid_mgmt_group(data->mgmt_group_cipher)) { > wpa_printf(MSG_DEBUG, > "%s: Unsupported management group cipher 0x%x (%08x)", > __func__, data->mgmt_group_cipher, This looks problematic for PMF.. Are you sure this does not result in unexpected behavior for BIP with Robust Management frames? This would likely need some changes in other locations and clear understanding on what to expect to happen with IGTK. The drivers would need to be able to drop any unprotected group-addressed Robust Management frame in such configuration. That would depend on there being an IGTK configured. That would either need to be a random value from the AP or a random value generated by wpa_supplicant internally if no IGTK is received from the AP. As far as consistent implementation is concerned, that check for WPA_CIPHER_GTK_NOT_USED would belong in wpa_cipher_valid_mgmt_group() similarly to the way this is handled with wpa_cipher_valid_group().
> -----Original Message----- > From: Jouni Malinen <j@w1.fi> > Sent: Sunday, March 01, 2020 00:06 > To: Peer, Ilan <ilan.peer@intel.com> > Cc: hostap@lists.infradead.org > Subject: Re: [PATCH 09/12] common: Allow WPA_CIPHER_GTK_NOT_USED in > RSNE parsing > > On Mon, Feb 24, 2020 at 11:14:34AM +0200, Ilan Peer wrote: > > PASN authentication requires that group management cipher suite would > > be set to 00-0F-AC:7 in the RSN IE, so allow this value when parsing > > and validating the RSN IE. > > Can you please point me to the location in P802.11az/D2.0 that describes > this? > See section 12.13.2.2 (PASN Frame Construction and Processing). > > diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c > @@ > > -1369,7 +1369,8 @@ int wpa_parse_wpa_ie_rsn(const u8 *rsn_ie, size_t > > rsn_ie_len, > > > > if (left >= 4) { > > data->mgmt_group_cipher = rsn_selector_to_bitfield(pos); > > - if (!wpa_cipher_valid_mgmt_group(data- > >mgmt_group_cipher)) { > > + if (data->mgmt_group_cipher != > WPA_CIPHER_GTK_NOT_USED && > > + !wpa_cipher_valid_mgmt_group(data- > >mgmt_group_cipher)) { > > wpa_printf(MSG_DEBUG, > > "%s: Unsupported management group > cipher 0x%x (%08x)", > > __func__, data->mgmt_group_cipher, > > This looks problematic for PMF.. Are you sure this does not result in > unexpected behavior for BIP with Robust Management frames? This would > likely need some changes in other locations and clear understanding on what > to expect to happen with IGTK. The drivers would need to be able to drop > any unprotected group-addressed Robust Management frame in such > configuration. That would depend on there being an IGTK configured. That > would either need to be a random value from the AP or a random value > generated by wpa_supplicant internally if no IGTK is received from the AP. > I'm not sure about this. From what I understand, during PASN not multicast frames are allowed, so drivers are expected to drop any multicast frames. > As far as consistent implementation is concerned, that check for > WPA_CIPHER_GTK_NOT_USED would belong in > wpa_cipher_valid_mgmt_group() similarly to the way this is handled with > wpa_cipher_valid_group(). Ok. I'll move it there and see if other changes are required. Thanks, Ilan.
On Sun, Mar 01, 2020 at 08:30:56AM +0000, Peer, Ilan wrote: > > From: Jouni Malinen <j@w1.fi> > > On Mon, Feb 24, 2020 at 11:14:34AM +0200, Ilan Peer wrote: > > > PASN authentication requires that group management cipher suite would > > > be set to 00-0F-AC:7 in the RSN IE, so allow this value when parsing > > > and validating the RSN IE. > > > > Can you please point me to the location in P802.11az/D2.0 that describes > > this? > > See section 12.13.2.2 (PASN Frame Construction and Processing). Thanks. I'm not sure how I did not find that when searching through the draft.. Anyway, that is quite clear on the design. > > This looks problematic for PMF.. Are you sure this does not result in > > unexpected behavior for BIP with Robust Management frames? This would > > likely need some changes in other locations and clear understanding on what > > to expect to happen with IGTK. The drivers would need to be able to drop > > any unprotected group-addressed Robust Management frame in such > > configuration. That would depend on there being an IGTK configured. That > > would either need to be a random value from the AP or a random value > > generated by wpa_supplicant internally if no IGTK is received from the AP. > > I'm not sure about this. From what I understand, during PASN not multicast > frames are allowed, so drivers are expected to drop any multicast frames. I'm not that worried about the part of using this for PASN; I'm worried about the implications of this particular change to non-PASN cases of using PMF since 00-0F-AC:7 has not been used as a group management cipher suite selector in the existing use cases. I'm not at all convinced it would work securely and that's why it is important for the parser to reject that group management cipher suite. If this patch alone were applied that could result in the station accepting any unprotected group-addressed Robust Management frame which is clearly not what should happen. For this change to be acceptable, the 00-0F-AC:7 case with group management cipher suite needs to be first confirmed to work correctly in today's (non-PASN) PMF cases without introducing security vulnerabilities. That's what the steps noted in that paragraphs are needed (make sure a random IGTK value gets configured into the driver regardless of whether the AP sends an IGTK).
On Sun, 2020-03-01 at 17:16 +0200, Jouni Malinen wrote: > On Sun, Mar 01, 2020 at 08:30:56AM +0000, Peer, Ilan wrote: > > > From: Jouni Malinen <j@w1.fi> > > > On Mon, Feb 24, 2020 at 11:14:34AM +0200, Ilan Peer wrote: > > > > PASN authentication requires that group management cipher suite would > > > > be set to 00-0F-AC:7 in the RSN IE, so allow this value when parsing > > > > and validating the RSN IE. > > > > > > Can you please point me to the location in P802.11az/D2.0 that describes > > > this? > > > > See section 12.13.2.2 (PASN Frame Construction and Processing). > > Thanks. I'm not sure how I did not find that when searching through the > draft.. Anyway, that is quite clear on the design. > > > > This looks problematic for PMF.. Are you sure this does not result in > > > unexpected behavior for BIP with Robust Management frames? This would > > > likely need some changes in other locations and clear understanding on > > > what > > > to expect to happen with IGTK. The drivers would need to be able to drop > > > any unprotected group-addressed Robust Management frame in such > > > configuration. That would depend on there being an IGTK configured. That > > > would either need to be a random value from the AP or a random value > > > generated by wpa_supplicant internally if no IGTK is received from the > > > AP. > > > > I'm not sure about this. From what I understand, during PASN not multicast > > frames are allowed, so drivers are expected to drop any multicast frames. > > I'm not that worried about the part of using this for PASN; I'm worried > about the implications of this particular change to non-PASN cases of > using PMF since 00-0F-AC:7 has not been used as a group management > cipher suite selector in the existing use cases. I'm not at all > convinced it would work securely and that's why it is important for the > parser to reject that group management cipher suite. If this patch alone > were applied that could result in the station accepting any unprotected > group-addressed Robust Management frame which is clearly not what should > happen. > I think I understand your concern now. At least from what I can tell about mac80211 it does not have any handling for such a case, i.e., not allow any group addressed frames. I do not know how other drivers would handle this. > For this change to be acceptable, the 00-0F-AC:7 case with group > management cipher suite needs to be first confirmed to work correctly in > today's (non-PASN) PMF cases without introducing security > vulnerabilities. That's what the steps noted in that paragraphs are > needed (make sure a random IGTK value gets configured into the driver > regardless of whether the AP sends an IGTK). > I can change the implementation so this would be allowed only in the case of PASN. This should be simple enough. If you want me to do it differently let me know. Regards, Ilan.
On Sun, Mar 01, 2020 at 08:33:30PM +0000, Peer, Ilan wrote: > I think I understand your concern now. At least from what I can tell about > mac80211 it does not have any handling for such a case, i.e., not allow any > group addressed frames. I do not know how other drivers would handle this. I think the only safe way to do this is to configure a random IGTK so that the drivers would not need to have any special handling for this. > I can change the implementation so this would be allowed only in the case of > PASN. This should be simple enough. If you want me to do it differently let me > know. It's a bit ugly in the generic parser function, but I guess that's fine as an initial step. That said, it probably makes sense to extend non-PASN PMF case to support no-BIP-used option as well even if that has not really been used so far. Though, I'm not sure there is any easy way of deploying this on the AP side if most already deployed STAs reject such configuration in practice and won't connect. I'll try to remember to bring this up in the REVmd (and/or P802.11az) discussions as well to get the standard clear on what to expect as allowed set of cipher suite selectors for the different cases using the group management cipher.
> -----Original Message----- > From: Jouni Malinen <j@w1.fi> > Sent: Monday, March 02, 2020 00:07 > To: Peer, Ilan <ilan.peer@intel.com> > Cc: hostap@lists.infradead.org > Subject: Re: [PATCH 09/12] common: Allow WPA_CIPHER_GTK_NOT_USED in > RSNE parsing > > On Sun, Mar 01, 2020 at 08:33:30PM +0000, Peer, Ilan wrote: > > I think I understand your concern now. At least from what I can tell > > about > > mac80211 it does not have any handling for such a case, i.e., not > > allow any group addressed frames. I do not know how other drivers would > handle this. > > I think the only safe way to do this is to configure a random IGTK so that the > drivers would not need to have any special handling for this. > Agree. Should be simple enough. > > I can change the implementation so this would be allowed only in the > > case of PASN. This should be simple enough. If you want me to do it > > differently let me know. > > It's a bit ugly in the generic parser function, but I guess that's fine as an initial > step. That said, it probably makes sense to extend non-PASN PMF case to > support no-BIP-used option as well even if that has not really been used so > far. Though, I'm not sure there is any easy way of deploying this on the AP > side if most already deployed STAs reject such configuration in practice and > won't connect. > As previously agreed, lets wait for the 802.11az standard to be more stable and clear about this, and the specification allows to set 07 as group management cipher I'll align the code to match the expectation. Regards, Ilan.
diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c index 6cb9180ee8..bd79575d0f 100644 --- a/src/common/wpa_common.c +++ b/src/common/wpa_common.c @@ -1369,7 +1369,8 @@ int wpa_parse_wpa_ie_rsn(const u8 *rsn_ie, size_t rsn_ie_len, if (left >= 4) { data->mgmt_group_cipher = rsn_selector_to_bitfield(pos); - if (!wpa_cipher_valid_mgmt_group(data->mgmt_group_cipher)) { + if (data->mgmt_group_cipher != WPA_CIPHER_GTK_NOT_USED && + !wpa_cipher_valid_mgmt_group(data->mgmt_group_cipher)) { wpa_printf(MSG_DEBUG, "%s: Unsupported management group cipher 0x%x (%08x)", __func__, data->mgmt_group_cipher,
PASN authentication requires that group management cipher suite would be set to 00-0F-AC:7 in the RSN IE, so allow this value when parsing and validating the RSN IE. Signed-off-by: Ilan Peer <ilan.peer@intel.com> --- src/common/wpa_common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)