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

Whonix /etc/sudoers.d exceptions wildcard / asterix * hardening #2852

Closed
adrelanos opened this issue Jun 8, 2017 · 11 comments
Closed

Whonix /etc/sudoers.d exceptions wildcard / asterix * hardening #2852

adrelanos opened this issue Jun 8, 2017 · 11 comments
Labels
C: Whonix This issue impacts Qubes-Whonix
Milestone

Comments

@adrelanos
Copy link
Member

Whonix uses * in a few /etc/sudoers.d files.

Quote @marmarek #2695 (comment)

As for actual sudo configuration - I see several files in /etc/sudoers.d have commands with * as argument. If in practice the command is called only with one or two specific arguments, IMO it's better to be verbose here.

TODO: harden this. Don't use * wherever possible.

find . -type f | grep sudoers | xargs cat | grep '*'
sdwdate ALL=NOPASSWD: /bin/date *
sdwdate ALL=NOPASSWD: /usr/lib/sdwdate/sclockadj *
sdwdate ALL=NOPASSWD: /sbin/hwclock *
sdwdate ALL=NOPASSWD: /usr/lib/sdwdate/sclockadj_kill_helper *
ALL ALL=NOPASSWD: /usr/lib/sdwdate/restart_fresh *
user ALL=NOPASSWD: /usr/bin/clock-random-manual-cli *
user ALL=NOPASSWD: /usr/bin/clock-random-manual-gui *
%sudo ALL=NOPASSWD: /usr/bin/whonix-setup-wizard *
%sudo ALL=NOPASSWD: /usr/lib/whonix-setup-wizard/whonix-setup-wizard-setup *
#tunnel ALL=(ALL) NOPASSWD: /usr/sbin/openvpn *
%sudo ALL=NOPASSWD: /usr/bin/whonixsetup *
user ALL=(whonixcheck) NOPASSWD: /usr/lib/whonixcheck/whonixcheck *
user ALL=(whonixcheck) NOPASSWD: /bin/bash -x /usr/lib/whonixcheck/whonixcheck *
ALL ALL=NOPASSWD: /usr/lib/msgcollector/msgprogressbar_kill_helper *

Do you think you could work on this? @tasket @crat0z

@andrewdavidwong andrewdavidwong added C: Whonix This issue impacts Qubes-Whonix enhancement labels Jun 8, 2017
@andrewdavidwong andrewdavidwong added this to the Release 4.0 milestone Jun 8, 2017
@tasket
Copy link

tasket commented Jun 8, 2017

I can start looking at a few of the commands to see how they're used in Whonix under core use cases such as 'browse web site' and 'update system'. Just want to say that Whonix is not my strong suit, though.

Tightening these entries means for some commands like date, specifying additional parameters to narrow the modes of use while still leaving * in there or whatever EBNF syntax allows. Obviously, it can't be completely nailed-down if it accepts data to set the system time, for example.

Optimal configuration should take into account whether the command is used in a purely automatic function or user-initiated. With the former, system startup and cron should not be an issue because as a general rule these are run as root user. With the latter, it may be best to let the system trigger an auth prompt in certain cases.

Whether a user-initiated function is considered an admin tool or just an app (like Torbrowser) also matters; the latter type should be usable without any auth prompts. OTOH, admin functions launched from dom0 menu could simply specify a privileged user like root to avoid auth prompts.

@tasket
Copy link

tasket commented Jul 15, 2017

Initial look at sdwdate:

These rules are specified by the sdwdate documentation for using this service as "non-root user". Its precarious to second-guess Patrick's design; normally I would say this is a security issue internal to the tool. I would leave most of those (first 7) rules as-is, or re-factor sdwdate in some way such as running only the HTTP part as unpriv user.

Apart from that, the line referencing '/usr/lib/sdwdate/restart_fresh' is a script that takes no parameters anyway, so '*' can be removed there.


For whonix-setup-wizard, I think these lines

%sudo ALL=NOPASSWD: /usr/bin/whonix-setup-wizard *
%sudo ALL=NOPASSWD: /usr/lib/whonix-setup-wizard/whonix-setup-wizard-setup *

...can be changed to

%sudo ALL=NOPASSWD: /usr/bin/whonix-setup-wizard
%sudo ALL=NOPASSWD: /usr/bin/whonix-setup-wizard locale_settings
%sudo ALL=NOPASSWD: /usr/lib/whonix-setup-wizard/whonix-setup-wizard-setup

However, the script '/usr/bin/whonixsetup' appears to accept multiple 'command' options. Perhaps @adrelanos could elaborate on that.

@adrelanos
Copy link
Member Author

adrelanos commented Aug 28, 2017 via email

@tasket
Copy link

tasket commented Oct 31, 2017

FYI this is still on my todo list, I just want to get to VPN and template-reinstall fixes first.

adrelanos pushed a commit to Kicksecure/sdwdate that referenced this issue Aug 8, 2018
adrelanos pushed a commit to Kicksecure/sdwdate that referenced this issue Aug 8, 2018
adrelanos pushed a commit to Kicksecure/msgcollector that referenced this issue Aug 9, 2018
@adrelanos
Copy link
Member Author

The use of asterix * in /etc/sudoers.d was considerably reduced in Whonix 14 / git master.

find . -type f | grep sudoers | xargs cat | grep '*'
sdwdate ALL=NOPASSWD: /bin/date --set *
sdwdate ALL=NOPASSWD: /usr/lib/sdwdate/sclockadj *
#tunnel ALL=(ALL) NOPASSWD: /usr/sbin/openvpn *
user ALL=(whonixcheck) NOPASSWD: /usr/lib/whonixcheck/whonixcheck *
user ALL=(whonixcheck) NOPASSWD: /bin/bash -x /usr/lib/whonixcheck/whonixcheck *

Suggestions on how to get rid of the remaining ones?

@tasket
Copy link

tasket commented Aug 9, 2018

The easier ones should be whonixcheck and openvpn.

My first inclination is to disable the whonixcheck lines and then investigate where/why its being run from an unprivileged 'user' context.

Suggestion for openvpn is to specify a static --config file.ovpn option pointing to a file in a privileged dir like /etc/openvpn; this still allows the user flexibility in config options (they can put anything in file.ovpn) while closing the sudoers loophole.

adrelanos pushed a commit to adrelanos/whonixcheck that referenced this issue Aug 10, 2018
adrelanos pushed a commit to adrelanos/sdwdate that referenced this issue Aug 10, 2018
- quote sclockadj shell command
- quote /bin/date shell command
- output sclockadj command in log

QubesOS/qubes-issues#2852
@adrelanos
Copy link
Member Author

whonixcheck is sorted. Let's look into sdwdate / sclockadj. Two ideas:

  • A) Would it help to write the time (unixtime, just numbers) for /bin/date into a text file, add a shell wrapper that reads the argument from the text file? (Not sure writing to a text file would significantly mess up accuracy.) [We introduce hypothetical vulnerabilities by bash / file parsing.]
  • B) Would it help to call a shell wrapper from sdwdate like echo "1533921121.245112658" | sudo /usr/lib/sdwdate/date-wrapper [We introduce hypothetical vulnerabilities by bash / stdout parsing.]

/usr/lib/sdwdate/date-wrapper:

#!/bin/bash
date --set "@$1"

Then sudoers could be as simple as:

sdwdate ALL=NOPASSWD: /usr/lib/sdwdate/date-wrapper

  • C) Using sudoers.d supports shell-style wildcards. [We introduce hypothetical vulnerabilities by sudo's wildcards parsing code.]

sdwdate log:
Aug 10 17:15:03 work sdwdate[29542]: 2018-08-10 17:15:03 - sdwdate - INFO - Instantly setting the time by using command: sudo --non-interactive /bin/date --set "@1533921121.245112658"
/etc/sudoers.d/sdwdate:

sdwdate ALL=NOPASSWD: /bin/date --set *

I am not good at regex.

/bin/date: Could anyone provide a regex for @1533921121.245112658?

(An @ sign followed by 10 digits followed by . followed by 9 digits.)


Aug 10 17:28:08 work sdwdate[636]: 2018-08-10 17:28:08 - sdwdate - INFO - Gradually adjusting the time by running sclockadj using command: sudo --non-interactive /usr/lib/sdwdate/sclockadj "-24123423912"

sclockadj: Could anyone provide a regex for 24123423912?
sclockadj: Could anyone provide a regex for -24123423912?

(Maybe a - followed by "20" digits.)

(Probably not "20" but I would have to think about a good maximum and modify sdwdate accordingly.)

@adrelanos adrelanos changed the title Whonix /etc/sudoers.d exceptions asterix * hardening Whonix /etc/sudoers.d exceptions wildcard / asterix * hardening Aug 10, 2018
adrelanos pushed a commit to adrelanos/sdwdate that referenced this issue Aug 10, 2018
improve sclockadj parameter parsing

check output and exit code of /bin/date in case it fails due to sudoers.d hardening

QubesOS/qubes-issues#2852
@marmarek
Copy link
Member

marmarek commented Aug 30, 2018 via email

adrelanos pushed a commit to adrelanos/sdwdate that referenced this issue Sep 9, 2018
@adrelanos
Copy link
Member Author

In what context is it callled? Is it only sdwdate service running as unprivileged user?

Yes.

Maybe adding CAP_SYS_TIME to its systemd unit file is enough and no root access is needed at all?

Great suggestion, implemented!


The use of asterix / wildcard * in /etc/sudoers.d was elimited in Whonix git master.

#tunnel ALL=(ALL) NOPASSWD: /usr/sbin/openvpn *

The remaining one is a comment / documentation issue (out commented by default) and should be fixable as well.

@adrelanos
Copy link
Member Author

This looks really good in Whonix 15 (still).

find . -type f | grep sudoers | xargs cat | grep '*'

The following is a comment only.

#tunnel ALL=(ALL) NOPASSWD: /usr/sbin/openvpn *

Ideally it should get fixed too.

So no default sudo wildcard / asterix * in Whonix 15.

@adrelanos
Copy link
Member Author

This is good enough.

All solved. Except for that comment.

But even then I think it is OK. Documentation issue. User tunnel needs to be able to run openvpn with any combination of commands. Would be hard to restrict that and needs to be left as an exercise for the user. Added a documentation comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Whonix This issue impacts Qubes-Whonix
Projects
None yet
Development

No branches or pull requests

4 participants