Stored rpc credentials cleared if not included in environment variables or docker secrets

toddrob99 commented 2 years ago

Is there a pinned issue for this?

[X] I have read the pinned issues and could not find my issue

Is there an existing or similar issue/discussion for this?

[X] I have searched the existing issues
[X] I have searched the existing discussions

Is there any comment in the documentation for this?

[X] I have read the documentation, especially the FAQ and Troubleshooting parts

Is this related to a provider?

[X] I have checked the provider repo for issues
[X] My issue is NOT related to a provider

Are you using the latest release?

[X] I am using the latest release

Have you tried using the dev branch latest?

[X] I have tried using dev branch

Docker run config used

version: '3.3'
services:
  transmission:
    container_name: transmission
    cap_add:
      - NET_ADMIN
    volumes:
      - /volume1/Downloads:/downloads
      - /volume1/Downloads/transmission/completed:/data/completed
      - /volume1/Downloads/transmission/incomplete:/data/incomplete
      - /volume1/Downloads/transmission/watch:/data/watch
      - ./transmission-home:/data/transmission-home
      - ./config:/config
    environment:
      - PUID=1033
      - PGID=65537
      - TZ=America/New_York
      - OPENVPN_PROVIDER=NORDVPN
      - OPENVPN_OPTS=--inactive 3600 --ping 10 --ping-exit 60
      - LOCAL_NETWORK=192.168.0.0/23
      - WEBPROXY_ENABLED=true
      - WEBPROXY_PORT=8118
      - TRANSMISSION_RPC_AUTHENTICATION_REQUIRED=true
      - TRANSMISSION_RPC_ENABLED=true
      - TRANSMISSION_RPC_USERNAME=xxxx
      - TRANSMISSION_RPC_PASSWORD=xxxx
    logging:
      driver: json-file
      options:
        max-size: 10m
    ports:
      - '9091:9091'
      - '8118:8118'
    dns:
      - 8.8.8.8
      - 8.8.4.4
    restart: unless-stopped
    image: haugene/transmission-openvpn
    networks:
      - media-automation

  transmission-proxy:
    container_name: transmission-proxy
    links:
      - 'transmission:transmission'
    ports:
      - '8888:8080'
    image: haugene/transmission-openvpn-proxy
    networks:
      - media-automation

networks:
  media-automation:
    name: media-automation
    external: true

Current Behavior

Discussion in #2386

After upgrading to the latest version, rpc credentials are wiped out if not included in the container's environment variables or docker secrets. Logging in with blank username/password works, but previously-stored credentials do not.

Previously, I set the username/password via environment variables and they were written to transmission-credentials.txt, and then removed the environment variables to avoid having my password in the docker-compose.yml file. The values in transmission-credentials.txt would persist across container restarts/recreations.

It appears the logic introduced in #2321 to support docker secrets has broken this scenario, and the values in transmission-credentials.txt are wiped out if not still defined in environment variables.

Expected Behavior

Values in transmission-credentials.txt should not be cleared out upon container start if no credentials are defined in environment variables or docker secrets.

How have you tried to solve the problem?

Tested by adding rpc username/password back to environment variables and confirming that works. Then removed environment variables and observed transmission-credentials.txt was cleared out upon container restart. Found #2321 and reviewed the logic--it appears pre-existing values in transmission-credentials.txt are not considered.

Log output

TRANSMISSION_HOME is currently set to: /config/transmission-home
WARNING: Deprecated. Found old default transmission-home folder at /data/transmission-home, setting this as TRANSMISSION_HOME. This might break in future versions.
We will fallback to this directory as long as the folder exists. Please consider moving it to /config/<transmission-home>
Enforcing ownership on transmission config directory
Applying permissions to transmission config directory
Setting owner for transmission paths to 1033:65537
Setting permissions for download and incomplete directories
Mask: 002
Directories: 775
Files: 664
Setting permission for watch directory (775) and its files (664)

-------------------------------------
Transmission will run as
-------------------------------------
User name:   abc
User uid:    1033
User gid:    65537
-------------------------------------

Updating Transmission settings.json with values from env variables
Attempting to use existing settings.json for Transmission
Successfully used existing settings.json /data/transmission-home/settings.json
Overriding bind-address-ipv4 because TRANSMISSION_BIND_ADDRESS_IPV4 is set to 10.8.3.10
Overriding download-dir because TRANSMISSION_DOWNLOAD_DIR is set to /data/completed
Overriding incomplete-dir because TRANSMISSION_INCOMPLETE_DIR is set to /data/incomplete
Overriding rpc-authentication-required because TRANSMISSION_RPC_AUTHENTICATION_REQUIRED is set to true
Overriding rpc-enabled because TRANSMISSION_RPC_ENABLED is set to true
Overriding rpc-password because TRANSMISSION_RPC_PASSWORD is set to [REDACTED]
Overriding rpc-port because TRANSMISSION_RPC_PORT is set to 9091
Overriding rpc-username because TRANSMISSION_RPC_USERNAME is set to
Overriding watch-dir because TRANSMISSION_WATCH_DIR is set to /data/watch
sed'ing True to true
STARTING TRANSMISSION
Transmission startup script complete.
Privoxy: Starting
Privoxy: Using config file at /etc/privoxy/config
Privoxy: Setting port to 8118
Privoxy: Running as PID 155
Thu Nov 10 08:57:09 2022 Initialization Sequence Completed

HW/SW Environment

- OS: DSM 7.1.1-42962
- Docker: 20.10.3, build 55f0773

Anything else?

No response

myna-me commented 2 years ago

Same situation. If the settings.json file is edited and it contains the user name and password, the user gets erased. Don't know about the password, since i can't authenticate at that point.

If you look at @toddrob99 's log, you will see that it says: Overriding rpc-username because TRANSMISSION_RPC_USERNAME is set to and then nothing. That's exactly what it is setting the user name to...nothing.

btw, @toddrob99 , your log did show me something i didn't know: as of this version, they would like the transmission-home directory to be under /config and not under /data. Thanks for that. :) I have now moved it to the new location

toddrob99 commented 2 years ago

btw, @toddrob99 , your log did show me something i didn't know: as of this version, they would like the transmission-home directory to be under /config and not under /data. Thanks for that. :) I have now moved it to the new location

I haven't done anything with that yet. I think I just need to move my transmission-home folder inside my folder that's mapped to /config and remove the /data/transmission-home volume mapping. It's not clear to me if I also have to set a TRANSMISSION_HOME environment variable to /config/transmission-home or if that will be implied. I haven't taken the time to do it and see.

jmrushing commented 2 years ago

Is there a way to stop the container from generating these '*-credentials.txt' files??? Since last week's breaking changes (or maybe it was because I switched to using '/config'?), I noticed that 'openvpn-credentials.txt' and 'transmission-credentials.txt' are being creating in my main docker directory. These files show user/passwords in plaintext. This wasn't happening before. I set my rpc and openvpn logins in environment variables and I don't use docker secrets. Why and when did the container start dumping plaintext login files? I don't want my passwords stored by the container this way! :angry:

elisimpson commented 2 years ago

Something to note while this is being worked on is the permissions for the two password files are not optimal. They get generated (at least on my machine) as group and everyone readable, which isn't good, whereas the dht, settings, and log files aren't group and everyone readable, which might be by design or could be a mistake. If I modify the permissions manually they get re-written at some point.

The log file even warns about the password files being readable, which is strange because they're generated that way by the app.

ColinHebert commented 1 year ago

A bit about the current behaviour:

Credentials need to always be available as environment variables as openvpn/start.sh always erases the content of /config/transmission-credentials.txt with what is currently set in the environment variables (it was already the case pre #2321). This file is used by various scripts (scripts/route-pre-down.sh, openvpn/pia/update-port.sh) and isn't all that new (https://github.com/haugene/docker-transmission-openvpn/commit/f7a7c2d4b47016787b33111cb78582f9d2276b38).
transmission/start.sh invokes transmission/updateSettings.py which loads environment variables that relate to transmission settings and inject them into the settings.json. As part of #2142 in January last year, the creation of the environment variables TRANSMISSION_RPC_USERNAME and TRANSMISSION_RPC_PASSWORD is forced through the dockerfile. This means that now, every time transmission/updateSettings.py those two variables will always be set (even to an empty string) therefore updateSettings.py will always update the username and password to the environment variables. (NB: A possible better behaviour would have been checking whether the variable exists rather than forcing its existence)
2321 introduced system where the environment variables for RPC could be left unset, but would then be loaded from the docker secret file automatically. Unfortunately, this PR doesn't account for the fact that the environment variable should always be set (due to the first point) otherwise it defaults to an empty value (due to the second point). The flawed logic in the script leads to a first run with docker secret to work properly (as the credentials will be loaded in environment variables) but failing subsequent runs (as the rpc_creds and transmission-credentials.txt are identical, skipping the environment variable loading)

Overall, to my knowledge, it has not been the case that environment variables could "not be set" like @toddrob99 suggests. It would indeed have a working settings.json (at least before TRANSMISSION_RPC_USERNAME and TRANSMISSION_RPC_PASSWORD were forced in the Dockerfile), but transmission-credentials.txt would have been erased, leading to route-pre-down.sh and pia/update-port.sh to fail (albeit it might not be noticeable that those script would fail.

To @myna-me's point, any direct edit to settings.json will always be erased (due to the #2142/updateSettings.py interaction).

For @jmrushing, indeed the creds are stored in the config volume, this could/should be easily solved by replacing the paths to non volume paths.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Forage commented 1 year ago

If the current behaviour where the rpc-username and rpc-password params in settings.json are overwritten is as intended then this should at least be described in the docs. Now you have a selection of params that can/should be changed in that file and they are persistent and another selection of params that are not.

In my case I removed all transmission environment variables thinking they can all be set through the settings file directly now.

Ideally those params shouldn't be overwritten with nothing if no environment variables are provided to begin with, to prevent the unpredictable behaviour.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Forage commented 1 year ago

Don't close it yet as it remains an issue.

stale[bot] commented 1 year ago

Feel free to re-open this issue if you think it deserves another look.

darmach commented 2 months ago

How I hate those bots @Forage . Of course it deserves another look - rpc working but allowing an empty logon is not a sane behaviour. Let's at least disable rpc as a default...

haugene / docker-transmission-openvpn