diff --git a/api/docs/docs.go b/api/docs/docs.go index 3d566084..125b8da5 100644 --- a/api/docs/docs.go +++ b/api/docs/docs.go @@ -2071,6 +2071,42 @@ const docTemplate = `{ } } }, + "config.APIAuthConfig": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/config.APIUser" + } + } + } + }, + "config.APIUser": { + "type": "object", + "properties": { + "password_hash": { + "type": "string" + }, + "role": { + "type": "string" + }, + "username": { + "type": "string" + } + } + }, + "config.AuthConfig": { + "type": "object", + "properties": { + "api": { + "$ref": "#/definitions/config.APIAuthConfig" + } + } + }, "config.BufferConfig": { "type": "object", "properties": { @@ -2442,6 +2478,8 @@ const docTemplate = `{ "domain.EventType": { "type": "string", "enum": [ + "session.opened", + "session.closed", "stream.created", "stream.updated", "stream.started", @@ -2475,9 +2513,7 @@ const docTemplate = `{ "template.updated", "template.deleted", "stream.runtime_created", - "stream.runtime_expired", - "session.opened", - "session.closed" + "stream.runtime_expired" ], "x-enum-comments": { "EventDVRSegmentPruned": "retention loop deleted an aged-out segment", @@ -2494,6 +2530,8 @@ const docTemplate = `{ "EventStreamUpdated": "PUT /streams/{code} on existing record" }, "x-enum-descriptions": [ + "", + "", "", "PUT /streams/{code} on existing record", "", @@ -2527,11 +2565,11 @@ const docTemplate = `{ "", "", "", - "", - "", "" ], "x-enum-varnames": [ + "EventSessionOpened", + "EventSessionClosed", "EventStreamCreated", "EventStreamUpdated", "EventStreamStarted", @@ -2565,14 +2603,15 @@ const docTemplate = `{ "EventTemplateUpdated", "EventTemplateDeleted", "EventStreamRuntimeCreated", - "EventStreamRuntimeExpired", - "EventSessionOpened", - "EventSessionClosed" + "EventStreamRuntimeExpired" ] }, "domain.GlobalConfig": { "type": "object", "properties": { + "auth": { + "$ref": "#/definitions/config.AuthConfig" + }, "buffer": { "$ref": "#/definitions/config.BufferConfig" }, diff --git a/api/docs/swagger.json b/api/docs/swagger.json index 827bd88a..bc3160e7 100644 --- a/api/docs/swagger.json +++ b/api/docs/swagger.json @@ -2064,6 +2064,42 @@ } } }, + "config.APIAuthConfig": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/config.APIUser" + } + } + } + }, + "config.APIUser": { + "type": "object", + "properties": { + "password_hash": { + "type": "string" + }, + "role": { + "type": "string" + }, + "username": { + "type": "string" + } + } + }, + "config.AuthConfig": { + "type": "object", + "properties": { + "api": { + "$ref": "#/definitions/config.APIAuthConfig" + } + } + }, "config.BufferConfig": { "type": "object", "properties": { @@ -2435,6 +2471,8 @@ "domain.EventType": { "type": "string", "enum": [ + "session.opened", + "session.closed", "stream.created", "stream.updated", "stream.started", @@ -2468,9 +2506,7 @@ "template.updated", "template.deleted", "stream.runtime_created", - "stream.runtime_expired", - "session.opened", - "session.closed" + "stream.runtime_expired" ], "x-enum-comments": { "EventDVRSegmentPruned": "retention loop deleted an aged-out segment", @@ -2487,6 +2523,8 @@ "EventStreamUpdated": "PUT /streams/{code} on existing record" }, "x-enum-descriptions": [ + "", + "", "", "PUT /streams/{code} on existing record", "", @@ -2520,11 +2558,11 @@ "", "", "", - "", - "", "" ], "x-enum-varnames": [ + "EventSessionOpened", + "EventSessionClosed", "EventStreamCreated", "EventStreamUpdated", "EventStreamStarted", @@ -2558,14 +2596,15 @@ "EventTemplateUpdated", "EventTemplateDeleted", "EventStreamRuntimeCreated", - "EventStreamRuntimeExpired", - "EventSessionOpened", - "EventSessionClosed" + "EventStreamRuntimeExpired" ] }, "domain.GlobalConfig": { "type": "object", "properties": { + "auth": { + "$ref": "#/definitions/config.AuthConfig" + }, "buffer": { "$ref": "#/definitions/config.BufferConfig" }, diff --git a/api/docs/swagger.yaml b/api/docs/swagger.yaml index b1d5daf0..2c67c474 100644 --- a/api/docs/swagger.yaml +++ b/api/docs/swagger.yaml @@ -309,6 +309,29 @@ definitions: total: type: integer type: object + config.APIAuthConfig: + properties: + enabled: + type: boolean + users: + items: + $ref: '#/definitions/config.APIUser' + type: array + type: object + config.APIUser: + properties: + password_hash: + type: string + role: + type: string + username: + type: string + type: object + config.AuthConfig: + properties: + api: + $ref: '#/definitions/config.APIAuthConfig' + type: object config.BufferConfig: properties: capacity: @@ -639,6 +662,8 @@ definitions: type: object domain.EventType: enum: + - session.opened + - session.closed - stream.created - stream.updated - stream.started @@ -673,8 +698,6 @@ definitions: - template.deleted - stream.runtime_created - stream.runtime_expired - - session.opened - - session.closed type: string x-enum-comments: EventDVRSegmentPruned: retention loop deleted an aged-out segment @@ -692,6 +715,8 @@ definitions: EventStreamUpdated: PUT /streams/{code} on existing record x-enum-descriptions: - "" + - "" + - "" - PUT /streams/{code} on existing record - "" - "" @@ -725,9 +750,9 @@ definitions: - "" - "" - "" - - "" - - "" x-enum-varnames: + - EventSessionOpened + - EventSessionClosed - EventStreamCreated - EventStreamUpdated - EventStreamStarted @@ -762,10 +787,10 @@ definitions: - EventTemplateDeleted - EventStreamRuntimeCreated - EventStreamRuntimeExpired - - EventSessionOpened - - EventSessionClosed domain.GlobalConfig: properties: + auth: + $ref: '#/definitions/config.AuthConfig' buffer: $ref: '#/definitions/config.BufferConfig' hooks: diff --git a/cmd/server/main.go b/cmd/server/main.go index d1b0bb31..8df5c653 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -5,6 +5,7 @@ package main import ( + "bufio" "context" "errors" "fmt" @@ -15,6 +16,8 @@ import ( "syscall" "time" + "golang.org/x/crypto/bcrypt" + "github.com/ntt0601zcoder/open-streamer/config" "github.com/ntt0601zcoder/open-streamer/internal/api" "github.com/ntt0601zcoder/open-streamer/internal/api/handler" @@ -42,12 +45,46 @@ import ( ) func main() { + // `open-streamer hashpw [password]` prints a bcrypt hash for an API user's + // password_hash field, so operators never put plaintext in the config. + if len(os.Args) > 1 && os.Args[1] == "hashpw" { + if err := runHashPw(os.Args[2:]); err != nil { + fmt.Fprintln(os.Stderr, "hashpw:", err) + os.Exit(1) + } + return + } if err := run(); err != nil { slog.Error("server: fatal error", "err", err) os.Exit(1) } } +// runHashPw reads a password from the first arg, or from stdin when no arg is +// given (so it stays out of shell history), and prints its bcrypt hash. +func runHashPw(args []string) error { + var pw string + if len(args) > 0 { + pw = args[0] + } else { + fmt.Fprint(os.Stderr, "password: ") + line, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil && line == "" { + return fmt.Errorf("read password: %w", err) + } + pw = strings.TrimRight(line, "\r\n") + } + if pw == "" { + return errors.New("empty password") + } + h, err := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.DefaultCost) + if err != nil { + return fmt.Errorf("hash: %w", err) + } + fmt.Println(string(h)) + return nil +} + func run() error { // 1. Load StorageConfig from file/env — only storage settings come from viper. storageCfg, err := config.LoadStorage() @@ -192,6 +229,7 @@ func provideSubConfigs(i *do.RootScope, gcfg *domain.GlobalConfig) { do.ProvideValue(i, deref(gcfg.Sessions)) do.ProvideValue(i, deref(gcfg.Watermarks)) do.ProvideValue(i, deref(gcfg.Log)) + do.ProvideValue(i, deref(gcfg.Auth)) } func deref[T any](p *T) T { diff --git a/config/config.go b/config/config.go index 5ebffa59..4920276d 100644 --- a/config/config.go +++ b/config/config.go @@ -246,6 +246,33 @@ type HooksConfig struct { BatchMaxQueueItems int `mapstructure:"batch_max_queue_items" json:"batch_max_queue_items,omitempty" yaml:"batch_max_queue_items,omitempty"` } +// AuthConfig holds authentication settings. Today it covers only the +// control-plane HTTP API (admin); media/playback auth is a separate plane. +type AuthConfig struct { + API APIAuthConfig `mapstructure:"api" json:"api" yaml:"api"` +} + +// APIAuthConfig configures HTTP Basic auth on the management API. When +// Enabled is false (default) the API is open, preserving prior behaviour. +// Users are part of the global config (managed via the config API by a +// `write` user). On boot, if Enabled is true and Users is empty, an `admin` +// user with the `write` role is seeded from OPEN_STREAMER_API_ADMIN_PASSWORD +// when that env var is set — otherwise every request is rejected until users +// are defined (fail-closed). +type APIAuthConfig struct { + Enabled bool `mapstructure:"enabled" json:"enabled" yaml:"enabled"` + Users []APIUser `mapstructure:"users" json:"users,omitempty" yaml:"users,omitempty"` +} + +// APIUser is one management-API account. Role is "read" (GET/HEAD/OPTIONS only) +// or "write" (all methods). PasswordHash is a bcrypt hash — never store a +// plaintext password here (generate one with `open-streamer hashpw`). +type APIUser struct { + Username string `mapstructure:"username" json:"username" yaml:"username"` + PasswordHash string `mapstructure:"password_hash" json:"password_hash" yaml:"password_hash"` + Role string `mapstructure:"role" json:"role" yaml:"role"` +} + // LogConfig controls structured logging output. type LogConfig struct { Level string `mapstructure:"level" json:"level" yaml:"level"` // trace | debug | info | warn | error diff --git a/docs/audit/security-quality-audit.md b/docs/audit/security-quality-audit.md index 3fa0f3d5..0fcf05a8 100644 --- a/docs/audit/security-quality-audit.md +++ b/docs/audit/security-quality-audit.md @@ -45,6 +45,8 @@ **Fix:** Add a fail-closed auth middleware (constant-time bearer/API-key compare via `subtle.ConstantTimeCompare`, or mTLS) mounted on every admin route group (`/config*`, `/streams*`, `/templates*`, `/hooks*`, `/watermarks*`, `/vod*`, `/sessions*`); allowlist only `/healthz`, `/readyz`, and (per policy) `/metrics` and the media catch-all. Refuse to start (or bind `127.0.0.1` only) when no credential is configured and the bind host is non-loopback. Replace `middleware.RealIP` with a non-mutating client-IP extractor that trusts `X-Forwarded-For`/`X-Real-IP` only from a configured trusted-proxy CIDR (this also closes S-17). For DVR/media playback specifically, gate behind a signed-URL/per-stream token if the media port is exposed, or split admin and media onto separate listeners. +> ✅ **FIXED (control-plane auth)** in `fix/api-basic-auth` — HTTP Basic auth with two roles (`read` = GET/HEAD/OPTIONS, `write` = all methods) gates every management route. Users live in the global config (`auth.api.users`, bcrypt `password_hash`, managed like any other config and hot-reloaded via `runtime.Manager.diff` → `Server.SetAuthConfig` with an `atomic.Pointer` swap). New `internal/api/apiauth.go` (`Auth.Middleware`, bcrypt verify with a dummy-hash timing-safe unknown-user path, `roleAllowsMethod`); all admin routes wrapped in an auth `Group` in `buildRouter`, with `/healthz`/`/readyz`/`/metrics` and the media catch-all left public. Opt-in (`enabled: false` default → pass-through, no behaviour change) with a fail-closed bootstrap: enabled + no users seeds `admin`/`write` from `OPEN_STREAMER_API_ADMIN_PASSWORD`, else rejects all. `open-streamer hashpw` generates hashes so plaintext never enters config. **Adversarially reviewed**: found + fixed a routing bypass where flat admin routes let an unregistered HTTP method fall through chi's trie to the unauthenticated media catch-all — `/config*` and `/streams` are now subrouters that 405 in-group (regression test `TestAdminRouteUnmatchedMethodDoesNotBypassAuth`); no GET-triggered mutation escalation (stream restart/switch are POST-gated). Tests: `TestAPIAuth_*`, `TestRoleAllowsMethod`. **Scope:** this is the auth half of S-1. Still open (separate findings): the non-mutating client-IP extractor / trusted-proxy CIDR (RealIP → S-15/S-17), media-plane playback auth (B-plane / S-13), and redacting `password_hash`/secrets from `GET /config`+`/config/yaml` for `read` users (folded into **S-9**'s secret-redaction with round-trip preservation). + --- #### S-2 (CRITICAL) — Unauthenticated arbitrary host-file create/append (and blind SSRF) via hooks diff --git a/go.mod b/go.mod index a735ec51..fac84783 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/swaggo/swag v1.16.6 github.com/testcontainers/testcontainers-go v0.41.0 github.com/yapingcat/gomedia v0.0.0-20240906162731-17feea57090c + golang.org/x/crypto v0.51.0 golang.org/x/net v0.55.0 golang.org/x/sync v0.20.0 golang.org/x/sys v0.45.0 @@ -116,7 +117,6 @@ require ( go.opentelemetry.io/proto/otlp v1.0.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.51.0 // indirect golang.org/x/mod v0.35.0 // indirect golang.org/x/text v0.37.0 // indirect golang.org/x/tools v0.44.0 // indirect diff --git a/internal/api/apiauth.go b/internal/api/apiauth.go new file mode 100644 index 00000000..1f74ff55 --- /dev/null +++ b/internal/api/apiauth.go @@ -0,0 +1,175 @@ +package api + +// apiauth.go — HTTP Basic auth for the management (control-plane) API. +// +// Two roles only: "read" (GET/HEAD/OPTIONS) and "write" (all methods). Users +// live in the global config (config.AuthConfig.API) and are managed like any +// other config by a write user. Passwords are stored as bcrypt hashes — never +// plaintext. The whole feature is opt-in: when Enabled is false the middleware +// is a pass-through, preserving the prior open-API behaviour. +// +// Threading: the resolved user set is held in an atomic.Pointer so the config +// hot-reload path (runtime.Manager.diff) can swap it via SetConfig without +// rebuilding the router or restarting the server. + +import ( + "log/slog" + "net/http" + "os" + "strings" + "sync/atomic" + + "golang.org/x/crypto/bcrypt" + + "github.com/ntt0601zcoder/open-streamer/config" +) + +const ( + roleRead = "read" + roleWrite = "write" + + // adminPasswordEnv seeds a bootstrap `admin` write user when auth is + // enabled but no users are configured yet — avoids locking the operator + // out on first enable. + adminPasswordEnv = "OPEN_STREAMER_API_ADMIN_PASSWORD" + + basicRealm = "open-streamer" +) + +// dummyHash is a valid bcrypt hash of a random string. Comparing an unknown +// username's password against it keeps the auth-failure path's timing close to +// the success path, so response time can't be used to enumerate usernames. +var dummyHash = []byte("$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy") + +// apiUser is a resolved (validated) account. +type apiUser struct { + hash []byte + role string +} + +// authState is an immutable snapshot of the auth configuration. Swapped +// atomically on config reload. +type authState struct { + enabled bool + users map[string]apiUser +} + +// Auth enforces Basic auth + read/write roles on the management API. +type Auth struct { + state atomic.Pointer[authState] +} + +// newAuth builds the authenticator from config. Invalid users are skipped +// with a warning; an enabled-but-empty user set is left empty (every request is +// then rejected — fail-closed) unless the bootstrap env var seeds an admin. +func newAuth(cfg config.AuthConfig) *Auth { + a := &Auth{} + a.SetConfig(cfg) + return a +} + +// SetConfig swaps in a new auth configuration atomically (config hot-reload). +func (a *Auth) SetConfig(cfg config.AuthConfig) { + st := buildAuthState(cfg.API) + a.state.Store(st) + if st.enabled && len(st.users) == 0 { + slog.Warn("api: Basic auth is ENABLED but no users are configured — all management requests will be rejected; " + + "define auth.api.users or set " + adminPasswordEnv) + } +} + +// buildAuthState validates users and applies the bootstrap env seed. +func buildAuthState(cfg config.APIAuthConfig) *authState { + st := &authState{enabled: cfg.Enabled, users: make(map[string]apiUser)} + if !cfg.Enabled { + return st + } + for _, u := range cfg.Users { + name := strings.TrimSpace(u.Username) + role := strings.ToLower(strings.TrimSpace(u.Role)) + if name == "" || u.PasswordHash == "" { + slog.Warn("api: skipping auth user with empty username/password_hash", "username", name) + continue + } + if role != roleRead && role != roleWrite { + slog.Warn("api: skipping auth user with invalid role (want read|write)", "username", name, "role", u.Role) + continue + } + st.users[name] = apiUser{hash: []byte(u.PasswordHash), role: role} + } + // Bootstrap: seed an admin from the env var when no users are defined, so + // enabling auth doesn't immediately lock the operator out. + if len(st.users) == 0 { + if pw := os.Getenv(adminPasswordEnv); pw != "" { + if h, err := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.DefaultCost); err == nil { + st.users["admin"] = apiUser{hash: h, role: roleWrite} + slog.Warn("api: seeded bootstrap 'admin' (write) user from " + adminPasswordEnv + " — define auth.api.users to make it permanent") + } else { + slog.Error("api: failed to hash bootstrap admin password", "err", err) + } + } + } + return st +} + +// Middleware enforces auth on the wrapped (management) handlers. It is a +// pass-through when auth is disabled. +func (a *Auth) Middleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + st := a.state.Load() + if st == nil || !st.enabled { + next.ServeHTTP(w, r) + return + } + user, pass, ok := r.BasicAuth() + if !ok { + a.challenge(w) + return + } + role, authed := st.verify(user, pass) + if !authed { + a.challenge(w) + return + } + if !roleAllowsMethod(role, r.Method) { + http.Error(w, "forbidden: read-only user cannot perform this action", http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) +} + +func (a *Auth) challenge(w http.ResponseWriter) { + w.Header().Set("WWW-Authenticate", `Basic realm="`+basicRealm+`", charset="UTF-8"`) + http.Error(w, "unauthorized", http.StatusUnauthorized) +} + +// verify checks credentials and returns the user's role. An unknown username +// still runs a bcrypt comparison against a dummy hash so timing doesn't reveal +// whether the username exists. +func (st *authState) verify(user, pass string) (role string, ok bool) { + u, found := st.users[user] + if !found { + _ = bcrypt.CompareHashAndPassword(dummyHash, []byte(pass)) + return "", false + } + if bcrypt.CompareHashAndPassword(u.hash, []byte(pass)) != nil { + return "", false + } + return u.role, true +} + +// roleAllowsMethod maps the two roles to HTTP methods: read may only perform +// safe (non-mutating) methods; write may perform any. +func roleAllowsMethod(role, method string) bool { + if role == roleWrite { + return true + } + if role == roleRead { + switch method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + return true + } + } + return false +} diff --git a/internal/api/apiauth_test.go b/internal/api/apiauth_test.go new file mode 100644 index 00000000..943ac033 --- /dev/null +++ b/internal/api/apiauth_test.go @@ -0,0 +1,245 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "golang.org/x/crypto/bcrypt" + + "github.com/ntt0601zcoder/open-streamer/config" +) + +func mustHash(t *testing.T, pw string) string { + t.Helper() + h, err := bcrypt.GenerateFromPassword([]byte(pw), bcrypt.MinCost) // MinCost: fast tests + if err != nil { + t.Fatalf("bcrypt: %v", err) + } + return string(h) +} + +// okHandler returns 200 so the test can tell "passed the middleware" from a +// 401/403 the middleware produced. +func okHandler() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) +} + +func doReq(t *testing.T, h http.Handler, method, user, pass string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequestWithContext(context.Background(), method, "/config", nil) + if user != "" || pass != "" { + req.SetBasicAuth(user, pass) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +func TestRoleAllowsMethod(t *testing.T) { + t.Parallel() + cases := []struct { + role, method string + want bool + }{ + {roleRead, http.MethodGet, true}, + {roleRead, http.MethodHead, true}, + {roleRead, http.MethodOptions, true}, + {roleRead, http.MethodPost, false}, + {roleRead, http.MethodPut, false}, + {roleRead, http.MethodDelete, false}, + {roleRead, http.MethodPatch, false}, + {roleWrite, http.MethodGet, true}, + {roleWrite, http.MethodPost, true}, + {roleWrite, http.MethodDelete, true}, + {"bogus", http.MethodGet, false}, + } + for _, c := range cases { + if got := roleAllowsMethod(c.role, c.method); got != c.want { + t.Errorf("roleAllowsMethod(%q,%q)=%v want %v", c.role, c.method, got, c.want) + } + } +} + +func TestAPIAuth_DisabledPassesThrough(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{Enabled: false}}) + h := a.Middleware(okHandler()) + // No credentials, mutating method — must still pass (auth disabled). + if rec := doReq(t, h, http.MethodPost, "", ""); rec.Code != http.StatusOK { + t.Fatalf("disabled auth: code=%d want 200", rec.Code) + } +} + +func TestAPIAuth_RequiresAndValidatesCreds(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{ + {Username: "writer", PasswordHash: mustHash(t, "s3cret"), Role: roleWrite}, + }, + }}) + h := a.Middleware(okHandler()) + + t.Run("no_creds_401_with_challenge", func(t *testing.T) { + rec := doReq(t, h, http.MethodGet, "", "") + if rec.Code != http.StatusUnauthorized { + t.Fatalf("code=%d want 401", rec.Code) + } + if rec.Header().Get("WWW-Authenticate") == "" { + t.Error("missing WWW-Authenticate challenge") + } + }) + t.Run("wrong_password_401", func(t *testing.T) { + if rec := doReq(t, h, http.MethodGet, "writer", "wrong"); rec.Code != http.StatusUnauthorized { + t.Fatalf("code=%d want 401", rec.Code) + } + }) + t.Run("unknown_user_401", func(t *testing.T) { + if rec := doReq(t, h, http.MethodGet, "ghost", "s3cret"); rec.Code != http.StatusUnauthorized { + t.Fatalf("code=%d want 401", rec.Code) + } + }) + t.Run("valid_write_user_passes", func(t *testing.T) { + if rec := doReq(t, h, http.MethodPost, "writer", "s3cret"); rec.Code != http.StatusOK { + t.Fatalf("code=%d want 200", rec.Code) + } + }) +} + +func TestAPIAuth_ReadRoleCannotWrite(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{ + {Username: "ro", PasswordHash: mustHash(t, "pw"), Role: roleRead}, + }, + }}) + h := a.Middleware(okHandler()) + + if rec := doReq(t, h, http.MethodGet, "ro", "pw"); rec.Code != http.StatusOK { + t.Fatalf("read GET: code=%d want 200", rec.Code) + } + if rec := doReq(t, h, http.MethodPost, "ro", "pw"); rec.Code != http.StatusForbidden { + t.Fatalf("read POST: code=%d want 403", rec.Code) + } + if rec := doReq(t, h, http.MethodDelete, "ro", "pw"); rec.Code != http.StatusForbidden { + t.Fatalf("read DELETE: code=%d want 403", rec.Code) + } +} + +func TestAPIAuth_BootstrapFromEnv(t *testing.T) { + t.Setenv(adminPasswordEnv, "bootpw") + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{Enabled: true}}) // no users + h := a.Middleware(okHandler()) + + if rec := doReq(t, h, http.MethodPost, "admin", "bootpw"); rec.Code != http.StatusOK { + t.Fatalf("bootstrap admin write: code=%d want 200", rec.Code) + } + if rec := doReq(t, h, http.MethodGet, "admin", "nope"); rec.Code != http.StatusUnauthorized { + t.Fatalf("bootstrap admin wrong pw: code=%d want 401", rec.Code) + } +} + +func TestAPIAuth_EnabledNoUsersRejectsAll(t *testing.T) { + // No env seed, enabled, no users → fail-closed (every request 401). + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{Enabled: true}}) + h := a.Middleware(okHandler()) + if rec := doReq(t, h, http.MethodGet, "anyone", "anything"); rec.Code != http.StatusUnauthorized { + t.Fatalf("enabled-no-users: code=%d want 401", rec.Code) + } +} + +func TestAPIAuth_SkipsInvalidUsers(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{ + {Username: "", PasswordHash: mustHash(t, "x"), Role: roleWrite}, // empty name → skip + {Username: "norole", PasswordHash: mustHash(t, "x"), Role: "admin"}, // bad role → skip + {Username: "nohash", PasswordHash: "", Role: roleWrite}, // empty hash → skip + {Username: "good", PasswordHash: mustHash(t, "pw"), Role: roleWrite}, + }, + }}) + h := a.Middleware(okHandler()) + if rec := doReq(t, h, http.MethodPost, "norole", "x"); rec.Code != http.StatusUnauthorized { + t.Errorf("invalid-role user must be rejected: code=%d", rec.Code) + } + if rec := doReq(t, h, http.MethodPost, "good", "pw"); rec.Code != http.StatusOK { + t.Errorf("valid user must pass: code=%d", rec.Code) + } +} + +// TestAdminRouteUnmatchedMethodDoesNotBypassAuth locks the routing invariant +// from the adversarial review: an admin path registered via a subrouter must, +// for an UNREGISTERED method, stay inside the auth group (run the middleware, +// then 405) — it must NEVER fall through chi's trie to the public media +// catch-all, which would skip auth entirely. +func TestAdminRouteUnmatchedMethodDoesNotBypassAuth(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{{Username: "w", PasswordHash: mustHash(t, "pw"), Role: roleWrite}}, + }}) + + mediaHit := false + r := chi.NewRouter() + // Admin group (mirrors buildRouter): subrouter owns the /config subtree. + r.Group(func(ar chi.Router) { + ar.Use(a.Middleware) + ar.Route("/config", func(cr chi.Router) { + cr.Get("/", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + cr.Post("/", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + }) + }) + // Public media catch-all (no auth). + r.Group(func(mr chi.Router) { + mr.HandleFunc("/*", func(w http.ResponseWriter, _ *http.Request) { + mediaHit = true + w.WriteHeader(http.StatusOK) + }) + }) + + // PUT /config — method not registered on the subrouter. Without creds the + // auth middleware must still run (401), proving the request did NOT escape + // to the unauthenticated media handler. + if rec := doReq(t, r, http.MethodPut, "", ""); rec.Code != http.StatusUnauthorized || mediaHit { + t.Fatalf("PUT /config no-creds: code=%d mediaHit=%v want 401 & no media", rec.Code, mediaHit) + } + // With valid write creds the subrouter returns 405 (still in-group), not media. + if rec := doReq(t, r, http.MethodPut, "w", "pw"); rec.Code != http.StatusMethodNotAllowed || mediaHit { + t.Fatalf("PUT /config write-creds: code=%d mediaHit=%v want 405 & no media", rec.Code, mediaHit) + } + // A genuine media path still reaches the public handler unauthenticated. + mreq := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/somestream/index.m3u8", nil) + mrec := httptest.NewRecorder() + r.ServeHTTP(mrec, mreq) + if !mediaHit || mrec.Code != http.StatusOK { + t.Fatalf("media path: code=%d mediaHit=%v want 200 & media hit", mrec.Code, mediaHit) + } +} + +func TestAPIAuth_HotReload(t *testing.T) { + t.Parallel() + a := newAuth(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{{Username: "old", PasswordHash: mustHash(t, "old"), Role: roleWrite}}, + }}) + h := a.Middleware(okHandler()) + if rec := doReq(t, h, http.MethodGet, "old", "old"); rec.Code != http.StatusOK { + t.Fatalf("old user before reload: code=%d want 200", rec.Code) + } + // Swap config: remove old, add new. + a.SetConfig(config.AuthConfig{API: config.APIAuthConfig{ + Enabled: true, + Users: []config.APIUser{{Username: "new", PasswordHash: mustHash(t, "new"), Role: roleWrite}}, + }}) + if rec := doReq(t, h, http.MethodGet, "old", "old"); rec.Code != http.StatusUnauthorized { + t.Errorf("old user after reload: code=%d want 401", rec.Code) + } + if rec := doReq(t, h, http.MethodGet, "new", "new"); rec.Code != http.StatusOK { + t.Errorf("new user after reload: code=%d want 200", rec.Code) + } +} diff --git a/internal/api/server.go b/internal/api/server.go index b0bd0e8a..63a24415 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -56,6 +56,10 @@ type Server struct { // without DI. Used by the request-duration middleware below. m *metrics.Metrics + // apiAuth enforces Basic auth + read/write roles on the management API. + // Never nil (a disabled authenticator is a pass-through). + apiAuth *Auth + router *chi.Mux http *http.Server } @@ -98,9 +102,27 @@ func New(i do.Injector) (*Server, error) { if m, err := do.Invoke[*metrics.Metrics](i); err == nil { s.m = m } + // API auth (Basic, read/write roles). Config is optional — a missing + // provider (or Enabled=false) yields a pass-through authenticator, so the + // API stays open until an operator turns it on. + authCfg, _ := do.Invoke[config.AuthConfig](i) + s.apiAuth = newAuth(authCfg) return s, nil } +// SetAuthConfig hot-swaps the API auth configuration. Called by the runtime +// config-reload path so user/role edits take effect without an API restart. +func (s *Server) SetAuthConfig(cfg *config.AuthConfig) { + if s.apiAuth == nil { + return + } + c := config.AuthConfig{} + if cfg != nil { + c = *cfg + } + s.apiAuth.SetConfig(c) +} + // httpDurationMiddleware records request latency into the // open_streamer_http_request_duration_seconds histogram. Routes use chi // route patterns (e.g. "/streams/{code}") rather than full paths so label @@ -169,77 +191,104 @@ func (s *Server) buildRouter(serverCfg *config.ServerConfig) *chi.Mux { r.Use(s.httpDurationMiddleware) r.Use(middleware.Timeout(120 * time.Second)) + // Public, unauthenticated: liveness/readiness probes and the metrics + // scrape endpoint. Everything else (the management API) is gated below. r.Get("/healthz", healthz) r.Get("/readyz", readyz) r.Handle("/metrics", promhttp.Handler()) - r.Get("/config", s.configH.GetConfig) - r.Post("/config", s.configH.UpdateConfig) - r.Get("/config/defaults", s.configH.GetConfigDefaults) - r.Post("/config/transcoder/probe", s.configH.ProbeTranscoder) - r.Get("/config/yaml", s.configH.GetConfigYAML) - r.Put("/config/yaml", s.configH.ReplaceConfigYAML) - - r.Get("/swagger/doc.json", serveSwaggerJSON) - r.Get("/swagger", http.RedirectHandler("/swagger/", http.StatusMovedPermanently).ServeHTTP) - r.Get("/swagger/", serveSwaggerIndex) - - // Admin: /streams. Stream codes may contain '/' (namespacing) so the - // per-code routes are served by a catch-all dispatcher that parses - // (code, action) from the suffix. See dispatch.go for the rules. - r.Get("/streams", s.streamH.List) - r.HandleFunc("/streams/*", s.dispatchStreamsSubpath()) - - // Templates. Codes are single-segment (a-zA-Z0-9_-) so the chi - // single-param routes are fine — no catch-all dispatcher needed. - r.Route("/templates", func(r chi.Router) { - r.Get("/", s.templateH.List) - r.Route("/{code}", func(r chi.Router) { - r.Get("/", s.templateH.Get) - r.Post("/", s.templateH.Put) - r.Delete("/", s.templateH.Delete) + + // Management API: Basic auth (read = safe methods, write = all). The + // authenticator is a pass-through when auth is disabled in config, so this + // group's routes behave exactly as before until an operator enables it. + // All admin routes live INSIDE this group so a newly-added one is protected + // by default rather than accidentally left open. + r.Group(func(ar chi.Router) { + if s.apiAuth != nil { + ar.Use(s.apiAuth.Middleware) + } + + // NOTE: every admin path is registered via a subrouter (ar.Route), + // never a flat ar.Get/Post at this level. A subrouter owns its whole + // subtree, so an unmatched METHOD returns 405 from inside the auth + // group. A flat route would instead let an unregistered method fall + // through chi's trie to the public media catch-all `/*` below — + // bypassing auth entirely. Keep this invariant when adding routes. + ar.Route("/config", func(r chi.Router) { + r.Get("/", s.configH.GetConfig) + r.Post("/", s.configH.UpdateConfig) + r.Get("/defaults", s.configH.GetConfigDefaults) + r.Post("/transcoder/probe", s.configH.ProbeTranscoder) + r.Get("/yaml", s.configH.GetConfigYAML) + r.Put("/yaml", s.configH.ReplaceConfigYAML) }) - }) - r.Route("/sessions", func(r chi.Router) { - r.Get("/", s.sessionH.List) - r.Route("/{id}", func(r chi.Router) { - r.Get("/", s.sessionH.Get) - r.Delete("/", s.sessionH.Delete) + // Swagger is GET-only static API docs — flat routes are fine here: an + // unmatched method falls through to the media 404 with no mutation or + // secret exposed (unlike /config and /streams above). + ar.Get("/swagger/doc.json", serveSwaggerJSON) + ar.Get("/swagger", http.RedirectHandler("/swagger/", http.StatusMovedPermanently).ServeHTTP) + ar.Get("/swagger/", serveSwaggerIndex) + + // Admin: /streams. Stream codes may contain '/' (namespacing) so the + // per-code routes are served by a catch-all dispatcher that parses + // (code, action) from the suffix. See dispatch.go for the rules. + ar.Route("/streams", func(r chi.Router) { + r.Get("/", s.streamH.List) + r.HandleFunc("/*", s.dispatchStreamsSubpath()) }) - }) - r.Route("/hooks", func(r chi.Router) { - r.Get("/", s.hookH.List) - r.Post("/", s.hookH.Create) - r.Route("/{hid}", func(r chi.Router) { - r.Get("/", s.hookH.Get) - r.Put("/", s.hookH.Update) - r.Delete("/", s.hookH.Delete) - r.Post("/test", s.hookH.Test) + // Templates. Codes are single-segment (a-zA-Z0-9_-) so the chi + // single-param routes are fine — no catch-all dispatcher needed. + ar.Route("/templates", func(r chi.Router) { + r.Get("/", s.templateH.List) + r.Route("/{code}", func(r chi.Router) { + r.Get("/", s.templateH.Get) + r.Post("/", s.templateH.Put) + r.Delete("/", s.templateH.Delete) + }) }) - }) - r.Route("/watermarks", func(r chi.Router) { - r.Get("/", s.watermarkH.List) - r.Post("/", s.watermarkH.Upload) - r.Route("/{filename}", func(r chi.Router) { - r.Get("/", s.watermarkH.Get) - r.Delete("/", s.watermarkH.Delete) - r.Get("/raw", s.watermarkH.Raw) + ar.Route("/sessions", func(r chi.Router) { + r.Get("/", s.sessionH.List) + r.Route("/{id}", func(r chi.Router) { + r.Get("/", s.sessionH.Get) + r.Delete("/", s.sessionH.Delete) + }) + }) + + ar.Route("/hooks", func(r chi.Router) { + r.Get("/", s.hookH.List) + r.Post("/", s.hookH.Create) + r.Route("/{hid}", func(r chi.Router) { + r.Get("/", s.hookH.Get) + r.Put("/", s.hookH.Update) + r.Delete("/", s.hookH.Delete) + r.Post("/test", s.hookH.Test) + }) + }) + + ar.Route("/watermarks", func(r chi.Router) { + r.Get("/", s.watermarkH.List) + r.Post("/", s.watermarkH.Upload) + r.Route("/{filename}", func(r chi.Router) { + r.Get("/", s.watermarkH.Get) + r.Delete("/", s.watermarkH.Delete) + r.Get("/raw", s.watermarkH.Raw) + }) }) - }) - r.Route("/vod", func(r chi.Router) { - r.Get("/", s.vodH.List) - r.Post("/", s.vodH.Create) - r.Route("/{name}", func(r chi.Router) { - r.Get("/", s.vodH.Get) - r.Put("/", s.vodH.Update) - r.Delete("/", s.vodH.Delete) - r.Get("/files", s.vodH.ListFiles) - r.Post("/files", s.vodH.UploadFile) - r.Delete("/files/*", s.vodH.DeleteFile) - r.Get("/raw/*", s.vodH.Raw) + ar.Route("/vod", func(r chi.Router) { + r.Get("/", s.vodH.List) + r.Post("/", s.vodH.Create) + r.Route("/{name}", func(r chi.Router) { + r.Get("/", s.vodH.Get) + r.Put("/", s.vodH.Update) + r.Delete("/", s.vodH.Delete) + r.Get("/files", s.vodH.ListFiles) + r.Post("/files", s.vodH.UploadFile) + r.Delete("/files/*", s.vodH.DeleteFile) + r.Get("/raw/*", s.vodH.Raw) + }) }) }) diff --git a/internal/domain/global_config.go b/internal/domain/global_config.go index 3d841f9f..e4abc06d 100644 --- a/internal/domain/global_config.go +++ b/internal/domain/global_config.go @@ -19,4 +19,5 @@ type GlobalConfig struct { Sessions *config.SessionsConfig `json:"sessions,omitempty" yaml:"sessions,omitempty"` Watermarks *config.WatermarksConfig `json:"watermarks,omitempty" yaml:"watermarks,omitempty"` Log *config.LogConfig `json:"log,omitempty" yaml:"log,omitempty"` + Auth *config.AuthConfig `json:"auth,omitempty" yaml:"auth,omitempty"` } diff --git a/internal/runtime/manager.go b/internal/runtime/manager.go index 02fc3e79..11d2fd26 100644 --- a/internal/runtime/manager.go +++ b/internal/runtime/manager.go @@ -236,6 +236,12 @@ func (m *Manager) diff(old, new *domain.GlobalConfig) { return m.deps.APISrv.StartWithConfig(ctx, new.Server) }) + // API auth: hot-swap users/roles without restarting the HTTP server. The + // middleware reads the new set atomically on its next request. + if m.deps.APISrv != nil && configChanged(old.Auth, new.Auth) { + m.deps.APISrv.SetAuthConfig(new.Auth) + } + // Push the new listeners snapshot to ingestor + publisher BEFORE // diffService can decide to restart them — each Run() reads the cached // listeners value at startup, so the swap must happen before the new