diff --git a/pkg/service/grantlimits_test.go b/pkg/service/grantlimits_test.go new file mode 100644 index 000000000..e97638334 --- /dev/null +++ b/pkg/service/grantlimits_test.go @@ -0,0 +1,138 @@ +// Copyright 2024 LiveKit, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/livekit/livekit-server/pkg/config" + "github.com/livekit/protocol/auth" + "github.com/livekit/protocol/livekit" + "github.com/livekit/protocol/logger" +) + +// stubRoomAllocator is a RoomAllocator whose ValidateCreateRoom always succeeds. The embedded +// interface is nil, so any other method panics, which keeps the tests honest about what the +// join path actually calls. +type stubRoomAllocator struct{ RoomAllocator } + +func (stubRoomAllocator) ValidateCreateRoom(context.Context, livekit.RoomName) error { return nil } + +func grantRequest(grant *auth.ClaimGrants) *http.Request { + r := httptest.NewRequest(http.MethodPost, "/", nil) + return r.WithContext(WithGrants(r.Context(), grant, "apikey")) +} + +// ConnectLimitCase is a join token that is over one limit. Every join path must reject it. +// It is exported so that TestStartSessionConnectLimits in package service_test runs the same cases. +type ConnectLimitCase struct { + Name string + Limit config.LimitConfig + Grant func() *auth.ClaimGrants // fresh grant per call, since a join path may change it + Err error +} + +func ConnectLimitCases() []ConnectLimitCase { + return []ConnectLimitCase{ + { + Name: "identity exceeds", + Limit: config.LimitConfig{MaxParticipantIdentityLength: 5}, + Grant: grantWith(func(g *auth.ClaimGrants) { g.Identity = "toolongidentity" }), + Err: ErrParticipantIdentityExceedsLimits, + }, + { + Name: "room name exceeds", + Limit: config.LimitConfig{MaxRoomNameLength: 3}, + Grant: grantWith(func(g *auth.ClaimGrants) { g.Video.Room = "toolongroom" }), + Err: ErrRoomNameExceedsLimits, + }, + { + Name: "name exceeds", + Limit: config.LimitConfig{MaxParticipantNameLength: 5}, + Grant: grantWith(func(g *auth.ClaimGrants) { g.Name = "toolongname" }), + Err: ErrNameExceedsLimits, + }, + { + Name: "metadata exceeds", + Limit: config.LimitConfig{MaxMetadataSize: 5}, + Grant: grantWith(func(g *auth.ClaimGrants) { g.Metadata = "abcdefg" }), + Err: ErrMetadataExceedsLimits, + }, + { + Name: "attributes exceeds", + Limit: config.LimitConfig{MaxAttributesSize: 5}, + Grant: grantWith(func(g *auth.ClaimGrants) { g.Attributes = map[string]string{"k": "abcdefg"} }), + Err: ErrAttributeExceedsLimits, + }, + } +} + +func baseGrant() *auth.ClaimGrants { + return &auth.ClaimGrants{ + Identity: "id", + Name: "name", + Video: &auth.VideoGrant{RoomJoin: true, Room: "room"}, + } +} + +func grantWith(mutate func(*auth.ClaimGrants)) func() *auth.ClaimGrants { + return func() *auth.ClaimGrants { + g := baseGrant() + mutate(g) + return g + } +} + +// TestConnectLimits checks that both HTTP join paths (WHIP ingest and the signal connection via +// ValidateConnectRequest) reject every ConnectLimitCase. +func TestConnectLimits(t *testing.T) { + for _, tc := range ConnectLimitCases() { + t.Run("WHIP/"+tc.Name, func(t *testing.T) { + svc := &WHIPService{config: &config.Config{Limit: tc.Limit}, roomAllocator: stubRoomAllocator{}} + _, code, err := svc.validateCreate(httptest.NewRecorder(), grantRequest(tc.Grant())) + require.ErrorIs(t, err, tc.Err) + require.Equal(t, http.StatusBadRequest, code) + }) + + t.Run("signal/"+tc.Name, func(t *testing.T) { + // nil router skips the node-limit check, reaching the grant checks + _, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(tc.Grant()), tc.Limit, ValidateConnectRequestParams{}, nil, stubRoomAllocator{}) + require.ErrorIs(t, err, tc.Err) + require.Equal(t, http.StatusBadRequest, code) + }) + } + + t.Run("signal/identity exceeds after publish suffix", func(t *testing.T) { + g := baseGrant() + g.Video.SetCanPublish(true) + // "id" fits a limit of 5, but the final identity "id#abcd" does not + params := ValidateConnectRequestParams{publish: "abcd"} + _, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(g), config.LimitConfig{MaxParticipantIdentityLength: 5}, params, nil, stubRoomAllocator{}) + require.ErrorIs(t, err, ErrParticipantIdentityExceedsLimits) + require.Equal(t, http.StatusBadRequest, code) + }) + + t.Run("within limits", func(t *testing.T) { + limit := config.LimitConfig{MaxParticipantIdentityLength: 5, MaxRoomNameLength: 10, MaxParticipantNameLength: 5, MaxMetadataSize: 5, MaxAttributesSize: 5} + _, code, err := ValidateConnectRequest(logger.GetLogger(), grantRequest(baseGrant()), limit, ValidateConnectRequestParams{}, nil, stubRoomAllocator{}) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + }) +} diff --git a/pkg/service/roommanager.go b/pkg/service/roommanager.go index 6a70c6b11..95bd515df 100644 --- a/pkg/service/roommanager.go +++ b/pkg/service/roommanager.go @@ -297,11 +297,8 @@ func (r *RoomManager) StartSession( sessionStartTime := time.Now() if pi.Identity != "" && pi.Grants != nil { - if !r.config.Limit.CheckMetadataSize(pi.Grants.Metadata) { - return ErrMetadataExceedsLimits - } - if !r.config.Limit.CheckAttributesSize(pi.Grants.Attributes) { - return ErrAttributeExceedsLimits + if err := checkConnectLimits(r.config.Limit, string(pi.Identity), livekit.RoomName(pi.CreateRoom.GetName()), pi.Grants); err != nil { + return err } } diff --git a/pkg/service/roommanager_test.go b/pkg/service/roommanager_test.go index a8208ff2c..b517cccd5 100644 --- a/pkg/service/roommanager_test.go +++ b/pkg/service/roommanager_test.go @@ -169,6 +169,28 @@ func TestRoomCompositeEgressOnCreateRoom(t *testing.T) { }) } +// TestStartSessionConnectLimits checks that StartSession on the RTC node rejects the same +// over-limit joins as the HTTP join paths, without relying on the caller's validation. +func TestStartSessionConnectLimits(t *testing.T) { + for _, tc := range service.ConnectLimitCases() { + t.Run(tc.Name, func(t *testing.T) { + conf, err := config.NewConfig("", true, nil, nil) + require.NoError(t, err) + conf.Limit = tc.Limit + rm := newTestRoomManager(t, conf, nil) + + g := tc.Grant() + err = rm.StartSession(context.Background(), routing.ParticipantInit{ + Identity: livekit.ParticipantIdentity(g.Identity), + Grants: g, + Client: &livekit.ClientInfo{}, + CreateRoom: &livekit.CreateRoomRequest{Name: g.Video.Room}, + }, &routingfakes.FakeMessageSource{}, &routingfakes.FakeMessageSink{}, false) + require.ErrorIs(t, err, tc.Err) + }) + } +} + const roomPresetConfig = ` room: room_configurations: diff --git a/pkg/service/utils.go b/pkg/service/utils.go index 0ee1d55bd..e4a22ba8a 100644 --- a/pkg/service/utils.go +++ b/pkg/service/utils.go @@ -366,9 +366,6 @@ func ValidateConnectRequest( if claims.Identity == "" { return res, http.StatusBadRequest, ErrIdentityEmpty } - if !limitConfig.CheckParticipantIdentityLength(claims.Identity) { - return res, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, limitConfig.MaxParticipantIdentityLength) - } if claims.RoomConfig != nil { if err := claims.RoomConfig.CheckCredentials(); err != nil { @@ -384,8 +381,30 @@ func ValidateConnectRequest( if res.roomName == "" { return res, http.StatusBadRequest, ErrNoRoomName } - if !limitConfig.CheckRoomNameLength(string(res.roomName)) { - return res, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, limitConfig.MaxRoomNameLength) + + if len(params.metadata) != 0 { + // Make sure grant has GetCanUpdateOwnMetadata set + if !claims.Video.GetCanUpdateOwnMetadata() { + return res, http.StatusUnauthorized, rtc.ErrPermissionDenied + } + claims.Metadata = params.metadata + } + + // Add extra attributes to the participant + if len(params.attributes) != 0 { + // Make sure grant has GetCanUpdateOwnMetadata set + if !claims.Video.GetCanUpdateOwnMetadata() { + return res, http.StatusUnauthorized, rtc.ErrPermissionDenied + } + if claims.Attributes == nil { + claims.Attributes = make(map[string]string, len(params.attributes)) + } + for k, v := range params.attributes { + if v == "" { + continue // do not allow deleting existing attributes + } + claims.Attributes[k] = v + } } // this is new connection for existing participant - with publish only permissions @@ -399,6 +418,11 @@ func ValidateConnectRequest( claims.Identity += "#" + params.publish } + // after the publish suffix, so the identity checked is the one StartSession sees + if err := checkConnectLimits(limitConfig, claims.Identity, res.roomName, claims); err != nil { + return res, http.StatusBadRequest, err + } + // room allocator validations err = roomAllocator.ValidateCreateRoom(r.Context(), res.roomName) if err != nil { @@ -425,36 +449,33 @@ func ValidateConnectRequest( SetRoomConfiguration(createRequest, claims.GetRoomConfiguration()) res.createRoomRequest = createRequest - if len(params.metadata) != 0 { - // Make sure grant has GetCanUpdateOwnMetadata set - if !claims.Video.GetCanUpdateOwnMetadata() { - return res, http.StatusUnauthorized, rtc.ErrPermissionDenied - } - claims.Metadata = params.metadata - } - - // Add extra attributes to the participant - if len(params.attributes) != 0 { - // Make sure grant has GetCanUpdateOwnMetadata set - if !claims.Video.GetCanUpdateOwnMetadata() { - return res, http.StatusUnauthorized, rtc.ErrPermissionDenied - } - if claims.Attributes == nil { - claims.Attributes = make(map[string]string, len(params.attributes)) - } - for k, v := range params.attributes { - if v == "" { - continue // do not allow deleting existing attributes - } - claims.Attributes[k] = v - } - } - res.grants = claims res.tokenExpiresAt = GetTokenExpiresAt(r.Context()) return res, http.StatusOK, nil } +// checkConnectLimits checks a join's identity, room name, participant name, metadata and +// attributes against the configured limits. Every join path runs it: the signal connection, +// WHIP, and StartSession on the RTC node, which does not trust the caller's validation. +func checkConnectLimits(limitConfig config.LimitConfig, identity string, roomName livekit.RoomName, grants *auth.ClaimGrants) error { + if !limitConfig.CheckParticipantIdentityLength(identity) { + return fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, limitConfig.MaxParticipantIdentityLength) + } + if !limitConfig.CheckRoomNameLength(string(roomName)) { + return fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, limitConfig.MaxRoomNameLength) + } + if !limitConfig.CheckParticipantNameLength(grants.Name) { + return fmt.Errorf("%w: max length %d", ErrNameExceedsLimits, limitConfig.MaxParticipantNameLength) + } + if !limitConfig.CheckMetadataSize(grants.Metadata) { + return fmt.Errorf("%w: max size %d", ErrMetadataExceedsLimits, limitConfig.MaxMetadataSize) + } + if !limitConfig.CheckAttributesSize(grants.Attributes) { + return fmt.Errorf("%w: max size %d", ErrAttributeExceedsLimits, limitConfig.MaxAttributesSize) + } + return nil +} + func IsRTCPath(path string) bool { return path == "/rtc" || path == "/rtc/v1" } diff --git a/pkg/service/whipservice.go b/pkg/service/whipservice.go index 4cdedf333..48ee38466 100644 --- a/pkg/service/whipservice.go +++ b/pkg/service/whipservice.go @@ -131,15 +131,12 @@ func (s *WHIPService) validateCreate(w http.ResponseWriter, r *http.Request) (*c if roomName == "" { return nil, http.StatusUnauthorized, errors.New("room name cannot be empty") } - if !s.config.Limit.CheckRoomNameLength(string(roomName)) { - return nil, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, s.config.Limit.MaxRoomNameLength) - } - if claims.Identity == "" { return nil, http.StatusBadRequest, ErrIdentityEmpty } - if !s.config.Limit.CheckParticipantIdentityLength(claims.Identity) { - return nil, http.StatusBadRequest, fmt.Errorf("%w: max length %d", ErrParticipantIdentityExceedsLimits, s.config.Limit.MaxParticipantIdentityLength) + + if err := checkConnectLimits(s.config.Limit, claims.Identity, roomName, claims); err != nil { + return nil, http.StatusBadRequest, err } if err := s.roomAllocator.ValidateCreateRoom(r.Context(), roomName); err != nil {