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

appRoot and sub dirs / files owned by root #1257

Closed
mlbiam opened this issue Nov 18, 2018 · 22 comments
Closed

appRoot and sub dirs / files owned by root #1257

mlbiam opened this issue Nov 18, 2018 · 22 comments

Comments

@mlbiam
Copy link

mlbiam commented Nov 18, 2018

Description of the issue:

The resulting image has all files in appRoot owned by root, even when the user parameter is set in the configuration. While my own container appears to work, it would be better to not have these files owned by root. It could cause unknown results on restrictive container platforms (like OpenShift).

Expected behavior:

If the user option is set, then all the files created by the build should be owned by that user id/name for consistency.

Steps to reproduce:

<plugin>
        <groupId>com.google.cloud.tools</groupId>
        <artifactId>jib-maven-plugin</artifactId>
        <version>0.10.0</version>
        <configuration>
          <from>
            <image>mlbiam/openunison-jib-builder:latest</image>
          </from>
          <container>
            <user>431</user>
            <appRoot>/usr/local/openunison/work/webapp</appRoot>
            <entrypoint>/usr/local/openunison/bin/run_openunison.sh</entrypoint>
          </container>
          <to>
            <image>mlbiam/testadlogin</image>
          </to>
        </configuration>
      </plugin>

run a build and look at /usr/local/openunison/work/webapp all the files are owned by root:

openunison@openunison-7b7b6df97b-bks92:/$ cd /usr/local/openunison/work/webapp
openunison@openunison-7b7b6df97b-bks92:~/work/webapp$ ls -lh
total 24K
drwxr-xr-x  2 root root 4.0K Jan  1  1970 META-INF
drwxr-xr-x  1 root root 4.0K Jan  1  1970 WEB-INF
drwxr-xr-x  4 root root 4.0K Jan  1  1970 auth
-rw-r--r--  1 root root 1.2K Jan  1  1970 favicon.ico
drwxr-xr-x 10 root root 4.0K Jan  1  1970 k8stoken
drwxr-xr-x 10 root root 4.0K Jan  1  1970 scale
openunison@openunison-7b7b6df97b-bks92:~/work/webapp$ id
uid=431(openunison) gid=433(openunison) groups=433(openunison)

Environment:

Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T14:33:14-04:00)
Maven home: /usr/local/Cellar/maven/3.5.4/libexec
Java version: 1.8.0_181, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk1.8.0_181.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "10.14", arch: "x86_64", family: "mac"

jib-maven-plugin Configuration:

<plugin>
        <groupId>com.google.cloud.tools</groupId>
        <artifactId>jib-maven-plugin</artifactId>
        <version>0.10.0</version>
        <configuration>
          <from>
            <image>mlbiam/openunison-jib-builder:latest</image>
          </from>
          <container>
            <user>431</user>
            <appRoot>/usr/local/openunison/work/webapp</appRoot>
            <entrypoint>/usr/local/openunison/bin/run_openunison.sh</entrypoint>
          </container>
          <to>
            <image>mlbiam/testadlogin</image>
          </to>
        </configuration>
      </plugin>

Additional Information:

I'd be happy to create a PR for this

@coollog
Copy link
Contributor

coollog commented Nov 20, 2018

Hi @mlbiam , do you have any more details about potential "unknown results"?

It could cause unknown results on restrictive container platforms (like OpenShift).

@mlbiam
Copy link
Author

mlbiam commented Nov 20, 2018

@coollog i've run into plenty of libraries that don't properly write to $TMP so if a library tries to write ephemeral data where its running as one user but the local directory is owned by another you can easily run into permission errors.

@chanseokoh
Copy link
Member

chanseokoh commented Nov 22, 2018

For those who came here, let me first emphasize that the best practice is that your <appRoot> should be immutable. If you find you need to set the ownership of <appRoot> to the user that runs the container, it probably means you have to modify the application at runtime. You should double-check if allowing your app to mutate itself is really necessary.

Of course, your application running as non-root may want to write ephemeral data as files. But you should not write them under <appRoot> in general. Probably those data should be written to a tmpfs or a separate volume directory mounted at runtime. And such a volume might be mounted it at <appRoot>/logs if necessary. But if it's because of log files, you should first check out this Stack Overflow answer.

