Ezka77 / xen-orchestra-ce

:whale: Docker & docker-compose files to deploy Xen Orchestra Community Edition (ie: from sources)
GNU General Public License v3.0
98 stars 35 forks source link

XO's nodejs process runs as a seemingly random user #30

Closed magikmw closed 4 years ago

magikmw commented 4 years ago

While running the containers the node.js process is ran as a random non-docker user. It took me a moment to pinpoint what's going on, but I've found this line in the Dockerfile:

https://github.com/Ezka77/xen-orchestra-ce/blob/c1ed8658fec0cd92184c7d8198264ce8513459c6/alpine/Dockerfile#L62

When container is ran the nodejs process' owner is set to a user with UID 1000 on host machine, whoever that might be. Other than potential security issues, I'm not sure if it has any functional impact.

Ezka77 commented 4 years ago

Hi magikmw,

Yes it comes from my own configuration, I have to update this part with a RUN mkdir -p /home/node : same outcome for the build. But it will have some side effect in the running container.

About your random non-docker user : I don't think it comes from this line, the container will run with the user id running the command line, but as the entrypoint script use the $USER env var if your user is not known in the container ... you will have an undefined behavior I think.

To fix your issue you can use the docker-compose file to set the $USER var, there is an example with root or use the --user option from docker run command line.

I'll patch the Dockerfile and the entrypoint script and I'll do some tests around this issue asap. Let me know if it works for you with one of the above fix.

magikmw commented 4 years ago

Hi, thanks for a quick response.

I've tried running with USER set in environment, but any value other than root throws out an error for the XO container:

chown: unknown user/group xo:xo

Maybe concerning this? https://github.com/Ezka77/xen-orchestra-ce/blob/c1ed8658fec0cd92184c7d8198264ce8513459c6/alpine/Dockerfile#L78-L79

Edit: Also, as for this:

I don't think it comes from this line, the container will run with the user id running the command line

I'm pretty sure it does, I'm invoking docker with sudo, and my host's user with UID:GID 1000:1000 runs the nodejs proceses (that has nothing to do with docker, and is just a standard user account). As an experiment I've deleted the user and now nodejs processes show up as owned just by 1000.


I'm not that versed with docker to know what's going on exactly, so any solution would work for me, as long as I can specify my own user.

Ezka77 commented 4 years ago

Well first my advise is to not run docker command with sudo ... that make no sense as the command is only a way to communicate with the docker daemon. Configure your user to communicate with the daemon.

... the container will run with the user id running the command line

I've take a shortcut with this part and it's wrong, what I should write is : the container will run with the user id configured by the docker run command line with the --user option. But it will not help you with your issue (you've try that with the USER var set in env and doing so will fail sooner I think).

If I understand well your needs, you have to map your host user with the user inside the container. You can do that in Dockerfile (but I think it's bad as you need to build an image for a specific user) or in the entry-point script by creating the user, fixing all permissions, and running your process but it will not be great neither as the xo-server process like to run as root.

I've made some changes in the latest images to run xo-server as root in the container with the minimum capabilities for the NFS mount (and avoid the privileged flag). At least the user will not be "random" anymore.

But I'm sorry, I'll not fix your issue soon as I think you need to build a specific image for your needs or at least a specific entry-point. If you come with a nice solution I'll be happy merge your changes but the only things I can think of are overly complicated (eg: nss wrapper from postgres docker). You will not need to use the nss wrapper I think but at least you will have to insert these kind of lines in the container system files. You can make it by hand or with a command line like useradd.

Hope it helps.

magikmw commented 4 years ago

Thanks for taking the time with this. I'll go over your suggestions and see if I can come up with anything sensible.

jmccoy555 commented 4 years ago

@Ezka77 > I've made some changes in the latest images to run xo-server as root in the container with the minimum capabilities for the NFS mount (and avoid the privileged flag). At least the user will not be "random" anymore.

Didn't allow me to mount my NFS share without the privileged flag, although I am running in Kubernetes so maybe there's something gong on there.

Works

        securityContext:
          privileged: true 

Doesn't work, with error 32

        securityContext:
          capabilities:
            add:
            - SYS_ADMIN

And the nag banner is back..... 😞

Ezka77 commented 4 years ago

Ha ! yes I've read something like that with Kubernetes. Anyway I'll go back with the privileged flag if it is simpler for you.

Yes the banner is back, but it should be back from some times now ... the patch wasn't used for months.

jmccoy555 commented 4 years ago

Don't mind. If its better for the Docker users, then maybe add a note. I can push my yaml if you'd like an example? I'll give it a tidy up, but I'm no Kubernetes expert.

Also, I got fed up with the banner appearing on every refresh last night and made a couple of patches, one to remove the popup and banner, and another to tidy up the menu and remove XOA, Hub, Proxies and XOSAN, which are all pointless tabs (other than serving as examples of the benefit of using the XOA version) in the CE version. I can push them too if you like? If I knew JS it would be cool to get it to check Docker Hub for a new version..... maybe one day when I have time and get bored.

olivierlambert commented 4 years ago

Hi!

I'm not really fond of "global" modifications that will hide parts of the application that could move later or stuff associated with XOA. For us, it's a way to show XOA Free and/or Pro users they could have more extra services within XOA. Since XOA is our main revenue stream for both XO and XCP-ng, this will reduce a stream of people potentially interested by that, also reducing our capabilities to develop the product further/faster. We are not Facebook or Google, each $ earned by selling it is directly and 100% invested into XO or XCP-ng.

We have no way to differentiate a pro user from a home labber, and as I explained in the past, if it's the only "price" to pay for home users, why trying to dry a potential source of future XOA users? (that are paying the software you are using for free already with all features).

I mean, if you do that, as a home user, on your own install, I'm completely fine with it. But we have no control on how people are installing it, and this repo is well used for even pro user to deploy XO from the sources.

I hope you understand my point of view.

jmccoy555 commented 4 years ago

Hi @olivierlambert I sort of understand (but then again OPNsense and FreeNAS don’t annoy me) and don't want to hijack this repo, or start a political war; I know there is a long thread on the forum about the pop ups etc., but why does the banner have to appear on every refresh.... its a bit much. Why can't it be linked to the cookie with the pop up? It is just a bit 'naggy' for my liking. I know I have no support; I've made that choice. Maybe if you had a Community+ level, with no support (even though in reality you have the patience of a saint and answer a lot of questions where people simply haven't even read the documentation or tried the search button) but enabling the features you would get another income stream, and another avenue of 'testers' to find issues, but I'm sure you've done some clever calculations to show that wouldn't work for you. Maybe this could be a configurable 'switch' in the docker file, the default to expose the 'Advertisements'.... does this really prompt people to suddenly pay for support? I'm sure if it bothers any pro users, they'll be much better coders (read that as hacker) than me anyway and have far more fancy ways to 'tidy' things up. Anyway, good news is this is easy to tinker with and I’ll happily just build it locally. I'm certainly one for hoping that XCP-ng and XO continue to grow and develop, for you as a business and for the community.

😃

olivierlambert commented 4 years ago

OPNsense and FreeNAS got a user base that's far larger than Xen Orchestra, because if more a niche market (so there's no scale effect for us). If it bother a pro user, the shortest path is to subscribe, because you are a company, not yourself in your basement ;)

