wefork / wekan

The open-source Trello-like kanban (built with Meteor)
https://wekan.io
MIT License
61 stars 12 forks source link

Allow Superuser issue for newer versions of meteor. #75

Closed stephenmoloney closed 7 years ago

stephenmoloney commented 7 years ago

With newer versions of meteor, an error will be encountered during the docker build and the build process crashes just after the following output:

Even with METEOR_ALLOW_SUPERUSER or --allow-superuser, permissions in your app
directory will be incorrect if you ever attempt to perform any Meteor tasks as
a normal user. If you need to fix your permissions, run the following command
from the root of your project:

  sudo chown -Rh <username> .meteor/local

More on this issue here

I'm not sure of the 'correct' solution.

Perhaps adding a meteor user instead of using root?

meteor build --directory --allow-superuser /opt/app_build got me past this for now but perhaps leave this issue open until someone who knows more about it can suggest the best approach.

PR in the meantime.

martingabelmann commented 7 years ago

meteor-launchpad is using the METEOR_ALLOW_SUPERUSER env which seems to be compatible with old and new meteor versions. However, they setup a node user which owns the actual app files (I dont know why, maybe because some apps complain about permissions but for wekan this is not the case(?)).
I would not add to much complexity as long as there are no other "bugs" than the install warning message.

xet7 commented 7 years ago

@stephenmoloney

Correct way would be to make it run in unprivileged Docker container.

With those allow-superuser etc I get feeling like security goes out of the window, and it's better to run Sandstorm.

martingabelmann commented 7 years ago

@xet7 docker containers are always unprivileged per default (i.e. have no access to cgroups and stuff if you mean this).

Im not that familiar with meteor: which user is usually supposed to run the app? If its not root, then this also should not be the case within the container but if its just a "convention" of meteor to be build as non-root while the actual app runs as root I wouldn’t care (also the article that was linked in the original meteor issue from above is no more up2date (it actually wasnt at the time of the comment) since this particular exploit was fixed in docker 1.0).

xet7 commented 7 years ago

@martingabelmann

I mean, that user account inside docker container that runs node.js should not be able to run apt-get etc commands, only run application. There can be other definitions for it, but I need to look is there anything else to it.

Generally you just create new user (any username) with for example useradd or similar command, chown required files to that user, and have node run as that user at startup.

stephenmoloney commented 7 years ago

OK, put the PR https://github.com/wefork/wekan/pull/74 on hold for now.

Sounds like creating a user and chown required files to that user, and have node run as that user at startup. is the correct way to go.

martingabelmann commented 7 years ago

Ok then there should be a USER tag right infront of the CMD tag (after doing the permissions). Though I guess this wont work for port 80 since one need to be root to listen on that port.

Thats what I dont understand about meteor apps. Usually http server daemons are started as root where the main process opens the network socket and then spawns http workers (as nonroot) that are able to access the socket somehow. Maybe this can be fixed by listening on a non-privileged port (e.g. 8080) within the container and then spawn a container with -p 80:8080.

xet7 commented 7 years ago

@martingabelmann

Try similar what Caddy does at: https://caddyserver.com/docs/faq

setcap cap_net_bind_service=+ep ./caddy
xet7 commented 7 years ago

@martingabelmann

Actually, that -p 80:8080 way in docker is more normal way to map ports inside container to any port outside of it, so adding cap_net_bind_service is not necessary.

xet7 commented 7 years ago

@martingabelmann

Oh, well actually for binding to 80 outside of container docker daemon maybe needs that privilege? If you don't have any webserver proxying to docker container in front. I have had Nginx or Caddy in front of docker containers that run in different localhost ports.

martingabelmann commented 7 years ago

Actually, that -p 80:8080 way in docker is more normal way to map ports inside container to any port outside of it, so adding cap_net_bind_service is not necessary.

Yes. Also the capadd functionality of docker seems not to support this (but maybe will in future) https://github.com/docker/docker/issues/8460

Oh, well actually for binding to 80 outside of container docker daemon maybe needs that privilege?

The docker daemon itself is of course privileged to do that (it is run by root, and in fact the proc in the container is not listening to 80 on the hosts physical device but the docker daemon that then is doing the forwarding with iptables).

Since its recommended to have a proxy for https authentifiaction infront it is not necessary to expose a port from wekan container to the host at all (in production). Instead one should link the proxy container (which exposes 80 and 442 to the host) to the wekan container and then have something like http://wekan-container-name:8080 in the reverse proxy config. Fort testing or private networks the -p 80:8080 option is also very simple.

stephenmoloney commented 7 years ago

Added a user wekan to perform the meteor build. gosu needed if you want to keep it inside a single RUN command as discussed before on another issue. PR 81

stephenmoloney commented 7 years ago

I think I will close this issue as #81 was merged.