[PATCH 1/2] hostapd: run as user 'network'
Daniel Golle
daniel at makrotopia.org
Mon Dec 7 17:48:51 EST 2020
On Mon, Dec 07, 2020 at 08:57:05PM +0100, Hauke Mehrtens wrote:
> On 12/7/20 7:14 PM, Daniel Golle wrote:
> > Granting capabilities CAP_NET_ADMIN and CAP_NET_RAW allows running
> > hostapd and wpa_supplicant without root priviledges.
> > Add ubus acl allowing the necessary ubus interactions for the 'network'
> > user running hostapd/wpa_supplicant.
> > To still allow netifd to acquire the PID of wpa_supplicant and hostapd
> > introduce a new ubus method 'getpid' for both of them and make use of
> > that instead of querying procd (which will not work, as procd will
> > return the PID of ujail)
> >
> > Signed-off-by: Daniel Golle <daniel at makrotopia.org>
> > ---
> > package/network/services/hostapd/Makefile | 9 ++++++--
> > .../network/services/hostapd/files/hostapd.sh | 2 +-
> > .../network/services/hostapd/files/wpad.init | 16 ++++++++++++++
> > .../network/services/hostapd/files/wpad.json | 22 +++++++++++++++++++
> > .../services/hostapd/files/wpad_acl.json | 10 +++++++++
> > .../services/hostapd/src/src/ap/ubus.c | 16 ++++++++++++++
> > .../hostapd/src/wpa_supplicant/ubus.c | 16 ++++++++++++++
> > 7 files changed, 88 insertions(+), 3 deletions(-)
> > create mode 100644 package/network/services/hostapd/files/wpad.json
> > create mode 100644 package/network/services/hostapd/files/wpad_acl.json
> >
> > diff --git a/package/network/services/hostapd/Makefile b/package/network/services/hostapd/Makefile
> > index 61b2a548ef..386c17f99d 100644
> > --- a/package/network/services/hostapd/Makefile
> > +++ b/package/network/services/hostapd/Makefile
> > @@ -7,7 +7,7 @@
> > include $(TOPDIR)/rules.mk
> > PKG_NAME:=hostapd
> > -PKG_RELEASE:=18
> > +PKG_RELEASE:=19
> > PKG_SOURCE_URL:=http://w1.fi/hostap.git
> > PKG_SOURCE_PROTO:=git
> > @@ -149,6 +149,7 @@ define Package/hostapd/Default
> > TITLE:=IEEE 802.1x Authenticator
> > URL:=http://hostap.epitest.fi/
> > DEPENDS:=$(DRV_DEPENDS) +hostapd-common +libubus
> > + USERID:=network=101:network=101
>
> Do we have some central uid assignment or at least a list of all used uids?
I grep for 'USERID:=' in all feeds to find out existing assignments
and try to use the same UID/GID values as assigned in FreeBSD (which
does have a central uid/gid allocation scheme)
I vaguely remember a discussion about that some years ago and I'm not
sure what we have concluded back then...
> We should prevent that someone uses 101 on some other package.
Actually UID and GID 101 are defined for the user 'network' in the
base-files package (among with user 'daemon' and user 'ftp', why ever).
> Do you plan to add more services under the network user?
I must admit the choice to use the existing 'network' user was kinda
arbitrary. Unless it would make things really much much easier while
not loosing a lot of security I'd always have a seperate user for
each service.
Potential candidates are:
netifd: in it's current form netifd is not easily restricted, as it
touches a lot of sysctl which isn't covered by capabilities nor can
we chown files in /proc/sys, it launches protocol handler processes
which need priviledges, ...
pppd: would traditionally run as the 'uucp' user, giving it access to
modem chardevs. As it is lauched by netifd rather than being a
procd service we'd have to think of a way that netifd can leverage
procd's jail feautres.
The same applies for odhcpd and odhcp6c, both would be easy to contain
(eg. using seccomp in addition to running non-root with only the
necessary capabilities), but currently netifd's built-in process
management can't do that for us.
>
> > PROVIDES:=hostapd
> > CONFLICTS:=$(HOSTAPD_PROVIDERS)
> > HOSTAPD_PROVIDERS+=$(1)
> > @@ -232,6 +233,7 @@ define Package/wpad/Default
> > SUBMENU:=WirelessAPD
> > TITLE:=IEEE 802.1x Auth/Supplicant
> > DEPENDS:=$(DRV_DEPENDS) +hostapd-common +libubus
> > + USERID:=network=101:network=101
> > URL:=http://hostap.epitest.fi/
> > PROVIDES:=hostapd wpa-supplicant
> > CONFLICTS:=$(HOSTAPD_PROVIDERS) $(SUPPLICANT_PROVIDERS)
> > @@ -346,6 +348,7 @@ define Package/wpa-supplicant/Default
> > TITLE:=WPA Supplicant
> > URL:=http://hostap.epitest.fi/wpa_supplicant/
> > DEPENDS:=$(DRV_DEPENDS) +hostapd-common +libubus
> > + USERID:=network=101:network=101
> > PROVIDES:=wpa-supplicant
> > CONFLICTS:=$(SUPPLICANT_PROVIDERS)
> > SUPPLICANT_PROVIDERS+=$(1)
> > @@ -597,10 +600,12 @@ define Install/supplicant
> > endef
> > define Package/hostapd-common/install
> > - $(INSTALL_DIR) $(1)/lib/netifd $(1)/etc/rc.button $(1)/etc/hotplug.d/ieee80211 $(1)/etc/init.d
> > + $(INSTALL_DIR) $(1)/etc/capabilities $(1)/etc/rc.button $(1)/etc/hotplug.d/ieee80211 $(1)/etc/init.d $(1)/lib/netifd $(1)/usr/share/acl.d
> > $(INSTALL_DATA) ./files/hostapd.sh $(1)/lib/netifd/hostapd.sh
> > $(INSTALL_BIN) ./files/wpad.init $(1)/etc/init.d/wpad
> > $(INSTALL_BIN) ./files/wps-hotplug.sh $(1)/etc/rc.button/wps
> > + $(INSTALL_DATA) ./files/wpad_acl.json $(1)/usr/share/acl.d
> > + $(INSTALL_DATA) ./files/wpad.json $(1)/etc/capabilities
> > endef
> > define Package/hostapd/install
> > diff --git a/package/network/services/hostapd/files/hostapd.sh b/package/network/services/hostapd/files/hostapd.sh
> > index 41b04e6029..781a34028a 100644
> > --- a/package/network/services/hostapd/files/hostapd.sh
> > +++ b/package/network/services/hostapd/files/hostapd.sh
> > @@ -1365,6 +1365,7 @@ wpa_supplicant_run() {
> > _wpa_supplicant_common "$ifname"
> > ubus wait_for wpa_supplicant
> > + local supplicant_pid=$(ubus call wpa_supplicant getpid | jsonfilter -l 1 -e @.pid)
> > ubus call wpa_supplicant config_add "{ \
> > \"driver\": \"${_w_driver:-wext}\", \"ctrl\": \"$_rpath\", \
> > \"iface\": \"$ifname\", \"config\": \"$_config\" \
> > @@ -1376,7 +1377,6 @@ wpa_supplicant_run() {
> > [ "$ret" != 0 ] && wireless_setup_vif_failed WPA_SUPPLICANT_FAILED
> > - local supplicant_pid=$(ubus call service list '{"name": "wpad"}' | jsonfilter -l 1 -e "@['wpad'].instances['supplicant'].pid")
> > wireless_add_process "$supplicant_pid" "/usr/sbin/wpa_supplicant" 1
> > return $ret
> > diff --git a/package/network/services/hostapd/files/wpad.init b/package/network/services/hostapd/files/wpad.init
> > index 3198e9801f..6c60d539a2 100644
> > --- a/package/network/services/hostapd/files/wpad.init
> > +++ b/package/network/services/hostapd/files/wpad.init
> > @@ -9,17 +9,33 @@ NAME=wpad
> > start_service() {
> > if [ -x "/usr/sbin/hostapd" ]; then
> > mkdir -p /var/run/hostapd
> > + chown network:network /var/run/hostapd
> > procd_open_instance hostapd
> > procd_set_param command /usr/sbin/hostapd -s -g /var/run/hostapd/global
> > procd_set_param respawn
> > + [ -x /sbin/ujail -a -e /etc/capabilities/wpad.json ] && {
> > + procd_add_jail netifd
> > + procd_set_param capabilities /etc/capabilities/wpad.json
> > + procd_set_param user network
> > + procd_set_param group network
> > + procd_set_param no_new_privs 1
> > + }
> > procd_close_instance
> > fi
> > if [ -x "/usr/sbin/wpa_supplicant" ]; then
> > mkdir -p /var/run/wpa_supplicant
> > + chown network:network /var/run/wpa_supplicant
> > procd_open_instance supplicant
> > procd_set_param command /usr/sbin/wpa_supplicant -n -s -g /var/run/wpa_supplicant/global
> > procd_set_param respawn
> > + [ -x /sbin/ujail -a -e /etc/capabilities/wpad.json ] && {
> > + procd_add_jail netifd
> > + procd_set_param capabilities /etc/capabilities/wpad.json
> > + procd_set_param user network
> > + procd_set_param group network
> > + procd_set_param no_new_privs 1
> > + }
> > procd_close_instance
> > fi
> > }
> > diff --git a/package/network/services/hostapd/files/wpad.json b/package/network/services/hostapd/files/wpad.json
> > new file mode 100644
> > index 0000000000..ae3edd4fea
> > --- /dev/null
> > +++ b/package/network/services/hostapd/files/wpad.json
> > @@ -0,0 +1,22 @@
> > +{
> > + "bounding": [
> > + "CAP_NET_RAW",
> > + "CAP_NET_ADMIN"
> > + ],
> > + "effective": [
> > + "CAP_NET_RAW",
> > + "CAP_NET_ADMIN"
> > + ],
> > + "ambient": [
> > + "CAP_NET_RAW",
> > + "CAP_NET_ADMIN"
> > + ],
> > + "permitted": [
> > + "CAP_NET_RAW",
> > + "CAP_NET_ADMIN"
> > + ],
> > + "inheritable": [
> > + "CAP_NET_RAW",
> > + "CAP_NET_ADMIN"
> > + ]
> > +}
> > diff --git a/package/network/services/hostapd/files/wpad_acl.json b/package/network/services/hostapd/files/wpad_acl.json
> > new file mode 100644
> > index 0000000000..c77ccd8ea0
> > --- /dev/null
> > +++ b/package/network/services/hostapd/files/wpad_acl.json
> > @@ -0,0 +1,10 @@
> > +{
> > + "user": "network",
> > + "access": {
> > + "service": {
> > + "methods": [ "event" ]
> > + }
> > + },
> > + "publish": [ "hostapd", "hostapd.*", "wpa_supplicant", "wpa_supplicant.*" ],
> > + "send": [ "bss.*", "wps_credentials" ]
> > +}
> > diff --git a/package/network/services/hostapd/src/src/ap/ubus.c b/package/network/services/hostapd/src/src/ap/ubus.c
> > index 8546d2ce69..e59db00bd3 100644
> > --- a/package/network/services/hostapd/src/src/ap/ubus.c
> > +++ b/package/network/services/hostapd/src/src/ap/ubus.c
> > @@ -690,6 +690,21 @@ hostapd_config_remove(struct ubus_context *ctx, struct ubus_object *obj,
> > return UBUS_STATUS_OK;
> > }
> > +static int
> > +hostapd_getpid(struct ubus_context *ctx, struct ubus_object *obj,
> > + struct ubus_request_data *req, const char *method,
> > + struct blob_attr *msg)
> > +{
> > +
That newline is not supposed to be here now that I see it. Fixed.
> > + blob_buf_init(&b, 0);
> > +
> > + blobmsg_add_u32(&b, "pid", getpid());
> > +
> > + ubus_send_reply(ctx, req, b.head);
> > +
> > + return UBUS_STATUS_OK;
> > +}
> > +
> > enum {
> > CSA_FREQ,
> > CSA_BCN_COUNT,
> > @@ -1363,6 +1378,7 @@ void hostapd_ubus_free_bss(struct hostapd_data *hapd)
> > static const struct ubus_method daemon_methods[] = {
> > UBUS_METHOD("config_add", hostapd_config_add, config_add_policy),
> > UBUS_METHOD("config_remove", hostapd_config_remove, config_remove_policy),
> > + UBUS_METHOD_NOARG("getpid", hostapd_getpid),
> > };
> > static struct ubus_object_type daemon_object_type =
> > diff --git a/package/network/services/hostapd/src/wpa_supplicant/ubus.c b/package/network/services/hostapd/src/wpa_supplicant/ubus.c
> > index 4bb92a7b66..f9fa1645d7 100644
> > --- a/package/network/services/hostapd/src/wpa_supplicant/ubus.c
> > +++ b/package/network/services/hostapd/src/wpa_supplicant/ubus.c
> > @@ -311,9 +311,25 @@ wpas_config_remove(struct ubus_context *ctx, struct ubus_object *obj,
> > return UBUS_STATUS_OK;
> > }
> > +static int
> > +wpas_getpid(struct ubus_context *ctx, struct ubus_object *obj,
> > + struct ubus_request_data *req, const char *method,
> > + struct blob_attr *msg)
> > +{
> > +
Removed that newline as well.
> > + blob_buf_init(&b, 0);
> > +
> > + blobmsg_add_u32(&b, "pid", getpid());
> > +
> > + ubus_send_reply(ctx, req, b.head);
> > +
> > + return UBUS_STATUS_OK;
> > +}
> > +
>
> Can we unify these two functions wpas_getpid() and hostapd_getpid()? They do
> the same thing and when we use wpad they should be both in the same binary.
>
> I think this is not needed now, but would be nice.
>
I agree, but well, you know the code, it wouldn't exactly be trivial
to do that. As that function is really really small I didn't bother
diving down that rabbit hole...
> > static const struct ubus_method wpas_daemon_methods[] = {
> > UBUS_METHOD("config_add", wpas_config_add, wpas_config_add_policy),
> > UBUS_METHOD("config_remove", wpas_config_remove, wpas_config_remove_policy),
> > + UBUS_METHOD_NOARG("getpid", wpas_getpid),
> > };
> > static struct ubus_object_type wpas_daemon_object_type =
> >
More information about the openwrt-devel
mailing list