Bug 249403 - Abstract priv_check() for wireless operations
Summary: Abstract priv_check() for wireless operations
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: wireless (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Bjoern A. Zeeb
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-17 18:36 UTC by martin
Modified: 2020-11-05 13:52 UTC (History)
3 users (show)

See Also:
bz: mfc-stable12?
bz: mfc-stable11-


Attachments
abstract priv_check() for iee80211 and add more arguments to allow a NetBSD kauth based implementation (1.88 KB, patch)
2020-09-17 18:36 UTC, martin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description martin 2020-09-17 18:36:52 UTC
Created attachment 218030 [details]
abstract priv_check() for iee80211 and add more arguments to allow a NetBSD kauth based implementation

The kernel authorization system in NetBSD is (optionally) quite fine grained and needs a bit more data to take decisions. This creates an ifdef mess in my adaption of the FreeBSD wifi stack, but it could be easily avoided with a minor change similar to the attached patch.

The definitions for the macro should move to the OS specific header (so only the FreeBSD definition in ieee80211_freebsd.h), I left them all together so the reason for the change is more obvious.
Comment 1 Adrian Chadd freebsd_committer freebsd_triage 2020-09-17 18:44:38 UTC
Hi!

I think we should put the priv checks in as a method in ieee80211_freebsd.c, so your ieee80211_netbsd.c can implement it as well. (same with the dfbsd and haiku users when they update.)

What do you think instead of creating some stub priv check functions rather than using system defines, and we'll then have to define them in our ieee80211_{os}.c file?

Thanks! We're also super open to adding whatever else makes portability easier, so if you find freebsd specific things that you're #ifdef'ing do let us know and let's figure out a better way to abstract it.
Comment 2 Bjoern A. Zeeb freebsd_committer freebsd_triage 2020-09-18 18:10:30 UTC
Hi Martin,

if adrian's suggestion would work for you I'll go and implement that?

/bz
Comment 3 martin 2020-09-18 18:13:41 UTC
Sure, that works fine for me - thanks!
Comment 4 Bjoern A. Zeeb freebsd_committer freebsd_triage 2020-09-23 22:11:32 UTC
Hi,

I have uploaded a diff for review:
https://reviews.freebsd.org/D26541

Drop me a private email if you have an account.
I am not sure if that will work for you current code (given vap != ic) but based on your diff, you also have a "vap" so should be fine.
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-10-18 21:34:20 UTC
A commit references this bug:

Author: bz
Date: Sun Oct 18 21:34:05 UTC 2020
New revision: 366817
URL: https://svnweb.freebsd.org/changeset/base/366817

Log:
  net80211: factor out the priv(9) checks into OS specifc code.

  Factor out the priv(9) checks into OS specifc code so other OSes can equally
  implement them.  This sorts out those XXX in the net80211 code.
  We provide 3 arguments (cmd, vap, ifp) where available to the functions, in
  order to allow other OSes to use that data but also in case we'd add auditing
  to these check to have the information available. For now the arguments are
  marked __unused.

  PR:		249403
  Reported by:	martin(NetBSD)
  Reviewed by:	adrian, martin(NetBSD)
  MFC after:	10 days
  Sponsored by:	Rubicon Communications, LLC (d/b/a "Netgate")
  Differential Revision:	https://reviews.freebsd.org/D26541

Changes:
  head/sys/net80211/ieee80211_freebsd.c
  head/sys/net80211/ieee80211_freebsd.h
  head/sys/net80211/ieee80211_ioctl.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2020-11-05 12:18:08 UTC
A commit references this bug:

Author: bz
Date: Thu Nov  5 12:17:51 UTC 2020
New revision: 367378
URL: https://svnweb.freebsd.org/changeset/base/367378

Log:
  MFC r366817:

    net80211: factor out the priv(9) checks into OS specifc code.

    Factor out the priv(9) checks into OS specifc code so other OSes can equally
    implement them.  This sorts out those XXX in the net80211 code.
    We provide 3 arguments (cmd, vap, ifp) where available to the functions, in
    order to allow other OSes to use that data but also in case we'd add auditing
    to these check to have the information available. For now the arguments are
    marked __unused.

  PR:		249403

Changes:
_U  stable/12/
  stable/12/sys/net80211/ieee80211_freebsd.c
  stable/12/sys/net80211/ieee80211_freebsd.h
  stable/12/sys/net80211/ieee80211_ioctl.c
Comment 7 Bjoern A. Zeeb freebsd_committer freebsd_triage 2020-11-05 13:52:54 UTC
Thanks for bringing this up!