diff mbox series

[RFC,1/2] cfg80211: skip strict validation for UDPATE_OWE command

Message ID 20200213090657.28841-2-sergey.matyukevich.os@quantenna.com (mailing list archive)
State Rejected
Delegated to: Johannes Berg
Headers show
Series cfg80211: minor updates for WPA3 OWE support | expand

Commit Message

Sergey Matyukevich Feb. 13, 2020, 9:07 a.m. UTC
Do not perform strict validation of UPDATE_OWE command.
Otherwise, kernel rejects command executed by hostapd.

Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
---
 net/wireless/nl80211.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Johannes Berg Feb. 13, 2020, 9:12 a.m. UTC | #1
On Thu, 2020-02-13 at 09:07 +0000, Sergey Matyukevich wrote:
> Do not perform strict validation of UPDATE_OWE command.
> Otherwise, kernel rejects command executed by hostapd.

Can't we fix hostapd? I mean, it's a relatively new command, so why
shouldn't we validate it strictly?

johannes
Sergey Matyukevich Feb. 13, 2020, 11:13 a.m. UTC | #2
On Thu, Feb 13, 2020 at 10:12:47AM +0100, Johannes Berg wrote:
> 
> [External Email]: This email arrived from an external source - Please exercise caution when opening any attachments or clicking on links.
> 
> On Thu, 2020-02-13 at 09:07 +0000, Sergey Matyukevich wrote:
> > Do not perform strict validation of UPDATE_OWE command.
> > Otherwise, kernel rejects command executed by hostapd.
> 
> Can't we fix hostapd? I mean, it's a relatively new command, so why
> shouldn't we validate it strictly?

That is why this patch is RFC: see cover email :)

Sure, I can fix hostapd instead. Could you point me at some good
starting point to look at ? Are there any user-space tools that
started to use strict validation ?

P.S.
If you are ok with the second patch, then could you just take it
from this series ? Or should I resubmit ?

Regards,
Sergey
Johannes Berg Feb. 13, 2020, 11:16 a.m. UTC | #3
On Thu, 2020-02-13 at 11:13 +0000, Sergey Matyukevich wrote:
> On Thu, Feb 13, 2020 at 10:12:47AM +0100, Johannes Berg wrote:
> > [External Email]: This email arrived from an external source - Please exercise caution when opening any attachments or clicking on links.
> > 
> > On Thu, 2020-02-13 at 09:07 +0000, Sergey Matyukevich wrote:
> > > Do not perform strict validation of UPDATE_OWE command.
> > > Otherwise, kernel rejects command executed by hostapd.
> > 
> > Can't we fix hostapd? I mean, it's a relatively new command, so why
> > shouldn't we validate it strictly?
> 
> That is why this patch is RFC: see cover email :)

Ah, was on the phone and hadn't read that yet ...

> Sure, I can fix hostapd instead. Could you point me at some good
> starting point to look at ? Are there any user-space tools that
> started to use strict validation ?

It's not really opt-in or not, it's entirely a kernel choice.

> P.S.
> If you are ok with the second patch, then could you just take it
> from this series ? Or should I resubmit ?

I can just take it.

johannes
Sergey Matyukevich Feb. 13, 2020, 11:21 a.m. UTC | #4
> On Thu, 2020-02-13 at 11:13 +0000, Sergey Matyukevich wrote:
> > On Thu, Feb 13, 2020 at 10:12:47AM +0100, Johannes Berg wrote:
> > > [External Email]: This email arrived from an external source - Please exercise caution when opening any attachments or clicking on links.
> > > 
> > > On Thu, 2020-02-13 at 09:07 +0000, Sergey Matyukevich wrote:
> > > > Do not perform strict validation of UPDATE_OWE command.
> > > > Otherwise, kernel rejects command executed by hostapd.
> > > 
> > > Can't we fix hostapd? I mean, it's a relatively new command, so why
> > > shouldn't we validate it strictly?
> > 
> > That is why this patch is RFC: see cover email :)
> 
> Ah, was on the phone and hadn't read that yet ...
> 
> > Sure, I can fix hostapd instead. Could you point me at some good
> > starting point to look at ? Are there any user-space tools that
> > started to use strict validation ?
> 
> It's not really opt-in or not, it's entirely a kernel choice.

I mean, I don't know what userspace is supposed to do in the case,
when kernel is doing strict validation. So I was asking about any
pointers to docs or userspace tools that already do netlink
machinery appropriately.

> > P.S.
> > If you are ok with the second patch, then could you just take it
> > from this series ? Or should I resubmit ?
> 
> I can just take it.

Ok

Regards,
Sergey
Johannes Berg Feb. 13, 2020, 11:26 a.m. UTC | #5
On Thu, 2020-02-13 at 11:21 +0000, Sergey Matyukevich wrote:
> > 
> > > Sure, I can fix hostapd instead. Could you point me at some good
> > > starting point to look at ? Are there any user-space tools that
> > > started to use strict validation ?
> > 
> > It's not really opt-in or not, it's entirely a kernel choice.
> 
> I mean, I don't know what userspace is supposed to do in the case,
> when kernel is doing strict validation. So I was asking about any
> pointers to docs or userspace tools that already do netlink
> machinery appropriately.

Userspace is just supposed to create well-formed messages :-)

But ... it really is a kernel bug. The nl80211_policy is missing an
entry for NL80211_ATTR_STATUS_CODE, so for *strict* commands, it's
assumed to not be supported. Need to add something like

	[NL80211_ATTR_STATUS_CODE] = { .type = NLA_U16, },

to the nl80211_policy, or perhaps with a range indicating that 0 isn't
valid or something.

johannes
Sergey Matyukevich Feb. 13, 2020, 11:43 a.m. UTC | #6
> On Thu, 2020-02-13 at 11:21 +0000, Sergey Matyukevich wrote:
> > > 
> > > > Sure, I can fix hostapd instead. Could you point me at some good
> > > > starting point to look at ? Are there any user-space tools that
> > > > started to use strict validation ?
> > > 
> > > It's not really opt-in or not, it's entirely a kernel choice.
> > 
> > I mean, I don't know what userspace is supposed to do in the case,
> > when kernel is doing strict validation. So I was asking about any
> > pointers to docs or userspace tools that already do netlink
> > machinery appropriately.
> 
> Userspace is just supposed to create well-formed messages :-)
> 
> But ... it really is a kernel bug. The nl80211_policy is missing an
> entry for NL80211_ATTR_STATUS_CODE, so for *strict* commands, it's
> assumed to not be supported. Need to add something like
> 
> 	[NL80211_ATTR_STATUS_CODE] = { .type = NLA_U16, },
> 
> to the nl80211_policy, or perhaps with a range indicating that 0 isn't
> valid or something.

Great. Thanks for explanation. Would you mind if I send a fix ?
Or now you have your own plans for this ?

Regards,
Sergey
Johannes Berg Feb. 13, 2020, 12:10 p.m. UTC | #7
On Thu, 2020-02-13 at 11:43 +0000, Sergey Matyukevich wrote:
> > 
> Great. Thanks for explanation. Would you mind if I send a fix ?
> Or now you have your own plans for this ?

Please do :)

johannes
diff mbox series

Patch

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 33fe6ac1c242..92e0723c21fa 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -14837,6 +14837,7 @@  static const struct genl_ops nl80211_ops[] = {
 	},
 	{
 		.cmd = NL80211_CMD_UPDATE_OWE_INFO,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_update_owe_info,
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |