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

Implement privilege separation in tlog-rec #5

Closed
spbnick opened this issue Feb 26, 2016 · 17 comments
Closed

Implement privilege separation in tlog-rec #5

spbnick opened this issue Feb 26, 2016 · 17 comments
Labels
Milestone

Comments

@spbnick
Copy link
Member

spbnick commented Feb 26, 2016

Make tlog-rec executable SUID/SGID to a dedicated user/group. Drop back to the
original user/group before exec'ing the shell in the fork. This will keep
tlog-rec safe from the modification by the recorded user and would also allow
authenticated filtering of the log messages.

Upon RPM installation, a special user and group both named tlog should be
created, and tlog-rec should be owned by them and marked SUID and SGID.

After forking the shell process, tlog-rec should drop to the real UID/GID with
setuid(getuid()) and setgid(getgid()).

For additional security, consider retreiving the effective user/group IDs and
setting them to the real IDs with seteuid/setegid right after starting,
passing them around (say as euid/egid) and only setting them back with
seteuid/setegid when necessary. I.e. when reading the configuration and
when opening the logging socket/file.

Also consider calling setreuid(euid, euid)/setregid(egid, egid) in the
parent process after forking the shell, so the parent (recording) process
would not be able to regain the user's real IDs. However, ignore EPERM,
perhaps producing a warning, as POSIX says that might not be permitted, even
though it works on Linux and some BSDs.

To verify:

  • Check that all user/group IDs in /proc/PID/status of the user shell are
    the user's IDs.
  • If setreuid/setregid use is implemented, check that all user/group IDs in
    /proc/PID/status of the parent recording process are the tlog's IDs.
    Otherwise, check that real IDs are user's IDs and others are tlog IDs.
  • Check that log file is created with tlog UID/GID, when using the file
    writer.
  • Check that journald and rsyslog record tlog UID/GID as the logger's
    credentials, and rsyslog can filter by them, when using the syslog writer.
@spbnick spbnick modified the milestone: SSSD Integration Feb 26, 2016
@spbnick spbnick added the risky label Feb 26, 2016
@lslebodn
Copy link

You might want to drop supplementary groups with "setgroups()".
It is especially useful for increasing privileges. We already have some code in sssd. You might inspire.
https://github.com/SSSD/sssd/blob/master/src/util/become_user.c

It would be good to to clear/reduce environment variables "clearenv()" as a first think in binaries with SUID adn SGID. It is also good to use restrictive umask for such binaries.

I am not sure how much time do you have but it would be good to use linux capabilities as a hardening. For changing UID/GID it should be enough to have (CAP_SETGID and CAP_SETUID).
I can be achieved with libcap/libcap-ng or it can be done in distribution specific packaging.
The same as ping on fedora. BTW *BSD or platforms can still relay on SUID and SGID;

sh$ getcap /usr/bin/ping
/usr/bin/ping = cap_net_admin,cap_net_raw+ep

It's possible to find more info in man 7 capabilities

@spbnick
Copy link
Member Author

spbnick commented Jun 8, 2016

Thanks a lot for review, Lukas!

Yes, we can drop the supplementary groups in the original (recording) process after forking the child, good idea.

We cannot clear the environment variables as the first thing, because we need to pass them to the child, recorded shell. However, we can remove any tlog-specific variables in the fork, before executing the child, and we can remove everything unnecessary in the parent after forking the child. We also have issue #3 tracking that.

Since we're started in place of the user shell, we're started with the target user's credentials and we cannot set any capabilities in general. That's why tlog-rec must be SUID/SGID to a special user to access special resources and be protected from the recorded user. So, if I understand it correctly, we don't need to change capabilities, as we don't need, and don't get any. Also, tlog-rec shouldn't ever run as a superuser, unless it is set SUID/SGID root, or the logging-in user is root, but that's an exception and shouldn't happen.

However, thanks for the mention of the capabilities, I used that opportunity to learn more about them :)

@spbnick spbnick modified the milestones: v3, SSSD Integration Jan 17, 2017
@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Ah, right, tlog can't drop the supplementary groups with setgroups, because it doesn't run as root.

@lslebodn
Copy link

Ah, right, tlog can't drop the supplementary groups with setgroups, because it doesn't run as root.

hmm, I'm confused. It can call setresgid and setresuid but it cannot call setgroups. That looks suspicious.

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Ah, yeah, setresgid/uid can do that. From their manpage:

An unprivileged process may change its real UID, effective UID, and saved set-user-ID, each to one of: the current real UID, the current effective UID or the current saved set-user-ID.

@lslebodn
Copy link

The man page for setresuid/gid says that it require capabilities CAP_SETGID and CAP_SETUID.
The man page for setgroups says that it requires capability CAP_SETGID,

And with suid/sgid bit you are roll with all calabilities

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Hmm, sorry, I'm not sure what you mean. Could you please elaborate? Thank you.

@lslebodn
Copy link

I confused as well. As I already eplained tlog has enough rights to run setgroups.
So could you explain where is the problem?

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Hmm, perhaps I ran setgroups with EUID/EGID set to RUID/RGID and that's why setgroups returned EPERM. I'll check if it works with original EUID/EGID.

@spbnick spbnick reopened this Jan 19, 2017
@lslebodn
Copy link

lslebodn commented Jan 19, 2017 via email

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Nope, EPERM, even if called right at the start of main(), no matter SUID/SGID, or not.

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Oh, and yeah, I'm calling setgroups(0, NULL).

@lslebodn
Copy link

lslebodn commented Jan 19, 2017 via email

@spbnick
Copy link
Member Author

spbnick commented Jan 19, 2017

Which UIDs/GIDs krb5_child has when it invokes setgroups?

@lslebodn
Copy link

lslebodn commented Jan 19, 2017 via email

@spbnick
Copy link
Member Author

spbnick commented Jan 20, 2017

Right, so it's suid-root, which is different from suid to a regular user, and that's why I assume setgroups works there and not in tlog. Yeah, you answered my question, thank you :)

@spbnick spbnick closed this as completed Jan 20, 2017
@spbnick
Copy link
Member Author

spbnick commented Jan 20, 2017

Once I change the owner of tlog-rec to root, setgroups works fine.

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

No branches or pull requests

2 participants