From 7d1b9af8588228991305803df9c67ff839c645ad Mon Sep 17 00:00:00 2001 From: Bolke de Bruin Date: Thu, 30 Apr 2026 19:09:15 +0200 Subject: [PATCH] Drop baked-in TLS cert, run as 1001, refuse known placeholder secrets The dev container image generated a TLS keypair at build time and shipped it inside the image, so every pull of the same image tag was serving the same private key. The entrypoint also reverted to USER 0 to support a dead `createusers.txt` loop and a `chmod u+s` that was a no-op (set on a binary owned by 1001). Net result was that any RCE in the gateway landed as root and the wire-trust posture relied on a shared private key. Stop generating the cert at build time: the runtime image now carries openssl and the entrypoint mints an ephemeral self-signed cert at first start when no cert is mounted at the configured path. Each container instance gets its own key. Drop USER 0 entirely; the entrypoint runs as 1001 throughout. Prune the dead createusers loop and the `chmod u+s`. Separately, the README and the dev compose files publish a small set of literal placeholder values for SessionKey, SessionEncryptionKey, and the various Token*Key fields. Operators copy-paste these into real deployments. Refuse to start when any of those literals appear in the corresponding config field. --- CHANGELOG.md | 11 ++++ UPGRADING.md | 44 ++++++++++++++ cmd/rdpgw/config/configuration.go | 44 ++++++++++++++ cmd/rdpgw/config/configuration_test.go | 79 ++++++++++++++++++++++++++ dev/docker/Dockerfile | 22 ++----- dev/docker/run.sh | 45 ++++++--------- 6 files changed, 202 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75f1bef..89a9ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. ### Changed +- The dev container image no longer ships a baked-in TLS key/cert. + The runtime image carries `openssl` and the entrypoint generates an + ephemeral self-signed cert at first start if none is mounted at the + configured path; each container instance gets its own key. The + entrypoint also runs entirely as UID 1001 (no more `USER 0`). +- rdpgw refuses to start when any of `Server.SessionKey`, + `Server.SessionEncryptionKey`, `Security.PAATokenSigningKey`, + `Security.PAATokenEncryptionKey`, `Security.UserTokenSigningKey`, + `Security.UserTokenEncryptionKey`, or `Security.QueryTokenSigningKey` + matches a published placeholder value from `README.md` / the dev + compose files. See [UPGRADING.md](UPGRADING.md) for the full list. - `rdpgw-auth` now creates its socket with mode `0660` and accepts only connections whose peer UID is on an allow-list (default: the daemon's own UID). Operators running rdpgw and rdpgw-auth as different users diff --git a/UPGRADING.md b/UPGRADING.md index 37569db..797ae79 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -2,6 +2,50 @@ ## Unreleased +### Container image no longer bakes a TLS cert and runs as UID 1001 + +Two changes to `dev/docker/Dockerfile` and the dev image's entrypoint: + +* The build step that ran `openssl genrsa` / `openssl x509` and shipped + the resulting key/cert in the image is gone. The runtime image now + contains `openssl` and the entrypoint mints an ephemeral self-signed + cert at first start when no cert is mounted at the configured path. + Each container instance gets its own key. +* The entrypoint runs as UID 1001 throughout. The previous `USER 0` + block, the dead `createusers.txt` loop in `run.sh`, and the + `chmod u+s` on `rdpgw-auth` (which set the bit on a file owned by + 1001 and was therefore a no-op anyway) have been removed. + +If you mount your own cert (recommended for any non-dev deployment), +make sure `RDPGW_SERVER__CERT_FILE` and `RDPGW_SERVER__KEY_FILE` point +to the mounted paths. Otherwise the entrypoint generates a fresh +self-signed pair at the configured locations on first start. + +If your deployment relies on `rdpgw-auth` running with elevated +privileges from the same image, run it from a separate container or +use Linux capabilities -- see +[docs/pam-authentication.md](docs/pam-authentication.md) for the +privilege-separation model. + +### Refuse to start when known placeholder secrets are configured + +These literal values appearing in `Server.SessionKey`, +`Server.SessionEncryptionKey`, `Security.PAATokenSigningKey`, +`Security.PAATokenEncryptionKey`, `Security.UserTokenSigningKey`, +`Security.UserTokenEncryptionKey`, or `Security.QueryTokenSigningKey` +will now cause rdpgw to refuse to start: + +``` +thisisasessionkeyreplacethisjetzt +thisisasessionkeyreplacethisjetz +thisisasessionkeyreplacethisnunu! +thisisasessionkeyreplacethisnunu +thisisasessionencryptionkey12345 +``` + +These are the published example values from `README.md` and the dev +compose files. Replace them with unique 32-character strings. + ### `rdpgw-auth` only accepts connections from the daemon's own UID by default The auth daemon previously created its socket world-writable diff --git a/cmd/rdpgw/config/configuration.go b/cmd/rdpgw/config/configuration.go index 3019f2f..4f2f962 100644 --- a/cmd/rdpgw/config/configuration.go +++ b/cmd/rdpgw/config/configuration.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "log" "os" "strings" @@ -13,6 +14,45 @@ import ( "github.com/knadh/koanf/v2" ) +// knownDefaultSecrets is the list of placeholder session/token-key values +// that appear in README.md and the dev compose files. Operators that +// copy-paste the examples into a real deployment would be running with +// widely-published keys, so the gateway refuses to start when one of these +// is set for any of the session-/token-binding fields. +var knownDefaultSecrets = []string{ + "thisisasessionkeyreplacethisjetzt", + "thisisasessionkeyreplacethisjetz", + "thisisasessionkeyreplacethisnunu!", + "thisisasessionkeyreplacethisnunu", + "thisisasessionencryptionkey12345", +} + +func checkDefaultSecrets(c *Configuration) error { + fields := []struct { + name string + value string + }{ + {"server.sessionkey", c.Server.SessionKey}, + {"server.sessionencryptionkey", c.Server.SessionEncryptionKey}, + {"security.paatokensigningkey", c.Security.PAATokenSigningKey}, + {"security.paatokenencryptionkey", c.Security.PAATokenEncryptionKey}, + {"security.usertokensigningkey", c.Security.UserTokenSigningKey}, + {"security.usertokenencryptionkey", c.Security.UserTokenEncryptionKey}, + {"security.querytokensigningkey", c.Security.QueryTokenSigningKey}, + } + for _, f := range fields { + if f.value == "" { + continue + } + for _, def := range knownDefaultSecrets { + if f.value == def { + return fmt.Errorf("%s is set to a known placeholder value (%q); replace it with a unique secret before starting", f.name, def) + } + } + } + return nil +} + const ( TlsDisable = "disable" TlsAuto = "auto" @@ -219,6 +259,10 @@ func Load(configFile string) Configuration { k.UnmarshalWithConf("Client", &Conf.Client, koanfTag) k.UnmarshalWithConf("Kerberos", &Conf.Kerberos, koanfTag) + if err := checkDefaultSecrets(&Conf); err != nil { + log.Fatalf("refusing to start: %s", err) + } + if len(Conf.Security.PAATokenEncryptionKey) != 32 { Conf.Security.PAATokenEncryptionKey, _ = security.GenerateRandomString(32) log.Printf("No valid `security.paatokenencryptionkey` specified (empty or not 32 characters). Setting to random") diff --git a/cmd/rdpgw/config/configuration_test.go b/cmd/rdpgw/config/configuration_test.go index cb204a7..05d93b0 100644 --- a/cmd/rdpgw/config/configuration_test.go +++ b/cmd/rdpgw/config/configuration_test.go @@ -53,6 +53,85 @@ func TestAuthenticationConstants(t *testing.T) { } } +func TestCheckDefaultSecrets(t *testing.T) { + const placeholder = "thisisasessionkeyreplacethisjetzt" + + cases := []struct { + name string + mutate func(*Configuration) + wantField string + }{ + { + name: "session key", + mutate: func(c *Configuration) { c.Server.SessionKey = placeholder }, + wantField: "server.sessionkey", + }, + { + name: "session encryption key", + mutate: func(c *Configuration) { c.Server.SessionEncryptionKey = placeholder }, + wantField: "server.sessionencryptionkey", + }, + { + name: "paa signing key", + mutate: func(c *Configuration) { c.Security.PAATokenSigningKey = placeholder }, + wantField: "security.paatokensigningkey", + }, + { + name: "paa encryption key", + mutate: func(c *Configuration) { c.Security.PAATokenEncryptionKey = placeholder }, + wantField: "security.paatokenencryptionkey", + }, + { + name: "user signing key", + mutate: func(c *Configuration) { c.Security.UserTokenSigningKey = placeholder }, + wantField: "security.usertokensigningkey", + }, + { + name: "user encryption key", + mutate: func(c *Configuration) { c.Security.UserTokenEncryptionKey = placeholder }, + wantField: "security.usertokenencryptionkey", + }, + { + name: "query signing key", + mutate: func(c *Configuration) { c.Security.QueryTokenSigningKey = placeholder }, + wantField: "security.querytokensigningkey", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := &Configuration{} + tc.mutate(c) + err := checkDefaultSecrets(c) + if err == nil { + t.Fatalf("checkDefaultSecrets accepted a placeholder value in %s", tc.wantField) + } + if got := err.Error(); !contains(got, tc.wantField) { + t.Errorf("error message %q should mention the field %q", got, tc.wantField) + } + }) + } +} + +func TestCheckDefaultSecretsAllowsRandomValues(t *testing.T) { + c := &Configuration{} + c.Server.SessionKey = "5aa3a1568fe8421cd7e127d5ace28d2d" + c.Server.SessionEncryptionKey = "d3ecd7e565e56e37e2f2e95b584d8c0c" + c.Security.PAATokenSigningKey = "0123456789abcdef0123456789abcdef" + if err := checkDefaultSecrets(c); err != nil { + t.Errorf("checkDefaultSecrets rejected non-placeholder values: %v", err) + } +} + +func contains(haystack, needle string) bool { + for i := 0; i+len(needle) <= len(haystack); i++ { + if haystack[i:i+len(needle)] == needle { + return true + } + } + return false +} + func TestHeaderConfigValidation(t *testing.T) { cases := []struct { name string diff --git a/dev/docker/Dockerfile b/dev/docker/Dockerfile index 3123609..e7fb0c9 100644 --- a/dev/docker/Dockerfile +++ b/dev/docker/Dockerfile @@ -2,21 +2,11 @@ FROM golang:1.24-alpine as builder # Install CA certificates explicitly in builder -RUN apk --no-cache add git gcc musl-dev linux-pam-dev openssl +RUN apk --no-cache add git gcc musl-dev linux-pam-dev # add user RUN adduser --disabled-password --gecos "" --home /opt/rdpgw --uid 1001 rdpgw -# certificate generation -RUN mkdir -p /opt/rdpgw && cd /opt/rdpgw && \ - random=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 32 | head -n 1) && \ - openssl genrsa -des3 -passout pass:$random -out server.pass.key 2048 && \ - openssl rsa -passin pass:$random -in server.pass.key -out key.pem && \ - rm server.pass.key && \ - openssl req -new -sha256 -key key.pem -out server.csr \ - -subj "/C=US/ST=VA/L=SomeCity/O=MyCompany/OU=MyDivision/CN=rdpgw" && \ - openssl x509 -req -days 365 -in server.csr -signkey key.pem -out server.pem - # build rdpgw and set rights ARG CACHEBUST RUN git clone https://github.com/bolkedebruin/rdpgw.git /app && \ @@ -25,12 +15,13 @@ RUN git clone https://github.com/bolkedebruin/rdpgw.git /app && \ CGO_ENABLED=0 GOOS=linux go build -trimpath -tags '' -ldflags '' -o '/opt/rdpgw/rdpgw' ./cmd/rdpgw && \ CGO_ENABLED=1 GOOS=linux go build -trimpath -tags '' -ldflags '' -o '/opt/rdpgw/rdpgw-auth' ./cmd/auth && \ chmod +x /opt/rdpgw/rdpgw && \ - chmod +x /opt/rdpgw/rdpgw-auth && \ - chmod u+s /opt/rdpgw/rdpgw-auth + chmod +x /opt/rdpgw/rdpgw-auth FROM alpine:latest -# Install CA certificates in final stage -RUN apk --no-cache add linux-pam musl tzdata ca-certificates && update-ca-certificates +# Install CA certificates and (for the dev compose) openssl so the +# entrypoint can mint an ephemeral self-signed cert at startup. No cert +# is baked into the image, so each container instance gets its own key. +RUN apk --no-cache add linux-pam musl tzdata ca-certificates openssl && update-ca-certificates # make tempdir in case filestore is used ADD tmp.tar / @@ -47,6 +38,5 @@ COPY --from=builder /app/cmd/rdpgw/templates /opt/rdpgw/templates # Copy assets directory from the app source COPY --chown=1001 --from=builder /app/assets /opt/rdpgw/assets -USER 0 WORKDIR /opt/rdpgw ENTRYPOINT ["/bin/sh", "/run.sh"] diff --git a/dev/docker/run.sh b/dev/docker/run.sh index e99dec1..ee82667 100755 --- a/dev/docker/run.sh +++ b/dev/docker/run.sh @@ -1,34 +1,25 @@ #!/bin/sh +set -e -USER=rdpgw +cd /opt/rdpgw -file="/root/createusers.txt" -if [ -f $file ] - then - while IFS=: read -r username password is_sudo - do - echo "Username: $username, Password: **** , Sudo: $is_sudo" - - if getent passwd "$username" > /dev/null 2>&1 - then - echo "User Exists" - else - adduser -s /sbin/nologin "$username" - echo "$username:$password" | chpasswd - fi - done <"$file" +# Generate an ephemeral self-signed cert at first start when one is not +# already mounted/present at the configured path. Each container instance +# gets its own key; nothing is baked into the image. This is intended for +# the dev compose stack -- production deployments should mount a real +# certificate, or set Tls=auto so rdpgw obtains one from Let's Encrypt. +CERT="${RDPGW_SERVER__CERT_FILE:-/opt/rdpgw/server.pem}" +KEY="${RDPGW_SERVER__KEY_FILE:-/opt/rdpgw/key.pem}" +if [ ! -f "${CERT}" ] && [ ! -f "${KEY}" ]; then + echo "Generating ephemeral self-signed cert at ${CERT} / ${KEY} (dev only)" + openssl req -x509 -newkey rsa:2048 -keyout "${KEY}" -out "${CERT}" \ + -sha256 -days 365 -nodes \ + -subj "/CN=rdpgw-ephemeral" fi -cd /opt/rdpgw || exit 1 - -if [ -n "${RDPGW_SERVER__AUTHENTICATION}" ]; then - if [ "${RDPGW_SERVER__AUTHENTICATION}" = "local" ]; then - echo "Starting rdpgw-auth" - /opt/rdpgw/rdpgw-auth & - fi +if [ "${RDPGW_SERVER__AUTHENTICATION}" = "local" ]; then + echo "Starting rdpgw-auth" + /opt/rdpgw/rdpgw-auth & fi -# drop privileges and run the application -su -c /opt/rdpgw/rdpgw "${USER}" -- "$@" & -wait -exit $? +exec /opt/rdpgw/rdpgw "$@"