config: the OIDC client secret, admin TLS, and a stop on plaintext admin
server-test / test (push) Successful in 34s

Two gaps found while answering where configuration lives.

The confidential admin client needs a secret and there was nowhere to put one -
I had added the issuer and both client ids but not the secret the admin login
actually needs. It now reads from ECHOLOT_OIDC_CLIENT_SECRET, and preferably
from ECHOLOT_OIDC_CLIENT_SECRET_FILE: a secret in the environment is readable by
anything that can see /proc/<pid>/environ and lands in every dump of the unit's
config, whereas a path is one file whose permissions an operator can reason
about. (/etc/echolot-server.env was also 0644; now 0600 on fmr.)

And the server now refuses to serve the admin UI in plaintext on a non-loopback
address. The session cookie is a bearer credential for everything the server can
do, and the OIDC authorization code arrives in a URL; in the clear, both belong
to anyone on the path - and on a globally routable address that is the internet.
A hard stop rather than a warning, because a warning in a log is not read by the
person who most needs it, and because the safe answers are cheap: bind to
loopback and tunnel, or supply a certificate. ECHOLOT_ADMIN_INSECURE=1 overrides
it, so the decision is made rather than stumbled into.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
mrambossek
2026-08-01 17:49:26 +02:00
co-authored by Claude Fable 5
parent 3a4cb1c327
commit cd187f9ef5
+72
View File
@@ -11,6 +11,7 @@ import (
"flag" "flag"
"fmt" "fmt"
"io" "io"
"net"
"os" "os"
"strconv" "strconv"
"strings" "strings"
@@ -78,10 +79,40 @@ type Config struct {
// Break-glass admin username; the password lives hashed in the state store. // Break-glass admin username; the password lives hashed in the state store.
AdminUser string // ECHOLOT_ADMIN_USER / --admin-user AdminUser string // ECHOLOT_ADMIN_USER / --admin-user
// Secret for the *confidential* admin client. Prefer ECHOLOT_OIDC_CLIENT_SECRET_FILE: a path
// keeps the secret out of the environment, where it is readable by anything that can see
// /proc/<pid>/environ and lands in every dump of the unit's configuration.
OIDCClientSecret string // ECHOLOT_OIDC_CLIENT_SECRET / _FILE
// Where the admin UI is reachable, used to build the OIDC redirect URI. Must match what is
// registered at the IdP exactly.
AdminBaseURL string // ECHOLOT_ADMIN_BASE_URL / --admin-base-url
// TLS for the admin listener. Without these it serves plaintext, which is only acceptable on
// loopback — see checkAdminExposure.
AdminTLSCert string // ECHOLOT_ADMIN_TLS_CERT / --admin-tls-cert
AdminTLSKey string // ECHOLOT_ADMIN_TLS_KEY / --admin-tls-key
// Deliberate override for serving the admin UI in plaintext off loopback, so that decision
// is made rather than stumbled into.
AdminInsecure bool // ECHOLOT_ADMIN_INSECURE / --admin-insecure
// Mode // Mode
Docker bool // --docker (or autodetected; env ECHOLOT_DOCKER=1 forces) Docker bool // --docker (or autodetected; env ECHOLOT_DOCKER=1 forces)
} }
// secretOr reads ECHOLOT_<key>, or the contents of the file named by ECHOLOT_<key>_FILE.
//
// The file form exists because a secret in the environment is readable by anything that can see
// /proc/<pid>/environ and lands in every dump of the unit's configuration. A path costs nothing
// and keeps the value in one file whose permissions an operator can reason about.
func secretOr(key, def string) string {
if path := envOr(key+"_FILE", ""); path != "" {
if b, err := os.ReadFile(path); err == nil {
return strings.TrimSpace(string(b))
}
}
return envOr(key, def)
}
// envInt reads ECHOLOT_<key> as an integer with a fallback. // envInt reads ECHOLOT_<key> as an integer with a fallback.
func envInt(key string, def int) int { func envInt(key string, def int) int {
if v := envOr(key, ""); v != "" { if v := envOr(key, ""); v != "" {
@@ -130,6 +161,11 @@ func Load(args []string) (*Config, *Actions, error) {
fs.StringVar(&c.OIDCClientID, "oidc-client-id", envOr("OIDC_CLIENT_ID", ""), "confidential OIDC client id for the admin UI") fs.StringVar(&c.OIDCClientID, "oidc-client-id", envOr("OIDC_CLIENT_ID", ""), "confidential OIDC client id for the admin UI")
fs.StringVar(&c.OIDCAppClientID, "oidc-app-client-id", envOr("OIDC_APP_CLIENT_ID", ""), "public OIDC client id used by the Android app (PKCE)") fs.StringVar(&c.OIDCAppClientID, "oidc-app-client-id", envOr("OIDC_APP_CLIENT_ID", ""), "public OIDC client id used by the Android app (PKCE)")
fs.StringVar(&c.OIDCAdminGroup, "oidc-admin-group", envOr("OIDC_ADMIN_GROUP", ""), "group claim required for admin access; empty means nobody is an admin via OIDC") fs.StringVar(&c.OIDCAdminGroup, "oidc-admin-group", envOr("OIDC_ADMIN_GROUP", ""), "group claim required for admin access; empty means nobody is an admin via OIDC")
fs.StringVar(&c.OIDCClientSecret, "oidc-client-secret", secretOr("OIDC_CLIENT_SECRET", ""), "secret for the confidential admin client; prefer ECHOLOT_OIDC_CLIENT_SECRET_FILE")
fs.StringVar(&c.AdminBaseURL, "admin-base-url", envOr("ADMIN_BASE_URL", ""), "public URL of the admin UI, for the OIDC redirect (e.g. https://admin.example.net)")
fs.StringVar(&c.AdminTLSCert, "admin-tls-cert", envOr("ADMIN_TLS_CERT", ""), "TLS certificate for the admin listener")
fs.StringVar(&c.AdminTLSKey, "admin-tls-key", envOr("ADMIN_TLS_KEY", ""), "TLS key for the admin listener")
fs.BoolVar(&c.AdminInsecure, "admin-insecure", envOr("ADMIN_INSECURE", "") == "1", "allow the admin UI in plaintext off loopback (you are on your own)")
fs.StringVar(&c.PublicControlURL, "public-url", envOr("PUBLIC_URL", ""), "public control-plane URL for enrollment links, e.g. https://probe.example.net:8443") fs.StringVar(&c.PublicControlURL, "public-url", envOr("PUBLIC_URL", ""), "public control-plane URL for enrollment links, e.g. https://probe.example.net:8443")
fs.StringVar(&c.MinAppVersion, "min-app-version", envOr("MIN_APP_VERSION", "0.2.0"), "oldest app version this server will serve (SemVer, inclusive)") fs.StringVar(&c.MinAppVersion, "min-app-version", envOr("MIN_APP_VERSION", "0.2.0"), "oldest app version this server will serve (SemVer, inclusive)")
fs.StringVar(&c.MaxAppVersion, "max-app-version", envOr("MAX_APP_VERSION", "1.0.0"), "first app version this server will refuse (SemVer, exclusive); empty = unbounded") fs.StringVar(&c.MaxAppVersion, "max-app-version", envOr("MAX_APP_VERSION", "1.0.0"), "first app version this server will refuse (SemVer, exclusive); empty = unbounded")
@@ -160,6 +196,11 @@ func Load(args []string) (*Config, *Actions, error) {
!a.SetAdminPassword && !a.Version { !a.SetAdminPassword && !a.Version {
a.Help = true a.Help = true
} }
if a.Serve {
if err := c.checkAdminExposure(); err != nil {
return nil, nil, err
}
}
if c.Docker && (a.InstallSystemd || a.UninstallSystemd || a.SelfUpdate) { if c.Docker && (a.InstallSystemd || a.UninstallSystemd || a.SelfUpdate) {
return nil, nil, fmt.Errorf("systemd/self-update actions are native-mode only (container detected; override with ECHOLOT_DOCKER=0 if this is wrong)") return nil, nil, fmt.Errorf("systemd/self-update actions are native-mode only (container detected; override with ECHOLOT_DOCKER=0 if this is wrong)")
} }
@@ -188,6 +229,37 @@ should be something you asked for.
`) `)
} }
// checkAdminExposure refuses to serve an unencrypted admin UI on a non-loopback address.
//
// The admin session cookie is a bearer credential for everything this server can do, and the OIDC
// authorization code arrives in a URL. In plaintext, both are readable by anyone on the path — and
// on a globally routable address "the path" means the internet. This is a hard stop rather than a
// warning because a warning in a log is not read by the person who most needs it, and because the
// two safe answers are cheap: bind to loopback and tunnel, or supply a certificate.
func (c *Config) checkAdminExposure() error {
if c.AdminTLSCert != "" || c.AdminInsecure {
return nil
}
for _, addr := range Addrs(c.AdminListen) {
host, _, err := net.SplitHostPort(addr)
if err != nil {
continue
}
ip := net.ParseIP(strings.Trim(host, "[]"))
if host == "" || ip == nil || ip.IsLoopback() {
continue // loopback, or a name we cannot judge; RFC 8252 blesses loopback plaintext
}
return fmt.Errorf(
"refusing to serve the admin UI in plaintext on %s: the session cookie and the OIDC "+
"authorization code would cross the network in the clear.\n"+
" Fix it one of three ways:\n"+
" - bind to 127.0.0.1 and reach it over an SSH tunnel (no certificate needed)\n"+
" - set ECHOLOT_ADMIN_TLS_CERT and ECHOLOT_ADMIN_TLS_KEY\n"+
" - set ECHOLOT_ADMIN_INSECURE=1 if you genuinely mean it", addr)
}
return nil
}
// Addrs splits a comma-separated listen spec into individual addresses. // Addrs splits a comma-separated listen spec into individual addresses.
// Explicit per-address binds matter on multi-IP hosts: a wildcard bind // Explicit per-address binds matter on multi-IP hosts: a wildcard bind
// (":8443") would also claim addresses reserved for other purposes (e.g. an // (":8443") would also claim addresses reserved for other purposes (e.g. an