Skip to content
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

usbmuxd service: init #30518

Merged
merged 2 commits into from Jan 6, 2018
Merged

usbmuxd service: init #30518

merged 2 commits into from Jan 6, 2018

Conversation

infinisil
Copy link
Member

Motivation for this change

The usbmuxd daemon is required for accessing files on connected iOS devices (along with ifuse). Also apparently enabling it allows Plug-and-Play tethering for iPhones, which is awesome.

This was tested by me and @grahamc, but it would be nice to have others try it out. Just run sudo usbmuxd -f in a terminal and connect your cellular iOS device, should work just like that.

Since this easily provides internet access, it would be very handy to enable this in the installer media by default, I'm probably gonna do a PR for that after this is merged.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.


config = mkIf config.services.usbmuxd.enable {
systemd.services.usbmuxd = {
description = "usbmuxd";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to run as root? Have you tested your system with this module enabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this module, using it on my system (with unstable).

I tried to make it work with non-root, that is by having it being started by non-root, which gave me the problem that it wants to create /var/run/usbmuxd.pid (even when run in foreground), which isn't possible as non-root afaik. It complains if the file exists already and the location is hardcoded, would require patching to change it.

Now what I haven't tried is using its -U <user> which changes to the given user after startup, but since it still has root permissions at start anyways and the additional effort to create a user I decided it's not worth it. On second thought, -U would be a good idea, since the real danger of it isn't in usbmuxd itself, but in the devices you connect, which only happens after startup. I'll have a look at it again later today.

Note: User needs to be in disk group.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could actually make it work. 3 things are needed:

  1. Add RuntimeDirectory = "usbmuxd" to the service definition

  2. Patch src/main.c to change the path to the pid from /var/run/usbmuxd.pid to /run/usbmuxd/usbmuxd.pid.

  3. Patch src/main.c to change the path to the socket.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really worth adding a patch (and I'd think this shouldn't be applied to the package itself) just so the service can run as non-root for startup though? It already switches to the given user after startup with the -U option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that it's in our interest to make NixOS as safe as possible (within reason obviously) even if that means patching upstream to act more sane. Especially since the patching is just 2 sed invocations.

@infinisil
Copy link
Member Author

I refactored the service to use a custom user and group. I tested this extensively with 2 iOS devices and had no problems

serviceConfig = {
# Trigger the udev rule manually. This doesn't require replugging the
# device when first enabling the option to get it to work
ExecStartPre = "${pkgs.libudev}/bin/udevadm trigger -s usb -a idVendor=${apple}";
Copy link
Member Author

@infinisil infinisil Oct 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect of this is quiet small, but I think it's worth it for the additional magic: It makes the iOS device connect automatically when the device is connected already upon enabling this service or cfg.group is changed.

'';
};

user = mkOption {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need to have user+group configurable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usbmuxd does not create any files in the filesystem (other than a socket and a pid file at runtime). The reason I added a user option is because usbmuxd has a specific argument for that (-U). I don't think there's any point in making the group configurable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try it out with DynamicUser = true; then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work for the same reason as mentioned here: #30518 (comment)

@infinisil
Copy link
Member Author

I tried to do a patch for it (to not require to be run as root) here infinisil@33d7f1c, but it fails when connecting a device with

preflight_worker_handle_device_add: ERROR: Could not connect to lockdownd on device XXXXXXXXXXXXXXXXXXXXXXXXXX, lockdown error -8

@infinisil
Copy link
Member Author

Well I don't know how to make it work without starting as root. Can be merged like this imo, the service could be really useful for some people.

@grahamc grahamc merged commit 013580c into NixOS:master Jan 6, 2018
@infinisil infinisil deleted the usbmuxd-service branch April 26, 2022 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants