diff --git a/spec.yaml b/spec.yaml index c159a3f..11a6a70 100644 --- a/spec.yaml +++ b/spec.yaml @@ -2,7 +2,7 @@ openapi: 3.0.3 info: title: Dcntrlized Maestro API version: 1.0.0 - description: The API of the Maestro orchestration router. Maestro manages the lifecycle of Dcntrlized service containers and routes inter-service action calls to them. Every /api/* route except /api/auth/* and the deliberately public GET /api/instance (hub-app theme) requires an instance-owner session (cookies issued by the auth endpoints); in setup mode gated routes answer 428 ERR_SETUP_REQUIRED, otherwise 401 ERR_UNAUTHENTICATED. Mutating requests from browser origins are additionally checked against the request host (CSRF). + description: The API of the Maestro orchestration router. Maestro manages the lifecycle of Dcntrlized service containers and routes inter-service action calls to them. Every /api/* route except /api/auth/* and the deliberately public GET /api/instance (hub-app theme) requires an instance-owner session (cookies issued by the auth endpoints); in setup mode gated routes answer 428 ERR_SETUP_REQUIRED, otherwise 401 ERR_UNAUTHENTICATED (an unreadable owner store fails closed with 500 ERR_INTERNAL). Mutating requests from browser origins are additionally checked against the request host (CSRF). components: schemas: @@ -616,7 +616,8 @@ paths: The SPA bootstraps from this endpoint. All other /api/* routes are gated by the instance-owner session (onboarding theme M1): in setup mode they answer 428 ERR_SETUP_REQUIRED, otherwise 401 - ERR_UNAUTHENTICATED without a valid session. + ERR_UNAUTHENTICATED without a valid session; an unreadable owner store + fails closed with 500 ERR_INTERNAL everywhere. responses: '200': description: Gate state @@ -650,6 +651,8 @@ paths: description: Setup already completed (ERR_SETUP_COMPLETED) '429': description: Rate limited (ERR_RATE_LIMITED) + '500': + description: Owner store unreadable/unwritable (ERR_INTERNAL) /api/auth/login: post: @@ -674,6 +677,8 @@ paths: description: Unknown user or wrong password (ERR_INVALID_CREDENTIALS) '429': description: Rate limited (ERR_RATE_LIMITED) with Retry-After + '500': + description: Owner store unreadable (ERR_INTERNAL) /api/auth/refresh: post: @@ -721,6 +726,8 @@ paths: description: Not authenticated '429': description: Rate limited (ERR_RATE_LIMITED) + '500': + description: Owner store unreadable/unwritable (ERR_INTERNAL) /api/ws: get: diff --git a/src/handlers/auth/auth.go b/src/handlers/auth/auth.go index 52bdf98..8613a9c 100644 --- a/src/handlers/auth/auth.go +++ b/src/handlers/auth/auth.go @@ -22,6 +22,7 @@ package auth import ( "context" "encoding/json" + "errors" "net/http" "net/url" "os" @@ -99,14 +100,24 @@ type passwordChange struct { } // SetupRequired reports whether the instance still needs its first owner. -func (s *Service) SetupRequired(ctx context.Context) bool { +// Storage failures are returned, never folded into "setup required" — an +// unreadable owner store must fail closed (500), not advertise setup mode. +func (s *Service) SetupRequired(ctx context.Context) (bool, error) { count, err := s.owners.Count(ctx) - return err != nil || count == 0 + if err != nil { + return false, err + } + return count == 0, nil } // Status serves GET /api/auth/status — public; the SPA bootstraps from it. func (s *Service) Status(w http.ResponseWriter, r *http.Request) { - setupRequired := s.SetupRequired(r.Context()) + setupRequired, err := s.SetupRequired(r.Context()) + if err != nil { + logger.Error("Owner store unreadable: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + return + } username := "" if !setupRequired { if c, err := r.Cookie(accessCookie); err == nil { @@ -130,7 +141,13 @@ func (s *Service) Setup(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "ERR_INVALID_REQUEST") return } - if count, _ := s.owners.Count(r.Context()); count > 0 { + count, err := s.owners.Count(r.Context()) + if err != nil { + logger.Error("Owner store unreadable during setup: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + return + } + if count > 0 { writeError(w, http.StatusConflict, "ERR_SETUP_COMPLETED") return } @@ -140,8 +157,17 @@ func (s *Service) Setup(w http.ResponseWriter, r *http.Request) { } o, err := s.owners.Create(r.Context(), creds.Username, creds.Password, true) if err != nil { - s.throttle.Fail(creds.Username) - writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS") + switch { + case errors.Is(err, owner.ErrInvalidUsername), errors.Is(err, owner.ErrWeakPassword): + s.throttle.Fail(creds.Username) + writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS") + case errors.Is(err, os.ErrExist): + // Lost a setup race; the instance already has its first owner. + writeError(w, http.StatusConflict, "ERR_SETUP_COMPLETED") + default: + logger.Error("Owner store write failed during setup: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + } return } logger.Info("Owner account created: %s", o.Username) @@ -160,9 +186,14 @@ func (s *Service) Login(w http.ResponseWriter, r *http.Request) { return } o, err := s.owners.Verify(r.Context(), creds.Username, creds.Password) - if err != nil || o == nil { - s.throttle.Fail(creds.Username) - writeError(w, http.StatusUnauthorized, "ERR_INVALID_CREDENTIALS") + if err != nil { + if errors.Is(err, os.ErrNotExist) { + s.throttle.Fail(creds.Username) + writeError(w, http.StatusUnauthorized, "ERR_INVALID_CREDENTIALS") + } else { + logger.Error("Owner store unreadable during login: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + } return } s.throttle.Reset(creds.Username) @@ -298,8 +329,14 @@ func (s *Service) ChangePassword(w http.ResponseWriter, r *http.Request) { return } if _, err := store.ChangePassword(r.Context(), sub, change.CurrentPassword, change.NewPassword); err != nil { - s.throttle.Fail(sub) - writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS") + switch { + case errors.Is(err, os.ErrPermission), errors.Is(err, os.ErrNotExist): + s.throttle.Fail(sub) + writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS") + default: + logger.Error("Owner store write failed during password change: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + } return } // Every existing session dies: the password changed under them. @@ -315,7 +352,13 @@ func (s *Service) ChangePassword(w http.ResponseWriter, r *http.Request) { // get the uniform 401; mutating requests pass the CSRF origin check. func (s *Service) RequireSession(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if s.SetupRequired(r.Context()) { + setupRequired, err := s.SetupRequired(r.Context()) + if err != nil { + logger.Error("Owner store unreadable: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + return + } + if setupRequired { writeError(w, http.StatusPreconditionRequired, "ERR_SETUP_REQUIRED") return } @@ -342,7 +385,13 @@ func (s *Service) RequireSession(next http.Handler) http.Handler { // rejected handshakes close with 401 instead of switching protocols. func (s *Service) GateWebSocket(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if s.SetupRequired(r.Context()) { + setupRequired, err := s.SetupRequired(r.Context()) + if err != nil { + logger.Error("Owner store unreadable: %v", err) + writeError(w, http.StatusInternalServerError, "ERR_INTERNAL") + return + } + if setupRequired { writeError(w, http.StatusPreconditionRequired, "ERR_SETUP_REQUIRED") return } diff --git a/src/handlers/auth/auth_test.go b/src/handlers/auth/auth_test.go index 2d1bcb6..bf079e4 100644 --- a/src/handlers/auth/auth_test.go +++ b/src/handlers/auth/auth_test.go @@ -19,6 +19,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -394,3 +396,83 @@ func TestGateWebSocketRejectsAnonymousUpgrade(t *testing.T) { t.Fatalf("anonymous ws = %d, want 401", rec.Code) } } + +// newBrokenStoreServer wires the same mux as newTestServer but pre-corrupts +// owners.json so every owner-store read fails — the on-disk stand-in for an +// unreadable (e.g. root-owned after a pre-handoff boot) state directory. +func newBrokenStoreServer(t *testing.T) (*Service, http.Handler) { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "owners.json"), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + owners, err := owner.NewFileOwnerStore(dir) + if err != nil { + t.Fatal(err) + } + tokens, err := owner.NewTokenService(dir) + if err != nil { + t.Fatal(err) + } + sessions, err := owner.NewFileSessionStore(dir) + if err != nil { + t.Fatal(err) + } + s := New(owners, tokens, sessions) + + mux := http.NewServeMux() + mux.HandleFunc("/api/auth/status", s.Status) + mux.HandleFunc("/api/auth/setup", s.Setup) + mux.HandleFunc("/api/auth/login", s.Login) + mux.Handle("/api/gated", s.RequireSession(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }))) + return s, mux +} + +func assertErrorCode(t *testing.T, rec *httptest.ResponseRecorder, code string) { + t.Helper() + var e errorBody + if err := json.Unmarshal(rec.Body.Bytes(), &e); err != nil { + t.Fatalf("body %q: %v", rec.Body.String(), err) + } + if e.Error.Code != code { + t.Fatalf("error code = %q, want %q", e.Error.Code, code) + } +} + +// TestStorageFailuresAnswerInternalError pins the fail-closed contract: an +// unreadable owner store is a 500 everywhere — never a 4xx that blames the +// credentials (the ERR_INVALID_CREDENTIALS-on-EACCES bug) and never a 428 +// that re-advertises setup mode. +func TestStorageFailuresAnswerInternalError(t *testing.T) { + _, h := newBrokenStoreServer(t) + + rec := do(t, h, http.MethodGet, "/api/auth/status", "", nil) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", rec.Code) + } + assertErrorCode(t, rec, "ERR_INTERNAL") + + rec = do(t, h, http.MethodPost, "/api/auth/setup", + `{"username":"admin","password":"first-owner-pass"}`, nil) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("setup = %d, want 500", rec.Code) + } + assertErrorCode(t, rec, "ERR_INTERNAL") + + rec = do(t, h, http.MethodPost, "/api/auth/login", + `{"username":"admin","password":"first-owner-pass"}`, nil) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("login = %d, want 500", rec.Code) + } + assertErrorCode(t, rec, "ERR_INTERNAL") + + req := httptest.NewRequest(http.MethodGet, "/api/gated", nil) + rec2 := httptest.NewRecorder() + h.ServeHTTP(rec2, req) + if rec2.Code != http.StatusInternalServerError { + t.Fatalf("gated route = %d, want 500 (fail closed)", rec2.Code) + } + assertErrorCode(t, rec2, "ERR_INTERNAL") +} diff --git a/src/lib/containers/install.go b/src/lib/containers/install.go index ccdf09f..2f15051 100644 --- a/src/lib/containers/install.go +++ b/src/lib/containers/install.go @@ -202,6 +202,9 @@ func EnsureDataRoot() error { if err != nil { return err } + if err := healOwnerState(root, identity); err != nil { + return fmt.Errorf("failed to hand owner state dir to uid %d: %w", identity.uid, err) + } if err := os.Chown(root, identity.uid, identity.gid); err != nil { return fmt.Errorf("failed to chown data root %s to %d:%d: %w", root, identity.uid, identity.gid, err) } @@ -212,6 +215,33 @@ func EnsureDataRoot() error { return nil } +// healOwnerState gives the instance-owner gate's state directory to the +// runtime user. Boots before the privilege drop opened the gate's stores as +// root first, leaving a root-owned /maestro behind; after the handoff +// every write then failed with EACCES (surfacing as a bogus +// ERR_INVALID_CREDENTIALS on /api/auth/setup). Repairs such installs here, +// while the process can still chown. Missing dirs are fine — nothing to heal. +func healOwnerState(root string, id runtimeIdentity) error { + dir := filepath.Join(root, "maestro") + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("read owner state dir: %w", err) + } + if err := os.Chown(dir, id.uid, id.gid); err != nil { + return fmt.Errorf("chown %s: %w", dir, err) + } + for _, e := range entries { + p := filepath.Join(dir, e.Name()) + if err := os.Chown(p, id.uid, id.gid); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("chown %s: %w", p, err) + } + } + return nil +} + // runtimeIdentity is the user the container should run as, plus any // supplementary groups needed to keep working (e.g. the docker socket group). type runtimeIdentity struct { diff --git a/src/lib/owner/owner.go b/src/lib/owner/owner.go index 4ecc1e8..36e93b4 100644 --- a/src/lib/owner/owner.go +++ b/src/lib/owner/owner.go @@ -27,6 +27,7 @@ import ( "crypto/subtle" "encoding/base64" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -249,13 +250,18 @@ func (s *FileOwnerStore) Create(_ context.Context, username, password string, is func (s *FileOwnerStore) Verify(_ context.Context, username, password string) (*Owner, error) { o, err := s.Get(context.Background(), username) - if err != nil || o == nil { + if err != nil { + // Unknown user is a credentials miss; keep storage failures + // distinguishable so callers can answer 500 instead of 401. + if errors.Is(err, os.ErrNotExist) { + return nil, os.ErrNotExist + } + return nil, err + } + if !VerifyPassword(o.PasswordHash, password) { return nil, os.ErrNotExist } - if VerifyPassword(o.PasswordHash, password) { - return o, nil - } - return nil, os.ErrNotExist + return o, nil } func (s *FileOwnerStore) Get(_ context.Context, username string) (*Owner, error) { diff --git a/src/lib/owner/owner_test.go b/src/lib/owner/owner_test.go index c89a754..d21653f 100644 --- a/src/lib/owner/owner_test.go +++ b/src/lib/owner/owner_test.go @@ -166,3 +166,29 @@ func TestOwnerFilePermissions(t *testing.T) { t.Fatalf("owners.json mode = %o, want 600", perm) } } + +// TestVerifyDistinguishesStorageFailureFromUnknownUser pins the contract the +// auth handlers rely on: an unknown user or wrong password surfaces as +// os.ErrNotExist (→ 401), while a broken store keeps its own error (→ 500). +func TestVerifyDistinguishesStorageFailureFromUnknownUser(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + s, err := NewFileOwnerStore(dir) + if err != nil { + t.Fatal(err) + } + if _, err := s.Create(ctx, "admin", "verify-check-pass", true); err != nil { + t.Fatal(err) + } + if _, err := s.Verify(ctx, "ghost", "verify-check-pass"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("unknown user: %v", err) + } + + broken := []byte("{not json") + if err := os.WriteFile(filepath.Join(dir, "owners.json"), broken, 0o600); err != nil { + t.Fatal(err) + } + if _, err := s.Verify(ctx, "admin", "verify-check-pass"); err == nil || errors.Is(err, os.ErrNotExist) { + t.Fatalf("storage failure collapsed into ErrNotExist: %v", err) + } +} diff --git a/src/main.go b/src/main.go index eb84431..4938bbc 100644 --- a/src/main.go +++ b/src/main.go @@ -75,6 +75,14 @@ func main() { if err != nil { log.Fatalf("Failed to resolve data root: %v", err) } + // Drop to the host user BEFORE any state is created: the stores below + // mkdir/write under /maestro, and files made during the root + // phase stay root-owned after the handoff, locking the owner gate out of + // its own store (EACCES surfaced as a bogus ERR_INVALID_CREDENTIALS on + // /api/auth/setup). + if err := libcontainers.EnsureDataRoot(); err != nil { + log.Fatalf("Data root check failed: %v", err) + } maestroDir := filepath.Join(dataRoot, "maestro") ownerStore, err := owner.NewFileOwnerStore(maestroDir) if err != nil { @@ -100,9 +108,6 @@ func main() { log.Fatalf("Failed to init docker client: %v", err) } defer dc.Close() - if err := libcontainers.EnsureDataRoot(); err != nil { - log.Fatalf("Data root check failed: %v", err) - } // Provision the instance CA on first run (security theme M1). The CA is // the trust anchor for every peer edge on dcntrlized_net; services get // their identity certs from it at start/install.