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

Numerous sudo requirements, even if root #322

Open
stevelitt opened this issue Oct 2, 2018 · 24 comments
Open

Numerous sudo requirements, even if root #322

stevelitt opened this issue Oct 2, 2018 · 24 comments
Labels
enhancement An issue to improve current behavior

Comments

@stevelitt
Copy link

For user slitt to successfully dig, forge, lock, open and close, he needs all the following in sudoers:

slitt ALL=(root:ALL) /usr/bin/tomb
slitt ALL=(root:ALL) /usr/bin/losetup
slitt ALL=(root:ALL) /usr/bin/cryptsetup
slitt ALL=(root:ALL) /usr/bin/fsck
slitt ALL=(root:ALL) /usr/bin/tune2fs
slitt ALL=(root:ALL) /usr/bin/mkdir
slitt ALL=(root:ALL) /usr/bin/mount
slitt ALL=(root:ALL) /usr/bin/rmdir
slitt ALL=(root:ALL) /usr/bin/mkfs.ext4
slitt ALL=(root:ALL) /usr/bin/findmnt

I didn't see this list documented anywhere: It should be.

More unexpected, doing it as user root fails unless the following goes in sudoers:
root ALL=(root:ALL) /usr/bin/tomb
root ALL=(root:ALL) /usr/bin/losetup
root ALL=(root:ALL) /usr/bin/cryptsetup
root ALL=(root:ALL) /usr/bin/fsck
root ALL=(root:ALL) /usr/bin/tune2fs
root ALL=(root:ALL) /usr/bin/mkdir
root ALL=(root:ALL) /usr/bin/mount
root ALL=(root:ALL) /usr/bin/rmdir
root ALL=(root:ALL) /usr/bin/mkfs.ext4
root ALL=(root:ALL) /usr/bin/findmnt

Most folks wouldn't expect to need to give user root sudo rights: They'd assume if they're root they can go ahead and do it. And I'm pretty sure it would be easy to make that true. Most of the sudoing in tomb are done by _sudo(), so you can simply put a test in _sudo() to test for user root, and if user root simply execute the command without sudo.

There are a few places where actual sudo is used instead of _sudo(). These should be replaced by something like chksudo() which checks for the user being root, and if so, execute directly, if not, sudo it.

These are pretty important points, because what I found on my computer was that if some but not all of the necessary commands were in sudoers, things might appear to work and fail at a different point. I had hell to pay during the time I didn't have mkfs.ext4 in sudoers.

I hope you can check for root, and improve the docs to include the whole list. The man page claims all yo u need to do is:

username ALL=NOPASSWD: /usr/local/bin/tomb

But my experience tells me I need to do that for every command that needs privilege escalation.

@stevelitt
Copy link
Author

I should mention that years ago I commented out:

root ALL=(ALL) ALL

A person who has that line intact in their /etc/sudoers would not need all the lines for what root can do under sudo. I'm not too sure what percentage of people have "root ALL=(ALL) ALL" in their /etc/sudoers.

@jaromil
Copy link
Member

jaromil commented Oct 2, 2018

hi Steve! another part of Tomb your good insights can become a contribution. This is already a very clear report of the situation, meanwhile and indeed, I've just applied this _sudo wrapper to make it easy to steer things. I agree with the need for a check, already a good solution. I'd also love to drop the need for sudo making it optional perhaps, but haven't found a proper way yet (while still allowing users to use tomb, that is).

@stevelitt
Copy link
Author

I have two ideas that might possibly work. One is to suid root the tomb executable. Ugly, not all that secure, but I think it would work.

The second way could be made as secure as you want it. Create a daemon, running as user root, capable of performing any losetup, cryptsetup, fsck, tune2fs, mkdir, mount, rmdir, mkfs.ext4, and findmnt. If the tomb executable fails on any of these commands, it writes the daemon's socket in a very simple language telling the daemon to do the task. It then reads from that same socket the success or failure of the command, and any other necessary information.

For security, the daemon should default to localhost only at some unused port number. Because sysvinit and OpenRC still exist, the daemon should have the ability to background itself, although that shouldn't be the default.

There needs to be enough statefulness that the daemon takes its orders ONLY from tomb. I don't know how to do that offhand, but I think the problem has been solved before.

If this works, any user including root can use tomb, and there's not a sudo in sight.

If you're able to do this, I'll be glad to write the runit run script for the daemon, and perhaps I can also write the daemontools-encore and s6 run scripts for it.

@Narrat
Copy link
Collaborator

Narrat commented Oct 3, 2018

Just as a note, so we don't fragment a conversation about a specific issue/wish (dropping the need for sudo) in more than one topic :)
#197
Would really nice if this point from the wishlist could be ticked

@jaromil
Copy link
Member

jaromil commented Oct 3, 2018

Indeed I like to improve on this point in a way or another.

There is a third way also besides suid, that is using https://sup.dyne.org to either allow sup'ed execution of losetup, cryptsetup, fsck, tune2fs, mkdir, mount, rmdir, mkfs.ext4, and findmnt or of tomb alltogether (but I'm wary of making a script suid).

@roddhjav
Copy link
Contributor

roddhjav commented Oct 3, 2018

Your proposition is interesting, however, when I read this:

it writes the daemon's socket in a very simple language telling the daemon to do the task.

In my opinion, this is an open door to arbitrary code execution. Furthermore, it would increase the attacker surface, which is a thing Tomb tries to reduce as much as possible.

