create_ap: init at 0.4.6

[klntsky]

Motivation for this change

create_ap.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Assured whether relevant documentation is up to date
• Fits CONTRIBUTING.md.

---

Additionally, I've included a systemd service which provides a nix interface to some of the most frequently used configurable options (the other can also be set via extraConfig).

I have tested the following:

• AP creation
  • without passphrase
  • with passphrase
  • SHARE_METHOD
    • nat
    • bridge
    • none
  • ISOLATE_CLIENTS
  • HIDDEN
• dnsmasq-related
  • ETC_HOSTS
  • additional hosts (-e flag)
  • MAC_FILTER / MAC_FILTER_ACCEPT
• usage without network manager
  • AP creation (no need to retest all the options above, because nmcli is used only to set/unset the unmanaged status)
• other
  • --list-running / --list-clients / --stop

[infinisil]

Try using the DynamicUser systemd option and maybe some others regarding privileges (man systemd.exec) to be able to run this without root privileges.

[klntsky]

Sadly, it isn't quite easy: the script checks the user id.

if [[ $(id -u) -ne 0 ]]; then
    echo "You must run it as root." >&2
    exit 1
fi

https://github.com/oblique/create_ap/blob/016fd2b38f87d56344b87b588a6c887628884482/create_ap#L1270

I can dedicate some more time to patching it, though, if running as a separate user is required.

[infinisil]

Ah I see, in this case I think root is fine, but the security related systemd settings would be very much appreciated (ProtectSystem, ProtectHome and the Private* settings, see man systemd.exec).

If patching is simple and it works pretty much out of the box with non-root, then that would be even better. We're trying not to have services run as root in general after all, see #41092

What do you think would be better?

[klntsky]

We're trying not to have services run as root in general after all

I'm all for this and I am sure that it is possible - it's just that I have had not enough experience with systemd to get it working right now.

Please wait until I finally figure out how to set it up with DynamicUser on.

[klntsky]

One more thing to consider: the script is able to run and control multiple instances of itself in parallel (i.e. via --list-running / --stop).

But when started as a service in its current implementation, only one instance is possible.

Maybe it is a good idea to replace the single instance configuration with a list of them. Then it will be possible to define multiple configs and thus create multiple access points:

  services.create_ap = [
  { enable = true;  ... }
  { enable = true;  ... }
  { enable = true;  ... }
  ];

Or maybe it is even better to handle two cases: when config.services.create_ap is a list and when it is an attribute set.

[klntsky]

I have tested the following:

• AP creation
  • without passphrase
  • with passphrase
  • SHARE_METHOD
    • nat
    • bridge
    • none
  • ISOLATE_CLIENTS
  • HIDDEN
• dnsmasq-related
  • ETC_HOSTS
  • additional hosts (-e flag)
  • MAC_FILTER / MAC_FILTER_ACCEPT
• usage without network manager
  • AP creation (no need to retest all the options above, because nmcli is used only to set/unset the unmanaged status)
• other
  • [x] --list-running / --list-clients / --stop (does not work anymore)

[klntsky]

Now I need some time to check all possible usage scenarios with DynamicUser again.

[klntsky]

Without CAP_DAC_OVERRIDE it can't set unmanaged status to devices owned by NetworkManager, because of read-only file system.

If the device is already unmanaged, then everything's OK - that's why I caught this problem that late.

Strangely enough, ReadWritePaths = "-/etc/NetworkManager/" line that was added earlier didn't help (I was thinking that it did).

[infinisil]

Try to do an strace of the process to see which path it tries to write to.

[klntsky]

sed -i tries to create a temporary file.

It can be observed even from status output:

Jan 28 23:18:29 nixos create_ap[16451]: Network Manager found, set wlp2s0f0 as unmanaged device... sed: couldn't open temporary file /etc/NetworkManager/sedW6bpNK: Permission denied

# strace -f -o /tmp/strace.log -s 2048 -p 1 & systemctl start create_ap

21764 openat(AT_FDCWD, "/etc/NetworkManager/sedd6IF0R", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EROFS (Read-only file system)

/etc/NetworkManager/ is in ReadWritePaths at the moment.

[klntsky]

Well, everything works as it should:

Paths listed in ReadWritePaths= are accessible from within the namespace with the same access modes as from outside of it.

It is impossible to modify files in /etc/ while under DynamicUser without CAP_DAC_OVERRIDE.

[ryantm]

@8084 Can you please change the commit message to match https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md "init" and "create_ap" are flipped

[klntsky]

@ryantm OK, I'll --squash and force-push the commit with correct message after I'm done.

[infinisil]

Now that I've opened NixOS/rfcs#42, I'm not so sure anymore about this anymore. Because once we introduce such hardcoded defaults and extraConfig, it's hard to go back. Consider using the approach in the RFC instead.

[deliciouslytyped]

bump? I'm interested in this.

[infinisil]

@klntsky Would you mind if I pushed a commit to this PR to make it use the approach in NixOS/rfcs#42?

[klntsky]

@infinisil I definitely do not mind, thank you.

I don't have enough time myself to work on this PR, sadly.

[infinisil]

Cool, did that, somebody should test this again before merging though, I could try with my own laptop later perhaps, not sure if it can do it though

[ryantm]

@klntsky Can you test the latest version?

[stale]

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
2. Ask on the NixOS Discourse.
3. Ask on the #nixos channel on irc.freenode.net.

[klntsky]

As for now, I believe it's best to drop the service entirely and provide the binary only, due to lack of human powers to wrap it properly.