diff --git a/.changelog/25089.txt b/.changelog/25089.txt new file mode 100644 index 000000000..4e4358c45 --- /dev/null +++ b/.changelog/25089.txt @@ -0,0 +1,3 @@ +```release-note:security +event stream: fixes vulnerability CVE-2025-0937, where using a wildcard namespace to subscribe to the events API grants a user with "read" capabilites on any namespace, the ability to read events from all namespaces. +``` diff --git a/nomad/event_endpoint.go b/nomad/event_endpoint.go index 8c0e818c3..13146ff95 100644 --- a/nomad/event_endpoint.go +++ b/nomad/event_endpoint.go @@ -41,6 +41,10 @@ func (e *Event) stream(conn io.ReadWriteCloser) { } authErr := e.srv.Authenticate(nil, &args) + if authErr != nil { + handleJsonResultError(structs.ErrPermissionDenied, pointer.Of(int64(403)), encoder) + return + } // forward to appropriate region if args.Region != e.srv.config.Region { @@ -52,16 +56,27 @@ func (e *Event) stream(conn io.ReadWriteCloser) { } e.srv.MeasureRPCRate("event", structs.RateMetricRead, &args) - if authErr != nil { + + resolvedACL, err := e.srv.ResolveACL(&args) + if err != nil { handleJsonResultError(structs.ErrPermissionDenied, pointer.Of(int64(403)), encoder) + return + } + + validatedNses, err := e.validateACL(args.Namespace, args.Topics, resolvedACL) + if err != nil { + handleJsonResultError(structs.ErrPermissionDenied, pointer.Of(int64(403)), encoder) + return } // Generate the subscription request subReq := &stream.SubscribeRequest{ - Token: args.AuthToken, - Topics: args.Topics, - Index: uint64(args.Index), - Namespace: args.Namespace, + Token: args.AuthToken, + Topics: args.Topics, + Index: uint64(args.Index), + // Namespaces is set once, in the event a users ACL is updated to include + // more NSes, the current event stream will not include the new NSes. + Namespaces: validatedNses, Authenticate: func() error { if err := e.srv.Authenticate(nil, &args); err != nil { return err @@ -70,7 +85,8 @@ func (e *Event) stream(conn io.ReadWriteCloser) { if err != nil { return err } - return validateACL(args.Namespace, args.Topics, resolvedACL) + _, err = e.validateACL(args.Namespace, args.Topics, resolvedACL) + return err }, } @@ -225,7 +241,26 @@ func handleJsonResultError(err error, code *int64, encoder *codec.Encoder) { }) } -func validateACL(namespace string, topics map[structs.Topic][]string, aclObj *acl.ACL) error { +// validateACL handles wildcard namespaces by replacing it with all existing namespaces +// and validates the user has the appropriate ACL to read topics in each one. +func (e *Event) validateACL(namespace string, topics map[structs.Topic][]string, resolvedAcl *acl.ACL) ([]string, error) { + nses := []string{} + if namespace == structs.AllNamespacesSentinel { + ns, _ := e.srv.State().NamespaceNames() + nses = append(nses, ns...) + } else { + nses = append(nses, namespace) + } + + for _, ns := range nses { + if err := validateNsOp(ns, topics, resolvedAcl); err != nil { + return nil, err + } + } + return nses, nil +} + +func validateNsOp(namespace string, topics map[structs.Topic][]string, aclObj *acl.ACL) error { for topic := range topics { switch topic { case structs.TopicDeployment, diff --git a/nomad/event_endpoint_test.go b/nomad/event_endpoint_test.go index f419cec25..b75941445 100644 --- a/nomad/event_endpoint_test.go +++ b/nomad/event_endpoint_test.go @@ -312,7 +312,7 @@ OUTER: } } -func TestEventStream_validateACL(t *testing.T) { +func TestEventStream_validateNsOp(t *testing.T) { ci.Parallel(t) require := require.New(t) @@ -480,12 +480,74 @@ func TestEventStream_validateACL(t *testing.T) { testACL, err := acl.NewACL(tc.Management, []*acl.Policy{p}) require.NoError(err) - err = validateACL(tc.Namespace, tc.Topics, testACL) + err = validateNsOp(tc.Namespace, tc.Topics, testACL) require.Equal(tc.ExpectedErr, err) }) } } +func TestEventStream_validateACL(t *testing.T) { + ci.Parallel(t) + + s1, _, cleanupS := TestACLServer(t, nil) + defer cleanupS() + testutil.WaitForLeader(t, s1.RPC) + + ns1 := mock.Namespace() + + err := s1.State().UpsertNamespaces(0, []*structs.Namespace{ns1}) + must.NoError(t, err) + + testEvent := &Event{srv: s1} + + t.Run("single namespace ACL errors on wildcard", func(t *testing.T) { + policy, err := acl.Parse(mock.NamespacePolicy(ns1.Name, "", []string{acl.NamespaceCapabilityReadJob})) + must.NoError(t, err) + + // does not contain policy for default NS + testAcl, err := acl.NewACL(false, []*acl.Policy{policy}) + must.NoError(t, err) + + topics := map[structs.Topic][]string{ + structs.TopicJob: {"*"}, + } + _, err = testEvent.validateACL("*", topics, testAcl) + must.Error(t, err) + }) + + t.Run("all namespace ACL succeeds on wildcard", func(t *testing.T) { + policy1, err := acl.Parse(mock.NamespacePolicy("default", "", []string{acl.NamespaceCapabilityReadJob})) + must.NoError(t, err) + policy2, err := acl.Parse(mock.NamespacePolicy(ns1.Name, "", []string{acl.NamespaceCapabilityReadJob})) + must.NoError(t, err) + + testAcl, err := acl.NewACL(false, []*acl.Policy{policy1, policy2}) + must.NoError(t, err) + + topics := map[structs.Topic][]string{ + structs.TopicJob: {"*"}, + } + nses, err := testEvent.validateACL("*", topics, testAcl) + must.NoError(t, err) + must.Eq(t, nses, []string{"default", ns1.Name}) + }) + + t.Run("single namespace ACL succeeds with correct NS", func(t *testing.T) { + policy, err := acl.Parse(mock.NamespacePolicy("default", "", []string{acl.NamespaceCapabilityReadJob})) + must.NoError(t, err) + + testAcl, err := acl.NewACL(false, []*acl.Policy{policy}) + must.NoError(t, err) + + topics := map[structs.Topic][]string{ + structs.TopicJob: {"*"}, + } + nses, err := testEvent.validateACL("default", topics, testAcl) + must.NoError(t, err) + must.Eq(t, nses, []string{"default"}) + }) +} + // TestEventStream_ACL_Update_Close_Stream asserts that an active subscription // is closed after the token is no longer valid func TestEventStream_ACL_Update_Close_Stream(t *testing.T) { diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go index 1d5375cbb..875cba715 100644 --- a/nomad/fsm_test.go +++ b/nomad/fsm_test.go @@ -3676,7 +3676,7 @@ func TestFSM_ACLEvents(t *testing.T) { Topics: map[structs.Topic][]string{ tc.reqTopic: {"*"}, }, - Namespace: "default", + Namespaces: []string{"default"}, } sub, err := broker.Subscribe(subReq) @@ -3730,7 +3730,7 @@ func TestFSM_EventBroker_JobRegisterFSMEvents(t *testing.T) { Topics: map[structs.Topic][]string{ structs.TopicJob: {"*"}, }, - Namespace: "default", + Namespaces: []string{"default"}, } sub, err := broker.Subscribe(subReq) diff --git a/nomad/state/deployment_events_test.go b/nomad/state/deployment_events_test.go index 153f185ae..2946679fb 100644 --- a/nomad/state/deployment_events_test.go +++ b/nomad/state/deployment_events_test.go @@ -97,7 +97,7 @@ func EventsForIndex(t *testing.T, s *StateStore, index uint64) []structs.Event { Topics: map[structs.Topic][]string{ "*": {"*"}, }, - Namespace: "default", + Namespaces: []string{"default"}, Index: index, StartExactlyAtIndex: true, }) diff --git a/nomad/stream/subscription.go b/nomad/stream/subscription.go index 474829988..31c5277bb 100644 --- a/nomad/stream/subscription.go +++ b/nomad/stream/subscription.go @@ -6,6 +6,7 @@ package stream import ( "context" "errors" + "slices" "sync/atomic" "github.com/hashicorp/nomad/nomad/structs" @@ -48,9 +49,9 @@ type Subscription struct { } type SubscribeRequest struct { - Token string - Index uint64 - Namespace string + Token string + Index uint64 + Namespaces []string Topics map[structs.Topic][]string @@ -130,15 +131,10 @@ func filter(req *SubscribeRequest, events []structs.Event) []structs.Event { allTopicKeys := req.Topics[structs.TopicAll] - // Return all events if subscribed to all namespaces and all topics - if req.Namespace == "*" && len(allTopicKeys) == 1 && allTopicKeys[0] == string(structs.TopicAll) { - return events - } - var result []structs.Event for _, event := range events { - if req.Namespace != "*" && event.Namespace != "" && event.Namespace != req.Namespace { + if event.Namespace != "" && !slices.Contains(req.Namespaces, event.Namespace) { continue } diff --git a/nomad/stream/subscription_test.go b/nomad/stream/subscription_test.go index db3fe72e9..1f715eff4 100644 --- a/nomad/stream/subscription_test.go +++ b/nomad/stream/subscription_test.go @@ -44,8 +44,10 @@ func TestFilter_AllKeys(t *testing.T) { func TestFilter_PartialMatch_Topic(t *testing.T) { ci.Parallel(t) - events := make([]structs.Event, 0, 5) - events = append(events, structs.Event{Topic: "Test", Key: "One"}, structs.Event{Topic: "Test", Key: "Two"}, structs.Event{Topic: "Exclude", Key: "Two"}) + event1 := structs.Event{Topic: "Test", Key: "One"} + event2 := structs.Event{Topic: "Test", Key: "Two"} + event3 := structs.Event{Topic: "Exclude", Key: "Two"} + events := []structs.Event{event1, event2, event3} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ @@ -53,7 +55,7 @@ func TestFilter_PartialMatch_Topic(t *testing.T) { }, } actual := filter(req, events) - expected := []structs.Event{{Topic: "Test", Key: "One"}, {Topic: "Test", Key: "Two"}} + expected := []structs.Event{event1, event2} require.Equal(t, expected, actual) require.Equal(t, 2, cap(actual)) @@ -62,11 +64,10 @@ func TestFilter_PartialMatch_Topic(t *testing.T) { func TestFilter_Match_TopicAll_SpecificKey(t *testing.T) { ci.Parallel(t) - events := []structs.Event{ - {Topic: "Match", Key: "Two"}, - {Topic: "NoMatch", Key: "One"}, - {Topic: "OtherMatch", Key: "Two"}, - } + event1 := structs.Event{Topic: "Match", Key: "Two"} + event2 := structs.Event{Topic: "NoMatch", Key: "One"} + event3 := structs.Event{Topic: "OtherMatch", Key: "Two"} + events := []structs.Event{event1, event2, event3} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ @@ -75,21 +76,17 @@ func TestFilter_Match_TopicAll_SpecificKey(t *testing.T) { } actual := filter(req, events) - expected := []structs.Event{ - {Topic: "Match", Key: "Two"}, - {Topic: "OtherMatch", Key: "Two"}, - } + expected := []structs.Event{event1, event3} require.Equal(t, expected, actual) } func TestFilter_Match_TopicAll_SpecificKey_Plus(t *testing.T) { ci.Parallel(t) - events := []structs.Event{ - {Topic: "FirstTwo", Key: "Two"}, - {Topic: "Test", Key: "One"}, - {Topic: "SecondTwo", Key: "Two"}, - } + event1 := structs.Event{Topic: "FirstTwo", Key: "Two"} + event2 := structs.Event{Topic: "Test", Key: "One"} + event3 := structs.Event{Topic: "SecondTwo", Key: "Two"} + events := []structs.Event{event1, event2, event3} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ @@ -99,19 +96,16 @@ func TestFilter_Match_TopicAll_SpecificKey_Plus(t *testing.T) { } actual := filter(req, events) - expected := []structs.Event{ - {Topic: "FirstTwo", Key: "Two"}, - {Topic: "Test", Key: "One"}, - {Topic: "SecondTwo", Key: "Two"}, - } + expected := []structs.Event{event1, event2, event3} require.Equal(t, expected, actual) } func TestFilter_PartialMatch_Key(t *testing.T) { ci.Parallel(t) - events := make([]structs.Event, 0, 5) - events = append(events, structs.Event{Topic: "Test", Key: "One"}, structs.Event{Topic: "Test", Key: "Two"}) + event1 := structs.Event{Topic: "Test", Key: "One"} + event2 := structs.Event{Topic: "Test", Key: "Two"} + events := []structs.Event{event1, event2} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ @@ -119,7 +113,7 @@ func TestFilter_PartialMatch_Key(t *testing.T) { }, } actual := filter(req, events) - expected := []structs.Event{{Topic: "Test", Key: "One"}} + expected := []structs.Event{event1} require.Equal(t, expected, actual) require.Equal(t, 1, cap(actual)) @@ -147,66 +141,39 @@ func TestFilter_NoMatch(t *testing.T) { func TestFilter_Namespace(t *testing.T) { ci.Parallel(t) - events := make([]structs.Event, 0, 5) - events = append(events, structs.Event{Topic: "Test", Key: "One", Namespace: "foo"}, structs.Event{Topic: "Test", Key: "Two"}, structs.Event{Topic: "Test", Key: "Two", Namespace: "bar"}) + event1 := structs.Event{Topic: "Test", Key: "One", Namespace: "foo"} + event2 := structs.Event{Topic: "Test", Key: "Two", Namespace: "foo"} + event3 := structs.Event{Topic: "Test", Key: "Two", Namespace: "bar"} + events := []structs.Event{event1, event2, event3} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ "*": {"*"}, }, - Namespace: "foo", + Namespaces: []string{"foo"}, } actual := filter(req, events) - expected := []structs.Event{ - {Topic: "Test", Key: "One", Namespace: "foo"}, - {Topic: "Test", Key: "Two"}, - } + // expect namespace "bar" to be filtered out + expected := []structs.Event{event1, event2} require.Equal(t, expected, actual) - require.Equal(t, 2, cap(actual)) } -func TestFilter_NamespaceAll(t *testing.T) { - ci.Parallel(t) - - events := make([]structs.Event, 0, 5) - events = append(events, - structs.Event{Topic: "Test", Key: "One", Namespace: "foo"}, - structs.Event{Topic: "Test", Key: "Two", Namespace: "bar"}, - structs.Event{Topic: "Test", Key: "Three", Namespace: "default"}, - ) - - req := &SubscribeRequest{ - Topics: map[structs.Topic][]string{ - "*": {"*"}, - }, - Namespace: "*", - } - actual := filter(req, events) - expected := []structs.Event{ - {Topic: "Test", Key: "One", Namespace: "foo"}, - {Topic: "Test", Key: "Two", Namespace: "bar"}, - {Topic: "Test", Key: "Three", Namespace: "default"}, - } - require.Equal(t, expected, actual) -} - func TestFilter_FilterKeys(t *testing.T) { ci.Parallel(t) - events := make([]structs.Event, 0, 5) - events = append(events, structs.Event{Topic: "Test", Key: "One", FilterKeys: []string{"extra-key"}}, structs.Event{Topic: "Test", Key: "Two"}, structs.Event{Topic: "Test", Key: "Two"}) + event1 := structs.Event{Topic: "Test", Key: "One", FilterKeys: []string{"extra-key"}} + event2 := structs.Event{Topic: "Test", Key: "Two"} + event3 := structs.Event{Topic: "Test", Key: "Two"} + events := []structs.Event{event1, event2, event3} req := &SubscribeRequest{ Topics: map[structs.Topic][]string{ "Test": {"extra-key"}, }, - Namespace: "foo", } actual := filter(req, events) - expected := []structs.Event{ - {Topic: "Test", Key: "One", FilterKeys: []string{"extra-key"}}, - } + expected := []structs.Event{event1} require.Equal(t, expected, actual) require.Equal(t, 1, cap(actual))