budimanjojo / vyos-config

My VyOS IaC configuration
Apache License 2.0
7 stars 3 forks source link

fix: security settings of favonia/cloudflare-ddns #7

Closed favonia closed 1 month ago

favonia commented 1 month ago

Thanks for using my DDNS updater. Since version 1.13.0 (released on 16 July), the updater has stopped dropping superuser privileges by itself, instead relying on Docker's built-in mechanism to drop those privileges. The new way is safer, cleaner, and more reliable; but it requires an update to the configuration. In particular, the environment variables PUID=uid and PGID=gid should be replaced by user: "uid:gid" or --user uid:gid. I am on a mission to eliminate the old template from the internet. Please help me promote security best practices!

For more information about this design change, please read the CHANGELOG. If copyright ever matters, this PR itself is licensed under CC0, which should allow you to do whatever you want. Thank you again for your interest in the updater.

PS: I do not have a VyOS to test the script. Please let me know whether it works or not. BTW, it’s sad that VyOS does not seem to provide many other useful protections such as “dropping all Linux capabilities” or “making the filesystem read-only”.

budimanjojo commented 1 month ago

Thank you @favonia!

Unfortunately I'm still running a very old version of VyOS (I'm in the process of migrating away from VyOS because of their recent drama on not letting people freely build LTS image). The version of VyOS I'm running doesn't have uid and gid option yet, so I just locally tried using set container name cloudflare-ddns arguments "-u 1000 -g 1000" and it runs just fine.

But checking at the container log, it doesn't say anything about what is the current user running the container. How do I confirm that I'm running non root user?

budimanjojo commented 1 month ago

Seems like arguments is not the correct way to do this too, it's adding the argument to the container and not the docker command. So I guess there's no way for this change for me?

favonia commented 1 month ago

@budimanjojo That sounds sad. I did not realize an older VyOS might not have uid or gid available.

On the other hand, if you do not do anything, the latest Docker image (1.13.2, not 1.13.0) will use the user/group IDs 1000:1000. For the version 1.13.0 and later, you will receive a warning in the very beginning if you are running the updater as root. Otherwise it will remain quiet. That is, if it doesn't say anything, then you are not running it as root.

favonia commented 1 month ago

So I guess there are a few ways to move forward:

  1. Use Docker images of 1.13.2+ that will run as 1000:1000 by default, and live with it.
  2. Upgrade VyOS to a version that supports uid and gid.
  3. Migrate to something other than VyOS (?).

The main point is that PUID and PGID are useless now and I want to prevent people from copying the misleading template. :grin:

budimanjojo commented 1 month ago

Thank you! As I'm right now running 1.13.2 and I don't see any warning other than warning about PUID and PGID are useless now, I think I'm just gonna let it be. Mind if you update your commit to just remove the environment variable and not adding the uid and gid so I can accept this PR? Or I can do it myself too if you're not willing to do it.

I'm definitely going for number 3 (moving away from VyOS) in the future once I have the time to do it. Not going for number 2 because there are some breaking changes made in VyOS preventing me to update without doing more work that I don't feel like doing.

And thank you for even spending your time looking around at your users repo like this, I really appreciate it!

favonia commented 1 month ago

@budimanjojo Alright, done. (FYI: you can actually push commits to the branch of this PR directly.)

budimanjojo commented 1 month ago

Thank you!