privilege separation for wpa_supplicant #13

Closed
thestinger opened this Issue Aug 19, 2015 · 20 comments

Comments

Projects
None yet
4 participants
@thestinger
Contributor

thestinger commented Aug 19, 2015

https://w1.fi/cgit/hostap/plain/wpa_supplicant/README (Privilege separation section)

This involves enabling the feature in the build, adding a new uid/gid pair to system/core, wiring up the services/sockets in the device-specific init scripts (device/$vendor/$device), setting up SELinux rules for the new user (likely using the old domain as a starting point) and finally stripping down the old capabilities and SELinux domain.

@e-nomem

This comment has been minimized.

Show comment Hide comment
@e-nomem

e-nomem Jun 19, 2016

I'm going to link my PRs from the other projects so we can discuss all of them here.

I haven't tested the image I created for this project because I'm having issues with the emulator and my Nexus 6P is my daily driver. I can probably dig up my Nexus 5 from wherever I left it after upgrading though.

About the new uid/gid, is it necessary? It's easy enough to do, but the selinux policy is a lot more specific than the general uid/gid restrictions and it seems unnecessary.

e-nomem commented Jun 19, 2016

I'm going to link my PRs from the other projects so we can discuss all of them here.

I haven't tested the image I created for this project because I'm having issues with the emulator and my Nexus 6P is my daily driver. I can probably dig up my Nexus 5 from wherever I left it after upgrading though.

About the new uid/gid, is it necessary? It's easy enough to do, but the selinux policy is a lot more specific than the general uid/gid restrictions and it seems unnecessary.

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Jun 19, 2016

Contributor

@e-nomem It's still a good idea to use uid/gid isolation either way, since then SELinux will be building upon a basic isolation layer rather than the policies being responsible for doing everything.

Contributor

thestinger commented Jun 19, 2016

@e-nomem It's still a good idea to use uid/gid isolation either way, since then SELinux will be building upon a basic isolation layer rather than the policies being responsible for doing everything.

@e-nomem e-nomem referenced this issue in CopperheadOS/platform_system_core Jun 22, 2016

Closed

Enable privilege separation for wpa_supplicant #1

@zongo zongo referenced this issue Jan 8, 2017

Closed

webview crash #555

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 16, 2017

The benefits of doing this on Android are not clear to me. What am I missing?

On a regular Linux system OTOH I can see the benefit of moving privileged capabilities further away from the presumably unprivileged process that is sending control messages to wpa supplicant.

On Android, wpa_supplicant is controlled via a socket that only system_server may use (enforced by selinux). Wpa supplicant's permissions/capabilities are basically a subset of those already granted to system_server. A malicious system_server would really be de-escalating privilege by exploiting wpa supplicant.

The benefits of doing this on Android are not clear to me. What am I missing?

On a regular Linux system OTOH I can see the benefit of moving privileged capabilities further away from the presumably unprivileged process that is sending control messages to wpa supplicant.

On Android, wpa_supplicant is controlled via a socket that only system_server may use (enforced by selinux). Wpa supplicant's permissions/capabilities are basically a subset of those already granted to system_server. A malicious system_server would really be de-escalating privilege by exploiting wpa supplicant.

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Jan 16, 2017

Contributor

@jvanderstoep: To isolate the remote attack surface exposed by wpa_supplicant to access points like the authentication handling code. It wouldn't really be worth focusing on it if it didn't already support privilege separation but it should mostly just be a build / configuration setup task. The WiFi drivers (particularly qcacld-2.0...) have far more code and it's harder to deal with those but splitting this up would at least be a starting point.

Contributor

thestinger commented Jan 16, 2017

@jvanderstoep: To isolate the remote attack surface exposed by wpa_supplicant to access points like the authentication handling code. It wouldn't really be worth focusing on it if it didn't already support privilege separation but it should mostly just be a build / configuration setup task. The WiFi drivers (particularly qcacld-2.0...) have far more code and it's harder to deal with those but splitting this up would at least be a starting point.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 16, 2017

I have it building and have fixed at least one bug that was causing it to exit. Currently it's associating with my AP but timing out during authentication...

I have it building and have fixed at least one bug that was causing it to exit. Currently it's associating with my AP but timing out during authentication...

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 16, 2017

And by "fixed" I mean "worked around" a bug where it bails because wowlan hasn't been implemented for privsep.

And by "fixed" I mean "worked around" a bug where it bails because wowlan hasn't been implemented for privsep.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 19, 2017

Confirmed my PoC works on guest network. Still investigating why authentication fails on wpa2...

Confirmed my PoC works on guest network. Still investigating why authentication fails on wpa2...

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Jan 20, 2017

Contributor

Authentication is one of the things that gets delegated to the unprivileged process so maybe that process isn't working at all.

Contributor

thestinger commented Jan 20, 2017

Authentication is one of the things that gets delegated to the unprivileged process so maybe that process isn't working at all.

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Jan 20, 2017

Contributor

Well, I'm not really sure which one is really delegating to which. I guess they might do it the other way around in which case it's weirder for it to be broken. I'd expect that at least someone is using this although edge cases might not be tested.

Contributor

thestinger commented Jan 20, 2017

Well, I'm not really sure which one is really delegating to which. I guess they might do it the other way around in which case it's weirder for it to be broken. I'd expect that at least someone is using this although edge cases might not be tested.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 20, 2017

They're both working. I'm seeing commands from wpa_supplicant to wpa_priv and replies back using the "-dd" option on both for verbose logging. For example, wpa_supplicant tells wpa_priv to scan for SSIDs, which wpa_supplicant then sends up to system_server.

1|angler:/ # ps | grep wpa
wifi 4362 1 13120 3636 poll_sched 730ecfac2c S /system/bin/wpa_supplicant
wifi 4363 1 9224 2100 poll_sched 70dc068c2c S /system/bin/wpa_priv

And two selinux denials (of many) showing them communicating over their respective command sockets:

02-08 07:21:05.826 4384 4384 I wpa_supplicant: type=1400 audit(0.0:18): avc: denied { sendto } for path="/data/misc/wifi/wpa_priv/wlan0" scontext=u:r:wpa:s0 tcontext=u:r:wpa_priv:s0 tclass=unix_dgram_socket permissive=1
02-08 07:21:05.840 4385 4385 I wpa_priv: type=1400 audit(0.0:36): avc: denied { sendto } for path="/data/misc/wifi/sockets/wpa_privsep-4384-1" scontext=u:r:wpa_priv:s0 tcontext=u:r:wpa:s0 tclass=unix_dgram_socket permissive=1

Still looking...

They're both working. I'm seeing commands from wpa_supplicant to wpa_priv and replies back using the "-dd" option on both for verbose logging. For example, wpa_supplicant tells wpa_priv to scan for SSIDs, which wpa_supplicant then sends up to system_server.

1|angler:/ # ps | grep wpa
wifi 4362 1 13120 3636 poll_sched 730ecfac2c S /system/bin/wpa_supplicant
wifi 4363 1 9224 2100 poll_sched 70dc068c2c S /system/bin/wpa_priv

And two selinux denials (of many) showing them communicating over their respective command sockets:

02-08 07:21:05.826 4384 4384 I wpa_supplicant: type=1400 audit(0.0:18): avc: denied { sendto } for path="/data/misc/wifi/wpa_priv/wlan0" scontext=u:r:wpa:s0 tcontext=u:r:wpa_priv:s0 tclass=unix_dgram_socket permissive=1
02-08 07:21:05.840 4385 4385 I wpa_priv: type=1400 audit(0.0:36): avc: denied { sendto } for path="/data/misc/wifi/sockets/wpa_privsep-4384-1" scontext=u:r:wpa_priv:s0 tcontext=u:r:wpa:s0 tclass=unix_dgram_socket permissive=1

Still looking...

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 20, 2017

Yes, the unprivileged process does the authentication.

Yes, the unprivileged process does the authentication.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 20, 2017

Figured out the issue. It's failing to create the P2P device.

02-08 14:30:20.200 4384 4384 D wpa_supplicant: P2P: Failed to create P2P Device interface
02-08 14:30:20.200 4384 4384 I wpa_supplicant: P2P: Failed to enable P2P Device interface

Angler is a wifi-direct device and much of the P2P driver functionality hasn't been implemented for privsep yet.

It's failing here: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/wpa_supplicant/p2p_supplicant.c#3736

Which leads to: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/wpa_supplicant/driver_i.h#399

The if_add() function is not implemented for privsep :(

The nl80211 driver operations currently supported by the privsep driver are a tiny subset of the available operations.

nl80211 ops: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/src/drivers/driver_nl80211.c#9098
privsep ops: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/src/drivers/driver_privsep.c#813

Figured out the issue. It's failing to create the P2P device.

02-08 14:30:20.200 4384 4384 D wpa_supplicant: P2P: Failed to create P2P Device interface
02-08 14:30:20.200 4384 4384 I wpa_supplicant: P2P: Failed to enable P2P Device interface

Angler is a wifi-direct device and much of the P2P driver functionality hasn't been implemented for privsep yet.

It's failing here: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/wpa_supplicant/p2p_supplicant.c#3736

Which leads to: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/wpa_supplicant/driver_i.h#399

The if_add() function is not implemented for privsep :(

The nl80211 driver operations currently supported by the privsep driver are a tiny subset of the available operations.

nl80211 ops: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/src/drivers/driver_nl80211.c#9098
privsep ops: https://android.googlesource.com/platform/external/wpa_supplicant_8/+/nougat-mr1-dev/src/drivers/driver_privsep.c#813

@thestinger thestinger added the upstream label Jan 20, 2017

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Jan 20, 2017

Contributor

That's too bad. I thought this was pretty much ready to go beyond wiring everything up.

Contributor

thestinger commented Jan 20, 2017

That's too bad. I thought this was pretty much ready to go beyond wiring everything up.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 20, 2017

I might try to add a couple of these operations to the privsep driver. If getting the whole thing working is too burdensome, I can still submit these to the upstream project.

I might try to add a couple of these operations to the privsep driver. If getting the whole thing working is too burdensome, I can still submit these to the upstream project.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Jan 22, 2017

For posterity, a non-comprehensive list of the driver commands still needed for privsep:

wpa_drv_get_ifname
wpa_driver_get_radio_name
wpa_drv_get_hw_feature_data
wpa_drv_set_countermeasures
wpa_drv_flush_pmkid
wpa_drv_set_supp_port
wpa_driver_privsep_set_wowlan
wpa_drv_set_operstate
wpa_drv_probe_req_report
wpa_drv_setband
wpa_drv_set_p2p_powersave
wpa_driver_privsep_deauthenticate
wpa_drv_abort_scan
wpa_drv_if_remove
wpa_drv_if_add

I'm sure some others are needed too.

For posterity, a non-comprehensive list of the driver commands still needed for privsep:

wpa_drv_get_ifname
wpa_driver_get_radio_name
wpa_drv_get_hw_feature_data
wpa_drv_set_countermeasures
wpa_drv_flush_pmkid
wpa_drv_set_supp_port
wpa_driver_privsep_set_wowlan
wpa_drv_set_operstate
wpa_drv_probe_req_report
wpa_drv_setband
wpa_drv_set_p2p_powersave
wpa_driver_privsep_deauthenticate
wpa_drv_abort_scan
wpa_drv_if_remove
wpa_drv_if_add

I'm sure some others are needed too.

@lucasduffey

This comment has been minimized.

Show comment Hide comment
@lucasduffey

lucasduffey Feb 19, 2017

The same wpa_supplicant binary is used for both wpa_supplicant and p2p_supplicant

If two distinct binaries were produced instead of one, the code could be simplified. I've been looking into this for a couple weeks and it seems like a pretty massive undertaking.

@jvanderstoep In wpa_supplicant's android.config do you know if the AP_CONFIG flag is really necessary? It ends up building a http server + xml parser into wpa_supplicant which seems like overkill. I'm pretty sure it could work for copperhead, but curious for core android. Also curious if p2p_supplicant is actually used for anything.

lucasduffey commented Feb 19, 2017

The same wpa_supplicant binary is used for both wpa_supplicant and p2p_supplicant

If two distinct binaries were produced instead of one, the code could be simplified. I've been looking into this for a couple weeks and it seems like a pretty massive undertaking.

@jvanderstoep In wpa_supplicant's android.config do you know if the AP_CONFIG flag is really necessary? It ends up building a http server + xml parser into wpa_supplicant which seems like overkill. I'm pretty sure it could work for copperhead, but curious for core android. Also curious if p2p_supplicant is actually used for anything.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Feb 19, 2017

On angler, only p2p_supplicant is ever used. Someone finally noticed this and removed the distinction in all devices on AOSP master, calling both wpa_supplicant (commit message pasted below). Basically if a device supports p2p, it now runs wpa_supplicant with the p2p flags. There's no need to distinguish between the two in the rc file.

AP mode is for when you're broadcasting your own mobile hotspot. I suspect that removing CONFIG_AP would break this feature.

commit 92ed7730c13b8f598944123592876ec57480b6cb
Author: Christopher Wiley wiley@google.com
Date: Thu Aug 11 15:38:29 2016 -0700

Do not define superfluous wpa_supplicant service

Bug: 30816535
Change-Id: Ib894268fb2ea044c37bd3eaa44d7289e24d75702
(cherry picked from commit d19c39841c33cb7cf14b3e5a3d3e3c167c97ad35)

On angler, only p2p_supplicant is ever used. Someone finally noticed this and removed the distinction in all devices on AOSP master, calling both wpa_supplicant (commit message pasted below). Basically if a device supports p2p, it now runs wpa_supplicant with the p2p flags. There's no need to distinguish between the two in the rc file.

AP mode is for when you're broadcasting your own mobile hotspot. I suspect that removing CONFIG_AP would break this feature.

commit 92ed7730c13b8f598944123592876ec57480b6cb
Author: Christopher Wiley wiley@google.com
Date: Thu Aug 11 15:38:29 2016 -0700

Do not define superfluous wpa_supplicant service

Bug: 30816535
Change-Id: Ib894268fb2ea044c37bd3eaa44d7289e24d75702
(cherry picked from commit d19c39841c33cb7cf14b3e5a3d3e3c167c97ad35)
@lucasduffey

This comment has been minimized.

Show comment Hide comment
@lucasduffey

lucasduffey Feb 19, 2017

yeah because there's also hostapd which I believe is intended for the mobile hotspot, but I could be mistaken. It looks like they share much of the same code.

Still need to look into it future, but the CONFIG_AP might be redundant for wpa_supplicant

I marked a few more potentially removable flags @ #595, but still testing.

lucasduffey commented Feb 19, 2017

yeah because there's also hostapd which I believe is intended for the mobile hotspot, but I could be mistaken. It looks like they share much of the same code.

Still need to look into it future, but the CONFIG_AP might be redundant for wpa_supplicant

I marked a few more potentially removable flags @ #595, but still testing.

@jvanderstoep

This comment has been minimized.

Show comment Hide comment
@jvanderstoep

jvanderstoep Feb 19, 2017

Yes, it's entirely possible that it's fully supported by hostapd and completely unnecessary in wpa_supplicant. Should be easy to test.

Yes, it's entirely possible that it's fully supported by hostapd and completely unnecessary in wpa_supplicant. Should be easy to test.

@thestinger

This comment has been minimized.

Show comment Hide comment
@thestinger

thestinger Aug 22, 2017

Contributor

I'm not going to consider this in scope for the time being because it's not as simple as I thought it would be and move of wpa_supplicant to vendor will also add an extra barrier for us to do it downstream since we may not be generating our own vendor.img anymore with Android O on Pixels, depending on how verity works with it now and other issues.

Contributor

thestinger commented Aug 22, 2017

I'm not going to consider this in scope for the time being because it's not as simple as I thought it would be and move of wpa_supplicant to vendor will also add an extra barrier for us to do it downstream since we may not be generating our own vendor.img anymore with Android O on Pixels, depending on how verity works with it now and other issues.

@thestinger thestinger closed this Aug 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment