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.

Sign up for GitHub

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

Jump to bottom

spice-gtk: fix usb redirection #35214

Merged
merged 3 commits into from
Mar 4, 2018
Merged

spice-gtk: fix usb redirection #35214

merged 3 commits into from
Mar 4, 2018
+17 −5

Conversation

xeji
Copy link
Contributor

@xeji xeji commented Feb 20, 2018

Motivation for this change

Build spice-gtk with polkit and acl to enable usb redirection in virt-viewer and virt-manager.
Fixes #27199.
usb redirection requires a setuid wrapper, see comment in code.

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.

Sorry, something went wrong.

Mic92
Mic92 reviewed Feb 20, 2018
View reviewed changes
pkgs/development/libraries/spice-gtk/default.nix
# the helper then uses polkit to check access
# in nixos, enable this with
# security.wrappers.spice-client-glib-usb-acl-helper.source =
# "${pkgs.spice_gtk}/bin/spice-client-glib-usb-acl-helper.real";
Copy link
Member

Choose a reason for hiding this comment

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

This could become a nixos module later.

@Mic92
Copy link
Member

Mic92 commented Feb 20, 2018

@GrahamcOfBorg build spice-gtk

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Partial log (click to expand)

error: attribute ‘spice-gtk’ in selection path ‘spice-gtk’ not found

@xeji
Copy link
Contributor Author

xeji commented Feb 20, 2018

@Mic92 it's spice_gtk

@Mic92
Copy link
Member

Mic92 commented Feb 20, 2018

@GrahamcOfBorg build spice_gtk

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Partial log (click to expand)

error: attribute 'spice-gtk' in selection path 'spice-gtk' not found

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘spice-gtk-0.34’ in /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/lnl7-mac/pkgs/development/libraries/spice-gtk/default.nix:54 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Partial log (click to expand)

�[31;1merror:�[0m attribute 'spice-gtk' in selection path 'spice-gtk' not found

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

shrinking /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/bin/spice-client-glib-usb-acl-helper
shrinking /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/bin/spicy-screenshot
shrinking /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/lib/libspice-controller.so.0.0.0
shrinking /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/lib/libspice-client-gtk-3.0.so.5.0.0
shrinking /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/lib/libspice-client-glib-2.0.so.8.6.0
gzipping man pages under /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/share/man/
strip is /nix/store/adidfx4pa7vmvby0gjqqmiwg2x49yr27-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/lib  /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34/bin 
patching script interpreter paths in /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34
checking for references to /tmp/nix-build-spice-gtk-0.34.drv-0 in /nix/store/k30jhwrlayqb2ll1ni8kr7r1ra6bd0zn-spice-gtk-0.34...

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

shrinking /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/bin/spice-client-glib-usb-acl-helper
shrinking /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/lib/libspice-client-gtk-3.0.so.5.0.0
shrinking /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/lib/libspice-client-glib-2.0.so.8.6.0
shrinking /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/lib/libspice-controller.so.0.0.0
gzipping man pages under /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/share/man/
strip is /nix/store/skd6ix5ipkyhxzq7naylj4digawakl4j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/lib  /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34/bin
patching script interpreter paths in /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34
checking for references to /build in /nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34...
/nix/store/vdnnyc8q2g6h0a9qzayy8x4746amnl9c-spice-gtk-0.34

xeji added 3 commits March 4, 2018 17:47
@xeji
spice-gtk: clean up unnecessary code
14c6c28
@xeji
spice-gtk: fix usb redirection
1894a2a 
Build with polkit and acl to enable usb redirection
in virt-viewer and virt-manager. Fixes NixOS#27199
usb redirection requires a setuid wrapper, see comment in code.
@xeji
spice-gtk: set maintainer (previously none)
54b432f
@xeji xeji force-pushed the spice-gtk-usbredir branch from 85863c8 to 54b432f Compare March 4, 2018 16:52
@xeji
Copy link
Contributor Author

xeji commented Mar 4, 2018

rebased on current master

@fpletz fpletz merged commit d6c4788 into NixOS:master Mar 4, 2018
@xeji xeji deleted the spice-gtk-usbredir branch March 4, 2018 20:08
@xeji xeji mentioned this pull request Mar 9, 2018
Closed
@d4g
Copy link
Contributor

d4g commented Mar 15, 2018 •
edited
Loading

I would actually suggest to resolve this not via suid wapper, but with an option to generate an appropriate group for usb devices and authorize users to access them. suid seems a little bit of an overkill, just for USB access?

My configuration:

  # add new group for usb
  users.groups = { usb = {}; };

 # let all usb devices be in the usb group
  services.udev.extraRules = ''
    KERNEL=="*", SUBSYSTEMS=="usb", MODE="0664", GROUP="usb"
  '';

@xeji
Copy link
Contributor Author

xeji commented Mar 15, 2018

While this approach will work, it does statically give all such users access to all usb devices, whether they are currently in an active session or not - which is fine in a single- or trusted-user environment.

The upstream solution follows a more dynamic approach: it uses spice-client-glib-usb-acl-helper (the setuid program), which confirms via polkit that the user has an active session, then adds a device acl entry for that user.

Anyway, you can activate either approach by manual config (yes, this needs documentation).
But if we do provide a NixOS option for convenience, I recommend the upstream setuid+pokit solution because it is closer to "least privilege".

@d4g
Copy link
Contributor

d4g commented Mar 15, 2018

Ok, makes sence.
Issue #37110 might also be related.

@xeji
Copy link
Contributor Author

xeji commented Mar 15, 2018

Thanks for the hint, looks related indeed.

@xeji xeji mentioned this pull request Mar 16, 2018
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@xeji @Mic92 @GrahamcOfBorg @d4g @fpletz