@stevelitt
Copy link
Author

Arbitrary code execution must be guarded against. First, the daemon must sanitize its input. Second, both the daemon and tomb must demand an identifier from each other, and modify that identifier on every interaction. At some point a badguy would have to substantially reproduce tomb to fake out the daemon, and if they want to go that far, they could do the same thing to the current tomb.

You're right, preventing arbitrary code execution is difficult.

@stevelitt
Copy link
Author

Jaromil,

The sub idea sounds great. Compile it with losetup, cryptsetup, fsck, tune2fs, mkdir, mount, rmdir, mkfs.ext4, and findmnt, call it a different name (tomb_commander or whatever) and the deed is done. Nobody would even need to configure it: It's ONLY for Tomb.

@jaromil
Copy link
Member

jaromil commented Oct 4, 2018

OK! eliminating the daemon option then I'd like to adopt two solutions:

  • add a check if id=0 in _sudo then call command directly
  • add extras/tomb-sup with a pre-configured makefile hashing all needed bins in sup
    and of course update the documentation accordingly

This looks like a great improvement to me! thanks!

@stevelitt
Copy link
Author

Sounds like a huge improvement to me. Your idea goes a long way to prevent the out-of-whack state problems that seems to happen every once in a while. Please be sure to include chown and chgrp in the list of executables: those are necessary to enable user whoever to write to /media/tombname.

@jaromil
Copy link
Member

jaromil commented Oct 5, 2018

The problem with chown and chgrp is that once their execution is granted to any user then the whole system security is compromised, since anyone can write any file. It defeats the purpose of giving user-only access to tombs. This has been always my show-stopper when designing this part of tomb, main reason to stick to root ALL=(ALL) ALL and allowing tomb alone to single out its use of chgrp/chown.

@jaromil jaromil added the enhancement An issue to improve current behavior label Oct 5, 2018
@stevelitt
Copy link
Author

Fair enough. And experiments show me that if I open it without -p the first time, add all the data, with various users and groups, close it, and then on all subsequent opens open it with -p, everything works fine, owners and groups aren't changed, and I don't need to use chmod or chgrp to be able to write to it from user slitt.

I'd like to point out that there's still the issue that if one accidentally forgets -p when they needed it, the original owner/group info is irretrievably lost.

For me this won't be much of a problem. I back up trees to .tgz files, which are all owned and grouped slitt, and put the .tgz's in the tomb. Also, if I need to, I can make a shellscript to run tomb with -p even though the shellscript isn't given a -p. So for me this won't be a problem.

@Narrat Narrat mentioned this issue Dec 3, 2018
@kennethdhau
Copy link

I'm not too familiar with Tomb's code but if there's still consideration of a daemon, you could use a UNIX socket with SO_PEERCRED to validate the UID/GID of the requesting agent against an access control mechanism and maybe use a readlink lookup against /proc to validate the running process (not that this is a terribly strong security barrier).

As for the design, of an IPC language, depending on the extent of the necessary features, you could potentially lock it down to be similarly secure as any other input mechanism (optarg, etc.)

@hyiltiz
Copy link

hyiltiz commented Oct 23, 2020

Is it even possible to use tomb without sudo access though?

@stevelitt
Copy link
Author

stevelitt commented Oct 23, 2020 via email

@hyiltiz
Copy link

hyiltiz commented Oct 23, 2020

Is there no way I could use tomb in a (POSIX, say Debian) system where I am just a regular user with no special privileges at all?

@roddhjav
Copy link
Contributor

Is there no way I could use tomb in a (POSIX, say Debian) system where I am just a regular user with no special privileges at all?

Tomb requires special privileges because it is based on cryptsetup and losetup that require special privileges. So, by construction, a regular user without special privileges cannot run tomb.

@jaromil
Copy link
Member

jaromil commented Oct 24, 2020

@roddhjav see manpage on configuring sudoers

@kennethdhau
Copy link

kennethdhau commented Oct 24, 2020 via email

@jaromil
Copy link
Member

jaromil commented Apr 8, 2021

bump to signal #412 at least something is moving thanks to @heat-wave !

@AimoE
Copy link

AimoE commented May 21, 2021

This bug report partly explains my experience, but I reported #415 anyway, to emphasizet the variety of the consequences of the underlying problem.

@chri2
Copy link

chri2 commented Oct 22, 2022

For user slitt to successfully dig, forge, lock, open and close, he needs all the following in sudoers:

slitt ALL=(root:ALL) /usr/bin/tomb
slitt ALL=(root:ALL) /usr/bin/losetup
[...]

  • Shouldn't the configuration contain some restrains on the options used for each command? And if so, is it even possible to close down the allowed parameters to something narrow?
  • If one needs sudo for /usr/bin/tomb wouldn't it be better to do something like I mentioned here (let tomb call itself for certain operations)

@jaromil
Copy link
Member

jaromil commented Oct 22, 2022

@chri2 I very much like the idea of the script calling itself to wrap around sudo permissions (as well doas etc.)

@jaromil jaromil added this to the portable milestone Nov 14, 2022
@jaromil
Copy link
Member

jaromil commented Jan 29, 2024

Issue related to #498 and #470 more recent issues. Hope we manage to achieve independence from sudo very soon!

@jaromil jaromil removed this from the portable milestone May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An issue to improve current behavior
Projects
None yet
Development

No branches or pull requests

8 participants