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().
2026-11-04 08:40:20 +00:00 · 2026-10-01 11:17:45 +00:00
1 parent 40240ed02b
commit 3ad985f36e
@@ -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)
 	}
@@ -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()...)
+}
@@ -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