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

Bug: can't work with Docker secrets #172

Closed
V33m opened this issue Jun 18, 2022 · 15 comments · Fixed by #173
Closed

Bug: can't work with Docker secrets #172

V33m opened this issue Jun 18, 2022 · 15 comments · Fixed by #173
Labels
bug Something isn't working as expected

Comments

@V33m
Copy link

V33m commented Jun 18, 2022

Hi, what an awesome service you have created!

I've a minor thing as I can't seem to figure out how to fully disable ipv6 using docker-compose. Hence my logs are full of

😞 Failed to send HTTP(S) request to "https://[REMOVED]/cdn-cgi/trace": Get "https://[REMOVED]/cdn-cgi/trace": dial tcp [REMOVED]:443: connect: cannot assign requested address
😞 Failed to detect the IPv6 address
🔧 If you are using Docker, Kubernetes, or other frameworks, IPv6 networks often require additional setups.
🔧 Read more about IPv6 networks in the README at https://github.com/favonia/cloudflare-ddns

Under highlights it says "Ability to enable or disable IPv4 and IPv6 individually." I've tried to disable ipv6 by adding the following to the env in docker-compose: - IP6_PROVIDER=none

If I set IP4_POLICY=none which is listed as a valid value in the readme I get:

   🔸 Use default IP4_POLICY=cloudflare.trace
   😡 Failed to parse "none": not a valid policy
@favonia
Copy link
Owner

favonia commented Jun 18, 2022

Hi! Thanks for the reporting. There was a recent change in the names of the environment variables by PR #167 so that the README on the main branch (which is about the development version) does not match the stable version you are using. (The older README is at https://github.com/favonia/cloudflare-ddns/blob/v1.4.0/README.markdown.) Sorry about the trouble. While the development version recognizes both the old IP6_POLICY=unmanaged (with a warning) and the new IP6_PROVIDER=none, the stable (older) version only recognizes IP6_POLICY=unmanaged. That's why IP6_PROVIDER=none has no effects.