@mlbiam just in case if you are blocked because of the owner permission issue on <appRoot>, here's a dirty workaround to make it globally writable (which I think is not a good solution). Let's say you want /usr/local/openunison/work/webapp (your <appRoot>) to be globally writable.

  1. Create an empty usr/local/openunison/work/webapp directory under ${project.basedir}/src/main/jib. That is, you will have ${project.basedir}/src/main/jib/usr/local/openunison/work/webapp.
  2. Configure its permission mode to be 757, for example:
          <container>
            <user>431</user>
            <appRoot>/usr/local/openunison/work/webapp</appRoot>
            <entrypoint>/usr/local/openunison/bin/run_openunison.sh</entrypoint>
          </container>
          <extraDirectories>
            <permissions>
              <permission>
                <file>/usr/local/openunison/work/webapp</file>
                <mode>757</mode>
              </permission>
            </permissions>
          </extraDirectories>

However, you may have to repeat this process for some of the sub-directories of <appRoot>.

Alternatively, one could also think of preparing a custom base image with desired owners, but it won't work in this <appRoot> case due to #1270. However, it may work for some sub-directories and files of <appRoot> that Jib won't touch.

That said, these are all dirty workarounds that I don't like at all.

@GoogleContainerTools/java-tools maybe for the <appRoot> contents, it isn't completely unreasonable to have owner:group match the running user. Alternatively, we may reconsider #892.

@eoftedal
Copy link

Just ran into this issue as well, where a jetty image would work, because jetty runs as jetty, but the added files and folders were added as root...

@slowr
Copy link
Contributor

slowr commented Oct 21, 2019

I have this issue as well because I am running all the containers with a pod security policy that does not allow root users. A workaround would be to give permissions to all of the files for everyone or use the variable that is provided as user for uid/guid on the copied files.

@chanseokoh
Copy link
Member

A workaround would be to give permissions to all of the files for everyone

@slowr you mean giving write permissions to everyone. What exactly is the issue you're seeing? It is usually the best practice to leave the app directory read-only and immutable at run-time, and if you are modifying the app root for some reason, I think it's better to understand who modifies it for what purposes first. That said, I am curious of your use-case too.

@slowr
Copy link
Contributor

slowr commented Oct 21, 2019

you mean giving write permissions to everyone.

No, I mean giving execute permissions to everyone.

I want to keep the files as read-only but I want the OWNER of the files to also be the same as the USER that executes it and all my containers run as non root user.

Also, I can't find a way to make the contents of an extraDirectory executables (works for one file but if I put a folder it does not seem to do anything).

For now, I compiled a custom jar that sets the uid/gid to 1000 by default.
ReproducibleLayerBuilder.java:131

      entry.setGroupId(1000);
      entry.setUserId(1000);
      entry.setUserName("user");
      entry.setGroupName("user");

@chanseokoh
Copy link
Member

chanseokoh commented Oct 21, 2019

No, I mean giving execute permissions to everyone.

Sorry, I am a little confused. This issue is about ownership and not about permissions, and usually, you give executable permissions to some binaries/scripts but you don't need (or want) to change the ownership. For example, everyone can execute /bin/ls on Linux, but the binary is owned by root.

And note that Jib by default gives executable permissions to the directories it puts in the image. If you need to give executable permissions to some files other than directories, what Jib provides is <extraDirectories><permissions> (which is intended only for <extraDirectories> files). And usually you don't need to touch the ownership. The reason of the original issue was that the user wanted to modify the app root at runtime, and changing the ownership is one way to achieve that. And in your case, I still don't see why you need to change the ownership, if this is only about execute permissions.

I want to keep the files as read-only but I want the OWNER of the files to also be the same as the USER that executes it and all my containers run as non root user.

So, I do see that you know about <exrtraDirectories><permissions>, but I don't get what exactly the problem you are seeing and why you need to change the ownership. Regardless of file ownership, you could run the container as 1000, 1001, 1002, nobody:nogroup, etc., and they would normally work.

Also, I can't find a way to make the contents of an extraDirectory executables (works for one file but if I put a folder it does not seem to do anything).

