Giter Site home page Giter Site logo

Comments (22)

alexanderadam avatar alexanderadam commented on June 9, 2024 3

@BYK this is probably a misunderstanding regarding Docker's "default". root is usually only required for setting up the image because administrative tools require super user rights for obvious reasons.

But in the end of building the Dockerfile, the image should switch to a non-privileged user (this way no entrypoint/commend could get privileged access). The official documentation is very clear about users and mention it even the "Best Practices" part:

If a service can run without privileges, use USER to change to a non-root user.

And I can't see any reason to let Sentry run as root.
So if you really want to follow Docker's recommendation, you should avoid using root.

If we require people to use a less privileged user, I'm sure we'll get more issues as people tend to skip reading the docs most of the time.

Do you have any particular reasons for that assumption? Is there any functionality that wouldn't work without having root?

from docker-sentry.

BYK avatar BYK commented on June 9, 2024 1

Hi! We use gosu but I'm not sure whether we really need root permissions or if we drop them afterwards. My only guess would be for accessing any protected ports but that should also not be necessary as docker allows port mapping. @mattrobenolt can you help here?

@maxdietrich, in the meantime, you can try removing the gosu command from the entrypoint file and use USER in the docker file to switch to a different user. You'd need to create this user and set a specific GID and UID as recommended here: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#user

from docker-sentry.

BYK avatar BYK commented on June 9, 2024 1

I don't think there would be any issues without root as we already don't run Sentry w/ root as @mattrobenolt pointed out above: https://github.com/getsentry/docker-sentry/blob/master/9.1/docker-entrypoint.sh#L19

It is just done in the entrypoint file. I'm guessing we can move that logic to the last step of the image builds if I understand your comment correctly?

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024 1

That is definitely an option, and we'd have to guarantee that our uid+guid doesn't change. We don't currently enforce that.

from docker-sentry.

maxdietrich avatar maxdietrich commented on June 9, 2024

Thanks for your response! In the mean time we decided to go for the cloud hosted solution instead of running it ourselves though. I'd leave the issue open until that question is resolved, since this could probably affect others who would like to run this in Openshift as well, but feel free to close it if you want.

from docker-sentry.

BYK avatar BYK commented on June 9, 2024

I'll talk to @mattrobenolt to get clarification and then keep it open or close accordingly. Glad you got around the issue (bonus points for using the cloud version 😁 )

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

So yeah, out of the box, it drops permissions with gosu if it is run by root. But you can also run it as a user with docker run --user if you’d like and it should be just fine. It’d bypass using gosu and just use whatever user you specify.

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

For context, this happens here. https://github.com/getsentry/docker-sentry/blob/master/9.1/docker-entrypoint.sh#L19

from docker-sentry.

BYK avatar BYK commented on June 9, 2024

Closing as I think @mattrobenolt's answer is sufficient and there's no action we can take immediately.

from docker-sentry.

alexanderadam avatar alexanderadam commented on June 9, 2024

Just for clarification:

  1. What exactly is the advantage of not having the image run as user per default?
  2. And what exactly is the advantage of not having the switch to a proper unprivileged user within the Dockerfile (and therefore not even having an attack surface if the command is overriden)?

from docker-sentry.

BYK avatar BYK commented on June 9, 2024

@alexanderadam we optimize for defaults and Docker's default is to run as root by default. If we require people to use a less privileged user, I'm sure we'll get more issues as people tend to skip reading the docs most of the time.

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

The main reason is for the file mount: https://github.com/getsentry/docker-sentry/blob/master/9.1/docker-entrypoint.sh#L20-L21

This needs to be owned by the user sentry is running at so it has permissions to read/write. If you just mount a VOLUME on your own, you'd need to match the uid/guid of the user running inside the container. You can do this on your own, but out of the box, it'll otherwise be the wrong permission.

from docker-sentry.

alexanderadam avatar alexanderadam commented on June 9, 2024

@mattrobenolt I guess adding documentation to use a proper uid (or at least a writable directory) for Docker volumes might be okay, doesn't it? Or do I oversimplify here?

from docker-sentry.

alexanderadam avatar alexanderadam commented on June 9, 2024

That is definitely an option

Awesome, that sounds good!

we'd have to guarantee that our uid+guid

Of course. You mean something like useradd -r -m -g sentry sentry --uid 1001, right?

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

Yeah, most likely. Not sure off the top of my head.

from docker-sentry.

alexanderadam avatar alexanderadam commented on June 9, 2024

So would it be okay to reopen this issue then, now that it's validity got 'restored'? πŸ˜‰

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

I mean, if you insist. I'm not sure this is a real issue. This is just a pattern that a lot of the official images follow.

I guess the only actionable thing here is for us to update documentation and explicitly choose a uid+guid for our sentry user, since we still won't be changing the default USER instruction.

from docker-sentry.

alexanderadam avatar alexanderadam commented on June 9, 2024

oh, then I misunderstood this conversation.
Why exactly wouldn't you consider adding USER to the Dockerfile?

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

This needs to be owned by the user sentry is running at so it has permissions to read/write. If you just mount a VOLUME on your own, you'd need to match the uid/guid of the user running inside the container. You can do this on your own, but out of the box, it'll otherwise be the wrong permission.

It's a bad default if it's just broken. This isn't always a trivial thing to do depending on your deployment.

This is basically the pattern of every official docker image, anything that writes data to disk.

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

See https://github.com/docker-library/postgres/blob/master/12/docker-entrypoint.sh#L31-L49

They all provide the ability for you to do your own thing by passing --user, which we support. But without having root privileges when it starts up, the onboarding experience is bad since it's broken, and forcing people to mount a volume with very specific permissions is not always possible. For example, I have no idea how you'd achieve this in Kubernetes without some initContainer that ran as root instead, just to change permissions. In which case, that's literally what's happening here.

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

I looked into this a bit more to make sure I wasn't overlooking anything, and you'd need to do this in Kubernetes if the container ran as a non-root user: https://stackoverflow.com/a/51195446

At this point, you have to spin up a container before your main container as root, just to run chown, then the actual container is free to run as a non-root user since permissions were swapped.

So if the goal is to not run a container as root, well, you still need to run a container as root. Just a different container. So changing this behavior out of the box is only going to cause other issues for similar scenarios.

If this is a concern if yours, you can very well just pass --user manually and do what you want. That's why the container was designed to work this way. If this behavior doesn't work, let us know and I'll be sure to fix it. But I don't want to change the default behaviors here. The processes already don't run as root. The only thing that runs as root is the docker-entrypoint.sh file, which quickly exec's and drops it's own permissions. You can work around this clearly by passing your own --user.

from docker-sentry.

mattrobenolt avatar mattrobenolt commented on June 9, 2024

I think this would also work? https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod

But in this case again, you just set all permissions right there yourself. You can just easily set the entire securityContext at one time for all bits to be the same permission including the volume.

If we have a different default USER, that means someone would still need to configure a securityContext anyways. I'd argue that while oyu're in there overriding securityContext, you might as well just set them all. There'd be no reason to just set fsGroup without runAsUser and runAsGroup.

I'm happy to document these options, but strong -1 on changing the default since this is more complex. I'm not even sure if you can mimic this with a normal docker volume, which you'd use in docker-compose. I don't know much about that environment.

from docker-sentry.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.