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

📢📢 URGENT! Docker WGDashboard Container Security Vulnerability! 📢📢 #333

Closed
DaanSelen opened this issue Aug 22, 2024 · 23 comments
Labels
bug Something isn't working

Comments

@DaanSelen
Copy link
Collaborator

For everyone using my work on the Docker image, I have recently discovered a crucial mistake with the creation of the image.

The Private key of my templated wg0.conf file is NOT random, and was prebaked in the image.
This has changed with the new version of dselen/wgdashboard:dev and dselen/wgdashboard:latest hosted on the Docker Hub.

Please update to this image if you are able to!
And if you are unable to use the new image, please configure a change of the private in your instance! This is to keep your data and connections secure.

I would like to sincerely apologise for my mistake, and if anyone needs help reach out to me!

@DaanSelen DaanSelen added the bug Something isn't working label Aug 22, 2024
@DaanSelen
Copy link
Collaborator Author

To add more context, if you built your own image you are safe! But if you pulled it from a repository. You must either delete the conf volume and let the container create a new one with a unique key. Or modify the private key entry yourself.

This is how to change the key in an existing file.

privateKey=$(wg genkey)
sed -i "s|^PrivateKey =$|PrivateKey = ${privateKey}|g" /etc/wireguard/wg0.conf

You also need to change the Public key in the endpoints, you can extract the public key with:
If you still have the environment variable from the first command simple run:

echo "${privateKey}" | wg pubkey

And you will get the public key printed.

@donaldzou donaldzou pinned this issue Aug 23, 2024
@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 23, 2024

@DaanSelen made a fix #334

@DaanSelen
Copy link
Collaborator Author

@DaanSelen made a fix #334

Hello @NOXCIS, may I ask where you fixed it? I see you removed it in its entirety?

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 23, 2024

@DaanSelen
Screenshot 2024-08-23 at 1 50 05 AM

  • I removed the steps for creating a wireguard config via passing Docker enviorment variables via compose, because its inherently insecure due to the nature of wireguard having kernel access. For example shell script via the postup feilds, etc. Secondly v4 has made it unnecessary.

  • Switched to Alpine because a debian base image is too bloated and vulnerability filled for the use case. Also for the ability to build multi arch image by using precompile binaries where possible. In short, removal and change of inherent insecurities with minimal project flow disruption.

enter image description here

@DaanSelen
Copy link
Collaborator Author

@NOXCIS inspired by your efforts and your repository, I have made an Alpine Linux image. Please review https://github.com/DaanSelen/WGDashboard. Regarding the size limitations I got an actual running container about on the same level as the one made by your fork. However I was keen on keeping the same features as I previously had crafted for WGDashboard.

CONTAINER ID   IMAGE                       COMMAND                  CREATED          STATUS                     PORTS       NAMES                SIZE
e189c27def35   dselen/wgdashboard:alpine   "/bin/bash /entrypoi…"   4 minutes ago    Up 4 minutes (healthy)     10086/tcp   peaceful_agnesi      11.3MB (virtual 140MB)
1d477a16b01f   dselen/wgdashboard:nox      "/home/app/entrypoin…"   5 minutes ago    Up 5 minutes (unhealthy)               ecstatic_albattani   49.6MB (virtual 114MB)

Regarding CVE-2018-20225. This is at the moment still unpatched, so I cannot remove that. However your image also has this CVE present, but moved to the runtime bash script.

Regarding the image I published, which is outdated (thanks to the documentation) it still points to my own registry, which should be moved to the Docker Hub registry managed by me, refer to https://hub.docker.com/repository/docker/dselen/wgdashboard/general

I'd like to hear from you.

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 23, 2024

@DaanSelen

  • Reimplementation of the automatic configuration creation is no issue as long as its done from the wgd.sh script. Docker treats the entrypoint script as the actual runtime script, all thats doing is calling the wgd.sh start and install arguments, and the cascade follows, etc. I have an adaption here where wireguard config are generated in the setup script and a master peer is outputted to terminal. The master peer is the sole way to access the dashboard as i only expose wireguard UDP ports in the container image.

  • CVE-2018-20225 still hasn't been patched, so the best option is mitigation. Even though the setup script isn't using --extra-index-url option when installing pip or performing the install requirements function. Its best practice to further isolate pip behind the venv, versus the docker image that acts as your first layer of defense.

  • I've recently pushed some updates to my fork to show how config generation can be better handled. However the ultimate solution will require the dashboard.py to modifed slightly to accept passed docker enviorment vars before the wg-dashboard.ini is created as external charges are overwritten after init.

