[GitHub] [guacamole-client] benrubson edited a comment on issue #469: GUACAMOLE-890: Security: Allow image to run as non-root user

[GitBox <gi...@apache.org>]

benrubson edited a comment on issue #469: GUACAMOLE-890: Security: Allow image to run as non-root user
URL: https://github.com/apache/guacamole-client/pull/469#issuecomment-605352752
 
 
   > > However, it is solving a problem that _we_ shouldn't be solving. If we move to the supported tomcat image, then it runs with a non-root user without any of these changes. Surely that is _much_ better than anything else.
   > 
   > I wholeheartedly agree.
   
   > Allowing the image to run as non-root is a worthy goal, but one that should be implemented within the Tomcat image. Once that's the case (it sounds like it already is?), (...)
   
   No, as already stated [here](https://github.com/apache/guacamole-client/pull/469#issuecomment-580483759), it's not.
   Some details again.
   Please read carefully, sorry I'll certainly repeat / reuse some arguments given above.
   
   Here is then the directory structure from the last `tomcat:8.5-jdk8`  image :
   _(as a ref base image update PR from `jre` to `jdk` is here : #485)_
   
   ```
   # ls -l /usr/local/tomcat/
   -rw-r--r-- 1 root root 19318 Mar 11 10:06 BUILDING.txt
   -rw-r--r-- 1 root root  5408 Mar 11 10:06 CONTRIBUTING.md
   -rw-r--r-- 1 root root 57011 Mar 11 10:06 LICENSE
   -rw-r--r-- 1 root root  1726 Mar 11 10:06 NOTICE
   -rw-r--r-- 1 root root  3255 Mar 11 10:06 README.md
   -rw-r--r-- 1 root root  7136 Mar 11 10:06 RELEASE-NOTES
   -rw-r--r-- 1 root root 16262 Mar 11 10:06 RUNNING.txt
   drwxr-xr-x 2 root root  4096 Mar 17 23:14 bin
   drwxr-xr-x 1 root root  4096 Mar 27 21:39 conf
   drwxr-xr-x 2 root root  4096 Mar 17 23:14 include
   drwxr-xr-x 2 root root  4096 Mar 17 23:13 lib
   drwxrwxrwx 1 root root  4096 Mar 27 21:39 logs
   drwxr-xr-x 3 root root  4096 Mar 17 23:14 native-jni-lib
   drwxrwxrwx 2 root root  4096 Mar 17 23:13 temp
   drwxr-xr-x 2 root root  4096 Mar 17 23:13 webapps
   drwxr-xr-x 7 root root  4096 Mar 11 10:04 webapps.dist
   drwxrwxrwx 2 root root  4096 Mar 11 10:03 work
   ```
   
   As you can see, `logs`, `temp` and `work` directories of the default `CATALINA_BASE` have been made world-writable.
   But still, the `webapps` directory is not world-writable, so a non-root user won't be able to link `guacamole.war` into it. So the Guacamole `start.sh` script will [fail](https://github.com/apache/guacamole-client/blob/1.1.0/guacamole-docker/bin/start.sh#L579).
   So, there's still something missing.
   
   You could then propose to ask the Tomcat image maintainers to `chmod 777 webapps`.
   But, and as you already stated [here](https://github.com/apache/guacamole-client/pull/469#discussion_r373092391) @mike-jumper, the goal (of running as non-root user) should not be achieved removing restrictions.
   In other words, from a security point of view, the method used by the image maintainers does not seem to be the good one.
   
   To confirm this, here's now the default directory structure from a Debian 10 Tomcat installation :
   ```
   # ls -l /var/lib/tomcat9/
   lrwxrwxrwx 1 root   root     12 Jun 13  2019 conf -> /etc/tomcat9
   drwxr-xr-x 2 tomcat tomcat 4096 Jun 13  2019 lib
   lrwxrwxrwx 1 root   root     17 Jun 13  2019 logs -> ../../log/tomcat9
   drwxr-xr-x 2 root   root   4096 Mar 27 22:51 policy
   drwxrwxr-x 3 tomcat tomcat 4096 Mar 27 22:51 webapps
   lrwxrwxrwx 1 root   root     19 Jun 13  2019 work -> ../../cache/tomcat9
   # ls -ld /var/log/tomcat9/ /var/cache/tomcat9/
   drwxr-x--- 3 tomcat tomcat 4096 Mar 27 22:51 /var/cache/tomcat9/
   drwxr-s--- 2 tomcat adm    4096 Mar 27 22:51 /var/log/tomcat9/
   ```
   
   As you can see here, `logs`, `work` and `webapps` directories are only writable by the `tomcat` user.
   It then does not makes sense to make them world-writable so that another user can start its application... Would be a messy / crappy configuration...
   
   On the contrary, as explained [above](https://github.com/apache/guacamole-client/pull/469#discussion_r392684827), and as explained in the [Tomcat documentation](http://tomcat.apache.org/tomcat-8.5-doc/introduction.html#CATALINA_HOME_and_CATALINA_BASE), we should, in this case, create and use a dedicated proper `CATALINA_BASE` directory structure.
   
   This is what this PR does...
   It creates the 5 runtime directories which are then readable / writable only by the user running the application. Link the application into it, and start from it...
   There's no reason why we should absolutely run the Guacamole application from the default runtime directories owned by `root`...
   
   Sincerely, it really looks like the way to go.
   We follow the Tomcat documentation, the resulting PR is really tiny, it has no side effect, no runtime or performance impact, it allows to run as non-root user, and enforces directory structure security...
   I don't see why we wouldn't go for it...
   
   Thank you again if you could then reconsider this 👍

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services