Yeah, the Jib configuration works for a single file or directory. If you are talking about recursively adding executable bits to all the files under a directory, then unfortunately you have to list all of them in the configuration. (We don't have such an option to apply permissions recursively, as we thought you usually give different permissions to different files and directories.)

@slowr
Copy link
Contributor

slowr commented Oct 21, 2019

The issue derives from the fact that I have a strict policy of running scripts that owner is also the user and since the user is non root I cannot run them.

Also, I can't find a way to make the contents of an extraDirectory executables (works for one file but if I put a folder it does not seem to do anything).

Yeah, the Jib configuration works for a single file or directory. If you are talking about recursively adding executable bits to all the files under a directory, then unfortunately you have to list all of them in the configuration. (We don't have such an option to apply permissions recursively, as we thought you usually give different permissions to different files and directories.)

Oh I see, the problem is that I have a folder with binaries so listing 100 files in the configuration is a bit more difficult than said but thanks for clarifying that the current implementation does not support this.

@chanseokoh
Copy link
Member

The issue derives from the fact that I have a strict policy of running scripts that owner is also the user and since the user is non root I cannot run them.

I see. Thanks for the explanation. Some question as I don't know well about this on Kubernetes: can this owernship policy be enforced through the pod security policy? If so, is it further possible that Kubernetes blocks executing scripts if the ownership policy is not met? Briefly looking at the pod security policy doc, it doesn't seem to offer control over file ownership. Or, is it more of a company policy?

@slowr
Copy link
Contributor

slowr commented Oct 21, 2019

The issue derives from the fact that I have a strict policy of running scripts that owner is also the user and since the user is non root I cannot run them.

I see. Thanks for the explanation. Some question as I don't know well about this on Kubernetes: can this owernship policy be enforced through the pod security policy? If so, is it further possible that Kubernetes blocks executing scripts if the ownership policy is not met? Briefly looking at the pod security policy doc, it doesn't seem to offer control over file ownership. Or, is it more of a company policy?

the latter one :) this is mostly code-base oriented and not psp.

@slowr
Copy link
Contributor

slowr commented Oct 21, 2019

@chanseokoh I created a patch on a custom branch that takes the user variable specified and applies it as an uid/gid inside the TarArchiveEntry. You can take a look here and tell me if it's something worth of making a PR here.

The only problem I came across is that it does not work if you specify uname and gname but only uid and gid. Cheers!

Edit:
Regarding the permissions I used this to create the map for setting permissions on all files:

project(':deployment').fileTree("bin").collectEntries { [("/usr/local/data/bin/$it.name".toString()): '755'] }

@markdbuck
Copy link

Here's my use case for wanting the user parameter to be respected for setting ownership of files under <appRoot>. First, for security reasons, we don't run our containers as root which is the reason that I am use the user parameter. Second, I am trying to combine jib with skaffold's file sync capability so that when a Java file is changed during the dev cycle, the whole image doesn't need to be rebuilt. I can do a mvn compile and then the changed class file will be synced into the running container and live reloaded by the Tomcat server. This is prevented by the fact that the class files are owned by root and group members only have read/execute permission. Therefore, the file sync fails. I have tested it successfully if I run as root.

@carljmosca
Copy link

There's a comment above which references OpenShift. I think @coollog asked a follow up question but I did not notice an answer and I thought it might be worth mentioning.

The approach Red Hat has taken (default OpenShift configuration) is to run as an "arbitrary user" (the unique UID is unknown until runtime by design) and is included in the root group. Therefore files/directories set to include the root group as needed at image build time are accessible/writeable.

This approach allows us the desired flexibility and the security associated with running as a non-root user. For example, doing a

RUN chmod g=u /etc/passwd

at image build time, allows us to add the random/arbitrary user with a desired name at runtime if/as needed.

@chanseokoh
Copy link
Member

chanseokoh commented Jan 2, 2020

For those who came here, let me first emphasize that the best practice is that your <appRoot> should be immutable. If you find you need to set the ownership of <appRoot> to the user that runs the container, it likely means you have to modify the application at runtime. You should double-check if allowing your app to mutate itself is really necessary. (Of course, your application running as non-root may want to write ephemeral data as files. But you should not modify application files under <appRoot> in general. Probably those data should be written to a tmpfs or a separate volume directory mounted at runtime. And such a volume may be mounted it at <appRoot>/logs if necessary.)

@markdbuck however, we'd want Jib to be seamlessly integrated with Skaffold, so your argument below is very compelling.

I am trying to combine jib with skaffold's file sync capability so that when a Java file is changed during the dev cycle, the whole image doesn't need to be rebuilt. I can do a mvn compile and then the changed class file will be synced into the running container and live reloaded by the Tomcat server.

However, in some sense, this is only to enable quick development/debugging cycle and unnecessary for production. It's still best to keep the application directory owned by root in production. The actual underlying issue is the limitation of Skaffold that it cannot sync files owned by root when the container is configured to run as a different user. Perhaps Skaffold might be able to overcome this limitation, e.g., by using ephemeral containers, but I admit it may not happen anytime soon.

We are still looking for a compelling use-case where an application must mutate itself in practice (i.e., requiring the application root directory to be owned by the container user), since this is basically against the best practice we'd like to guard. @carljmosca can you elaborate the OpenShift case? I read your comment, but it's unclear how exactly OpenShift mutates <appRoot> and why?

@carljmosca
Copy link

carljmosca commented Jan 2, 2020

Thank you for the follow up @chanseokoh

I agree that we should not need to set the ownership but there are times when we encounter software that only runs as user xyz (and we cannot change that requirement).

I believe the Red Hat approach is to create the user at runtime. These are the first examples I could find just now:

https://github.com/RHsyseng/container-rhel-examples/blob/master/starter-arbitrary-uid/Dockerfile
https://github.com/RHsyseng/container-rhel-examples/blob/master/starter-arbitrary-uid/bin/uid_entrypoint

One ends up with an arbitrary UID which has a known username and is included in the root group.

In the end, I agree that it may be less secure but running as a non-root, non-predictable UID in a distroless container makes me wonder if the UIDs are actually predictable. Is this the case?

Currently we have this requirement (known username) with software for which we do and do not have sources. How else might we address it?

@carljmosca
Copy link

@chanseokoh I am not sure I made it clear above but I did not want to modify that comment again.

The arbitrary UID scenario I am referencing is the default configuration for OpenShift as discussed here

My question above about if the UID is predictable was not necessarily about OpenShift. I was asking in general but I am not asking that you reveal anything that you cannot or should not.

@mlbiam
Copy link
Author

mlbiam commented Jan 2, 2020

honestly after opening this issue I don't have any concrete use cases for it. I run our containers built by jib on both k8s and openshift without issue. @chanseokoh is right that the best practice is to not write to the container at all unless you have an emptyDir volume mount in which case the root ownership isn't an issue. This is how you get around the issues in openshift trying to write to the container ephemeral data with a random uid. I don't like the inconsistency but i don't have any concrete reasons to get rid of it and would be fine closing this issue.

@carljmosca
Copy link

@mlbiam I was talking about the username issue that we have to deal with in some situations...what are the "unknown results" you initially mentioned for OCP?

@mlbiam
Copy link
Author

mlbiam commented Jan 2, 2020

The more issues i see with poorly designed libraries and containers the less i'm apt to give up security for usability. in this case if your container is writing ephemeral data it should be to a mount. its painful to step through and enumerate it when data does get created on odd places but its just not worth opening an exploitable path. The workaround is worth the pain.

@markdbuck
Copy link

@chanseokoh is correct that my use case for skaffold/jib integration is for the quick development cycle and not for production. However, it is still an important one for my organization. I have made it work well with skaffold/docker build where I can control the file permissions in the image. I make the class files group writeable and then using my IDE, I can edit a Java source file, press save and the rest is automatic. The IDE compiles the Java, skaffold detects the modified class file and syncs it into the container, and then Tomcat reloads it. No waiting for the image to build and push to the registry. I wish I could have made it work with jib but understand your perspective.

@chanseokoh
Copy link
Member

chanseokoh commented Jun 11, 2020

@mlbiam @eoftedal @mserdur @markdbuck @tdittmann @slowr @ch-wc @carljmosca @hansenc
The Jib Extension Framework is now available with the latest Jib versions. You can easily extend and tailor the Jib plugins behavior to your liking.

Since there's a very compelling reason to allow the container to be mutable at runtime (e.g., running Skaffold to update class files on Kubernetes during development), we've decided to support this use case through an extension:

For general information about using and writing extensions, take a look at the Jib Extensions repo.

Use of these extensions is still discouraged unless absolutely needed.

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

No branches or pull requests

7 participants