On Wed, Oct 26, 2016 at 07:36:27AM +0200, Johannes Berg wrote:
> > +++ b/net/wireless/nl80211.c
> > + [NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
> > + .len = FILS_MAX_KEK_LEN },
> > + [NL80211_ATTR_FILS_NONCES] = { .type = NLA_BINARY,
> > + .len = 2 * FILS_NONCE_LEN },
> If you remove the type = NLA_BINARY and just leave the type zero, then
> you'll get *minimum* length validation, rather than limiting the
> maximum length. That seems more appropriate for the nonces?
FILS_KEK is variable length, but FILS_NONCES should be exactly 2 *
FILS_NONCE_LEN. I didn't remember the minimum length case here, but yes,
that sounds reasonable for FILS_NONCES.
> > + if (info->attrs[NL80211_ATTR_FILS_NONCES]) {
> > + if (nla_len(info->attrs[NL80211_ATTR_FILS_NONCES])
> > !=
> > + 2 * FILS_NONCE_LEN)
> > + return -EINVAL;
>
> You're validating the *exact* length here, which unfortunately nlattr
> doesn't support right now, but perhaps we can live with checking that
> it's at least that many bytes, and using only 2*nonces? We do that for
> most other attributes (like MAC addresses).
This was because of the .len above ending up allowing shorter values..
Since we do that minimum length check only for number of other
attributes, I guess we can do it here as well and ignore whatever else
the user space might have added incorrectly.
> Or do we expect to extend this to more than 2 nonces in the future, at
> which point we'll need the length?
No, this should remain exactly two nonces and each of those having
exactly 16 octets.
--
Jouni Malinen PGP id EFC895FA
