From 40240ed02bdfe4ad3914292c32cac0a52dfc719f Mon Sep 17 00:00:00 2001 From: xvkong233 <88175491+xvkong233@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:56:05 +0800 Subject: [PATCH 1/2] fix(nat): strip only dashboard-issued Authorization credentials at NAT ingress Since 2640b86 (v2.2.11, 2026-07-20) the NAT ingress deleted the entire Authorization header, breaking self-hosted apps behind the tunnel that authenticate with their own Bearer tokens (e.g. the Qinglong panel login loop). Strip only credentials issued by this dashboard and forward foreign ones: - controller.IsDashboardCredential classifies an Authorization value as dashboard-issued via a signature-only JWT parser derived from the auth middleware (same key, HS256 pinned, claims validation deliberately skipped so an expired panel token is still recognized and kept out of the tunnel) or an api_tokens lookup for PAT-shaped values. Unknown PATs, foreign signatures, other schemes and missing values pass through. When classification cannot complete (parser not ready, DB nil or error) the verdict fails closed (strip) and the log carries a fixed reason only, never credential content. - Both NAT capability variants (default, and agent-compat legacy / invalid-capability / post-consume paths) replace the unconditional Del("Authorization") with stripDashboardCredential. The rpc package receives the classifier through SetNATDashboardCredentialGate, the same injection pattern as SetMCPKillSwitchObserver; an unwired gate keeps the historical strip-everything behavior. Tests: controller classification semantics (panel JWT, expired panel JWT and panel PAT stripped; foreign-signed JWT, unknown nzp_ token, random strings, non-Bearer schemes and missing header pass through; parser unavailable, DB nil and DB error fail closed with credential-free logs) plus rpc unit and flow coverage for both variants (dashboard credentials never reach the forwarded bytes; foreign and missing Authorization are forwarded untouched). Compatibility: no API, database or frontend changes; NAT backends that do not use Authorization behave exactly as before. --- cmd/dashboard/controller/controller.go | 3 + .../controller/dashboard_credential.go | 104 ++++++++++++ .../controller/dashboard_credential_test.go | 150 ++++++++++++++++++ cmd/dashboard/main.go | 5 + cmd/dashboard/rpc/nat.go | 9 +- .../rpc/nat_capability_agentcompat.go | 14 +- .../rpc/nat_capability_agentcompat_test.go | 122 ++++++++++++++ cmd/dashboard/rpc/nat_capability_default.go | 6 +- .../rpc/nat_capability_default_flow_test.go | 50 +++++- .../rpc/nat_capability_default_test.go | 74 ++++++++- .../nat_capability_flow_agentcompat_test.go | 61 ++++++- cmd/dashboard/rpc/nat_credential.go | 41 +++++ cmd/dashboard/rpc/nat_credential_test.go | 37 +++++ 13 files changed, 662 insertions(+), 14 deletions(-) create mode 100644 cmd/dashboard/controller/dashboard_credential.go create mode 100644 cmd/dashboard/controller/dashboard_credential_test.go create mode 100644 cmd/dashboard/rpc/nat_credential.go create mode 100644 cmd/dashboard/rpc/nat_credential_test.go diff --git a/cmd/dashboard/controller/controller.go b/cmd/dashboard/controller/controller.go index 1823d2c4..a3dd50b0 100644 --- a/cmd/dashboard/controller/controller.go +++ b/cmd/dashboard/controller/controller.go @@ -57,6 +57,9 @@ func routers(r *gin.Engine, frontendDist fs.FS) { if err := authMiddleware.MiddlewareInit(); err != nil { log.Fatal("authMiddleware.MiddlewareInit Error:" + err.Error()) } + // Signature-only view of the auth middleware for IsDashboardCredential: + // same key and pinned algorithm, claims validation deliberately skipped. + dashboardCredentialJWTParser = newDashboardCredentialJWTParser(authMiddleware) // /mcp — Model Context Protocol endpoint, authenticated by PAT only (闸 1 + 闸 2)。 // 不放在 /api/v1 下：MCP client 配置 URL 更短，且 MCP transport 协议演进与 REST API // 解耦。鉴权一律走 apiTokenAuthMiddleware；不接受 JWT 以避免浏览器误触。 diff --git a/cmd/dashboard/controller/dashboard_credential.go b/cmd/dashboard/controller/dashboard_credential.go new file mode 100644 index 00000000..e24ecdd7 --- /dev/null +++ b/cmd/dashboard/controller/dashboard_credential.go @@ -0,0 +1,104 @@ +package controller + +import ( + "errors" + "log" + "strings" + + jwt "github.com/golang-jwt/jwt/v4" + "gorm.io/gorm" + + ginjwt "github.com/appleboy/gin-jwt/v2" + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/service/singleton" +) + +// dashboardCredentialJWTParser verifies that a JWT was signed by this +// dashboard. It is derived from the auth middleware created in routers() and +// exists only for IsDashboardCredential; it must never authenticate dashboard +// requests on its own. +var dashboardCredentialJWTParser *ginjwt.GinJWTMiddleware + +// newDashboardCredentialJWTParser derives a signature-only parser from the +// dashboard auth middleware. It keeps the exact key value and the pinned HS256 +// signing algorithm of initParams() — there is no second read of +// singleton.Conf.JWTSecretKey — and disables claims validation so expiry does +// not change the verdict: an expired token is still proof the dashboard issued +// it, and keeping expired dashboard tokens out of the NAT tunnel prevents a +// careless backend from treating "any JWT" as its own credential. The live +// auth middleware keeps full claims validation; dashboard authentication is +// not relaxed by this parser in any way. +func newDashboardCredentialJWTParser(authMiddleware *ginjwt.GinJWTMiddleware) *ginjwt.GinJWTMiddleware { + parser := *authMiddleware + parser.ParseOptions = []jwt.ParserOption{jwt.WithoutClaimsValidation()} + return &parser +} + +// IsDashboardCredential reports whether an Authorization header value was +// issued by this dashboard: a panel-signed JWT or a panel API token (PAT). +// +// NAT ingress uses this to enforce the tunnel credential invariant: values +// classified as dashboard-issued are stripped before the request is tunneled +// to the agent, while foreign credentials are forwarded untouched because the +// NAT backend may authenticate with credentials of its own. +// +// Decision policy mirrors the dashboard's own auth middlewares: only the +// case-sensitive "Bearer " scheme is considered; PAT-shaped values are +// resolved against the api_tokens table; everything else is treated as a JWT. +// Whenever the classification cannot be completed (parser or config not ready, +// database failure) the value is conservatively reported as a dashboard +// credential so the caller strips it; only a deterministic proof that the +// value was not issued by this dashboard (scheme mismatch, unknown PAT hash, +// failed signature verification) returns false. +func IsDashboardCredential(authz string) bool { + raw := strings.TrimSpace(authz) + if !strings.HasPrefix(raw, "Bearer ") { + return false + } + plaintext := strings.TrimSpace(strings.TrimPrefix(raw, "Bearer ")) + if strings.HasPrefix(plaintext, model.APITokenPrefix) { + return isDashboardAPIToken(plaintext) + } + return isDashboardSignedJWT(plaintext) +} + +// isDashboardAPIToken resolves a PAT-shaped value against the api_tokens +// table. The plaintext token is only ever hashed here; it is never logged. +func isDashboardAPIToken(plaintext string) bool { + if singleton.DB == nil { + logDashboardCredentialUnclassifiable("api token lookup unavailable") + return true + } + var tok model.APIToken + err := singleton.DB.Where("token_hash = ?", model.HashAPIToken(plaintext)).First(&tok).Error + if err == nil { + return true + } + if errors.Is(err, gorm.ErrRecordNotFound) { + // Deterministic proof the token was not issued by this dashboard: + // forwarding it leaks nothing the dashboard ever validated. + return false + } + logDashboardCredentialUnclassifiable("api token lookup failed") + return true +} + +// isDashboardSignedJWT verifies the token signature with the dashboard's +// pinned algorithm and key. Claims validation is disabled on purpose (see +// newDashboardCredentialJWTParser): expiry does not change who issued the +// token. +func isDashboardSignedJWT(token string) bool { + if dashboardCredentialJWTParser == nil { + // routers() has not run yet (startup incomplete): fail closed. + logDashboardCredentialUnclassifiable("dashboard credential parser unavailable") + return true + } + _, err := dashboardCredentialJWTParser.ParseTokenString(token) + return err == nil +} + +// logDashboardCredentialUnclassifiable records a classification failure. The +// Authorization value itself is never included in the message. +func logDashboardCredentialUnclassifiable(reason string) { + log.Printf("NEZHA>> NAT ingress: Authorization value could not be classified (%s); stripping it as a precaution", reason) +} diff --git a/cmd/dashboard/controller/dashboard_credential_test.go b/cmd/dashboard/controller/dashboard_credential_test.go new file mode 100644 index 00000000..9ad1070e --- /dev/null +++ b/cmd/dashboard/controller/dashboard_credential_test.go @@ -0,0 +1,150 @@ +package controller + +import ( + "bytes" + "log" + "testing" + "time" + + jwt "github.com/appleboy/gin-jwt/v2" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/service/singleton" +) + +func setupDashboardCredentialTest(t *testing.T) { + t.Helper() + originalConf, originalDB, originalParser := singleton.Conf, singleton.DB, dashboardCredentialJWTParser + t.Cleanup(func() { + singleton.Conf, singleton.DB, dashboardCredentialJWTParser = originalConf, originalDB, originalParser + }) + + singleton.Conf = &singleton.ConfigClass{Config: &model.Config{}} + singleton.Conf.JWTSecretKey = "dashboard-credential-classification-test-key" + singleton.Conf.JWTTimeout = 1 + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + t.Cleanup(func() { _ = sqlDB.Close() }) + require.NoError(t, db.AutoMigrate(&model.APIToken{})) + singleton.DB = db + + // Same construction path as routers(): jwt.New(initParams()). + authMiddleware, err := jwt.New(initParams()) + require.NoError(t, err) + dashboardCredentialJWTParser = newDashboardCredentialJWTParser(authMiddleware) +} + +// mintDashboardCredentialJWT signs a JWT with the dashboard key through +// initParams(), the same key source the production verifier uses. A negative +// timeout mints an already-expired token. +func mintDashboardCredentialJWT(t *testing.T, timeout time.Duration) string { + t.Helper() + params := initParams() + params.Timeout = timeout + mw, err := jwt.New(params) + require.NoError(t, err) + token, _, err := mw.TokenGenerator(nil) + require.NoError(t, err) + return token +} + +func mintForeignJWT(t *testing.T) string { + t.Helper() + params := initParams() + params.Key = []byte("foreign-signing-key-not-held-by-the-dashboard") + mw, err := jwt.New(params) + require.NoError(t, err) + token, _, err := mw.TokenGenerator(nil) + require.NoError(t, err) + return token +} + +func TestIsDashboardCredentialClassification(t *testing.T) { + setupDashboardCredentialTest(t) + + dashboardJWT := mintDashboardCredentialJWT(t, time.Hour) + expiredDashboardJWT := mintDashboardCredentialJWT(t, -time.Hour) + foreignJWT := mintForeignJWT(t) + + dashboardPAT := model.APITokenPrefix + "dashboard-credential-pat" + require.NoError(t, singleton.DB.Create(&model.APIToken{ + UserID: 1, + Name: "nat-classification", + TokenHash: model.HashAPIToken(dashboardPAT), + }).Error) + + t.Run("dashboard signed jwt is a dashboard credential", func(t *testing.T) { + require.True(t, IsDashboardCredential("Bearer "+dashboardJWT)) + }) + + t.Run("expired dashboard signed jwt is still a dashboard credential", func(t *testing.T) { + // Signature verification ignores exp on purpose: an expired token is + // still proof the dashboard issued it. + require.True(t, IsDashboardCredential("Bearer "+expiredDashboardJWT)) + }) + + t.Run("dashboard api token is a dashboard credential", func(t *testing.T) { + require.True(t, IsDashboardCredential("Bearer "+dashboardPAT)) + }) + + t.Run("foreign signed jwt is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredential("Bearer "+foreignJWT)) + }) + + t.Run("unknown api token is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredential("Bearer "+model.APITokenPrefix+"unknown-to-the-dashboard")) + }) + + t.Run("random string is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredential("Bearer random-not-a-token")) + }) + + t.Run("non bearer scheme is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredential("Basic "+dashboardJWT)) + // Scheme matching is case-sensitive, as in both dashboard auth middlewares. + require.False(t, IsDashboardCredential("bearer "+dashboardJWT)) + }) + + t.Run("empty or missing header is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredential("")) + require.False(t, IsDashboardCredential("Bearer ")) + }) +} + +func TestIsDashboardCredentialFailsClosedWhenParserUnavailable(t *testing.T) { + setupDashboardCredentialTest(t) + dashboardCredentialJWTParser = nil + + token := mintDashboardCredentialJWT(t, time.Hour) + require.True(t, IsDashboardCredential("Bearer "+token)) +} + +func TestIsDashboardCredentialFailsClosedWhenAPITokenLookupUnavailable(t *testing.T) { + setupDashboardCredentialTest(t) + singleton.DB = nil + + require.True(t, IsDashboardCredential("Bearer "+model.APITokenPrefix+"unresolvable")) +} + +func TestIsDashboardCredentialFailsClosedOnDatabaseError(t *testing.T) { + setupDashboardCredentialTest(t) + sqlDB, err := singleton.DB.DB() + require.NoError(t, err) + require.NoError(t, sqlDB.Close()) + + var logs bytes.Buffer + originalOutput := log.Writer() + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(originalOutput) }) + + plaintext := model.APITokenPrefix + "unresolvable-after-db-failure" + require.True(t, IsDashboardCredential("Bearer "+plaintext)) + // The credential content itself must never reach the log. + require.NotContains(t, logs.String(), plaintext) +} diff --git a/cmd/dashboard/main.go b/cmd/dashboard/main.go index 69b905db..50d02590 100644 --- a/cmd/dashboard/main.go +++ b/cmd/dashboard/main.go @@ -169,6 +169,11 @@ func main() { grpcHandler := rpc.ServeRPC() httpHandler := controller.ServeWeb(frontendDist) controller.InitUpgrader() + // The NAT ingress strips dashboard-issued Authorization values (panel JWT / + // panel PAT) before tunneling a request to the agent and forwards foreign + // credentials untouched. ServeWeb built the JWT parser the classifier + // reuses, so the gate is wired right after it. + rpc.SetNATDashboardCredentialGate(controller.IsDashboardCredential) muxHandler := newHTTPandGRPCMux(httpHandler, grpcHandler) muxServerHTTP := &http.Server{ diff --git a/cmd/dashboard/rpc/nat.go b/cmd/dashboard/rpc/nat.go index d5b3acb6..5c519493 100644 --- a/cmd/dashboard/rpc/nat.go +++ b/cmd/dashboard/rpc/nat.go @@ -80,7 +80,14 @@ func ServeNAT(w http.ResponseWriter, r *http.Request, natConfig *model.NAT) { return } - // Authorization authenticates Dashboard access before NAT ingress; it must not become an origin credential for the configured NAT backend. + // Credential invariant at NAT ingress: credentials issued by this + // dashboard — the panel-signed JWT (verified by signature only, expiry + // deliberately ignored) and the panel API token — must never become origin + // credentials for the configured NAT backend. prepareNATCapability has + // already stripped every Authorization value IsDashboardCredential + // classified as dashboard-issued; whatever remains is either absent or a + // foreign credential, which is forwarded untouched because the NAT backend + // may authenticate with credentials of its own. wWrapped, err := utils.NewRequestWrapper(r, w) if err != nil { return diff --git a/cmd/dashboard/rpc/nat_capability_agentcompat.go b/cmd/dashboard/rpc/nat_capability_agentcompat.go index d346ae77..e2c3bc67 100644 --- a/cmd/dashboard/rpc/nat_capability_agentcompat.go +++ b/cmd/dashboard/rpc/nat_capability_agentcompat.go @@ -22,16 +22,24 @@ type natCapabilityLease struct { func prepareNATCapability(request *http.Request, natConfig *model.NAT) (natCapabilityLease, error) { values := request.Header.Values(agentcompatcontract.IOStreamCapabilityHeader) if len(values) == 0 { - request.Header.Del("Authorization") + // No capability header: legacy NAT path. Strip dashboard-issued + // Authorization values (panel JWT or panel PAT) so they never become + // origin credentials; foreign ones are ordinary request data. + stripDashboardCredential(request) return natCapabilityLease{}, nil } request.Header.Del(agentcompatcontract.IOStreamCapabilityHeader) if len(values) != 1 || values[0] == "" { - request.Header.Del("Authorization") + // Invalid capability: nothing is forwarded, but dashboard-issued + // credentials still must not survive into errors or task data. + stripDashboardCredential(request) return natCapabilityLease{}, errors.New("invalid NAT capability") } access, handle, err := serviceRPC.NezhaHandlerSingleton.ConsumeAgentCompatNATCapabilityForProfile(values[0], natConfig.ServerID, natConfig.ID) - request.Header.Del("Authorization") + // From here the request bytes may reach the agent (active lease) or the + // request ends in a 503; either way dashboard-issued Authorization values + // are stripped, foreign ones are forwarded untouched. + stripDashboardCredential(request) if err != nil { return natCapabilityLease{}, errors.New("invalid NAT capability") } diff --git a/cmd/dashboard/rpc/nat_capability_agentcompat_test.go b/cmd/dashboard/rpc/nat_capability_agentcompat_test.go index 94910c40..e1e369b4 100644 --- a/cmd/dashboard/rpc/nat_capability_agentcompat_test.go +++ b/cmd/dashboard/rpc/nat_capability_agentcompat_test.go @@ -4,6 +4,7 @@ package rpc import ( "bytes" + "context" "log" "net/http" "strings" @@ -59,7 +60,128 @@ func TestPrepareNATCapabilityRejectsDuplicateHeaderAfterRemovingAllValues(t *tes } } +func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentials(t *testing.T) { + // Given — legacy path without a capability header, then the invalid + // capability path: both must drop dashboard-issued Authorization values. + installNATCredentialTestGate(t) + for name, credential := range map[string]string{ + "dashboard jwt": natTestDashboardJWT, + "expired dashboard jwt": natTestDashboardExpiredJWT, + "dashboard api token": natTestDashboardAPIToken, + } { + t.Run(name+"/legacy path", func(t *testing.T) { + request := &http.Request{Header: make(http.Header)} + request.Header.Set("Authorization", credential) + + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + if err != nil { + t.Fatalf("legacy NAT path returned error: %v", err) + } + if lease.active { + t.Fatal("legacy NAT path unexpectedly activated") + } + if got := request.Header.Get("Authorization"); got != "" { + t.Fatalf("legacy NAT path retained dashboard credential %q", got) + } + }) + t.Run(name+"/invalid capability path", func(t *testing.T) { + request := &http.Request{Header: make(http.Header)} + request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, "malformed") + request.Header.Set("Authorization", credential) + + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + if err == nil { + t.Fatal("malformed capability unexpectedly accepted") + } + if lease.active { + t.Fatal("malformed capability unexpectedly activated") + } + if got := request.Header.Get("Authorization"); got != "" { + t.Fatalf("invalid capability path retained dashboard credential %q", got) + } + }) + } +} + +func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentialAfterConsumingCapability(t *testing.T) { + // Given — a well-formed capability that consumes successfully: the request + // is about to be tunneled, so the dashboard credential must be gone. + installNATCredentialTestGate(t) + handler := serviceRPC.NewNezhaHandler() + original := serviceRPC.NezhaHandlerSingleton + serviceRPC.NezhaHandlerSingleton = handler + t.Cleanup(func() { serviceRPC.NezhaHandlerSingleton = original }) + + capability, err := handler.RegisterAgentCompatIOStreamCapability(context.Background(), serviceRPC.AgentCompatCapabilityRegistration{ + Owner: serviceRPC.AgentCompatCapabilityOwner{PATID: 1, UserID: 2}, + Purpose: serviceRPC.AgentCompatCapabilityNAT, + TargetServerID: 81, + ResourceID: 91, + ServerAccessAllowed: true, + }) + if err != nil { + t.Fatalf("register capability: %v", err) + } + request := &http.Request{Header: make(http.Header)} + request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, capability.String()) + request.Header.Set("Authorization", natTestDashboardJWT) + + // When + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + // Then + if err != nil { + t.Fatalf("valid NAT capability rejected: %v", err) + } + if !lease.active { + t.Fatal("valid NAT capability did not activate") + } + if got := request.Header.Get("Authorization"); got != "" { + t.Fatalf("dashboard credential retained after capability consumption: %q", got) + } +} + +func TestPrepareNATCapabilityAgentCompatKeepsForeignAndMissingAuthorization(t *testing.T) { + // Given — legacy path: foreign credentials are ordinary request data and + // an absent header must stay untouched. + installNATCredentialTestGate(t) + for name, values := range map[string][]string{ + "foreign jwt": {natTestForeignJWT}, + "foreign random": {natTestForeignRandom}, + "empty value": {""}, + "missing header": nil, + } { + t.Run(name, func(t *testing.T) { + request := &http.Request{Header: make(http.Header)} + for _, value := range values { + request.Header.Set("Authorization", value) + } + + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + if err != nil { + t.Fatalf("legacy NAT path returned error: %v", err) + } + if lease.active { + t.Fatal("legacy NAT path unexpectedly activated") + } + got := request.Header.Values("Authorization") + if len(got) != len(values) { + t.Fatalf("legacy NAT path changed Authorization to %q, want %q", got, values) + } + for i := range values { + if got[i] != values[i] { + t.Fatalf("legacy NAT path changed Authorization to %q, want %q", got, values) + } + } + }) + } +} + func TestServeNATAgentCompatSensitiveHeadersStayOutOfErrorsAndLogs(t *testing.T) { + installNATCredentialTestGate(t) request := &http.Request{Header: make(http.Header)} request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, "capability-secret") request.Header.Set("Authorization", "Bearer deterministic-secret") diff --git a/cmd/dashboard/rpc/nat_capability_default.go b/cmd/dashboard/rpc/nat_capability_default.go index 8773477b..865c8f08 100644 --- a/cmd/dashboard/rpc/nat_capability_default.go +++ b/cmd/dashboard/rpc/nat_capability_default.go @@ -18,7 +18,11 @@ type natCapabilityLease struct { } func prepareNATCapability(request *http.Request, _ *model.NAT) (natCapabilityLease, error) { - request.Header.Del("Authorization") + // The default build has no capability protocol: every request takes the + // legacy NAT path, so any dashboard-issued Authorization value (panel JWT + // or panel PAT) is stripped before the request is tunneled to the agent. + // Foreign Authorization values are ordinary request data and stay put. + stripDashboardCredential(request) return natCapabilityLease{}, nil } diff --git a/cmd/dashboard/rpc/nat_capability_default_flow_test.go b/cmd/dashboard/rpc/nat_capability_default_flow_test.go index c3698afa..30c7d71b 100644 --- a/cmd/dashboard/rpc/nat_capability_default_flow_test.go +++ b/cmd/dashboard/rpc/nat_capability_default_flow_test.go @@ -19,6 +19,7 @@ import ( func TestServeNATDefaultForwardsCapabilityHeaderAsOrdinaryData(t *testing.T) { fixture := newServeNATFixture(t) + installNATCredentialTestGate(t) connection := newServeNATConn() agent := &serveNATAgent{readErr: io.EOF, writeDone: make(chan struct{})} request := &http.Request{ @@ -28,7 +29,7 @@ func TestServeNATDefaultForwardsCapabilityHeaderAsOrdinaryData(t *testing.T) { Body: &serveNATBody{}, } request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, "ordinary-data") - request.Header.Set("Authorization", "Bearer deterministic-secret") + request.Header.Set("Authorization", natTestForeignRandom) fixture.taskStream.onSend = func(task *proto.Task) error { var nat model.TaskNAT if err := json.Unmarshal([]byte(task.Data), &nat); err != nil { @@ -58,7 +59,50 @@ func TestServeNATDefaultForwardsCapabilityHeaderAsOrdinaryData(t *testing.T) { if !strings.Contains(forwarded, strings.ToLower(agentcompatcontract.IOStreamCapabilityHeader)+": ordinary-data") { t.Fatal("default NAT flow did not forward capability header as ordinary request data") } - if strings.Contains(forwarded, "Authorization:") || strings.Contains(forwarded, "deterministic-secret") { - t.Fatal("default NAT flow forwarded Authorization credentials") + if !strings.Contains(forwarded, strings.ToLower("Authorization: "+natTestForeignRandom)) { + t.Fatal("default NAT flow stripped a foreign credential that does not belong to the dashboard") + } +} + +func TestServeNATDefaultDoesNotForwardDashboardCredentials(t *testing.T) { + fixture := newServeNATFixture(t) + installNATCredentialTestGate(t) + connection := newServeNATConn() + agent := &serveNATAgent{readErr: io.EOF, writeDone: make(chan struct{})} + request := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Scheme: "http", Host: "example.test", Path: "/nat"}, + Header: make(http.Header), + Body: &serveNATBody{}, + } + request.Header.Set("Authorization", natTestDashboardJWT) + fixture.taskStream.onSend = func(task *proto.Task) error { + var nat model.TaskNAT + if err := json.Unmarshal([]byte(task.Data), &nat); err != nil { + return err + } + return fixture.handler.AgentConnected(nat.StreamID, agent) + } + done := make(chan struct{}) + deadline, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + go func() { + ServeNAT(&serveNATResponseWriter{conn: connection}, request, &model.NAT{ServerID: fixture.server.ID, Host: "target.example"}) + close(done) + }() + select { + case <-agent.writeDone: + case <-deadline.Done(): + t.Fatal("agent did not receive ordinary NAT request") + } + _ = connection.Close() + select { + case <-done: + case <-deadline.Done(): + t.Fatal("ServeNAT did not finish") + } + forwarded := strings.ToLower(string(agent.writtenBytes())) + if strings.Contains(forwarded, "authorization:") || strings.Contains(forwarded, "dashboard-signed-jwt") { + t.Fatal("default NAT flow forwarded dashboard credentials") } } diff --git a/cmd/dashboard/rpc/nat_capability_default_test.go b/cmd/dashboard/rpc/nat_capability_default_test.go index 7ff5e686..71154084 100644 --- a/cmd/dashboard/rpc/nat_capability_default_test.go +++ b/cmd/dashboard/rpc/nat_capability_default_test.go @@ -12,9 +12,10 @@ import ( func TestPrepareNATCapabilityDefaultPreservesHeaderAsOrdinaryRequestData(t *testing.T) { // Given + installNATCredentialTestGate(t) request := &http.Request{Header: make(http.Header)} request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, "ordinary-data") - request.Header.Set("Authorization", "Bearer deterministic-secret") + request.Header.Set("Authorization", natTestForeignRandom) // When lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) @@ -29,7 +30,74 @@ func TestPrepareNATCapabilityDefaultPreservesHeaderAsOrdinaryRequestData(t *test if got := request.Header.Get(agentcompatcontract.IOStreamCapabilityHeader); got != "ordinary-data" { t.Fatalf("default NAT capability hook changed header to %q", got) } - if got := request.Header.Get("Authorization"); got != "" { - t.Fatalf("default NAT capability hook retained Authorization %q", got) + if got := request.Header.Get("Authorization"); got != natTestForeignRandom { + t.Fatalf("default NAT capability hook changed foreign Authorization to %q", got) + } +} + +func TestPrepareNATCapabilityDefaultStripsDashboardCredentials(t *testing.T) { + // Given + installNATCredentialTestGate(t) + for name, credential := range map[string]string{ + "dashboard jwt": natTestDashboardJWT, + "expired dashboard jwt": natTestDashboardExpiredJWT, + "dashboard api token": natTestDashboardAPIToken, + } { + t.Run(name, func(t *testing.T) { + request := &http.Request{Header: make(http.Header)} + request.Header.Set("Authorization", credential) + + // When + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + // Then + if err != nil { + t.Fatalf("default NAT capability hook returned error: %v", err) + } + if lease.active { + t.Fatal("default NAT capability hook unexpectedly activated") + } + if got := request.Header.Get("Authorization"); got != "" { + t.Fatalf("default NAT capability hook retained dashboard credential %q", got) + } + }) + } +} + +func TestPrepareNATCapabilityDefaultKeepsForeignAndMissingAuthorization(t *testing.T) { + // Given + installNATCredentialTestGate(t) + for name, values := range map[string][]string{ + "foreign jwt": {natTestForeignJWT}, + "foreign random": {natTestForeignRandom}, + "empty value": {""}, + "missing header": nil, + } { + t.Run(name, func(t *testing.T) { + request := &http.Request{Header: make(http.Header)} + for _, value := range values { + request.Header.Set("Authorization", value) + } + + // When + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + // Then + if err != nil { + t.Fatalf("default NAT capability hook returned error: %v", err) + } + if lease.active { + t.Fatal("default NAT capability hook unexpectedly activated") + } + got := request.Header.Values("Authorization") + if len(got) != len(values) { + t.Fatalf("default NAT capability hook changed Authorization to %q, want %q", got, values) + } + for i := range values { + if got[i] != values[i] { + t.Fatalf("default NAT capability hook changed Authorization to %q, want %q", got, values) + } + } + }) } } diff --git a/cmd/dashboard/rpc/nat_capability_flow_agentcompat_test.go b/cmd/dashboard/rpc/nat_capability_flow_agentcompat_test.go index 2ed5a003..0bfe381a 100644 --- a/cmd/dashboard/rpc/nat_capability_flow_agentcompat_test.go +++ b/cmd/dashboard/rpc/nat_capability_flow_agentcompat_test.go @@ -23,6 +23,7 @@ import ( func TestServeNATAgentCompatPublishesExactStreamAfterRequestTransfer(t *testing.T) { fixture := newServeNATFixture(t) + installNATCredentialTestGate(t) capability, err := fixture.handler.RegisterAgentCompatIOStreamCapability(context.Background(), serviceRPC.AgentCompatCapabilityRegistration{ Owner: serviceRPC.AgentCompatCapabilityOwner{PATID: 1, UserID: 2}, Purpose: serviceRPC.AgentCompatCapabilityNAT, @@ -44,7 +45,7 @@ func TestServeNATAgentCompatPublishesExactStreamAfterRequestTransfer(t *testing. Body: io.NopCloser(strings.NewReader("payload")), } request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, capability.String()) - request.Header.Set("Authorization", "Bearer deterministic-secret") + request.Header.Set("Authorization", natTestForeignJWT) request.Header.Set("X-Ordinary-NAT", "ordinary") fixture.taskStream.onSend = func(task *proto.Task) error { var nat model.TaskNAT @@ -106,8 +107,62 @@ func TestServeNATAgentCompatPublishesExactStreamAfterRequestTransfer(t *testing. require.Contains(t, forwarded, expected) } require.NotContains(t, forwarded, agentcompatcontract.IOStreamCapabilityHeader) - require.NotContains(t, forwarded, "Authorization:") - require.NotContains(t, forwarded, "deterministic-secret") + // Foreign credentials are ordinary request data for the NAT backend and + // must survive the transfer; dashboard-issued values are stripped before + // any byte leaves the dashboard. + require.Contains(t, forwarded, "Authorization: "+natTestForeignJWT) +} + +func TestServeNATAgentCompatDoesNotForwardDashboardCredentials(t *testing.T) { + fixture := newServeNATFixture(t) + installNATCredentialTestGate(t) + capability, err := fixture.handler.RegisterAgentCompatIOStreamCapability(context.Background(), serviceRPC.AgentCompatCapabilityRegistration{ + Owner: serviceRPC.AgentCompatCapabilityOwner{PATID: 1, UserID: 2}, + Purpose: serviceRPC.AgentCompatCapabilityNAT, + TargetServerID: fixture.server.ID, + ResourceID: 91, + ServerAccessAllowed: true, + }) + require.NoError(t, err) + connection := newServeNATConn() + writer := &serveNATResponseWriter{conn: connection} + agent := &serveNATAgent{readErr: io.EOF, writeDone: make(chan struct{})} + request := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Scheme: "http", Host: "example.test", Path: "/nat"}, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("payload")), + } + request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, capability.String()) + request.Header.Set("Authorization", natTestDashboardJWT) + fixture.taskStream.onSend = func(task *proto.Task) error { + var nat model.TaskNAT + if err := json.Unmarshal([]byte(task.Data), &nat); err != nil { + return err + } + return fixture.handler.AgentConnected(nat.StreamID, agent) + } + done := make(chan struct{}) + go func() { + ServeNAT(writer, request, &model.NAT{Common: model.Common{ID: 91}, ServerID: fixture.server.ID, Host: "target.example"}) + close(done) + }() + select { + case <-agent.writeDone: + case <-time.After(time.Second): + t.Fatal("agent did not receive the transferred NAT request") + } + require.NoError(t, connection.Close()) + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("ServeNAT did not complete") + } + forwarded := strings.ToLower(string(agent.writtenBytes())) + require.NotContains(t, forwarded, "authorization:") + for _, dashboardCredential := range []string{natTestDashboardJWT, natTestDashboardExpiredJWT, natTestDashboardAPIToken} { + require.NotContains(t, forwarded, strings.ToLower(dashboardCredential)) + } } type orderedNATAgent struct { diff --git a/cmd/dashboard/rpc/nat_credential.go b/cmd/dashboard/rpc/nat_credential.go new file mode 100644 index 00000000..eb6470a8 --- /dev/null +++ b/cmd/dashboard/rpc/nat_credential.go @@ -0,0 +1,41 @@ +package rpc + +import "net/http" + +// natDashboardCredentialGate classifies an Authorization header value as a +// credential issued by this dashboard (panel-signed JWT or panel API token). +// main() injects controller.IsDashboardCredential; the setter keeps the rpc +// package free of a dependency on the controller package, the same pattern as +// SetReceiptGateListener and SetMCPKillSwitchObserver. +var natDashboardCredentialGate func(authz string) bool + +// SetNATDashboardCredentialGate wires the dashboard-credential classifier used +// by the NAT ingress. Passing nil restores the fail-closed default in +// stripDashboardCredential. +func SetNATDashboardCredentialGate(fn func(authz string) bool) { + natDashboardCredentialGate = fn +} + +// stripDashboardCredential removes the Authorization header when it carries a +// credential issued by this dashboard (panel-signed JWT or panel API token). +// Such credentials authenticate the dashboard itself and must never become +// origin credentials for the NAT backend. Foreign credentials are forwarded +// untouched: the NAT backend may authenticate with credentials of its own, and +// this dashboard must not make policy for them. +func stripDashboardCredential(request *http.Request) { + if natDashboardCredentialGate == nil { + // Gate not wired (early startup or tests): fail closed and keep the + // historical behavior of stripping every Authorization header. + request.Header.Del("Authorization") + return + } + // A multi-valued Authorization header is malformed traffic, but any + // dashboard-issued value in it must still not survive into the tunnel, so + // the whole header is dropped when one is recognized. + for _, value := range request.Header.Values("Authorization") { + if natDashboardCredentialGate(value) { + request.Header.Del("Authorization") + return + } + } +} diff --git a/cmd/dashboard/rpc/nat_credential_test.go b/cmd/dashboard/rpc/nat_credential_test.go new file mode 100644 index 00000000..0d5690d1 --- /dev/null +++ b/cmd/dashboard/rpc/nat_credential_test.go @@ -0,0 +1,37 @@ +package rpc + +import ( + "testing" + + "github.com/nezhahq/nezha/model" +) + +// Values standing in for the credentials this dashboard issues. The fake gate +// below mirrors the contract of controller.IsDashboardCredential; the real +// classification semantics — panel JWTs accepted regardless of expiry, +// foreign-signed JWTs and random strings rejected — are pinned by the +// controller package tests (dashboard_credential_test.go). +const ( + natTestDashboardJWT = "Bearer dashboard-signed-jwt" + natTestDashboardExpiredJWT = "Bearer dashboard-signed-expired-jwt" + natTestDashboardAPIToken = "Bearer " + model.APITokenPrefix + "dashboard-issued-pat" + natTestForeignJWT = "Bearer foreign-signed-jwt" + natTestForeignRandom = "Bearer foreign-random-string" +) + +// installNATCredentialTestGate wires a fake dashboard-credential classifier +// for the duration of the test: exactly the dashboard-issued test values above +// are classified as dashboard credentials, everything else is foreign. +func installNATCredentialTestGate(t *testing.T) { + t.Helper() + original := natDashboardCredentialGate + natDashboardCredentialGate = func(authz string) bool { + switch authz { + case natTestDashboardJWT, natTestDashboardExpiredJWT, natTestDashboardAPIToken: + return true + default: + return false + } + } + t.Cleanup(func() { natDashboardCredentialGate = original }) +} From 3ad985f36ef6dc31fa2e8a33965b02a016cf14db Mon Sep 17 00:00:00 2001 From: xvkong233 <88175491+xvkong233@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:02:15 +0800 Subject: [PATCH 2/2] fix(nat): scrub dashboard-issued credentials from all accepted token channels The initial selective strip covered only the Authorization header, but the dashboard also accepts its JWT via the ?token= query parameter and the nz-jwt cookie (jwt.go TokenLookup channels). A dashboard credential arriving through those channels would still reach a NAT backend. Apply the same classify-then-strip rule to all three channels at NAT ingress: values verified as dashboard-issued are removed, foreign values are preserved byte-for-byte. Adds a ServeNAT flow test exercising the live classifier and a wiring test pinning the gate injection in main(). --- .../controller/dashboard_credential.go | 30 ++- .../dashboard_credential_nat_flow_test.go | 250 ++++++++++++++++++ .../controller/dashboard_credential_test.go | 46 ++++ .../dashboard_credential_wiring_test.go | 27 ++ cmd/dashboard/main.go | 18 +- cmd/dashboard/rpc/nat.go | 9 +- .../rpc/nat_capability_agentcompat.go | 16 +- .../rpc/nat_capability_agentcompat_test.go | 90 ++++++- cmd/dashboard/rpc/nat_capability_default.go | 9 +- .../rpc/nat_capability_default_test.go | 58 +++- cmd/dashboard/rpc/nat_credential.go | 189 +++++++++++-- cmd/dashboard/rpc/nat_credential_test.go | 146 ++++++++-- 12 files changed, 806 insertions(+), 82 deletions(-) create mode 100644 cmd/dashboard/controller/dashboard_credential_nat_flow_test.go create mode 100644 cmd/dashboard/dashboard_credential_wiring_test.go diff --git a/cmd/dashboard/controller/dashboard_credential.go b/cmd/dashboard/controller/dashboard_credential.go index e24ecdd7..00af53ea 100644 --- a/cmd/dashboard/controller/dashboard_credential.go +++ b/cmd/dashboard/controller/dashboard_credential.go @@ -43,19 +43,33 @@ func newDashboardCredentialJWTParser(authMiddleware *ginjwt.GinJWTMiddleware) *g // NAT backend may authenticate with credentials of its own. // // Decision policy mirrors the dashboard's own auth middlewares: only the -// case-sensitive "Bearer " scheme is considered; PAT-shaped values are -// resolved against the api_tokens table; everything else is treated as a JWT. -// Whenever the classification cannot be completed (parser or config not ready, -// database failure) the value is conservatively reported as a dashboard -// credential so the caller strips it; only a deterministic proof that the -// value was not issued by this dashboard (scheme mismatch, unknown PAT hash, -// failed signature verification) returns false. +// case-sensitive "Bearer " scheme is considered; the value after the scheme is +// classified by IsDashboardCredentialValue, which is also the entry point for +// the panel's scheme-less TokenLookup channels. Only a deterministic proof +// that the value was not issued by this dashboard (scheme mismatch, unknown +// PAT hash, failed signature verification) returns false. func IsDashboardCredential(authz string) bool { raw := strings.TrimSpace(authz) if !strings.HasPrefix(raw, "Bearer ") { return false } - plaintext := strings.TrimSpace(strings.TrimPrefix(raw, "Bearer ")) + return IsDashboardCredentialValue(strings.TrimPrefix(raw, "Bearer ")) +} + +// IsDashboardCredentialValue reports whether a raw credential value — the part +// after the "Bearer " scheme, exactly as the panel's other TokenLookup +// channels carry it (the ?token= query parameter and the nz-jwt cookie, see +// initParams) — was issued by this dashboard: a panel-signed JWT or a panel +// API token (PAT). +// +// Failure policy is identical to IsDashboardCredential: whenever the +// classification cannot be completed (parser or config not ready, database +// failure) the value is conservatively reported as a dashboard credential so +// the caller strips it; only a deterministic proof that the value was not +// issued by this dashboard (unknown PAT hash, failed signature verification) +// returns false. +func IsDashboardCredentialValue(value string) bool { + plaintext := strings.TrimSpace(value) if strings.HasPrefix(plaintext, model.APITokenPrefix) { return isDashboardAPIToken(plaintext) } diff --git a/cmd/dashboard/controller/dashboard_credential_nat_flow_test.go b/cmd/dashboard/controller/dashboard_credential_nat_flow_test.go new file mode 100644 index 00000000..26a8f6ff --- /dev/null +++ b/cmd/dashboard/controller/dashboard_credential_nat_flow_test.go @@ -0,0 +1,250 @@ +package controller + +import ( + "bufio" + "bytes" + "context" + "io" + "net" + "net/http" + "net/url" + "sync" + "testing" + "time" + + "github.com/goccy/go-json" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + + "github.com/nezhahq/nezha/cmd/dashboard/rpc" + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/proto" + rpcService "github.com/nezhahq/nezha/service/rpc" + "github.com/nezhahq/nezha/service/singleton" +) + +// setupNATFlowTest extends setupDashboardCredentialTest with the minimal +// ServeNAT environment: a registered server with a task stream and a fresh +// handler singleton, mirroring the rpc package's serveNAT fixture. +func setupNATFlowTest(t *testing.T) (*rpcService.NezhaHandler, *model.Server, *natFlowTaskStream) { + t.Helper() + setupDashboardCredentialTest(t) + + require.NoError(t, singleton.DB.AutoMigrate(&model.Server{})) + originalServerShared, originalHandler := singleton.ServerShared, rpcService.NezhaHandlerSingleton + t.Cleanup(func() { + singleton.ServerShared, rpcService.NezhaHandlerSingleton = originalServerShared, originalHandler + }) + + serverRow := &model.Server{Common: model.Common{ID: 7}, UUID: "nat-credential-flow-test", Name: "nat-credential-flow-test"} + require.NoError(t, singleton.DB.Create(serverRow).Error) + singleton.ServerShared = singleton.NewServerClass() + handler := rpcService.NewNezhaHandler() + rpcService.NezhaHandlerSingleton = handler + + server, ok := singleton.ServerShared.Get(serverRow.ID) + require.True(t, ok) + taskStream := &natFlowTaskStream{} + server.SetTaskStream(taskStream) + + // Wire the gate exactly as cmd/dashboard/main.go does, with the real + // classifiers built by setupDashboardCredentialTest. + rpc.SetNATDashboardCredentialGate(IsDashboardCredential, IsDashboardCredentialValue) + t.Cleanup(func() { rpc.SetNATDashboardCredentialGate(nil, nil) }) + + return handler, server, taskStream +} + +// TestServeNATWithRealClassifierStripsDashboardCredentialsFromAllChannels pins +// the main() wiring contract end to end: with the real classifiers wired as +// main() does, ServeNAT forwards a request whose dashboard-issued credentials +// — Authorization header, ?token= query parameter, nz-jwt cookie — are all +// stripped, while foreign values and untouched query/cookie data survive +// byte-for-byte. +func TestServeNATWithRealClassifierStripsDashboardCredentialsFromAllChannels(t *testing.T) { + handler, server, taskStream := setupNATFlowTest(t) + + dashboardJWT := mintDashboardCredentialJWT(t, time.Hour) + foreignJWT := mintForeignJWT(t) + // No api_tokens row hashes to this PAT, so the classifier proves it was + // not issued by this dashboard and it must survive the tunnel. + foreignPAT := model.APITokenPrefix + "foreign-issued-pat" + dashboardPAT := model.APITokenPrefix + "dashboard-issued-pat" + require.NoError(t, singleton.DB.Create(&model.APIToken{ + UserID: 1, + Name: "nat-flow", + TokenHash: model.HashAPIToken(dashboardPAT), + }).Error) + + connection := &natFlowConn{closed: make(chan struct{})} + agent := &natFlowAgent{writeDone: make(chan struct{})} + writer := &natFlowWriter{conn: connection} + request := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + dashboardJWT + "&token=" + url.QueryEscape(foreignJWT) + + "&token=" + foreignPAT + "&token=" + dashboardPAT}, + Header: make(http.Header), + Body: http.NoBody, + } + request.Header.Set("Authorization", "Bearer "+dashboardJWT) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+dashboardJWT+"; nz-jwt="+foreignJWT+"; nz-jwt="+foreignPAT) + request.Header.Set("X-Ordinary-Nat", "ordinary") + taskStream.onSend = func(task *proto.Task) error { + var nat model.TaskNAT + if err := json.Unmarshal([]byte(task.Data), &nat); err != nil { + return err + } + return handler.AgentConnected(nat.StreamID, agent) + } + + done := make(chan struct{}) + go func() { + rpc.ServeNAT(writer, request, &model.NAT{ServerID: server.ID, Host: "target.example"}) + close(done) + }() + select { + case <-agent.writeDone: + case <-time.After(time.Second): + t.Fatal("agent did not receive the transferred NAT request") + } + require.NoError(t, connection.Close()) + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("ServeNAT did not finish") + } + + forwarded := string(agent.writtenBytes()) + // Dashboard credentials are gone from every channel. + require.NotContains(t, forwarded, "Authorization:") + require.NotContains(t, forwarded, dashboardJWT) + require.NotContains(t, forwarded, dashboardPAT) + // Foreign credentials and untouched data survive byte-for-byte. + require.Contains(t, forwarded, "POST /nat?keep=1&token="+url.QueryEscape(foreignJWT)+"&token="+foreignPAT+" HTTP/1.1") + require.Contains(t, forwarded, "Cookie: sid=abc; nz-jwt="+foreignJWT+"; nz-jwt="+foreignPAT) + require.Contains(t, forwarded, "X-Ordinary-Nat: ordinary") +} + +// TestServeNATWithRealClassifierKeepsForeignCredentials pins that the real +// classifier deterministically proves foreign values were not issued by this +// dashboard and leaves every channel untouched. +func TestServeNATWithRealClassifierKeepsForeignCredentials(t *testing.T) { + handler, server, taskStream := setupNATFlowTest(t) + + foreignJWT := mintForeignJWT(t) + connection := &natFlowConn{closed: make(chan struct{})} + agent := &natFlowAgent{writeDone: make(chan struct{})} + writer := &natFlowWriter{conn: connection} + request := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Path: "/nat", RawQuery: "token=" + foreignJWT}, + Header: make(http.Header), + Body: http.NoBody, + } + request.Header.Set("Authorization", "Bearer "+foreignJWT) + request.Header.Set("Cookie", "nz-jwt="+foreignJWT) + taskStream.onSend = func(task *proto.Task) error { + var nat model.TaskNAT + if err := json.Unmarshal([]byte(task.Data), &nat); err != nil { + return err + } + return handler.AgentConnected(nat.StreamID, agent) + } + + done := make(chan struct{}) + go func() { + rpc.ServeNAT(writer, request, &model.NAT{ServerID: server.ID, Host: "target.example"}) + close(done) + }() + select { + case <-agent.writeDone: + case <-time.After(time.Second): + t.Fatal("agent did not receive the transferred NAT request") + } + require.NoError(t, connection.Close()) + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("ServeNAT did not finish") + } + + forwarded := string(agent.writtenBytes()) + require.Contains(t, forwarded, "POST /nat?token="+foreignJWT+" HTTP/1.1") + require.Contains(t, forwarded, "Authorization: Bearer "+foreignJWT) + require.Contains(t, forwarded, "Cookie: nz-jwt="+foreignJWT) +} + +type natFlowTaskStream struct { + onSend func(*proto.Task) error +} + +func (stream *natFlowTaskStream) Send(task *proto.Task) error { + if stream.onSend != nil { + return stream.onSend(task) + } + return nil +} +func (*natFlowTaskStream) Recv() (*proto.TaskResult, error) { return nil, io.EOF } +func (*natFlowTaskStream) SetHeader(metadata.MD) error { return nil } +func (*natFlowTaskStream) SendHeader(metadata.MD) error { return nil } +func (*natFlowTaskStream) SetTrailer(metadata.MD) {} +func (*natFlowTaskStream) Context() context.Context { return context.Background() } +func (*natFlowTaskStream) SendMsg(any) error { return nil } +func (*natFlowTaskStream) RecvMsg(any) error { return io.EOF } + +type natFlowWriter struct{ conn *natFlowConn } + +func (writer *natFlowWriter) Header() http.Header { return http.Header{} } +func (writer *natFlowWriter) Write(data []byte) (int, error) { return len(data), nil } +func (writer *natFlowWriter) WriteHeader(int) {} +func (writer *natFlowWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + go func() { _, _ = writer.conn.Read(make([]byte, 1)) }() + return writer.conn, bufio.NewReadWriter(bufio.NewReader(writer.conn), bufio.NewWriter(writer.conn)), nil +} + +type natFlowConn struct { + closed chan struct{} + closeOnce sync.Once +} + +func (conn *natFlowConn) Read([]byte) (int, error) { + <-conn.closed + return 0, io.EOF +} +func (conn *natFlowConn) Write(data []byte) (int, error) { return len(data), nil } +func (conn *natFlowConn) Close() error { + conn.closeOnce.Do(func() { close(conn.closed) }) + return nil +} +func (conn *natFlowConn) LocalAddr() net.Addr { return natFlowAddr{} } +func (conn *natFlowConn) RemoteAddr() net.Addr { return natFlowAddr{} } +func (conn *natFlowConn) SetDeadline(time.Time) error { return nil } +func (conn *natFlowConn) SetReadDeadline(time.Time) error { return nil } +func (conn *natFlowConn) SetWriteDeadline(time.Time) error { return nil } + +type natFlowAddr struct{} + +func (natFlowAddr) Network() string { return "test" } +func (natFlowAddr) String() string { return "test" } + +type natFlowAgent struct { + mu sync.Mutex + written bytes.Buffer + writeDone chan struct{} + once sync.Once +} + +func (agent *natFlowAgent) Read([]byte) (int, error) { return 0, io.EOF } +func (agent *natFlowAgent) Write(data []byte) (int, error) { + agent.mu.Lock() + agent.written.Write(data) + agent.mu.Unlock() + agent.once.Do(func() { close(agent.writeDone) }) + return len(data), nil +} +func (*natFlowAgent) Close() error { return nil } +func (agent *natFlowAgent) writtenBytes() []byte { + agent.mu.Lock() + defer agent.mu.Unlock() + return append([]byte(nil), agent.written.Bytes()...) +} diff --git a/cmd/dashboard/controller/dashboard_credential_test.go b/cmd/dashboard/controller/dashboard_credential_test.go index 9ad1070e..fbfe47ec 100644 --- a/cmd/dashboard/controller/dashboard_credential_test.go +++ b/cmd/dashboard/controller/dashboard_credential_test.go @@ -117,6 +117,52 @@ func TestIsDashboardCredentialClassification(t *testing.T) { }) } +// TestIsDashboardCredentialValueClassification pins the scheme-less classifier +// that backs IsDashboardCredential and drives the ?token= query parameter and +// nz-jwt cookie channels at NAT ingress (see IsDashboardCredentialValue). +func TestIsDashboardCredentialValueClassification(t *testing.T) { + setupDashboardCredentialTest(t) + + dashboardJWT := mintDashboardCredentialJWT(t, time.Hour) + expiredDashboardJWT := mintDashboardCredentialJWT(t, -time.Hour) + foreignJWT := mintForeignJWT(t) + + dashboardPAT := model.APITokenPrefix + "dashboard-credential-value-pat" + require.NoError(t, singleton.DB.Create(&model.APIToken{ + UserID: 1, + Name: "nat-value-classification", + TokenHash: model.HashAPIToken(dashboardPAT), + }).Error) + + t.Run("dashboard signed jwt is a dashboard credential", func(t *testing.T) { + require.True(t, IsDashboardCredentialValue(dashboardJWT)) + }) + t.Run("expired dashboard signed jwt is still a dashboard credential", func(t *testing.T) { + require.True(t, IsDashboardCredentialValue(expiredDashboardJWT)) + }) + t.Run("dashboard api token is a dashboard credential", func(t *testing.T) { + require.True(t, IsDashboardCredentialValue(dashboardPAT)) + }) + t.Run("foreign signed jwt is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredentialValue(foreignJWT)) + }) + t.Run("unknown api token is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredentialValue(model.APITokenPrefix+"unknown-to-the-dashboard")) + }) + t.Run("random string is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredentialValue("random-not-a-token")) + }) + t.Run("empty value is not a dashboard credential", func(t *testing.T) { + require.False(t, IsDashboardCredentialValue("")) + }) + t.Run("bearer scheme is not part of the value contract", func(t *testing.T) { + // The query and cookie channels carry the bare value; a value that + // itself starts with "Bearer " is not something the panel issues and + // fails signature verification. + require.False(t, IsDashboardCredentialValue("Bearer "+dashboardJWT)) + }) +} + func TestIsDashboardCredentialFailsClosedWhenParserUnavailable(t *testing.T) { setupDashboardCredentialTest(t) dashboardCredentialJWTParser = nil diff --git a/cmd/dashboard/dashboard_credential_wiring_test.go b/cmd/dashboard/dashboard_credential_wiring_test.go new file mode 100644 index 00000000..95be89ff --- /dev/null +++ b/cmd/dashboard/dashboard_credential_wiring_test.go @@ -0,0 +1,27 @@ +package main + +import ( + "testing" + + "github.com/nezhahq/nezha/cmd/dashboard/rpc" +) + +// TestWireNATDashboardCredentialGate pins the NAT ingress wiring contract: +// main() must connect the real dashboard-credential classifiers +// (controller.IsDashboardCredential / controller.IsDashboardCredentialValue) +// through wireNATDashboardCredentialGate before any listener serves traffic. +// That the wired functions really classify per channel is pinned end to end +// by the controller package's real-classifier NAT flow test. +func TestWireNATDashboardCredentialGate(t *testing.T) { + if rpc.NATDashboardCredentialGateWired() { + t.Fatal("NAT dashboard credential gate already wired before the test") + } + + wireNATDashboardCredentialGate() + + if !rpc.NATDashboardCredentialGateWired() { + t.Fatal("wireNATDashboardCredentialGate left the NAT dashboard credential gate unwired") + } + // Leave the process in the wired state main() establishes. + t.Cleanup(wireNATDashboardCredentialGate) +} diff --git a/cmd/dashboard/main.go b/cmd/dashboard/main.go index 50d02590..4b64c441 100644 --- a/cmd/dashboard/main.go +++ b/cmd/dashboard/main.go @@ -169,11 +169,7 @@ func main() { grpcHandler := rpc.ServeRPC() httpHandler := controller.ServeWeb(frontendDist) controller.InitUpgrader() - // The NAT ingress strips dashboard-issued Authorization values (panel JWT / - // panel PAT) before tunneling a request to the agent and forwards foreign - // credentials untouched. ServeWeb built the JWT parser the classifier - // reuses, so the gate is wired right after it. - rpc.SetNATDashboardCredentialGate(controller.IsDashboardCredential) + wireNATDashboardCredentialGate() muxHandler := newHTTPandGRPCMux(httpHandler, grpcHandler) muxServerHTTP := &http.Server{ @@ -233,6 +229,18 @@ func main() { close(errChan) } +// wireNATDashboardCredentialGate connects the NAT ingress to the real +// dashboard-credential classifiers. The NAT ingress strips dashboard-issued +// credentials (panel JWT / panel PAT) from every channel the panel's own +// TokenLookup accepts — the Authorization header, the ?token= query parameter +// and the nz-jwt cookie — before tunneling a request to the agent, and +// forwards foreign values untouched. ServeWeb built the JWT parser the +// classifiers reuse, so the gate is wired right after it and before any +// listener starts serving. +func wireNATDashboardCredentialGate() { + rpc.SetNATDashboardCredentialGate(controller.IsDashboardCredential, controller.IsDashboardCredentialValue) +} + func newHTTPandGRPCMux(httpHandler http.Handler, grpcHandler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { natConfig := singleton.NATShared.GetNATConfigByDomain(r.Host) diff --git a/cmd/dashboard/rpc/nat.go b/cmd/dashboard/rpc/nat.go index 5c519493..7b8be72b 100644 --- a/cmd/dashboard/rpc/nat.go +++ b/cmd/dashboard/rpc/nat.go @@ -84,10 +84,11 @@ func ServeNAT(w http.ResponseWriter, r *http.Request, natConfig *model.NAT) { // dashboard — the panel-signed JWT (verified by signature only, expiry // deliberately ignored) and the panel API token — must never become origin // credentials for the configured NAT backend. prepareNATCapability has - // already stripped every Authorization value IsDashboardCredential - // classified as dashboard-issued; whatever remains is either absent or a - // foreign credential, which is forwarded untouched because the NAT backend - // may authenticate with credentials of its own. + // already stripped them from every channel the panel's own TokenLookup + // accepts (Authorization header, ?token= query parameter, nz-jwt cookie); + // whatever remains on those channels is either absent or a foreign + // credential, which is forwarded untouched because the NAT backend may + // authenticate with credentials of its own. wWrapped, err := utils.NewRequestWrapper(r, w) if err != nil { return diff --git a/cmd/dashboard/rpc/nat_capability_agentcompat.go b/cmd/dashboard/rpc/nat_capability_agentcompat.go index e2c3bc67..16fdf503 100644 --- a/cmd/dashboard/rpc/nat_capability_agentcompat.go +++ b/cmd/dashboard/rpc/nat_capability_agentcompat.go @@ -23,23 +23,25 @@ func prepareNATCapability(request *http.Request, natConfig *model.NAT) (natCapab values := request.Header.Values(agentcompatcontract.IOStreamCapabilityHeader) if len(values) == 0 { // No capability header: legacy NAT path. Strip dashboard-issued - // Authorization values (panel JWT or panel PAT) so they never become - // origin credentials; foreign ones are ordinary request data. - stripDashboardCredential(request) + // credentials (panel JWT or panel PAT) from the Authorization header, + // the ?token= query parameter and the nz-jwt cookie so they never + // become origin credentials; foreign values are ordinary request data. + stripDashboardCredentials(request) return natCapabilityLease{}, nil } request.Header.Del(agentcompatcontract.IOStreamCapabilityHeader) if len(values) != 1 || values[0] == "" { // Invalid capability: nothing is forwarded, but dashboard-issued // credentials still must not survive into errors or task data. - stripDashboardCredential(request) + stripDashboardCredentials(request) return natCapabilityLease{}, errors.New("invalid NAT capability") } access, handle, err := serviceRPC.NezhaHandlerSingleton.ConsumeAgentCompatNATCapabilityForProfile(values[0], natConfig.ServerID, natConfig.ID) // From here the request bytes may reach the agent (active lease) or the - // request ends in a 503; either way dashboard-issued Authorization values - // are stripped, foreign ones are forwarded untouched. - stripDashboardCredential(request) + // request ends in a 503; either way dashboard-issued credentials are + // stripped from every TokenLookup channel, foreign ones are forwarded + // untouched. + stripDashboardCredentials(request) if err != nil { return natCapabilityLease{}, errors.New("invalid NAT capability") } diff --git a/cmd/dashboard/rpc/nat_capability_agentcompat_test.go b/cmd/dashboard/rpc/nat_capability_agentcompat_test.go index e1e369b4..2bfae4a8 100644 --- a/cmd/dashboard/rpc/nat_capability_agentcompat_test.go +++ b/cmd/dashboard/rpc/nat_capability_agentcompat_test.go @@ -7,6 +7,7 @@ import ( "context" "log" "net/http" + "net/url" "strings" "testing" @@ -62,16 +63,21 @@ func TestPrepareNATCapabilityRejectsDuplicateHeaderAfterRemovingAllValues(t *tes func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentials(t *testing.T) { // Given — legacy path without a capability header, then the invalid - // capability path: both must drop dashboard-issued Authorization values. + // capability path: both must drop dashboard-issued credentials from the + // Authorization header, the ?token= query parameter and the nz-jwt cookie. installNATCredentialTestGate(t) for name, credential := range map[string]string{ - "dashboard jwt": natTestDashboardJWT, - "expired dashboard jwt": natTestDashboardExpiredJWT, - "dashboard api token": natTestDashboardAPIToken, + "dashboard jwt": natTestDashboardJWTValue, + "expired dashboard jwt": natTestDashboardExpiredJWTValue, + "dashboard api token": natTestDashboardPATValue, } { t.Run(name+"/legacy path", func(t *testing.T) { - request := &http.Request{Header: make(http.Header)} - request.Header.Set("Authorization", credential) + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + credential}, + } + request.Header.Set("Authorization", "Bearer "+credential) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+credential) lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) @@ -82,13 +88,23 @@ func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentials(t *testing.T) t.Fatal("legacy NAT path unexpectedly activated") } if got := request.Header.Get("Authorization"); got != "" { - t.Fatalf("legacy NAT path retained dashboard credential %q", got) + t.Fatalf("legacy NAT path retained dashboard credential in Authorization %q", got) + } + if got := request.URL.RawQuery; got != "keep=1" { + t.Fatalf("legacy NAT path retained dashboard credential in query %q", got) + } + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("legacy NAT path retained dashboard credential in cookie %q", got) } }) t.Run(name+"/invalid capability path", func(t *testing.T) { - request := &http.Request{Header: make(http.Header)} + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + credential}, + } request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, "malformed") - request.Header.Set("Authorization", credential) + request.Header.Set("Authorization", "Bearer "+credential) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+credential) lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) @@ -99,7 +115,13 @@ func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentials(t *testing.T) t.Fatal("malformed capability unexpectedly activated") } if got := request.Header.Get("Authorization"); got != "" { - t.Fatalf("invalid capability path retained dashboard credential %q", got) + t.Fatalf("invalid capability path retained dashboard credential in Authorization %q", got) + } + if got := request.URL.RawQuery; got != "keep=1" { + t.Fatalf("invalid capability path retained dashboard credential in query %q", got) + } + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("invalid capability path retained dashboard credential in cookie %q", got) } }) } @@ -124,9 +146,13 @@ func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentialAfterConsumingC if err != nil { t.Fatalf("register capability: %v", err) } - request := &http.Request{Header: make(http.Header)} + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + natTestDashboardJWTValue}, + } request.Header.Set(agentcompatcontract.IOStreamCapabilityHeader, capability.String()) request.Header.Set("Authorization", natTestDashboardJWT) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+natTestDashboardJWTValue) // When lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) @@ -139,7 +165,13 @@ func TestPrepareNATCapabilityAgentCompatStripsDashboardCredentialAfterConsumingC t.Fatal("valid NAT capability did not activate") } if got := request.Header.Get("Authorization"); got != "" { - t.Fatalf("dashboard credential retained after capability consumption: %q", got) + t.Fatalf("dashboard credential retained in Authorization after capability consumption: %q", got) + } + if got := request.URL.RawQuery; got != "keep=1" { + t.Fatalf("dashboard credential retained in query after capability consumption: %q", got) + } + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("dashboard credential retained in cookie after capability consumption: %q", got) } } @@ -180,6 +212,40 @@ func TestPrepareNATCapabilityAgentCompatKeepsForeignAndMissingAuthorization(t *t } } +func TestPrepareNATCapabilityAgentCompatKeepsForeignAndMissingQueryAndCookie(t *testing.T) { + // Given — legacy path: foreign query tokens and cookies are ordinary + // request data, and absent channels must stay untouched. + installNATCredentialTestGate(t) + for name, tc := range map[string]struct{ rawQuery, cookie string }{ + "foreign values": {"keep=1&token=" + natTestForeignJWTValue + "&a=%2Fx", "sid=abc; nz-jwt=" + natTestForeignJWTValue}, + "empty values": {"keep=1&token=", "nz-jwt="}, + "missing channel": {"keep=1", "sid=abc"}, + } { + t.Run(name, func(t *testing.T) { + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: tc.rawQuery}, + } + request.Header.Set("Cookie", tc.cookie) + + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + if err != nil { + t.Fatalf("legacy NAT path returned error: %v", err) + } + if lease.active { + t.Fatal("legacy NAT path unexpectedly activated") + } + if got := request.URL.RawQuery; got != tc.rawQuery { + t.Fatalf("legacy NAT path changed query to %q, want %q", got, tc.rawQuery) + } + if got := request.Header.Get("Cookie"); got != tc.cookie { + t.Fatalf("legacy NAT path changed cookie to %q, want %q", got, tc.cookie) + } + }) + } +} + func TestServeNATAgentCompatSensitiveHeadersStayOutOfErrorsAndLogs(t *testing.T) { installNATCredentialTestGate(t) request := &http.Request{Header: make(http.Header)} diff --git a/cmd/dashboard/rpc/nat_capability_default.go b/cmd/dashboard/rpc/nat_capability_default.go index 865c8f08..b0480ce6 100644 --- a/cmd/dashboard/rpc/nat_capability_default.go +++ b/cmd/dashboard/rpc/nat_capability_default.go @@ -19,10 +19,11 @@ type natCapabilityLease struct { func prepareNATCapability(request *http.Request, _ *model.NAT) (natCapabilityLease, error) { // The default build has no capability protocol: every request takes the - // legacy NAT path, so any dashboard-issued Authorization value (panel JWT - // or panel PAT) is stripped before the request is tunneled to the agent. - // Foreign Authorization values are ordinary request data and stay put. - stripDashboardCredential(request) + // legacy NAT path, so any dashboard-issued credential (panel JWT or panel + // PAT) carried by the Authorization header, the ?token= query parameter or + // the nz-jwt cookie is stripped before the request is tunneled to the + // agent. Foreign values are ordinary request data and stay put. + stripDashboardCredentials(request) return natCapabilityLease{}, nil } diff --git a/cmd/dashboard/rpc/nat_capability_default_test.go b/cmd/dashboard/rpc/nat_capability_default_test.go index 71154084..40827939 100644 --- a/cmd/dashboard/rpc/nat_capability_default_test.go +++ b/cmd/dashboard/rpc/nat_capability_default_test.go @@ -4,6 +4,7 @@ package rpc import ( "net/http" + "net/url" "testing" "github.com/nezhahq/nezha/model" @@ -39,13 +40,17 @@ func TestPrepareNATCapabilityDefaultStripsDashboardCredentials(t *testing.T) { // Given installNATCredentialTestGate(t) for name, credential := range map[string]string{ - "dashboard jwt": natTestDashboardJWT, - "expired dashboard jwt": natTestDashboardExpiredJWT, - "dashboard api token": natTestDashboardAPIToken, + "dashboard jwt": natTestDashboardJWTValue, + "expired dashboard jwt": natTestDashboardExpiredJWTValue, + "dashboard api token": natTestDashboardPATValue, } { t.Run(name, func(t *testing.T) { - request := &http.Request{Header: make(http.Header)} - request.Header.Set("Authorization", credential) + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + credential}, + } + request.Header.Set("Authorization", "Bearer "+credential) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+credential) // When lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) @@ -58,7 +63,13 @@ func TestPrepareNATCapabilityDefaultStripsDashboardCredentials(t *testing.T) { t.Fatal("default NAT capability hook unexpectedly activated") } if got := request.Header.Get("Authorization"); got != "" { - t.Fatalf("default NAT capability hook retained dashboard credential %q", got) + t.Fatalf("default NAT capability hook retained dashboard credential in Authorization %q", got) + } + if got := request.URL.RawQuery; got != "keep=1" { + t.Fatalf("default NAT capability hook retained dashboard credential in query %q", got) + } + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("default NAT capability hook retained dashboard credential in cookie %q", got) } }) } @@ -101,3 +112,38 @@ func TestPrepareNATCapabilityDefaultKeepsForeignAndMissingAuthorization(t *testi }) } } + +func TestPrepareNATCapabilityDefaultKeepsForeignAndMissingQueryAndCookie(t *testing.T) { + // Given + installNATCredentialTestGate(t) + for name, tc := range map[string]struct{ rawQuery, cookie string }{ + "foreign values": {"keep=1&token=" + natTestForeignJWTValue + "&a=%2Fx", "sid=abc; nz-jwt=" + natTestForeignJWTValue}, + "empty values": {"keep=1&token=", "nz-jwt="}, + "missing channel": {"keep=1", "sid=abc"}, + } { + t.Run(name, func(t *testing.T) { + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: tc.rawQuery}, + } + request.Header.Set("Cookie", tc.cookie) + + // When + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + // Then + if err != nil { + t.Fatalf("default NAT capability hook returned error: %v", err) + } + if lease.active { + t.Fatal("default NAT capability hook unexpectedly activated") + } + if got := request.URL.RawQuery; got != tc.rawQuery { + t.Fatalf("default NAT capability hook changed query to %q, want %q", got, tc.rawQuery) + } + if got := request.Header.Get("Cookie"); got != tc.cookie { + t.Fatalf("default NAT capability hook changed cookie to %q, want %q", got, tc.cookie) + } + }) + } +} diff --git a/cmd/dashboard/rpc/nat_credential.go b/cmd/dashboard/rpc/nat_credential.go index eb6470a8..a9e82f4a 100644 --- a/cmd/dashboard/rpc/nat_credential.go +++ b/cmd/dashboard/rpc/nat_credential.go @@ -1,29 +1,69 @@ package rpc -import "net/http" +import ( + "net/http" + "net/url" + "strings" +) -// natDashboardCredentialGate classifies an Authorization header value as a -// credential issued by this dashboard (panel-signed JWT or panel API token). -// main() injects controller.IsDashboardCredential; the setter keeps the rpc -// package free of a dependency on the controller package, the same pattern as -// SetReceiptGateListener and SetMCPKillSwitchObserver. -var natDashboardCredentialGate func(authz string) bool +// Channel names of the panel's own TokenLookup (controller/jwt.go +// "header: Authorization, query: token, cookie: nz-jwt"). A dashboard JWT +// can therefore reach the NAT ingress through the Authorization header, the +// ?token= query parameter or the nz-jwt cookie, and all three must be +// classified before the request is tunneled to the agent. +const ( + natDashboardJWTCookieName = "nz-jwt" + natDashboardJWTQueryParameter = "token" +) -// SetNATDashboardCredentialGate wires the dashboard-credential classifier used -// by the NAT ingress. Passing nil restores the fail-closed default in -// stripDashboardCredential. -func SetNATDashboardCredentialGate(fn func(authz string) bool) { - natDashboardCredentialGate = fn +var ( + // natDashboardAuthorizationGate classifies an Authorization header value + // (case-sensitive "Bearer " scheme included) as a credential issued by + // this dashboard. + natDashboardAuthorizationGate func(authz string) bool + // natDashboardCredentialValueGate classifies a raw credential value — the + // part after the "Bearer " scheme, exactly as the ?token= query parameter + // and the nz-jwt cookie carry it — as a credential issued by this + // dashboard. + natDashboardCredentialValueGate func(value string) bool +) + +// SetNATDashboardCredentialGate wires the dashboard-credential classifiers +// used by the NAT ingress. main() injects +// controller.IsDashboardCredential and controller.IsDashboardCredentialValue; +// the setter keeps the rpc package free of a dependency on the controller +// package, the same pattern as SetReceiptGateListener and +// SetMCPKillSwitchObserver. A nil function fails closed: every value on that +// channel is then treated as dashboard-issued and stripped. +func SetNATDashboardCredentialGate(authorizationClass func(authz string) bool, valueClass func(value string) bool) { + natDashboardAuthorizationGate = authorizationClass + natDashboardCredentialValueGate = valueClass } -// stripDashboardCredential removes the Authorization header when it carries a -// credential issued by this dashboard (panel-signed JWT or panel API token). -// Such credentials authenticate the dashboard itself and must never become -// origin credentials for the NAT backend. Foreign credentials are forwarded -// untouched: the NAT backend may authenticate with credentials of its own, and -// this dashboard must not make policy for them. -func stripDashboardCredential(request *http.Request) { - if natDashboardCredentialGate == nil { +// NATDashboardCredentialGateWired reports whether the dashboard-credential +// classifiers have been wired. It exists so the main() wiring contract can be +// pinned by a test. +func NATDashboardCredentialGateWired() bool { + return natDashboardAuthorizationGate != nil && natDashboardCredentialValueGate != nil +} + +// stripDashboardCredentials removes credentials issued by this dashboard from +// every channel the panel's own TokenLookup accepts: the Authorization +// header, the ?token= query parameter and the nz-jwt cookie. Such credentials +// authenticate the dashboard itself and must never become origin credentials +// for the NAT backend. Foreign values are forwarded untouched: the NAT +// backend may authenticate with credentials of its own, and this dashboard +// must not make policy for them. +func stripDashboardCredentials(request *http.Request) { + stripDashboardAuthorizationHeader(request) + stripDashboardCredentialQuery(request) + stripDashboardCredentialCookie(request) +} + +// stripDashboardAuthorizationHeader removes the Authorization header when it +// carries a dashboard-issued credential. +func stripDashboardAuthorizationHeader(request *http.Request) { + if natDashboardAuthorizationGate == nil { // Gate not wired (early startup or tests): fail closed and keep the // historical behavior of stripping every Authorization header. request.Header.Del("Authorization") @@ -33,9 +73,116 @@ func stripDashboardCredential(request *http.Request) { // dashboard-issued value in it must still not survive into the tunnel, so // the whole header is dropped when one is recognized. for _, value := range request.Header.Values("Authorization") { - if natDashboardCredentialGate(value) { + if natDashboardAuthorizationGate(value) { request.Header.Del("Authorization") return } } } + +// stripDashboardCredentialQuery removes ?token= query parameters whose value +// is a dashboard-issued credential. Every other element of the query string — +// including foreign token values — is preserved byte-for-byte, so the +// forwarded request keeps the exact query the client sent. +func stripDashboardCredentialQuery(request *http.Request) { + if request.URL == nil || request.URL.RawQuery == "" { + return + } + elements := strings.Split(request.URL.RawQuery, "&") + kept := elements[:0] + changed := false + for _, element := range elements { + if isDashboardCredentialQueryElement(element) { + changed = true + continue + } + kept = append(kept, element) + } + if changed { + request.URL.RawQuery = strings.Join(kept, "&") + } +} + +// isDashboardCredentialQueryElement reports whether a raw query element +// carries a dashboard-issued credential in the panel's token parameter. +func isDashboardCredentialQueryElement(element string) bool { + rawName, rawValue, _ := strings.Cut(element, "=") + // The panel reads the parameter by its decoded name (gin-jwt c.Query), + // so an escaped key must be decoded before comparing. + name, err := url.QueryUnescape(rawName) + if err != nil || name != natDashboardJWTQueryParameter { + return false + } + if natDashboardCredentialValueGate == nil { + return true // fail closed + } + // A value that fails to decode is one the panel's own query parsing drops + // as well, so it can never be a panel credential. + decoded, err := url.QueryUnescape(rawValue) + if err != nil { + return false + } + return natDashboardCredentialValueGate(decoded) +} + +// stripDashboardCredentialCookie removes nz-jwt cookies whose value is a +// dashboard-issued credential. All other cookie pairs — including foreign +// nz-jwt values — are preserved verbatim. +func stripDashboardCredentialCookie(request *http.Request) { + lines := request.Header.Values("Cookie") + if len(lines) == 0 { + return + } + changed := false + kept := make([]string, 0, len(lines)) + for _, line := range lines { + nextLine, lineChanged := stripDashboardCredentialCookieLine(line) + changed = changed || lineChanged + if nextLine != "" { + kept = append(kept, nextLine) + } + } + if !changed { + return + } + request.Header.Del("Cookie") + for _, line := range kept { + request.Header.Add("Cookie", line) + } +} + +// stripDashboardCredentialCookieLine rewrites a single Cookie header line, +// dropping the dashboard-issued nz-jwt pairs. Kept pairs keep their raw form, +// whitespace included — except that a pair following a dropped one no longer +// carries the removed pair's share of the "; " separator. +func stripDashboardCredentialCookieLine(line string) (string, bool) { + pairs := strings.Split(line, ";") + kept := pairs[:0] + droppedBeforeFirstKept := true + changed := false + for _, pair := range pairs { + name, value, _ := strings.Cut(pair, "=") + if strings.TrimSpace(name) == natDashboardJWTCookieName && + (natDashboardCredentialValueGate == nil || isDashboardCredentialCookieValue(value)) { + changed = true + continue + } + if droppedBeforeFirstKept { + pair = strings.TrimLeft(pair, " ") + } + droppedBeforeFirstKept = false + kept = append(kept, pair) + } + if !changed { + return line, false + } + return strings.Join(kept, ";"), true +} + +// isDashboardCredentialCookieValue classifies a raw cookie value the same way +// the panel's cookie parsing would hand it to the JWT verifier: surrounding +// whitespace and quoting are presentation, not part of the credential. +func isDashboardCredentialCookieValue(rawValue string) bool { + value := strings.Trim(strings.TrimSpace(rawValue), `"`) + return natDashboardCredentialValueGate(value) +} diff --git a/cmd/dashboard/rpc/nat_credential_test.go b/cmd/dashboard/rpc/nat_credential_test.go index 0d5690d1..2f7a2be9 100644 --- a/cmd/dashboard/rpc/nat_credential_test.go +++ b/cmd/dashboard/rpc/nat_credential_test.go @@ -1,31 +1,43 @@ package rpc import ( + "net/http" + "net/url" "testing" "github.com/nezhahq/nezha/model" ) -// Values standing in for the credentials this dashboard issues. The fake gate -// below mirrors the contract of controller.IsDashboardCredential; the real -// classification semantics — panel JWTs accepted regardless of expiry, -// foreign-signed JWTs and random strings rejected — are pinned by the -// controller package tests (dashboard_credential_test.go). +// Values standing in for the credentials this dashboard issues. The fake +// gates below mirror the contract of controller.IsDashboardCredential and +// controller.IsDashboardCredentialValue; the real classification semantics — +// panel JWTs accepted regardless of expiry, foreign-signed JWTs and random +// strings rejected — are pinned by the controller package tests +// (dashboard_credential_test.go) and by the flow test that wires the real +// classifiers into ServeNAT (dashboard_credential_nat_flow_test.go). const ( - natTestDashboardJWT = "Bearer dashboard-signed-jwt" - natTestDashboardExpiredJWT = "Bearer dashboard-signed-expired-jwt" - natTestDashboardAPIToken = "Bearer " + model.APITokenPrefix + "dashboard-issued-pat" - natTestForeignJWT = "Bearer foreign-signed-jwt" - natTestForeignRandom = "Bearer foreign-random-string" + natTestDashboardJWTValue = "dashboard-signed-jwt" + natTestDashboardExpiredJWTValue = "dashboard-signed-expired-jwt" + natTestDashboardPATValue = model.APITokenPrefix + "dashboard-issued-pat" + natTestForeignJWTValue = "foreign-signed-jwt" + natTestForeignRandomValue = "foreign-random-string" + + natTestDashboardJWT = "Bearer " + natTestDashboardJWTValue + natTestDashboardExpiredJWT = "Bearer " + natTestDashboardExpiredJWTValue + natTestDashboardAPIToken = "Bearer " + natTestDashboardPATValue + natTestForeignJWT = "Bearer " + natTestForeignJWTValue + natTestForeignRandom = "Bearer " + natTestForeignRandomValue ) -// installNATCredentialTestGate wires a fake dashboard-credential classifier +// installNATCredentialTestGate wires fake dashboard-credential classifiers // for the duration of the test: exactly the dashboard-issued test values above -// are classified as dashboard credentials, everything else is foreign. +// are classified as dashboard credentials on every channel, everything else +// is foreign. func installNATCredentialTestGate(t *testing.T) { t.Helper() - original := natDashboardCredentialGate - natDashboardCredentialGate = func(authz string) bool { + originalAuthorization := natDashboardAuthorizationGate + originalValue := natDashboardCredentialValueGate + natDashboardAuthorizationGate = func(authz string) bool { switch authz { case natTestDashboardJWT, natTestDashboardExpiredJWT, natTestDashboardAPIToken: return true @@ -33,5 +45,109 @@ func installNATCredentialTestGate(t *testing.T) { return false } } - t.Cleanup(func() { natDashboardCredentialGate = original }) + natDashboardCredentialValueGate = func(value string) bool { + switch value { + case natTestDashboardJWTValue, natTestDashboardExpiredJWTValue, natTestDashboardPATValue: + return true + default: + return false + } + } + t.Cleanup(func() { + natDashboardAuthorizationGate = originalAuthorization + natDashboardCredentialValueGate = originalValue + }) +} + +func TestStripDashboardCredentialsQueryChannel(t *testing.T) { + // Given + installNATCredentialTestGate(t) + request := &http.Request{URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + natTestDashboardJWTValue + "&token=" + natTestForeignJWTValue + "&a=%2Fx"}} + + // When + stripDashboardCredentials(request) + + // Then + if got := request.URL.RawQuery; got != "keep=1&token="+natTestForeignJWTValue+"&a=%2Fx" { + t.Fatalf("query channel kept %q, want dashboard token dropped and foreign values byte-preserved", got) + } +} + +func TestStripDashboardCredentialsQueryChannelEscapedValue(t *testing.T) { + // Given — a dashboard token percent-encoded in the query must still be + // recognized, while the escaped foreign element stays verbatim. + installNATCredentialTestGate(t) + request := &http.Request{URL: &url.URL{Path: "/nat", RawQuery: "token=" + url.QueryEscape(natTestDashboardJWTValue) + "&token=" + url.QueryEscape(natTestForeignJWTValue)}} + + // When + stripDashboardCredentials(request) + + // Then + if got := request.URL.RawQuery; got != "token="+url.QueryEscape(natTestForeignJWTValue) { + t.Fatalf("query channel kept %q, want escaped dashboard token dropped and foreign element verbatim", got) + } +} + +func TestStripDashboardCredentialsCookieChannel(t *testing.T) { + // Given + installNATCredentialTestGate(t) + request := &http.Request{Header: make(http.Header)} + request.Header.Set("Cookie", "sid=abc; nz-jwt="+natTestDashboardJWTValue+"; nz-jwt="+natTestForeignJWTValue+"; theme=dark") + + // When + stripDashboardCredentials(request) + + // Then + if got := request.Header.Get("Cookie"); got != "sid=abc; nz-jwt="+natTestForeignJWTValue+"; theme=dark" { + t.Fatalf("cookie channel kept %q, want dashboard pair dropped and foreign pairs verbatim", got) + } +} + +func TestStripDashboardCredentialsCookieChannelQuotedValue(t *testing.T) { + // Given — quoting is presentation the panel's cookie parsing strips before + // verifying, so a quoted dashboard token must be recognized. + installNATCredentialTestGate(t) + request := &http.Request{Header: make(http.Header)} + request.Header.Set("Cookie", `nz-jwt="`+natTestDashboardJWTValue+`"; sid=abc`) + + // When + stripDashboardCredentials(request) + + // Then + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("cookie channel kept %q, want quoted dashboard token dropped", got) + } +} + +func TestPrepareNATCapabilityFailsClosedWithoutGate(t *testing.T) { + // Given — unwired gates: every value on every channel is treated as + // dashboard-issued and stripped, foreign ones included. + SetNATDashboardCredentialGate(nil, nil) + t.Cleanup(func() { SetNATDashboardCredentialGate(nil, nil) }) + request := &http.Request{ + Header: make(http.Header), + URL: &url.URL{Path: "/nat", RawQuery: "keep=1&token=" + natTestForeignJWTValue}, + } + request.Header.Set("Authorization", natTestForeignRandom) + request.Header.Set("Cookie", "sid=abc; nz-jwt="+natTestForeignJWTValue) + + // When + lease, err := prepareNATCapability(request, &model.NAT{Common: model.Common{ID: 91}, ServerID: 81}) + + // Then + if err != nil { + t.Fatalf("fail-closed NAT capability hook returned error: %v", err) + } + if lease.active { + t.Fatal("fail-closed NAT capability hook unexpectedly activated") + } + if got := request.Header.Get("Authorization"); got != "" { + t.Fatalf("fail-closed hook retained Authorization %q", got) + } + if got := request.URL.RawQuery; got != "keep=1" { + t.Fatalf("fail-closed hook kept query %q, want token parameter stripped", got) + } + if got := request.Header.Get("Cookie"); got != "sid=abc" { + t.Fatalf("fail-closed hook kept cookie %q, want nz-jwt pair stripped", got) + } }