-
-
Notifications
You must be signed in to change notification settings - Fork 17.6k
nixos/wpa_supplicant: harden and run as unprivileged user #427528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
abb06be
bf9788e
d7535dd
7871403
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,31 +48,6 @@ let | |
| else | ||
| networkList; | ||
|
|
||
| # Content of wpa_supplicant.conf | ||
| generatedConfig = concatStringsSep "\n" ( | ||
| (map mkNetwork allNetworks) | ||
| ++ optional cfg.userControlled.enable ( | ||
| concatStringsSep "\n" [ | ||
| "ctrl_interface=/run/wpa_supplicant" | ||
| "ctrl_interface_group=${cfg.userControlled.group}" | ||
| "update_config=1" | ||
| ] | ||
| ) | ||
| ++ [ "pmf=1" ] | ||
| ++ optional (cfg.secretsFile != null) "ext_password_backend=file:${cfg.secretsFile}" | ||
| ++ optional cfg.scanOnLowSignal ''bgscan="simple:30:-70:3600"'' | ||
| ++ optional (cfg.extraConfig != "") cfg.extraConfig | ||
| ); | ||
|
|
||
| configIsGenerated = with cfg; networks != { } || extraConfig != "" || userControlled.enable; | ||
|
|
||
| # the original configuration file | ||
| configFile = | ||
| if configIsGenerated then | ||
| pkgs.writeText "wpa_supplicant.conf" generatedConfig | ||
| else | ||
| "/etc/wpa_supplicant.conf"; | ||
|
|
||
| # Creates a network block for wpa_supplicant.conf | ||
| mkNetwork = | ||
| opts: | ||
|
|
@@ -104,6 +79,12 @@ let | |
| } | ||
| ''; | ||
|
|
||
| hasDeclarative = lib.any id [ | ||
| (cfg.networks != { }) | ||
| (cfg.extraConfig != "") | ||
| cfg.userControlled | ||
| ]; | ||
|
|
||
| # Creates a systemd unit for wpa_supplicant bound to a given (or any) interface | ||
| mkUnit = | ||
| iface: | ||
|
|
@@ -114,9 +95,11 @@ let | |
| configStr = | ||
| ( | ||
| if cfg.allowAuxiliaryImperativeNetworks then | ||
| "-c /etc/wpa_supplicant.conf -I ${configFile}" | ||
| "-c /etc/wpa_supplicant/imperative.conf -I /etc/wpa_supplicant/nixos.conf" | ||
| else if hasDeclarative then | ||
| "-c /etc/wpa_supplicant/nixos.conf" | ||
| else | ||
| "-c ${configFile}" | ||
| "-c /etc/wpa_supplicant/imperative.conf" | ||
| ) | ||
| + lib.concatMapStrings (p: " -I " + p) cfg.extraConfigFiles; | ||
| in | ||
|
|
@@ -128,32 +111,100 @@ let | |
| wants = [ "network.target" ]; | ||
| requires = deviceUnit; | ||
| wantedBy = [ "multi-user.target" ]; | ||
|
|
||
| stopIfChanged = false; | ||
| restartTriggers = [ config.environment.etc."wpa_supplicant/nixos.conf".source ]; | ||
|
|
||
| path = [ pkgs.wpa_supplicant ]; | ||
| # if `userControl.enable`, the supplicant automatically changes the permissions | ||
| # and owning group of the runtime dir; setting `umask` ensures the generated | ||
| # config file isn't readable (except to root); see nixpkgs#267693 | ||
| serviceConfig.UMask = "066"; | ||
| serviceConfig.RuntimeDirectory = "wpa_supplicant"; | ||
| serviceConfig.RuntimeDirectoryMode = "700"; | ||
| serviceConfig = { | ||
| User = "wpa_supplicant"; | ||
| Group = "wpa_supplicant"; | ||
| RuntimeDirectory = "wpa_supplicant"; | ||
| AmbientCapabilities = [ | ||
| "CAP_NET_ADMIN" | ||
| "CAP_NET_RAW" | ||
| ]; | ||
| CapabilityBoundingSet = [ | ||
| "CAP_NET_ADMIN" | ||
| "CAP_NET_RAW" | ||
| ]; | ||
| RootDirectory = "/run/wpa_supplicant"; | ||
| RootDirectoryStartOnly = true; | ||
| BindPaths = [ | ||
| "/etc/wpa_supplicant" # to write wpa_supplicant.conf{,.tmp} | ||
| "/run/wpa_supplicant" # to make control sockets | ||
| # to set up interfaces | ||
| "/proc/sys/net" | ||
| "/dev/rfkill" | ||
| ] | ||
| ++ lib.optional cfg.dbusControlled "/run/dbus" | ||
| ++ lib.optional cfg.allowAuxiliaryImperativeNetworks "/etc/wpa_supplicant"; | ||
fpletz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| BindReadOnlyPaths = [ | ||
| builtins.storeDir | ||
| "/etc/" | ||
| ] | ||
| ++ lib.optional (cfg.secretsFile != null) cfg.secretsFile; | ||
| DeviceAllow = "/dev/rfkill rw"; | ||
| LockPersonality = true; | ||
| MemoryDenyWriteExecute = true; | ||
| NoNewPrivileges = true; | ||
| PrivateDevices = true; | ||
rnhmjoj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| PrivateMounts = true; | ||
| PrivateTmp = true; | ||
| PrivateUsers = false; | ||
| ProtectClock = true; | ||
| ProtectControlGroups = true; | ||
| ProtectHome = true; | ||
| ProtectHostname = true; | ||
| ProtectKernelLogs = true; | ||
| ProtectKernelModules = true; | ||
| ProtectKernelTunables = true; | ||
| ProtectProc = "invisible"; | ||
| ProtectSystem = "strict"; | ||
| IPAddressDeny = "any"; | ||
| RemoveIPC = true; | ||
| RestrictAddressFamilies = [ | ||
| "AF_UNIX" | ||
| "AF_INET" | ||
| "AF_INET6" | ||
rnhmjoj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "AF_NETLINK" | ||
| "AF_PACKET" | ||
| ]; | ||
| RestrictNamespaces = true; | ||
| RestrictRealtime = true; | ||
| RestrictSUIDSGID = true; | ||
| SystemCallFilter = [ | ||
| "@system-service" | ||
| "~@keyring" | ||
| "~@resources" | ||
| ]; | ||
| SystemCallArchitectures = "native"; | ||
| UMask = "0077"; | ||
|
|
||
| ExecStartPre = | ||
| lib.optionals (cfg.allowAuxiliaryImperativeNetworks || !hasDeclarative) [ | ||
| # set up imperative config file | ||
| "+${pkgs.coreutils}/bin/touch /etc/wpa_supplicant/imperative.conf" | ||
| "+${pkgs.coreutils}/bin/chmod 664 /etc/wpa_supplicant/imperative.conf" | ||
| "+${pkgs.coreutils}/bin/chown -R wpa_supplicant:wpa_supplicant /etc/wpa_supplicant" | ||
rnhmjoj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ] | ||
| ++ lib.optionals cfg.userControlled [ | ||
| # set up client sockets directory | ||
| "+${pkgs.coreutils}/bin/mkdir /run/wpa_supplicant/client" | ||
| "+${pkgs.coreutils}/bin/chown wpa_supplicant:wpa_supplicant /run/wpa_supplicant/client" | ||
| "+${pkgs.coreutils}/bin/chmod g=u /run/wpa_supplicant/client" | ||
|
Comment on lines
+186
to
+195
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be done via tmpfiles instead?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I always see tmpfiles being recommended, but it's not a substitute for a privileged prestart script. Even if I add an
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is that not guaranteed? What should delete those files? If the user deletes them then that is their fault.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't know, but why would I increase the chance of failure when I can just set up everything needed right before starting the service? |
||
| ]; | ||
| }; | ||
|
|
||
| script = '' | ||
| ${optionalString (configIsGenerated && !cfg.allowAuxiliaryImperativeNetworks) '' | ||
| if [ -f /etc/wpa_supplicant.conf ]; then | ||
| echo >&2 "<3>/etc/wpa_supplicant.conf present but ignored. Generated ${configFile} is used instead." | ||
| fi | ||
| ''} | ||
|
|
||
| # ensure wpa_supplicant.conf exists, or the daemon will fail to start | ||
| ${optionalString cfg.allowAuxiliaryImperativeNetworks '' | ||
| touch /etc/wpa_supplicant.conf | ||
| ''} | ||
|
|
||
| iface_args="-s ${optionalString cfg.dbusControlled "-u"} -D${cfg.driver} ${configStr}" | ||
|
|
||
| ${ | ||
| if iface == null then | ||
| if iface != null then | ||
| '' | ||
| # add known interface to the daemon arguments | ||
| args="-i${iface} $iface_args" | ||
| '' | ||
| else if cfg.autoDetectInterfaces then | ||
| '' | ||
| # detect interfaces automatically | ||
|
|
||
|
|
@@ -176,10 +227,7 @@ let | |
| done | ||
| '' | ||
| else | ||
| '' | ||
| # add known interface to the daemon arguments | ||
| args="-i${iface} $iface_args" | ||
| '' | ||
| "args=$iface_args" | ||
| } | ||
|
|
||
| # finally start daemon | ||
|
|
@@ -205,7 +253,8 @@ in | |
| "wlan1" | ||
| ]; | ||
| description = '' | ||
| The interfaces {command}`wpa_supplicant` will use. If empty, it will | ||
| The interfaces {command}`wpa_supplicant` will use. If empty and | ||
| [](#opt-networking.wireless.autoDetectInterfaces) is true it will | ||
| automatically use all wireless interfaces. | ||
|
|
||
| ::: {.note} | ||
|
|
@@ -214,6 +263,10 @@ in | |
| ''; | ||
| }; | ||
|
|
||
| autoDetectInterfaces = mkEnableOption "automatic detection of wireless interfaces" // { | ||
| default = true; | ||
| }; | ||
|
|
||
| driver = mkOption { | ||
| type = types.str; | ||
| default = "nl80211,wext"; | ||
|
|
@@ -503,27 +556,36 @@ in | |
| ''; | ||
| }; | ||
|
|
||
| userControlled = { | ||
| enable = mkOption { | ||
| type = types.bool; | ||
| default = false; | ||
| description = '' | ||
| Allow normal users to control wpa_supplicant through wpa_gui or wpa_cli. | ||
| This is useful for laptop users that switch networks a lot and don't want | ||
| to depend on a large package such as NetworkManager just to pick nearby | ||
| access points. | ||
|
|
||
| When using a declarative network specification you cannot persist any | ||
| settings via wpa_gui or wpa_cli. | ||
| ''; | ||
| }; | ||
| userControlled = mkOption { | ||
| type = | ||
| with types; | ||
| coercedTo attrs ( | ||
| val: | ||
| if builtins.isAttrs val && val ? enable then | ||
| trace "Obsolete option `networking.wireless.userControlled.enable' is used. It was renamed to networking.wireless.userControlled" val.enable | ||
| else if builtins.isAttrs val && val ? group then | ||
| trace | ||
| "The option definition `networking.wireless.userControlled.group' no longer has any effect. The group is now fixed to `wpa_supplicant'." | ||
| (val.enable or false) | ||
| else if builtins.isBool val then | ||
| val | ||
| else | ||
| false | ||
| ) bool; | ||
| default = false; | ||
| description = '' | ||
| Allow users of the `wpa_supplicant` group to control wpa_supplicant | ||
| through wpa_gui or wpa_cli. | ||
| This is useful for laptop users that switch networks a lot and don't want | ||
| to depend on a large package such as NetworkManager just to pick nearby | ||
| access points. | ||
|
|
||
| group = mkOption { | ||
| type = types.str; | ||
| default = "wheel"; | ||
| example = "network"; | ||
| description = "Members of this group can control wpa_supplicant."; | ||
| }; | ||
| ::: {.note} | ||
| When networks are configured declaratively, you cannot persist any settings | ||
| via wpa_gui or wpa_cli, unless {option}`allowAuxiliaryImperativeNetworks` | ||
| is used. | ||
| ::: | ||
| ''; | ||
| }; | ||
|
|
||
| dbusControlled = mkOption { | ||
|
|
@@ -624,9 +686,33 @@ in | |
| } | ||
| ]; | ||
|
|
||
| users.groups.wpa_supplicant = { }; | ||
| users.users.wpa_supplicant = { | ||
| isSystemUser = true; | ||
| group = "wpa_supplicant"; | ||
| description = "WPA Supplicant user"; | ||
| }; | ||
|
|
||
| hardware.wirelessRegulatoryDatabase = true; | ||
|
|
||
| environment.systemPackages = [ pkgs.wpa_supplicant ]; | ||
|
|
||
| # NixOS-generated configuration files | ||
| environment.etc."wpa_supplicant/nixos.conf".text = concatStringsSep "\n" ( | ||
| (map mkNetwork allNetworks) | ||
| ++ optional cfg.userControlled ( | ||
| concatStringsSep "\n" [ | ||
| "ctrl_interface=/run/wpa_supplicant/control" | ||
| "ctrl_interface_group=wpa_supplicant" | ||
| "update_config=1" | ||
| ] | ||
| ) | ||
| ++ [ "pmf=1" ] | ||
| ++ optional (cfg.secretsFile != null) "ext_password_backend=file:${cfg.secretsFile}" | ||
| ++ optional cfg.scanOnLowSignal ''bgscan="simple:30:-70:3600"'' | ||
| ++ optional (cfg.extraConfig != "") cfg.extraConfig | ||
| ); | ||
|
|
||
| services.dbus.packages = optional cfg.dbusControlled pkgs.wpa_supplicant; | ||
|
|
||
| systemd.services = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to instead to rely on
systemd.services.wpa_supplicant.confinement?I remember that it was controversial when I wrote the Akkoma NixOS package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with this option: what's the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It creates a bind mount with only the store paths required to run the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store paths as in /nix/store paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My last touch with confinement was in postfix-tlspol where it was removed again because it is not easy to debug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. It does so by producing a systemd override configuration from the closure information with
BindReadOnlyPathdirectives for all required store paths: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/security/systemd-confinement.nix#L218I hacked together a similar thing for a systemd user service via Home Manager: ausweisapp.nix.txt
Instead of relying on
RootDirectorylike your proposal or the confinement module, it sets up an empty tmpfs withTemporaryFileSystem = "/:ro,nodev,noexec,nosuid", which I felt was cleaner.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a bit extreme. What's the benefit of locking down /nix/store? The only thing I can think of is preventing wpa_supplicant from stealing some world-readable "secret" that went into the store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the idea is not to hide confidential information but to reduce the set of executable code to the bare minimum required to operate the service.
Compared to other hardening features it may have a less favourable cost/benefit ratio, especially if run-time dependencies vary with configuration.