Skip to content

wip: no root for wpa-supplicant#305722

Closed
tomfitzhenry wants to merge 1 commit intoNixOS:masterfrom
tomfitzhenry:wpa-supplicant-non-root
Closed

wip: no root for wpa-supplicant#305722
tomfitzhenry wants to merge 1 commit intoNixOS:masterfrom
tomfitzhenry:wpa-supplicant-non-root

Conversation

@tomfitzhenry
Copy link
Contributor

@tomfitzhenry tomfitzhenry commented Apr 21, 2024

Description of changes

WIP running of wpa_supplicant using Linux capabilities rather than as a root process, as documented in https://w1.fi/cgit/hostap/tree/wpa_supplicant/README#n982 . The motivation is to reduce the privilege that this process has, in case it's exploited.

Tests are passing, but there's a few problems:

  • the patch is a bit gross
  • wpa_cli fails when executed as root
  • Would be nice if this was opt-in for now, if only because wpa_supplicant has all sorts of integrations (e.g. NetworkManager) that this might broken in esoteric hard-to-test ways.

@rnhmjoj thoughts on whether this change is desirable?

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 21, 2024
@tomfitzhenry tomfitzhenry requested a review from rnhmjoj April 21, 2024 11:56
@ofborg ofborg bot requested review from Ma27 and MarcWeber April 21, 2024 12:18
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 21, 2024
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Apr 21, 2024

@rnhmjoj thoughts on whether this change is desirable?

Of course it's desirable. Ideally no userspace service should run as root give that POSIX capabilities exist.
I would go even further and try to harder it as I'm doing in #276919 for dhcpcd.

the patch is a bit gross

Nah, we're doing much worst with wpa_supplicant already. See the AllowAuxilliaryImperativeNetworks patch.

wpa_cli fails when executed as root

This is strange... there must be something going on with the permissions on the wpa_supplcicant socket file.

Would be nice if this was opt-in for now, if only because wpa_supplicant has all sorts of integrations (e.g. NetworkManager) that this might have broken.

Well, there's no hurry: this will certainly have to wait for the 24.05 branch off.
In any case, we would need someone to test Networkmanager because, AFAIK, it's unmaintained in NixOS and I've never used it.

@tomfitzhenry
Copy link
Contributor Author

wpa_cli fails when executed as root

I had a look more into this, by adding verbose flags -dd to wpa_supplicant. wpa_cli -i wlan1 status as root causes wpa-supplicant (running as user wpa-supplicant) to print:

basic # [  890.773404] wpa_supplicant[661]: EAPOL: EAP Session-Id not available
basic # [  890.776694] wpa_supplicant[661]: CTRL-DEBUG: ctrl_sock-sendto: sock=15 sndbuf=212992 outq=0 send_len=124
basic # [  890.777858] wpa_supplicant[661]: wlan1: ctrl_iface sendto failed: 13 - Permission denied

https://stackoverflow.com/a/60424409 suggests wpa_supplicant replies on a client-provided socket. Here's the code: https://w1.fi/cgit/hostap/tree/wpa_supplicant/ctrl_iface_unix.c#n205 .

So what looks to be happening is that:

  • wpa_cli (as root) will create a socket, passing its filename when communicating over wpa_supplicant's ctrl socket
  • wpa_supplicant (running as wpa-supplicant) tries to write to wpa_cli's reply socket, but fails to lack of access

This makes sense: wpa-supplicant likely doesn't have access to the socket that wpa_cli (a private file owned by root?).

I'm not sure what the expected working behaviour is here, to have wpa_cli work with an unprivileged wpa_supplicant.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Apr 25, 2024

This makes sense: wpa-supplicant likely doesn't have access to the socket that wpa_cli (a private file owned by root?).

Yeah, strange design choice, though.
We could patch wpa_cli to create the socket with the right permissions for wpa_supplicant, but I don't think it's worth it. Still, as a breaking change it will have to be documented in the release notes.

@tomfitzhenry
Copy link
Contributor Author

tomfitzhenry commented Apr 25, 2024

I looked more into this. First, a bit of background on Unix domain datagram sockets.

wpa_cli and wpa_supplicant use Unix domain SOCK_DGRAM sockets for their query-response type control interface. (Perhaps they use SOCK_DGRAM rather than SOCK_STREAM to benefit from SOCK_DGRAM's perserving message boundaries, making it easy to write a simple query-response protocol.)

Per https://man7.org/linux/man-pages/man7/unix.7.html , wpa_cli must bind its socket to a filepath to receive replies. wpa_cli does that, binding to /tmp/wpa_ctrl-$PID-$COUNTER.

Per "Pathname socket ownership and permissions" of that man page, this socket must be writable by wpa_supplicant. wpa_cli doesn't change ownership of the socket it creates, so in this case, the socket is owned by root:root, and not writably by others, resulting in wpa_supplicant (running as use wpa-supplicant) being unable to send replies to wpa_cli.

I'm not sure what the expected working behaviour is here, to have wpa_cli work with an unprivileged wpa_supplicant.

AFAICT, wpa_cli isn't currently designed to work with an unprivileged wpa_supplicant (unless wpa_cli and wpa_supplicant run as the same user, as this PR's tests do). wpa_cli uses wpa_ctrl.c, which only does the necessary fchmod/lchown when #ifdef ANDROID: https://w1.fi/cgit/hostap/tree/src/common/wpa_ctrl.c?id=1dda619ed291edddf979d4513ddc59abf0a30c9e#n94 . I guess Android runs unprivileged wpa_supplicant , but doesn't use wpa_cli.

To ensure users of wpa_cli are able to interact with unprivileged wpa_supplicant, I think wpa_cli would have to be changed to fchmod/lchown to g+rw and group wpa-supplicant. That'd require some upstream changes.

Where does that leave us?

Well I'm not sure of the full implications of only allowing execution of wpa_cli via sudo -u wpa-supplicant, esp. wrt NetworkManager.

I'll try to find some time to get this fixed upstream.

References:

@tomfitzhenry
Copy link
Contributor Author

This isn't a bug. If anything, this is a wpa_supplicant feature request, and the NixOS issue tracker isn't the place for that. Closing.

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

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants