appropriate / docker-jetty

Formerly the location of the Docker official image for Jetty
https://registry.hub.docker.com/_/jetty/
46 stars 46 forks source link

USER jetty + /generate-jetty-start.sh = Permission denied #80

Closed jeremy-im closed 7 years ago

jeremy-im commented 7 years ago

/generate-jetty-start.sh: 3: /generate-jetty-start.sh: cannot create /jetty-start: Permission denied

md5 commented 7 years ago

Can you post your Dockerfile?

It looks like you need to temporarily switch to USER root for that command and then switch back to USER jetty.

gregw commented 7 years ago

@md5 This looks like it is going to be a common issue. Perhaps we should put some explicit code into the script so that it gives a reasonable warning and explanation that USER needs to be switched back to ROOT. We should then probably warn in the entry point if the USER is not switched back to jetty

md5 commented 7 years ago

@gregw Sure, that sounds good. I think it would also be good to put an example in the documentation.

gregw commented 7 years ago

@md5 actually switching USER frequently is against best practises. Perhaps we'd be better off moving /jetty-start' into$JETTY_BASE` so it can be written without changing users?

I'll prepare a PR for that....

md5 commented 7 years ago

I'm not sure the advice against switching USER frequently really applies here. We only switch it once in the Dockerfile for this image and I think having derived images switch to USER root to run commands that need root before going back to USER jetty isn't too bad. I suppose it could get a little out of hand if you had multiple layers of derived images.

As for whether it's actually a good practice to have jetty.start owned by jetty, I would argue that having all files that should not be writable by the jetty user from the running application owned by root would be more secure. This would help prevent attackers from gaining a foothold inside a long-running container by modifying files inside it. Using Docker's read-only mode goes one step further.

jeremy-im commented 7 years ago

Aplogies for my terseness in opening this issue. But you got to an understanding of my complaint in spite of that.

For my two cents, I think this is mostly about documentation. An example would cover it. And the README shown at hub.docker.com needs updating. Thanks!

gregw commented 7 years ago

@md5, I'm not sure this is a security issue as anybody that can write to $JETTY_BASE has complete control of the server anyway as they can replace any jars etc.

The jetty.start contents is just a preprocessed version of what is in $JETTY_BASE/start.d and you do not need root permission to change that, so it is a bit strange that root is needed to pre-process it. In fact it is a little strange that in order to prevent the need to run the JVM as root, we require the user to switch to root to run the JVM and then switch back.

Also currently if the /jetty.start does not exist, we create a temporary one in $TMPDIR. This change allows for the 'jetty.start` file to be in the same location regardless, which is a good simplification.

md5 commented 7 years ago

@gregw Sorry I wasn't more clear. I agree that having jetty.start owned by jetty doesn't introduce a new security issue. My intent was to bring up the existing issue that you mention about JARs, config files, etc being writable by the web process.

gregw commented 7 years ago

@md5 OK so you are saying that we should make most of $JETTY_BASE write protected from the jetty user, so that you can only change configuration in a Dockerfile by switching to root; configuring server; switching back to jetty. I can see reason in that, but we'd have to do an audit of all the jetty modules to make sure that none of them lazily change configuration on startup (I don't think any do, but I'm not 99% sure, specially with regards to third party modules like hawte).

If users really want that level of security, then I'd be more inclined to provide a script called protect-jetty-base.sh which could be run after configuration and it would remove write permission from directories and libraries. Thus an attacker would first need to run chmod before replacing any files.

md5 commented 7 years ago

I think any change to the permissions can be handled as a separate PR/issue.

My feeling at this point is that it's actually best handled in documentation, either describing how to run the equivalent of your protect-jetty-base.sh command (since it can probably be accomplished with find ... -exec) or by describing the volumes necessary to run with --read-only (i.e. where user jetty should actually be able to write).

gregw commented 7 years ago

@md5 is that comment in relation to #81 as it is, or to further changes that have been suggested to protect $JETTY_BASE. If the later, then I totally agree. I think #81 is a sensible step and means that users need only switch back to root for non-jetty related configuration.

md5 commented 7 years ago

I think #81 looks OK. I did just noticed an issue that I'll comment about on that PR.

Aside from that, I'm not sure which comment you're asking about.