rustdesk / rustdesk-server

RustDesk Server Program
https://rustdesk.com/server
GNU Affero General Public License v3.0
6.74k stars 1.45k forks source link

Allow keypair to be mounted as docker secrets #287

Open lonix1 opened 1 year ago

lonix1 commented 1 year ago

Is your feature request related to a problem? Please describe.

The self hosted docker option allows to mount the keypair as file mounts:

volumes:
  - ./id_ed25519:/data/id_ed25519:ro
  - ./id_ed25519.pub:/data/id_ed25519.pub:ro

Although this works, it's not ideal for such sensitive secrets.

Describe the solution you'd like

Please consider allowing the keypair to be mounted as proper docker secrets:

secrets:
  secret_private:
    file: id_ed25519
  secret_public:
    file: id_ed25519.pub

services:
  rustdesk:
    secrets:
      - secret_private
      - secret_public
    image: rustdesk/rustdesk-server-s6:latest
    # ...

Those secrets would be automatically mounted at the standard docker paths:

/run/secrets/secret_private
/run/secrets/secret_public

Then the app can load the secrets from those files directly.

So from rustdesk's perspective, the ONLY change is to read from /run/secrets/secret_private and /run/secrets/secret_public instead of /data/id_ed25519 and /data/id_ed25519.pub. That's it.

Describe alternatives you've considered

Mounting as file mounts, as shown above.

Additional context

Thanks for considering it.

dinger1986 commented 1 year ago

If you want to PR the change or docs or anything please feel free and the devs can review the changes 😀

paspo commented 1 year ago

If you want to use docker (or k8s) secrets, please refer to this section of the readme.

lonix1 commented 1 year ago

Thank you @paspo I didn't see that part of the docs. I changed to docker secrets and it works perfectly!

One note though:

It seems the app copies the secrets from /run/secrets/key_priv and /run/secrets/key_pub to /data/id_ed25519 and /data/id_ed25519.pub, respectively.

Surely they should be read from the original path only? The two paths have different attack surfaces, so this behaviour is circumventing the docker secrets mechanism.

paspo commented 1 year ago

Surely they should be read from the original path only? The two paths have different attack surfaces, so this behaviour is circumventing the docker secrets mechanism.

I agree with that.

Unfortunately this is not possible yet, because hbbr and hbbs binaries expects the key pair to be in specific files inside the current working directory.

That's why this scripts is going to copy the secrets in /data, by collecting from ENV or secrets; also, the start scripts for hbbs and hbbr doesn't have any parameter to specify the key location.

IMHO this can be resolved in 2 ways:

rustdesk commented 1 year ago

It seems the app copies the secrets from /run/secrets/key_priv and /run/secrets/key_pub to /data/id_ed25519 and /data/id_ed25519.pub, respectively.

How about a symbol link?

lonix1 commented 1 year ago

How about a symbol link?

My gut feeling tells me that it would basically be the same problem... BUT, I don't know for sure.

However, @paspo suggestion seems good. I've seen something like this in other apps:

secrets:
  - key_priv:
      filename: ./id_ed25519
  - key_pub:
      filename: ./id_ed25519.pub

services:
  rustdesk:
    secrets:
      - key_priv
      - key_pub
    environment:
      - RUSTDESK_PRIVATE_KEY: /run/secrets/key_priv     # <-----
      - RUSTDESK_PUBLIC_KEY: /run/secrets/key_pub       # <-----
    # ...

The app would then read the secrets from files specified by those env vars.

paspo commented 9 months ago

just to frame this a little more, you can start hbbs like this:

hbbs -k IRvYWCnv72zAFBYO4LT9h5yUfqLQXG0/DVw68yFfr55UrtLfm6TjcOcvo+nLtscsA4VxvKcVJfJYC2rpi2R2DA==

You don't even need a key file. But this approach has a big disadvantage: you have the secret key dumped in every process list! And BTW, the public key is also dumped on stdout at start, which is a minor issue, but still avoidable.