I guess the development version is stable enough for another release (there's some internal rewrite), and in the meanwhile you could use IP6_POLICY=unmanaged to disable IPv6.

@V33m
Copy link
Author

V33m commented Jun 18, 2022

Cool, thanks! One more thing, I'm using secrets for my other Docker services, yet I can't seem to get it to work as usual. Any suggestions? All secrets are in a secrets folder and are defined correctly in the docker-compose file (I'm using 8 secrets and for multiple containers)

  cloudflare-ddns:
    image: favonia/cloudflare-ddns:latest
    network_mode: host
    restart: always
    security_opt:
      - no-new-privileges:true
    environment:
      - PGID=1001
      - PUID=1002
      - CF_API_TOKEN_FILE=/run/secrets/cf_api_token
      - DOMAINS=REMOVED
      - PROXIED=true
      - IP6_POLICY=unmanaged
    secrets:
      - cf_api_token

Logs:

📖 Reading settings . . .
   😡 Failed to read "/run/secrets/cf_api_token": open /run/secrets/cf_api_token: invalid argument

@favonia favonia changed the title Can't seem to disable ipv6 Bug: open: invalid argument Jun 18, 2022
@favonia
Copy link
Owner

favonia commented Jun 18, 2022

This shouldn't happen and looks like a bug. Please give me some time to check things. (The code was tested against a fake file system during testing, and maybe there are some differences between a real file system and the fake one I missed.)

PS: I changed the title for me to track things, but feel free to change it to something you like better.

@favonia
Copy link
Owner

favonia commented Jun 18, 2022

A temporary workaround, while not ideal, is to use CF_API_TOKEN, but I suppose you already knew it.

@V33m
Copy link
Author

V33m commented Jun 18, 2022

Yeah, I'm currently using CF_API_TOKEN. Thanks again for the incredible work👍

@favonia
Copy link
Owner

favonia commented Jun 18, 2022

I think I found the problem: golang/go#44279

The issue is that os.DirFS("/") only supports non-root paths, so

os.DirFS("/").Open("/run/secrets/cf_api_token")

would fail but

os.DirFS("/").Open("run/secrets/cf_api_token")

would work (for all the wrong reasons). This was not caught by the current testing because I did not test the paths beginning with the slash /. The new code was introduced when Go 1.16 was released in hope for better testing (e.g., visualizing the entire file system). It would have worked with the old code that calls os.Open directly. I have to think a little bit about how to deal with this strange os API.

@favonia
Copy link
Owner

favonia commented Jun 18, 2022

@V33m I believe I fixed it but you might have to wait for the next release, or use edge instead of latest for the development version.

@V33m
Copy link
Author

V33m commented Jun 18, 2022

Great work! No worries, I'll wait for the next release, hopefully it's not months away🙂

@favonia
Copy link
Owner

favonia commented Jun 18, 2022

@V33m okay I think it's done---docker-compose pull and then docker-compose up -d should do the trick. Let me know if you encountered other issues. Thank you again for the reporting.

@favonia favonia added the bug Something isn't working as expected label Jun 18, 2022
@V33m
Copy link
Author

V33m commented Jun 18, 2022

Unfortunately I can't get it to work and I think it's related to the path. Regardless if I use /run/secrets/cf_api_token or run/secrets/cf_api_token, I see in my logs:

😡 Failed to read "run/secrets/cf_api_token": open //run/secrets/cf_api_token: permission denied

Isn't there a / too many? So the code should not include the / before the path

newpath, err := filepath.Rel("/", path)

Here is an example of how it looks for Traefik
- CF_DNS_API_TOKEN_FILE=/run/secrets/traefik_cf_dns_api_token

@favonia favonia changed the title Bug: open: invalid argument Bug: can't work with Docker secrets Jun 18, 2022
@favonia
Copy link
Owner

favonia commented Jun 18, 2022

The Rel function would actually remove the beginning /. (Sorry, this might be wrong, but it should not matter.) In Unix-like systems, // would not hurt.

The error seems to indicate that the group 1001 or the user 1002 might not have the permission to read the secrets. I need some time to check what's going on.

@favonia favonia reopened this Jun 18, 2022
@favonia
Copy link
Owner

favonia commented Jun 18, 2022

@V33m Sorry I made an incorrect comment about Rel. Could you check the read permission of the file---is it readable to the group 1001 or the user 1002? It seems I can reproduce the error message by intentionally restricting the read permission. The tool immediately drops all root privileges when it starts, so the file must be explicitly readable by the group or the user.

@favonia
Copy link
Owner

favonia commented Jun 18, 2022

@V33m Out of curiosity I checked the source code of Traefik. It does not seem to drop any root privileges, and if so, that's why it can read your secret file no matter what the permissions are. The root privileges might be necessary for Traefik, but this also means your system is more vulnerable when Traefik has a security bug. They seem to provide a "rootless" version at https://doc.traefik.io/traefik-enterprise/operations/rootless-image/ which I would recommend for serious business. (It might need more complicated configurations, though.)

@V33m
Copy link
Author

V33m commented Jun 18, 2022

You are absolutely right! I actually thought that running Docker in non-rootless mode, secrets were read and made available to each container. In addition to that I had configured another container with the same PUID/GUID and secrets where everything is fine. Turns out that the container does actuallty not use PUID/GUID...

Anyways, thanks for suggesting a rootless image of Traefik. I'll look into this for sure. I've been thinking about running Docker in rootless mode as well, but I guess that's much more of a hassle. Have a good weekend and thanks again for the awesome support!

@V33m V33m closed this as completed Jun 18, 2022
@favonia
Copy link
Owner

favonia commented Jul 25, 2024

@V33m This is a head-up---recent versions (1.13.0+) will benefit from an update to your configuration, and I am on the mission to eliminate the old template from this universe. The environment variables PUID and PGID in your configuration will be ignored from now on. In case you or anyone is confused by how to update the configuration, here's the tl;dr in your case:

  1. Add cap_drop: [all].
  2. Add user: "1000:1000" (or other IDs you want to use).
  3. Remove PUID=... and PGID=... (optional, but why not).

For more information, please see the CHANGELOG and README.

PS: the new template works for older versions of the updater as well!

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

Successfully merging a pull request may close this issue.

2 participants