/var/run/docker.sock should not be mounted

[geonode]

/var/run/docker.sock should not be mounted

[francbartoli]

Hi @frafra the socket is used internally by python docker sdk, i.e. here

[frafra]

Hi @francbartoli, thank you for your reply.
So it looks to me that setting GEONODE_LB_HOST_IP and avoid mounting /var/run/docker.sock is the only viable solution in a production environment. I think that should be the default, as mounting docker.sock inside geonode would be like giving root access to geonode to the entire machine.

[francbartoli]

@frafra I understand your concern about security. There could be cases where we cannot get rid of DinD. Do you think there is space to moderate the issue?

[francbartoli]

Maybe this workaround below can be applied for production:

• https://integratedcode.us/2016/04/08/user-namespaces-sharing-the-docker-unix-socket/
• https://integratedcode.us/2016/04/20/sharing-the-docker-unix-socket-with-unprivileged-containers-redux/

[frafra]

@francbartoli I personally see no reason to give to GeoNode access to Docker in the first place. It is not a service which should interact with other containers (like Traefik, Portainer, and so on) and it would be better to obtain the IP as a variable: it is both a security issue and an anti-pattern. Eventual future proposals for using docker.sock should be discussed independently. But that is just my opinion of course.

You can mitigate the security risk using AuthZ and setting up additional security measures on the server, but that defies the concept of using containers in the first place, and that would be even harder to digest for a sysadmin.

You could expose just the machine network to the container instead of sharing the socket, but this is also a bad idea imho.

There are other issues with this approach: many PaaS solutions do not allow you to do to this kind of operations, nor it would be compatible with systems that do not expose the socket or they do not have one (like podman).

As the public IP of a server should not change overtime (not very often at least), picking the variable from the environment seems reasonable to me, so I will proceed in this direction, but other users could be not aware of this security risk if this code remains unchanged.

[francbartoli]

A lot of good points @frafra indeed! That was thought for non sysadmin users who want to try the stack and are not even able to know and understand docker networking and get the docker0 ip address in native environments for setting up GEONODE_LB_HOST_IP. We have to still give them an utility to do that automagically imho. Something like:

geonode-docker-compose up

which can read the address and override the yml file for the above variable

[frafra]

It seems a very nice solution to me :-)

Maybe there is an even simpler solution if localhost is an acceptable default:

• Set GEONODE_LB_HOST_IP in docker-compose.yml as an environmental variable like this: GEONODE_LB_HOST_IP:${GEONODE_LB_HOST_IP}
• Set a default value for GEONODE_LB_HOST_IP (would 127.0.0.1 be ok?)
• Add an example for setting GEONODE_LB_HOST_IP in docker-compose-override.yml
• Update the documentation
• Remove any code related to docker.sock

Otherwise, we could both give a command line to set this environmental variable before using docker-compose up, or have a geonode-docker-compose script which sets this variable first and act as a wrapper.

[giohappy]

I understand your concerns @frafra, they were also mine months ago.
I think it's time to take a decision on this. I'm in favour of removing this "magic" which simplifies the setup but, as you descibed, brings a list of issues (one is the hack to make the Docker socket work on Windows).
@francbartoli my vote is to remove it but I want to hear from you what problems it could bring from your side.

[francbartoli]

Agree with you guys to remove it but I believe there will be consequences in some cases, for instance multiple geonode stacks on rancher and also I need to figure out native linux when that variable GEONODE_LB_HOST_IP is not set. Maybe it would be better to understand how to accomodate them too before doing it

[frafra]

@giohappy the "hack" to make the Docker socket work on Windows is gone, as there is no need to use the Docker socket anymore.

@francbartoli localhost:80 seems a reasonable default to me. People who relies on that automatic host/port detection should use the documented environmental variables. GEONODE_LB_HOST_IP and GEONODE_LB_HOST_PORT. We could provide a script to get the same behavior without using docker.sock, but that would be executed outside the container and I do not think anyone would use it in a production system with multiple instances: it would be ok just for testing, and in that case the defaults seems enough to me.

[afabiani]

Seems a reasonable solution to me also.

@francbartoli if you have some time, could you please evaluate it more carefully and eventually merge it? Let me know if you need help to test different use cases.

Thanks all.

[francbartoli]

@afabiani let's play for a while with this and then merge

[frafra]

Similar change could be proposed for the main geonode repository, but it seems to me that copying the files and snippets of code around could be a bad idea. For example, in the official geonode repository tasks.py has a function which tries to execute wait-for-databases.sh, which does not exist in that repository: the code has probably been copied from geonode-project.

If this pull request get accepted I can create a new one for geonode, but it would be a good idea to think if there is a better non-redundant and simpler/safer way to manage geonode-project.

[francbartoli]

@frafra that file has been copied from the geonode-docker repository to being able to override it during development with the option --build

[olivierdalang]

@frafra Yes, we definitely need one reference implementation for Docker in the central repo, from which the setup for geonode-project should somehow derive.

As a reminder, we now also have spcgeonode in the main repo (in the scripts folder), that has some advantages over the setup that lies in the root (e.g. already didn't have docker in docker, has proper versioning of the images, proper logging to stdout, additional features for deployment such as https and backups, cleaner settings in .env, etc...).

[frafra]

@olivierdalang Thanks; now that it has been merged in the main repository I will use it and continue to propose minor improvements on it, hoping it could become the preferred way to deal with Docker.

[afabiani]

Remember to close this issue if it does not apply anymore.

Also, if possible and with respect to the available resources from everyone, let's try to improve the documentation also which is currently a bit messed and outdated.

[frafra]

I would still suggest to apply the proposed patch, because having docker.sock exposed means that the container has the full control of the machine, so if there is a security issue in geonode the server could be compromised. But feel free to reject it if you prefer to do otherwise.

I am currently testing oliver's solution and I think I will use it for production with some minor modifications (it is easier to work on it directly than trying to fix/improve how Docker is used by geonode/geonode-project in my opinion).

I will try to improve the doc step-by-step whenever I encounter issues or errors. It will take time.

[afabiani]

@frafra very nice, thanks.

If the solution is better, more stable and well documented, we can envisage a GNIP (or just evaluating this during a PSC meeting) to substitute the current docker setup with the SPC one.

[francbartoli]

It seems a very nice solution to me :-)

@frafra please have a look at GNPIP #63

Maybe there is an even simpler solution if localhost is an acceptable default:

• Set GEONODE_LB_HOST_IP in docker-compose.yml as an environmental variable like this: GEONODE_LB_HOST_IP:${GEONODE_LB_HOST_IP}
• Set a default value for GEONODE_LB_HOST_IP (would 127.0.0.1 be ok?)
• Add an example for setting GEONODE_LB_HOST_IP in docker-compose-override.yml
• Update the documentation
• Remove any code related to docker.sock

Otherwise, we could both give a command line to set this environmental variable before using docker-compose up, or have a geonode-docker-compose script which sets this variable first and act as a wrapper.