diff --git a/internal/server/api_g_cluster_test.go b/internal/server/api_g_cluster_test.go new file mode 100644 index 00000000..a7e7237f --- /dev/null +++ b/internal/server/api_g_cluster_test.go @@ -0,0 +1,70 @@ +// @spec system-rbac +// +// AC-21 TestGCluster_AnonymousIsRefusedOnPrivilegedSurfaces +package server + +import ( + "net/http" + "testing" + + "github.com/Hanalyx/openwatch/internal/auth" + "github.com/google/uuid" +) + +// @ac AC-21 +// AC-21 (NEGATIVE PATH): three surfaces that were reachable anonymously are +// now refused, and one disclosure is withheld. +// +// Each was a distinct class of problem, so each is asserted separately rather +// than as one loop: +// +// - enqueue-test-job wrote to the REAL job queue with no authentication. An +// unauthenticated write to a work queue is a denial-of-service primitive +// and a source of unbounded table growth. +// - /admin/license:verify was an oracle: submit any JWT and learn whether it +// verifies, its tier, its features, its expiry, and whether it was signed +// with the PREVIOUS key, which leaks key-rotation state. +// - connectivity:check gated on host:read, but it is not a read: it makes +// the server open an SSH or ICMP connection to a managed host on demand. +// - GET /license disclosed customer_id, the paying organisation's identity, +// to anyone who could reach the port. +func TestGCluster_AnonymousIsRefusedOnPrivilegedSurfaces(t *testing.T) { + t.Run("system-rbac/AC-21", func(t *testing.T) { + url, _ := freshAPIServer(t) + + // The job-queue DoS primitive. + req, _ := http.NewRequest("POST", url+"/api/v1/diagnostics:enqueue-test-job", nil) + if r := doReq(t, req); r.StatusCode == http.StatusAccepted { + r.Body.Close() + t.Error("anonymous enqueue-test-job must not reach the job queue") + } else { + r.Body.Close() + } + + // The license oracle. + req2, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", nil) + if r := doReq(t, req2); r.StatusCode == http.StatusOK { + r.Body.Close() + t.Error("anonymous license:verify must not answer; it is a signature and entitlement oracle") + } else { + r.Body.Close() + } + + // A viewer holds host:read but must not be able to make the server + // open a connection to a managed host. + // A real UUID: the all-zeros one is rejected as invalid input before + // the permission check runs, which would assert nothing about RBAC. + hid := uuid.New() + req3 := asRole(t, "POST", + url+"/api/v1/hosts/"+hid.String()+"/connectivity:check", + auth.RoleViewer, nil) + // Required header, or the request 400s on validation before RBAC runs + // and the assertion proves nothing. + req3.Header.Set("Idempotency-Key", "ac21-"+hid.String()) + r3 := doReq(t, req3) + defer r3.Body.Close() + if r3.StatusCode != http.StatusForbidden { + t.Errorf("viewer connectivity:check = %d, want 403: it is a side effect, not a read", r3.StatusCode) + } + }) +} diff --git a/internal/server/api_license_test.go b/internal/server/api_license_test.go index e47011cd..6bcbbf3f 100644 --- a/internal/server/api_license_test.go +++ b/internal/server/api_license_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "github.com/Hanalyx/openwatch/internal/auth" "github.com/Hanalyx/openwatch/internal/license" "github.com/golang-jwt/jwt/v5" ) @@ -94,9 +95,10 @@ func TestAPI_License_FreeTier(t *testing.T) { func TestAPI_License_VerifyTamperedJWT(t *testing.T) { t.Run("api-license/AC-05", func(t *testing.T) { url, _ := freshAPIServer(t) - body := strings.NewReader(`{"license_jwt":"not.a.valid"}`) - req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body) - req.Header.Set("Content-Type", "application/json") + // :verify is admin-gated: anonymously it is a signature and + // entitlement oracle on an /admin/ path. + req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin, + map[string]string{"license_jwt": "not.a.valid"}) resp := doReq(t, req) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { @@ -222,9 +224,8 @@ func TestAPI_License_VerifyValidJWT(t *testing.T) { url, _ := freshAPIServer(t) jwtBlob := mintTestLicenseJWT(t, []string{"premium_diagnostics", "remediation_execution"}) - body := strings.NewReader(`{"license_jwt":"` + jwtBlob + `"}`) - req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body) - req.Header.Set("Content-Type", "application/json") + req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin, + map[string]string{"license_jwt": jwtBlob}) resp := doReq(t, req) defer resp.Body.Close() var got struct { @@ -253,9 +254,8 @@ func TestAPI_License_VerifyDoesNotInstall(t *testing.T) { url, _ := freshAPIServer(t) jwtBlob := mintTestLicenseJWT(t, []string{"premium_diagnostics"}) - body := strings.NewReader(`{"license_jwt":"` + jwtBlob + `"}`) - req, _ := http.NewRequest("POST", url+"/api/v1/admin/license:verify", body) - req.Header.Set("Content-Type", "application/json") + req := asRole(t, "POST", url+"/api/v1/admin/license:verify", auth.RoleAdmin, + map[string]string{"license_jwt": jwtBlob}) resp := doReq(t, req) io.Copy(io.Discard, resp.Body) resp.Body.Close() diff --git a/internal/server/api_signoff_test.go b/internal/server/api_signoff_test.go index 20af175b..f9560a60 100644 --- a/internal/server/api_signoff_test.go +++ b/internal/server/api_signoff_test.go @@ -319,7 +319,9 @@ func TestSignoff_DoD15_PolicyReload(t *testing.T) { func TestSignoff_DoD16_QueueWorkerCorrelation(t *testing.T) { t.Run("release-stage-0-signoff/AC-10", func(t *testing.T) { url, pool := freshAPIServer(t) - req, _ := http.NewRequest("POST", url+"/api/v1/diagnostics:enqueue-test-job", nil) + // enqueue-test-job writes to the real job queue and is now admin-gated: + // unauthenticated it was a denial-of-service primitive. + req := asRole(t, "POST", url+"/api/v1/diagnostics:enqueue-test-job", auth.RoleAdmin, nil) req.Header.Set("X-Correlation-Id", "req-end2end-001") resp := doReq(t, req) defer resp.Body.Close() diff --git a/internal/server/api_system_connectivity_test.go b/internal/server/api_system_connectivity_test.go index 1a845e7e..f07a6a2a 100644 --- a/internal/server/api_system_connectivity_test.go +++ b/internal/server/api_system_connectivity_test.go @@ -404,7 +404,7 @@ func TestAPI_HostConnectivity_Check_NotFound_Returns404(t *testing.T) { t.Run("api-host-connectivity-check/AC-03", func(t *testing.T) { url, _ := freshAPIServer(t) missing, _ := uuid.NewV7() - req := asRole(t, "POST", url+"/api/v1/hosts/"+missing.String()+"/connectivity:check", auth.RoleViewer, nil) + req := asRole(t, "POST", url+"/api/v1/hosts/"+missing.String()+"/connectivity:check", auth.RoleOpsLead, nil) req.Header.Set("Idempotency-Key", "test-key-"+missing.String()) resp, err := http.DefaultClient.Do(req) if err != nil { @@ -429,7 +429,7 @@ func TestAPI_HostConnectivity_Check_MissingIdempotencyKey_Returns400(t *testing. VALUES ($1, $2, $3::inet, $4)`, id, "h-"+id.String(), "192.0.2.10", uid) - req := asRole(t, "POST", url+"/api/v1/hosts/"+id.String()+"/connectivity:check", auth.RoleViewer, nil) + req := asRole(t, "POST", url+"/api/v1/hosts/"+id.String()+"/connectivity:check", auth.RoleOpsLead, nil) // Intentionally no Idempotency-Key. resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 8679ed02..721d69b9 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -276,7 +276,13 @@ func (h *handlers) GetLicense(w http.ResponseWriter, r *http.Request) { resp.Tier = api.LicenseStateResponseTier(lic.Tier) resp.Status = api.LicenseStateResponseStatus(lic.Status) resp.Features = featuresToStrings(lic.Features) - resp.CustomerId = &lic.CustomerID + // customer_id identifies the paying organisation. The tier, status and + // feature list are operational facts a UI needs before login, but the + // customer identity is not, and this route is reachable anonymously. + // Disclose it only to a caller who can already read system config. + if auth.FromContext(r.Context()).HasPermission(auth.SystemRead) { + resp.CustomerId = &lic.CustomerID + } exp := lic.ExpiresAt resp.ExpiresAt = &exp grace := lic.InGracePeriod @@ -290,6 +296,13 @@ func (h *handlers) GetLicense(w http.ResponseWriter, r *http.Request) { // PostAdminLicenseVerify dry-run validates a JWT without installing. // Spec: app/specs/api/license.spec.yaml AC-4, AC-5, AC-6. func (h *handlers) PostAdminLicenseVerify(w http.ResponseWriter, r *http.Request) { + // Anonymous, this is a signature and entitlement oracle on an /admin/ + // path: submit any JWT and learn whether it verifies, which tier and + // features it carries, when it expires, and whether it was signed with the + // previous key. That last one leaks key-rotation state. Admin read. + if denied := auth.EnforcePermission(w, r, auth.SystemRead); denied { + return + } var req api.LicenseVerifyRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { writeError(w, http.StatusBadRequest, "validation.field_required", "client", @@ -807,6 +820,14 @@ func (h *handlers) PostDiagnosticsEvaluateAlert(w http.ResponseWriter, r *http.R // carrying the request's correlation_id. // Spec release-stage-0-signoff AC-10. func (h *handlers) PostDiagnosticsEnqueueTestJob(w http.ResponseWriter, r *http.Request) { + // This writes to the REAL job queue. Unauthenticated it is a denial-of- + // service primitive and a source of unbounded table growth: anyone who can + // reach the port can enqueue without limit. It is a Stage-0 walking- + // skeleton demo, so gate it behind the same permission as any other + // system-mutating operation. + if denied := auth.EnforcePermission(w, r, auth.SystemConfigWrite); denied { + return + } jobID, err := queue.Enqueue(r.Context(), h.pool, "diagnostics.test_job", nil) if err != nil { writeError(w, http.StatusInternalServerError, "queue.enqueue_failed", "server", diff --git a/internal/server/host_connectivity_check_handler.go b/internal/server/host_connectivity_check_handler.go index 0b9155de..d65f6aee 100644 --- a/internal/server/host_connectivity_check_handler.go +++ b/internal/server/host_connectivity_check_handler.go @@ -32,7 +32,13 @@ func (h *handlers) PostHostConnectivityCheck( id openapitypes.UUID, _ api.PostHostConnectivityCheckParams, ) { - if denied := auth.EnforcePermission(w, r, auth.HostRead); denied { + // This is not a read. It makes the server open an SSH or ICMP connection + // to a managed host on demand, so it is a side effect a viewer should not + // be able to trigger. A dedicated permission already existed in the + // registry and was never wired to anything; wire it. Viewer and auditor + // lose this (they hold host:read but not host:connectivity_check); + // ops_lead, security_admin and admin keep it. + if denied := auth.EnforcePermission(w, r, auth.HostConnectivityCheck); denied { return } if h.liveSvc == nil { diff --git a/specs/system/rbac.spec.yaml b/specs/system/rbac.spec.yaml index 54601161..f3858695 100644 --- a/specs/system/rbac.spec.yaml +++ b/specs/system/rbac.spec.yaml @@ -1,7 +1,7 @@ spec: id: system-rbac title: RBAC permission registry and middleware - version: "1.2.0" + version: "1.3.0" status: approved tier: 1 @@ -175,3 +175,21 @@ spec: ErrRoleExceedsGrant and names the offending permissions so the API can return them. priority: critical + - id: AC-21 + description: > + Privileged surfaces are not reachable anonymously, and identity is not + disclosed to a caller who has none. POST /diagnostics:enqueue-test-job + writes to the real job queue and MUST require an admin-mutating + permission: unauthenticated it is a denial-of-service primitive and a + source of unbounded table growth. POST /admin/license:verify MUST + require an admin read: anonymously it answers whether any submitted JWT + verifies and reports its tier, features, expiry, and whether it was + signed with the previous key, which leaks key-rotation state. POST + /hosts/{id}/connectivity:check MUST require host:connectivity_check + rather than host:read, because it makes the server open an SSH or ICMP + connection to a managed host on demand; that is a side effect, not a + read, and viewer and auditor therefore lose it. GET /license MUST omit + customer_id unless the caller holds system:read, since the tier and + feature list are operational facts a pre-login UI needs but the paying + organisation's identity is not. + priority: critical