Regarding Community+, I already addressed that invoicing individuals will cost us (managing VAT in the EU zone) more than it could bring us in terms of revenue.

Again, the goal is to remind pro users (or their own customers, true story... it happened) to paid for using it to get support or extra services.

Due to the small market and the stakes involved (XCP-ng and XO), keep in mind that your contribution can be to keep that banner 😺 It doesn't remove usable XO feature. Would you prefer an Open Core model? I don't, and I think it's a reasonable compromise (than you can remove yourself on your install if you want to hack it!)

jmccoy555 commented 4 years ago

Fair enough, if this really brings you extra income and clicks then I understand, you seam like a team of genuine people. Just noticed the 2M pulls, hows about an official Docker image. Hopefully XCP-ng becomes a major player when this will not matter 😄 I think its clear what has really happened here looking at the previous ‘issues’ the quick ‘don’t do that’ response and looking at what 6776f4f achieved, coincidentally made from a now delete account. I don't expect a reply and I'm not going to either as this thread has been hijacked enough already!!

olivierlambert commented 4 years ago

Yes sorry for the hijack, but it's always good to be able to talk about this in a "factual" way and with arguments :+1: It's interesting and also a way for us to understand better our users (wherever and whoever they are).

Regarding Docker, it's a possibility, but personally I'm not ultra-fan of having to install Docker just to test a software. I feel the official doc with just cloning/build is simple enough. But I keep that in mind, this is a topic I'd like to address :)

Ezka77 commented 4 years ago

Ho woah ! Didn't expect so much comments here !

Well first thanks @olivierlambert to answer a polemical remark 😁 ! Indeed I've used some patches to remove the banner. More because other install methods use that kind of patch than the banner annoys me. Still one remains the url rewrite ! As many questions about the docker was asked to your team, it feels more fair that they come here first but I don't know the real effect of this patch as here, there isn't much activity. If it bothers you I can remove that last one (and I have no right to do this in fact, even providing a bundle is not allowed by AGPL3).

@jmccoy555 I'll not add any patch about the banner or UI changes, otherwise this will not be a docker about Xen-Orchestra Community Edition but more a XOA Community Edited 😁 but thanks.

olivierlambert commented 4 years ago

Do as you wish @Ezka77 I'm not doing a hunt to enforce aGPLv3 for individuals :wink:

jmccoy555 commented 4 years ago

Its not just Google and Facebook that are watching 🤫

olivierlambert commented 4 years ago

FYI, in our next release (this week) we added a cookie to be able to discard for 24h. Also in that banner, we'll have a link pointing to our doc, explaining the reasons (what I did here).

This way I think we are more transparent and we explain everything, also giving more room for people who are refreshing XO a lot (or opening new tabs), without bothering them.