gravitational / teleport

The easiest, and most secure way to access and protect all of your infrastructure.
https://goteleport.com
GNU Affero General Public License v3.0
17.7k stars 1.77k forks source link

Teleport should show a warning if running as root when it doesn't need root privileges #2759

Open webvictim opened 5 years ago

webvictim commented 5 years ago

~As discussed on Slack.~

~Currently, Teleport will happily run as root even when the config file or CLI parameters don't specify that a node process needs to run. auth and proxy will run fine as a non-root user, and it'd be a nice security feature to warn the user that root privileges aren't needed when node isn't enabled and drop to an unprivileged user.~

~This would also be useful inside a Docker container and help us to comply with security best practices.~

We should encourage users to use the principle of least privilege when deploying Teleport. As such, Teleport should show a warning when it starts if it is running as root but doesn't need root privileges - i.e. if it's not running a node process.

webvictim commented 5 years ago

cc @russjones

kontsevoy commented 5 years ago

@webvictim but why would that be a Teleport ticket?

"Back in the day" daemons used to have a configuration setting (like nginx, libvirtd or postfix) usually called user to indicate a system account to drop daemon privileges to. But since systemd (or upstart before it) took over, this functionality has moved to the supervisor, so users can already create a system user for any daemon, and tell systemd to drop Teleport to its level via teleport.service unit file.

Why would we need to duplicate this functionality inside teleport? It's already built-in into any even remotely modern Linux distro.

webvictim commented 5 years ago

@kontsevoy I agree in principle, but people don't always use systemd/upstart to run processes - we're using dumb-init within our own Docker containers for example which doesn't have this functionality.

The reason this ticket was created is because we always build our own official Docker containers with Teleport running as root - this is so that if the teleport daemon is running a node process and does need root privileges (to spawn sessions as any given user) it will have them. In many cases, though, teleport isn't running a node process at all and so it running as root is just an unnecessary security risk. This also makes us fall foul of numerous Kubernetes and Docker recommendations that containers should never be run as root unless absolutely necessary.

I figured that rather than trying to do some horrific config parsing within our container scripts to determine from the config whether we need to run as root or not, it would be a nicer feature to just do a simple check within the Teleport code like if (euid == 0 && nodeProcess == notRunning) { log.Warn("The teleport process has been started with root privileges but does not need them") }, perhaps along with willingly dropping its own privileges to an unprivileged user (the big question, of course, would be which user it should drop to if one isn't already configured)

gravitational-jenkins commented 5 years ago

official Docker containers with Teleport running as root

Which is fine given that modern containers can use user namespaces to map root to a less privileged user, limit the caps, set up selinux profiles and so on.

On Fri, Aug 9, 2019 at 8:47 AM Gus Luxton notifications@github.com wrote:

@kontsevoy https://github.com/kontsevoy I agree in principle, but people don't always use systemd/upstart to run processes - we're using dumb-init within our own Docker containers for example which doesn't have this functionality.

The reason this ticket was created is because we always build our own official Docker containers with Teleport running as root - this is so that if the teleport daemon is running a node process and does need root privileges (to spawn sessions as any given user) it will have them. In many cases, though, teleport isn't running a node process at all and so it running as root is just an unnecessary security risk. This also makes us fall foul of numerous Kubernetes and Docker recommendations that containers should never be run as root unless absolutely necessary.

I figured that rather than trying to do some horrific config parsing within our container scripts to determine from the config whether we need to run as root or not, it would be a nicer feature to just do a simple check within the Teleport code like if (euid == 0 && nodeProcess == notRunning) { log.Warn("The teleport process has been started with root privileges but does not need them") }, perhaps along with willingly dropping its own privileges to an unprivileged user (the big question, of course, would be which user it should drop to if one isn't already configured)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/gravitational/teleport/issues/2759?email_source=notifications&email_token=AD5GI6VC5QHJWQT22SGJIYTQDWGSRA5CNFSM4HTKXJPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD37BRNQ#issuecomment-519968950, or mute the thread https://github.com/notifications/unsubscribe-auth/AD5GI6QDOX76SZBNQJUZ52LQDWGSRANCNFSM4HTKXJPA .

-- You received this message because you are subscribed to the Google Groups "Tech Operations" group. To unsubscribe from this group and stop receiving emails from it, send an email to ops+unsubscribe@gravitational.io.

webvictim commented 5 years ago

Yes, but we're not providing any examples of how to do that within our own Helm charts or recommended deployments. Out of the box, our containers' configuration goes against security best practices (which is pretty poor for a company that wants to help others improve security)

klizhentas commented 5 years ago

within our own Helm charts or recommended deployments.

Our terraform / cloudformation do not run proxies and auth servers as root.

Our helm charts could be probably improved, I agree with this part.

webvictim commented 5 years ago

I've updated the scope of this ticket to adding a warning to Teleport when running as root if root privileges aren't needed.

I also created https://github.com/gravitational/teleport/issues/2909 to improve the security model used in our Helm and Docker examples.

stmuraka commented 4 years ago

I'd also like to see the Teleport containers to run as non-root. i.e. user teleport we're overriding the root user by setting a kube Security Context. it would be nice that it didn't have elevated privileges by default.

@benarent