@donaldzou
Copy link
Owner

@DaanSelen

  • Reimplementation of the automatic configuration creation is no issue as long as its done from the wgd.sh script. Docker treats the entrypoint script as the actual runtime script, all thats doing is calling the wgd.sh start and install arguments, and the cascade follows, etc. I have an adaption here where wireguard config are generated in the setup script and a master peer is outputted to terminal. The master peer is the sole way to access the dashboard as i only expose wireguard UDP ports in the container image.
  • CVE-2018-20225 still hasn't been patched, so the best option is mitigation. Even though the setup script isn't using --extra-index-url option when installing pip or performing the install requirements function. Its best practice to further isolate pip behind the venv, versus the docker image that acts as your first layer of defense.
  • I've recently pushed some updates to my fork to show how config generation can be better handled. However the ultimate solution will require the dashboard.py to modifed slightly to accept passed docker enviorment vars before the wg-dashboard.ini is created as external charges are overwritten after init.

I believe these are fixed?

@DaanSelen
Copy link
Collaborator Author

DaanSelen commented Aug 25, 2024

@DaanSelen

  • Reimplementation of the automatic configuration creation is no issue as long as its done from the wgd.sh script. Docker treats the entrypoint script as the actual runtime script, all thats doing is calling the wgd.sh start and install arguments, and the cascade follows, etc. I have an adaption here where wireguard config are generated in the setup script and a master peer is outputted to terminal. The master peer is the sole way to access the dashboard as i only expose wireguard UDP ports in the container image.
  • CVE-2018-20225 still hasn't been patched, so the best option is mitigation. Even though the setup script isn't using --extra-index-url option when installing pip or performing the install requirements function. Its best practice to further isolate pip behind the venv, versus the docker image that acts as your first layer of defense.
  • I've recently pushed some updates to my fork to show how config generation can be better handled. However the ultimate solution will require the dashboard.py to modifed slightly to accept passed docker enviorment vars before the wg-dashboard.ini is created as external charges are overwritten after init.

I believe these are fixed?

Hallo Donald, regarding the CVE there is no fix and automatic deployment is something we need to do from either Github action or by interval.

But yes @donaldzou i think my latest image, or the code in my fork is doing this

@donaldzou
Copy link
Owner

But yes @donaldzou i think my latest image, or the code in my fork is doing this

Do you mean @NOXCIS 's PR I just merged?

@DaanSelen
Copy link
Collaborator Author

DaanSelen commented Aug 25, 2024

But yes @donaldzou i think my latest image, or the code in my fork is doing this

Do you mean @NOXCIS 's PR I just merged?

No I did not, but I will look at what has changed tomorrow. Looks like a lot has changed and I do not know what specifically yet.

From what I see now, the Docker part is incomplete, Noxcis may have changed the Image compilation but all Documentation is still very outdated and all. For the inner workings on the image, it might work better, but I have to analyze it

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 25, 2024

@DaanSelen @donaldzou Its been a pleasure working with the two of you. There are some aspects of the docker implementation that need to be discussed before any further changes. I.E Docs. For now I'm burnt, happy hunting 😄.

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 25, 2024

@DaanSelen @donaldzou Oh and theres no attack surface for the CVE in question. Its Mitigated until pip patch.

  1. the --extra-index-url option is not utilized during pip actions anywhere
  2. Container Start sequence interrupt required for injection and exploitation.
  3. Python & Friends are not ran directly in docker file system layers

@donaldzou
Copy link
Owner

@DaanSelen @donaldzou Its been a pleasure working with the two of you. There are some aspects of the docker implementation that need to be discussed before any further changes. I.E Docs. For now I'm burnt, happy hunting 😄.

No problem, and thank you so much on helping this!!

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 26, 2024

@DaanSelen
I forgot to update the Healthcheck.
HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 CMD sh -c 'pgrep gunicorn > /dev/null && pgrep tail > /dev/null' || exit 1

@DaanSelen
Copy link
Collaborator Author

DaanSelen commented Aug 26, 2024

@DaanSelen I forgot to update the Healthcheck. HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 CMD sh -c 'pgrep gunicorn > /dev/null && pgrep tail > /dev/null' || exit 1

I'll look ino the Healthcheck, but the one your proposed checks for the gunicorn process, why this over a simple curl call?

It also does not work out-of-the-box anymore. So I have to analyze the changes, and streamline them further in the coming days.
And its configurations are faulty out of the box because of the Docker IP.

@DaanSelen DaanSelen unpinned this issue Aug 26, 2024
@DaanSelen
Copy link
Collaborator Author

@DaanSelen I forgot to update the Healthcheck. HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 CMD sh -c 'pgrep gunicorn > /dev/null && pgrep tail > /dev/null' || exit 1

What was the use of the multistage build? Was it to mitigate the Pip installation? Because the package is still present on your changed version. Docker Scout just does not show it.

06b0265790ba:/opt/wireguarddashboard/src# pip --version
pip 24.0 from /usr/lib/python3.12/site-packages/pip (python 3.12)

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 26, 2024

@DaanSelen 1. The curl call does not work under in prod or dev.
2. checking for both the gunicorn and tails processes is a better health check, because it works.
3.
Screenshot 2024-08-26 at 8 43 42 AM
The mitigation step is to keep pip install away from any active execution layers. Which in the case of docker is the entrypoint script to which the pip install is apart of the child process and not the parent entrypoint shell.

@DaanSelen
Copy link
Collaborator Author

@NOXCIS Can you elaborate more than "It does not work" and "because it works".

How does the curl call not work? It calls the web service and if it gets a response it marks it as healthy, true this is shallow. But so it looking if there is a Gunicorn process.

Please I would like to know why you think so!

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 26, 2024

@DaanSelen Honestly dont know why its not working for me, i just know that i don't need to have curl installed in the image. Its barebones and failsafe to check for gunicorn.

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 26, 2024

@DaanSelen objective is to have a final docker image that is so bare bones it nerfs privilege escalation and lateral movement from within the container if someone like me found a away in through the frontend. Hence why i was adamant on not shipping an image with a c lib, compiler, git, curl or anything that would be useful for exploitation.

@DaanSelen
Copy link
Collaborator Author

@DaanSelen objective is to have a final docker image that is so bare bones it nerfs privilege escalation and lateral movement from within the container if someone like me found a away in through the frontend. Hence why i was adamant on not shipping an image with a c lib, compiler, git, curl or anything that would be useful for exploitation.

I know that the less packages the better, but right now its a bit of a conflict between functionality and security. In my opinion the Docker Image should be made around WGDashboard, not WGDashboard around the docker image. The WGDashboard itself should be secure enough, otherwise exploiting a Docker container is a small task compared to exploiting a system which has WGDashboard installed as a SystemD Daemon.

@NOXCIS
Copy link
Contributor

NOXCIS commented Aug 26, 2024

@DaanSelen The Docker image is built around WGDashboard. The docker image is just the alpine OS with with WGDashboard & dependencies . Essentially a quasi VM whose sole purpose is to run WGDashboard. Full bare metal functionality still works, docker and Bare Metal share the same install sequence function in wgd.sh and call separate functions for their respective start sequences. Full functionality and full security can be done without compromise, its been done here. But security should never be compromised for the sake of "functionality" with a VPN Server Dashboard.

In short, the Dashboard runs with all of its functionality, however passing environment variables from docker compose is not an original function of the dashboard. Neither is hard coding said passed vars into the dockerfile a part of original functionality.
In v3 they were somewhat necessary as there was no way to create configs from within the dashboard. Doing so pokes holes in everything and allows for issues such as this one we are conversating under because the issue is hard baked into the image it self for everyone to pull from. Docker environment vars should be passed into a python venv with .env file to prevent vars passed from the docker space being executed in the python venv space.

As for the docker exploitation, It shares the host system kernel with its containers. I.E exploit docker exploit the host system at the root level.

@DaanSelen
Copy link
Collaborator Author

@DaanSelen The Docker image is built around WGDashboard. The docker image is just the alpine OS with with WGDashboard & dependencies . Essentially a quasi VM whose sole purpose is to run WGDashboard. Full bare metal functionality still works, docker and Bare Metal share the same install sequence function in wgd.sh and call separate functions for their respective start sequences. Full functionality and full security can be done without compromise, its been done here. But security should never be compromised for the sake of "functionality" with a VPN Server Dashboard.

In short, the Dashboard runs with all of its functionality, however passing environment variables from docker compose is not an original function of the dashboard. Neither is hard coding said passed vars into the dockerfile a part of original functionality. In v3 they were somewhat necessary as there was no way to create configs from within the dashboard. Doing so pokes holes in everything and allows for issues such as this one we are conversating under because the issue is hard baked into the image it self for everyone to pull from. Docker environment vars should be passed into a python venv with .env file to prevent vars passed from the docker space being executed in the python venv space.

As for the docker exploitation, It shares the host system kernel with its containers. I.E exploit docker exploit the host system at the root level.

Thanks for the info, and its true that some features in the Docker container are unique compared to the WGDashboard itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants