diff --git a/service/rpc/auth.go b/service/rpc/auth.go index 34b607fe..3ea7dd0c 100644 --- a/service/rpc/auth.go +++ b/service/rpc/auth.go @@ -2,6 +2,7 @@ package rpc import ( "context" + "fmt" "strings" petname "github.com/dustinkirkland/golang-petname" @@ -56,7 +57,10 @@ func (a *authHandler) Check(ctx context.Context) (uint64, error) { return 0, status.Error(codes.Unauthenticated, "客户端 UUID 不合法") } - clientID, hasID := singleton.ServerShared.UUIDToID(clientUUID) + clientID, hasID, err := authorizeAgentForUUID(userId, clientUUID) + if err != nil { + return 0, status.Error(codes.Unauthenticated, err.Error()) + } if !hasID { s := model.Server{UUID: clientUUID, Name: petname.Generate(2, "-"), Common: model.Common{ UserID: userId, @@ -73,3 +77,32 @@ func (a *authHandler) Check(ctx context.Context) (uint64, error) { return clientID, nil } + +// authorizeAgentForUUID resolves a client UUID to the dashboard's internal +// server ID, ensuring the resolved server is actually owned by the agent +// secret's owner. Previously Check returned the resolved server ID without +// verifying ownership, allowing an agent that knew another user's server +// UUID to impersonate it (poisoning monitoring state, triggering alerts). +// hasID=false means the UUID is unknown and the caller may register it as +// a new server for the secret owner. +// +// The error path also doubles as a leak-detection signal for operators: if +// an agent persistently fails with "client UUID does not belong to the +// agent secret owner", it pins down which user's secret has been reused +// against a server they don't own. +func authorizeAgentForUUID(userId uint64, clientUUID string) (clientID uint64, hasID bool, err error) { + cid, found := singleton.ServerShared.UUIDToID(clientUUID) + if !found { + return 0, false, nil + } + server, _ := singleton.ServerShared.Get(cid) + if server == nil { + // Cache inconsistency: UUID maps to an ID, but no server record exists. + // Treat as unknown (registration path) rather than impersonation. + return 0, false, nil + } + if server.UserID != userId { + return 0, false, fmt.Errorf("client UUID does not belong to the agent secret owner") + } + return cid, true, nil +} diff --git a/service/rpc/auth_test.go b/service/rpc/auth_test.go new file mode 100644 index 00000000..373c9849 --- /dev/null +++ b/service/rpc/auth_test.go @@ -0,0 +1,89 @@ +package rpc + +import ( + "testing" + + "gorm.io/driver/sqlite" + "gorm.io/gorm" + + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/service/singleton" +) + +// setupAuthAgentFixture seeds an in-memory DB and ServerShared with two +// servers belonging to different users so we can assert that a secret bound +// to user A cannot resolve a server UUID owned by user B. +func setupAuthAgentFixture(t *testing.T) func() { + t.Helper() + originalDB := singleton.DB + originalServerShared := singleton.ServerShared + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open db: %v", err) + } + if err := db.AutoMigrate(&model.Server{}); err != nil { + t.Fatalf("migrate: %v", err) + } + if err := db.Create(&model.Server{ + Common: model.Common{ID: 1, UserID: 100}, + UUID: "uuid-alice", + Name: "alice-srv", + }).Error; err != nil { + t.Fatalf("create alice: %v", err) + } + if err := db.Create(&model.Server{ + Common: model.Common{ID: 2, UserID: 200}, + UUID: "uuid-bob", + Name: "bob-srv", + }).Error; err != nil { + t.Fatalf("create bob: %v", err) + } + singleton.DB = db + singleton.ServerShared = singleton.NewServerClass() + + return func() { + singleton.DB = originalDB + singleton.ServerShared = originalServerShared + } +} + +func TestAuthorizeAgentForUUIDAcceptsOwnedServer(t *testing.T) { + defer setupAuthAgentFixture(t)() + + cid, hasID, err := authorizeAgentForUUID(100, "uuid-alice") + if err != nil { + t.Fatalf("alice with her own server UUID must not error, got %v", err) + } + if !hasID || cid != 1 { + t.Fatalf("expected (cid=1, hasID=true), got (cid=%d, hasID=%v)", cid, hasID) + } +} + +// Core regression: an agent presenting user A's secret but user B's server +// UUID must be rejected. Previously the code returned the resolved server ID +// without verifying the UserID matched the secret owner, allowing same-tenant +// (and worse — cross-tenant if UUID leaks) server impersonation. +func TestAuthorizeAgentForUUIDRejectsForeignServerUUID(t *testing.T) { + defer setupAuthAgentFixture(t)() + + _, _, err := authorizeAgentForUUID(100, "uuid-bob") // alice's secret + bob's UUID + if err == nil { + t.Fatalf("UUID owned by another user must be rejected") + } +} + +// An unknown UUID must NOT be treated as an impersonation attempt — it is +// the normal first-time registration path and the caller (Check) creates a +// new server bound to the secret owner. +func TestAuthorizeAgentForUUIDPermitsUnknownUUIDForRegistration(t *testing.T) { + defer setupAuthAgentFixture(t)() + + cid, hasID, err := authorizeAgentForUUID(100, "uuid-never-seen-before") + if err != nil { + t.Fatalf("unknown UUID must be permitted for new registration, got %v", err) + } + if hasID { + t.Fatalf("hasID must be false for unknown UUID, got cid=%d", cid) + } +}