diff --git a/cmd/dashboard/controller/fm.go b/cmd/dashboard/controller/fm.go index 87699114..85ac2386 100644 --- a/cmd/dashboard/controller/fm.go +++ b/cmd/dashboard/controller/fm.go @@ -46,7 +46,7 @@ func createFM(c *gin.Context) (*model.CreateFMResponse, error) { return nil, err } - rpc.NezhaHandlerSingleton.CreateStream(streamId) + rpc.NezhaHandlerSingleton.CreateStream(streamId, getUid(c)) fmData, _ := json.Marshal(&model.TaskFM{ StreamID: streamId, @@ -72,6 +72,12 @@ func createFM(c *gin.Context) (*model.CreateFMResponse, error) { // @Router /ws/file/{id} [get] func fmStream(c *gin.Context) (any, error) { streamId := c.Param("id") + // GHSA-style fix: io_stream sessions must be reachable only by their creator + // (or an admin). Without this, any authenticated user who learns a stream + // UUID can hijack a live file-manager session on the target server. + if !rpc.NezhaHandlerSingleton.IsStreamAuthorizedForUser(streamId, getUid(c), callerIsAdmin(c)) { + return nil, singleton.Localizer.ErrorT("permission denied") + } if _, err := rpc.NezhaHandlerSingleton.GetStream(streamId); err != nil { return nil, err } diff --git a/cmd/dashboard/controller/stream_ownership_test.go b/cmd/dashboard/controller/stream_ownership_test.go new file mode 100644 index 00000000..ae88f233 --- /dev/null +++ b/cmd/dashboard/controller/stream_ownership_test.go @@ -0,0 +1,117 @@ +package controller + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/pkg/i18n" + "github.com/nezhahq/nezha/service/rpc" + "github.com/nezhahq/nezha/service/singleton" +) + +func ensureLocalizerForStreamTests(t *testing.T) { + t.Helper() + if singleton.Localizer == nil { + singleton.Localizer = i18n.NewLocalizer("en_US", "nezha", "translations", i18n.Translations) + } + // upgrader stays nil — these tests must reject the caller BEFORE WS upgrade. + // If a test ever reaches the upgrade path it will panic on nil upgrader, + // surfacing the regression. +} + +// decodeCommonResponseError returns Success and Error of a CommonResponse[any]. +func decodeCommonResponseError(t *testing.T, body []byte) (bool, string) { + t.Helper() + var resp struct { + Success bool `json:"success"` + Error string `json:"error"` + } + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("decode response: %v body=%s", err, string(body)) + } + return resp.Success, resp.Error +} + +func setAuthUser(c *gin.Context, userID uint64, role model.Role) { + c.Set(model.CtxKeyAuthorizedUser, &model.User{ + Common: model.Common{ID: userID}, + Role: role, + }) +} + +func TestTerminalStreamRejectsForeignMember(t *testing.T) { + gin.SetMode(gin.TestMode) + ensureLocalizerForStreamTests(t) + rpc.NezhaHandlerSingleton = rpc.NewNezhaHandler() + rpc.NezhaHandlerSingleton.CreateStream("alice-terminal", 100) + + r := gin.New() + r.Use(func(c *gin.Context) { + setAuthUser(c, 200, model.RoleMember) // bob + c.Next() + }) + r.GET("/ws/terminal/:id", commonHandler(terminalStream)) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ws/terminal/alice-terminal", nil) + r.ServeHTTP(w, req) + + success, errMsg := decodeCommonResponseError(t, w.Body.Bytes()) + assert.False(t, success, "foreign member must not be authorized to attach to alice's terminal") + assert.Contains(t, errMsg, "permission denied") + + // And the existing stream must NOT have been torn down by the failed attempt. + _, stillExists := rpc.NezhaHandlerSingleton.StreamOwnership("alice-terminal") + assert.True(t, stillExists, "rejected attempt must not destroy the legitimate session") +} + +func TestFMStreamRejectsForeignMember(t *testing.T) { + gin.SetMode(gin.TestMode) + ensureLocalizerForStreamTests(t) + rpc.NezhaHandlerSingleton = rpc.NewNezhaHandler() + rpc.NezhaHandlerSingleton.CreateStream("alice-fm", 100) + + r := gin.New() + r.Use(func(c *gin.Context) { + setAuthUser(c, 200, model.RoleMember) + c.Next() + }) + r.GET("/ws/file/:id", commonHandler(fmStream)) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ws/file/alice-fm", nil) + r.ServeHTTP(w, req) + + success, errMsg := decodeCommonResponseError(t, w.Body.Bytes()) + assert.False(t, success, "foreign member must not be authorized to attach to alice's FM session") + assert.Contains(t, errMsg, "permission denied") + + _, stillExists := rpc.NezhaHandlerSingleton.StreamOwnership("alice-fm") + assert.True(t, stillExists, "rejected attempt must not destroy the legitimate FM session") +} + +func TestTerminalStreamRejectsUnknownStreamID(t *testing.T) { + gin.SetMode(gin.TestMode) + ensureLocalizerForStreamTests(t) + rpc.NezhaHandlerSingleton = rpc.NewNezhaHandler() + + r := gin.New() + r.Use(func(c *gin.Context) { + setAuthUser(c, 100, model.RoleMember) + c.Next() + }) + r.GET("/ws/terminal/:id", commonHandler(terminalStream)) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/ws/terminal/nonexistent", nil) + r.ServeHTTP(w, req) + + success, _ := decodeCommonResponseError(t, w.Body.Bytes()) + assert.False(t, success, "unknown stream id must produce an error response") +} diff --git a/cmd/dashboard/controller/terminal.go b/cmd/dashboard/controller/terminal.go index 7899f1b7..e109920c 100644 --- a/cmd/dashboard/controller/terminal.go +++ b/cmd/dashboard/controller/terminal.go @@ -44,7 +44,7 @@ func createTerminal(c *gin.Context) (*model.CreateTerminalResponse, error) { return nil, err } - rpc.NezhaHandlerSingleton.CreateStream(streamId) + rpc.NezhaHandlerSingleton.CreateStream(streamId, getUid(c)) terminalData, _ := json.Marshal(&model.TerminalTask{ StreamID: streamId, @@ -72,6 +72,13 @@ func createTerminal(c *gin.Context) (*model.CreateTerminalResponse, error) { // @Router /ws/terminal/{id} [get] func terminalStream(c *gin.Context) (any, error) { streamId := c.Param("id") + // GHSA-style fix: io_stream sessions must be reachable only by their creator + // (or an admin). Without this, any authenticated user who learns a stream + // UUID — via Referer leak, access logs, browser history — can hijack a live + // terminal and gain shell access to the target server. + if !rpc.NezhaHandlerSingleton.IsStreamAuthorizedForUser(streamId, getUid(c), callerIsAdmin(c)) { + return nil, singleton.Localizer.ErrorT("permission denied") + } if _, err := rpc.NezhaHandlerSingleton.GetStream(streamId); err != nil { return nil, err } diff --git a/cmd/dashboard/rpc/rpc.go b/cmd/dashboard/rpc/rpc.go index 68c84cd6..ca148708 100644 --- a/cmd/dashboard/rpc/rpc.go +++ b/cmd/dashboard/rpc/rpc.go @@ -138,7 +138,10 @@ func ServeNAT(w http.ResponseWriter, r *http.Request, natConfig *model.NAT) { return } - rpcService.NezhaHandlerSingleton.CreateStream(streamId) + // NAT streams are anonymous HTTP-facing tunnels; they are NOT reachable + // via /ws/terminal or /ws/file (which check stream ownership), so the + // creator user ID does not need to identify a real user. + rpcService.NezhaHandlerSingleton.CreateStream(streamId, 0) defer rpcService.NezhaHandlerSingleton.CloseStream(streamId) taskData, err := json.Marshal(model.TaskNAT{ diff --git a/service/rpc/io_stream.go b/service/rpc/io_stream.go index 87ef0aa2..161be3b4 100644 --- a/service/rpc/io_stream.go +++ b/service/rpc/io_stream.go @@ -11,6 +11,7 @@ import ( ) type ioStreamContext struct { + creatorUserID uint64 userIo io.ReadWriteCloser agentIo io.ReadWriteCloser userIoConnectCh chan struct{} @@ -31,16 +32,47 @@ var bufPool = sync.Pool{ }, } -func (s *NezhaHandler) CreateStream(streamId string) { +func (s *NezhaHandler) CreateStream(streamId string, creatorUserID uint64) { s.ioStreamMutex.Lock() defer s.ioStreamMutex.Unlock() s.ioStreams[streamId] = &ioStreamContext{ + creatorUserID: creatorUserID, userIoConnectCh: make(chan struct{}), agentIoConnectCh: make(chan struct{}), } } +// IsStreamAuthorizedForUser checks whether the requesting user may attach to +// the stream. A stream is reachable only by its creator or by an admin; any +// other authenticated user must be rejected. Unknown streams are always +// rejected. +func (s *NezhaHandler) IsStreamAuthorizedForUser(streamId string, userID uint64, isAdmin bool) bool { + creator, found := s.StreamOwnership(streamId) + if !found { + return false + } + if isAdmin { + return true + } + return creator == userID +} + +// StreamOwnership returns the user ID that created the stream and whether the +// stream is still tracked. Callers must compare the returned creator against +// the requesting user before attaching to the stream — without this the +// channel becomes a session-hijack primitive (terminal/file manager RCE). +func (s *NezhaHandler) StreamOwnership(streamId string) (uint64, bool) { + s.ioStreamMutex.RLock() + defer s.ioStreamMutex.RUnlock() + + ctx, ok := s.ioStreams[streamId] + if !ok { + return 0, false + } + return ctx.creatorUserID, true +} + func (s *NezhaHandler) GetStream(streamId string) (*ioStreamContext, error) { s.ioStreamMutex.RLock() defer s.ioStreamMutex.RUnlock() diff --git a/service/rpc/io_stream_test.go b/service/rpc/io_stream_test.go index ea90387c..3fb5e08d 100644 --- a/service/rpc/io_stream_test.go +++ b/service/rpc/io_stream_test.go @@ -12,7 +12,7 @@ func TestIOStream(t *testing.T) { const testStreamID = "ffffffff-ffff-ffff-ffff-ffffffffffff" - handler.CreateStream(testStreamID) + handler.CreateStream(testStreamID, 0) userIo, agentIo := newPipeReadWriter(), newPipeReadWriter() defer func() { userIo.Close() @@ -105,3 +105,71 @@ func newPipeReadWriter() io.ReadWriteCloser { io.WriteCloser }{r, w} } + +func TestStreamOwnershipReturnsCreatorUserID(t *testing.T) { + h := NewNezhaHandler() + h.CreateStream("alice-stream", 100) + + creator, found := h.StreamOwnership("alice-stream") + if !found { + t.Fatalf("expected stream to be found after CreateStream") + } + if creator != 100 { + t.Fatalf("expected creator user ID 100, got %d", creator) + } +} + +func TestStreamOwnershipReturnsNotFoundForUnknownID(t *testing.T) { + h := NewNezhaHandler() + if _, found := h.StreamOwnership("nonexistent"); found { + t.Fatalf("expected unknown stream id to report not-found") + } +} + +func TestStreamOwnershipPreservesPerStreamCreator(t *testing.T) { + h := NewNezhaHandler() + h.CreateStream("alice-stream", 100) + h.CreateStream("bob-stream", 200) + + aliceCreator, _ := h.StreamOwnership("alice-stream") + bobCreator, _ := h.StreamOwnership("bob-stream") + if aliceCreator != 100 || bobCreator != 200 { + t.Fatalf("expected per-stream creator IDs alice=100 bob=200, got alice=%d bob=%d", + aliceCreator, bobCreator) + } +} + +func TestIsStreamAuthorizedForUserAllowsCreator(t *testing.T) { + h := NewNezhaHandler() + h.CreateStream("alice-stream", 100) + + if !h.IsStreamAuthorizedForUser("alice-stream", 100, false) { + t.Fatalf("creator must be authorized to attach to their own stream") + } +} + +func TestIsStreamAuthorizedForUserDeniesForeignMember(t *testing.T) { + h := NewNezhaHandler() + h.CreateStream("alice-stream", 100) + + if h.IsStreamAuthorizedForUser("alice-stream", 200, false) { + t.Fatalf("foreign member must not be authorized — session hijack would be possible") + } +} + +func TestIsStreamAuthorizedForUserAllowsAdmin(t *testing.T) { + h := NewNezhaHandler() + h.CreateStream("alice-stream", 100) + + if !h.IsStreamAuthorizedForUser("alice-stream", 999, true) { + t.Fatalf("admin must be authorized to attach regardless of creator") + } +} + +func TestIsStreamAuthorizedForUserDeniesUnknownStream(t *testing.T) { + h := NewNezhaHandler() + + if h.IsStreamAuthorizedForUser("nonexistent", 100, true) { + t.Fatalf("unknown stream id must not authorize even admin") + } +}