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

nixos/xss-lock: specify a default locker #53404

Merged
merged 1 commit into from Jan 5, 2019
Merged

nixos/xss-lock: specify a default locker #53404

merged 1 commit into from Jan 5, 2019

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Jan 4, 2019

Having a default locker is less error-prone and more convenient.
Incorrect values might leave the machine vulnerable since there is no
fallback.

Motivation for this change
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.

@Mic92 Mic92 requested a review from infinisil as a code owner January 4, 2019 14:45
@Mic92 Mic92 requested a review from Ma27 January 4, 2019 14:45
@Mic92
Copy link
Member Author

Mic92 commented Jan 4, 2019

test is still building.

@Mic92
Copy link
Member Author

Mic92 commented Jan 4, 2019

Test works!

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Admittedly I was unsure what a sensitive default is in this case when I wrote this module as I don't know enough about X internals and how locking works (and if there are differences between desktop environments).

The implementation seems fine though, so if this is a sensitive default I'm totally 👍 for merging.

@xeji
Copy link
Contributor

xeji commented Jan 4, 2019

I support the idea of setting a default screen locker. But i'm not sure i3lock-fancy is a good default choice since it has a rather large closure size. Here's nix path-info -S closure size for some screen locker packages:

slock-1.4              29379808
xlockmore-5.56         45961680
i3lock-2.10            70629384
i3lock-fancy-unstable-2017-12-14_rev3734fba   110151208
xsecurelock-1.1       433157808

@Mic92
Copy link
Member Author

Mic92 commented Jan 4, 2019

@xeji what the is the compromise between UX and closure-size? The fancy one could go to the example section.

@xeji
Copy link
Contributor

xeji commented Jan 5, 2019

what the is the compromise between UX and closure-size?

I would recommend non-fancy i3lock as default.
slock (my personal favorite) is very light but it's a suckless.org tool - "to configure, edit config.h and recompile" - probably not what most users want.
xlockmore looks and feels a little outdated.

Having a default locker is less error-prone and more convenient.
Incorrect values might leave the machine vulnerable since there is no
fallback.
@Mic92 Mic92 merged commit 8a2389e into NixOS:master Jan 5, 2019
@Mic92 Mic92 deleted the xsslock branch January 5, 2019 15:44
@Mic92
Copy link
Member Author

Mic92 commented Jan 20, 2019

In case someone is interested. I also added the dim-screen script as described in the manpage, which will dim the screen before locking it: https://github.com/Mic92/dotfiles/blob/bb297e471a52c3feba6756c857c72db6bbc5d208/nixos/vms/modules/xss-lock.nix#L14

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

Successfully merging this pull request may close these issues.

None yet

4 participants