Vulnerability GHSA-h246-wpgf-vmq5

Summary

Incomplete fix of GHSA-5v7c-xpfp-p65m: the secure-session (OTP step-up) requirement added to the nginx, cert, dns, backup, site, and stream mutation routers was not applied to the parallel api/cluster router, so cluster node management and cluster-wide nginx reload/restart run with only a JWT and no step-up. An authenticated user whose JWT is stolen or persisted, but who does not hold a fresh secure-session, can perform cluster node CRUD (including reading and rewriting a node token secret) and trigger cluster-wide nginx reload/restart. Confirmed at HEAD 2cb7ee9102c9d87274de2fa104db804841d140a0.

The defect

GHSA-5v7c-xpfp-p65m required a fresh secure session (an OTP step-up beyond the JWT) for sensitive mutation routes. The fix wrapped the nginx, cert, dns, backup, site, and stream mutation handlers in middleware.RequireSecureSession() and added reload/restart to the MCP sensitive-tool list. It did not touch the physically separate api/cluster package, whose router registers the same class of sensitive operations on the bare authenticated group.

The fixed sibling, api/nginx/router.go:

o := r.Group("", middleware.RequireSecureSession())   // line 28
{
    o.POST("nginx/reload", Reload)                     // line 30
    o.POST("nginx/restart", Restart)                   // line 31
}

The missed router, api/cluster/router.go, registers every sensitive operation directly on r with no RequireSecureSession wrapper:

nodeGroup := r.Group("nodes")                          // line 9, no step-up
{
    nodeGroup.POST("", AddNode)                        // line 12
    nodeGroup.POST("/:id", EditNode)                   // line 13
    nodeGroup.DELETE("/:id", DeleteNode)               // line 14
}
r.POST("nodes/reload_nginx", ReloadNginx)              // line 17
r.POST("nodes/restart_nginx", RestartNginx)            // line 18
r.POST("namespaces", AddNamespace)                     // line 22
r.POST("namespaces/:id", ModifyNamespace)              // line 23
r.DELETE("namespaces/:id", DeleteNamespace)            // line 24

Both routers mount on the same group in router/routers.go: g := root.Group("/", middleware.AuthRequired(), middleware.Proxy()) (line 87); nginx.InitRouter(g) creates its own RequireSecureSession subgroup, cluster.InitRouter(g) does not. So the cluster routes inherit only AuthRequired and Proxy, exactly the pre-fix posture the advisory closed for the nginx routes. AuthRequired has no role gate, so any authenticated user reaches them.

Attacker model and impact

An authenticated, OTP-enabled user who does not present a fresh X-Secure-Session-ID (the parent advisory's model: a stolen or persisted JWT used without the step-up). Such a user can: add/edit/delete cluster nodes (AddNode/EditNode store an AES-serialized node token, the secret used to control a remote node, so this reads back and rewrites a cross-node credential); trigger nodes/reload_nginx and nodes/restart_nginx across the cluster (the exact reload/restart action class the fix protected on the single-node path); and add/modify/delete/reorder namespaces.

Proof of concept: with a valid JWT but no fresh secure session, call POST /api/nodes (AddNode) or POST /api/nodes/reload_nginx. The nginx equivalent POST /api/nginx/reload returns the secure-session challenge; the cluster route succeeds.

Verification

Source-verified at HEAD: the nginx router wraps reload/restart in RequireSecureSession, the cluster router registers node/namespace/reload/restart directly on the group with no such wrapper, and both mount on the AuthRequired+Proxy-only group. I did not stand up a live nginx-ui.

Suggested fix

Wrap the cluster router's mutation handlers (node CRUD, nodes/reload_nginx, nodes/restart_nginx, namespace CRUD) in middleware.RequireSecureSession(), mirroring api/nginx/router.go. Secondary lower-confidence items worth checking in the same pass: the system/restart handler, the core-upgrade websocket, and the upstream socket PUT also appear to run without the step-up (I did not fully trace these).