On Wed, Oct 26, 2016 at 07:30:00AM +0200, Johannes Berg wrote:
>
> > if (req->auth_data_len >= 4) {
> > - __le16 *pos = (__le16 *) req->auth_data;
> > - auth_data->sae_trans = le16_to_cpu(pos[0]);
> > - auth_data->sae_status = le16_to_cpu(pos[1]);
> > + if (req->auth_type == NL80211_AUTHTYPE_SAE) {
> > + __le16 *pos = (__le16 *) req->auth_data;
> > + auth_data->sae_trans = le16_to_cpu(pos[0]);
> > + auth_data->sae_status = le16_to_cpu(pos[1]);
> > + }
> > memcpy(auth_data->data, req->auth_data + 4,
> > req->auth_data_len - 4);
> > auth_data->data_len += req->auth_data_len - 4;
>
> Hmm. Do we really want to still skip the first four bytes of the data
> userspace passed? That seems a bit strange to me. The docs in nl80211.h
> do say it that way now, but should we really include a dummy
> Authentication transaction sequence number field?
This is admittedly a bit strange design with that special case needed
for SAE. If we were to design the SAE case now in combination with FILS,
I guess this would be quite different (e.g., separate attributes for
Authentication transaction sequence number and Status code). Unlike the
mesh use case with SAE, FILS is only between an AP and a station and as
such, there would not really be a case where the station would send an
Authentication frame with non-zero Status code.
A future amendment might define a new authentication algorithm that ends
up using more than a single Authentication frame exchange. In such a
case, we would actually have need for Authentication transaction
sequence number even though FILS doesn't need it.
I think I'd rather maintain a consistent attribute design for all
authentication algorithms and leave this as-is now. Another option would
be to not apply the rename SAE attributes patch and define something new
as a more generic solution, but I'm not sure there is sufficient
justification for the added complexity since we cannot really get rid of
the current SAE design any time soon.
--
Jouni Malinen PGP id EFC895FA
