From 09e3aef859e8a114fdf030d3fc47066ea52f0af3 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Fri, 12 Jul 2024 09:57:17 +0530 Subject: [PATCH] Check size limits on metadata and name set from client. (#2850) * Send error response when update metadata fails. Keeping it simple for the first implementation. - Send error response only if request_id != 0 - Two kinds of errors notified o does not have permissions - NOT_ALLOWED o attributes exceeds size limits - INVALID_ARGUMENT * Check size limits on metadata and name set from client. Added a name length limit also. * check name length in service update participant path also * limit check in limit config * update protocol * longer keys --- go.mod | 12 +- go.sum | 28 ++-- pkg/config/config.go | 26 ++++ pkg/rtc/errors.go | 4 +- pkg/rtc/participant.go | 40 +++--- pkg/rtc/room.go | 20 --- pkg/rtc/signalhandler.go | 30 ++++- pkg/rtc/types/interfaces.go | 4 +- .../typesfakes/fake_local_participant.go | 121 ++++++++++++------ pkg/rtc/types/typesfakes/fake_room.go | 80 ------------ pkg/service/auth_test.go | 2 +- pkg/service/errors.go | 1 + pkg/service/roommanager.go | 18 ++- pkg/service/roomservice.go | 26 ++-- test/integration_helpers.go | 2 +- 15 files changed, 208 insertions(+), 206 deletions(-) diff --git a/go.mod b/go.mod index e26348d5c..506815a88 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/jxskiss/base62 v1.1.0 github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 github.com/livekit/mediatransportutil v0.0.0-20240625074155-301bb4a816b7 - github.com/livekit/protocol v1.19.2-0.20240708223954-14b61d08b08f + github.com/livekit/protocol v1.19.2-0.20240711102651-18c869b1089e github.com/livekit/psrpc v0.5.3-0.20240616012458-ac39c8549a0a github.com/mackerelio/go-osstat v0.2.5 github.com/magefile/mage v1.15.0 @@ -40,7 +40,7 @@ require ( github.com/pion/webrtc/v3 v3.2.44 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.19.1 - github.com/redis/go-redis/v9 v9.5.3 + github.com/redis/go-redis/v9 v9.5.4 github.com/rs/cors v1.11.0 github.com/stretchr/testify v1.9.0 github.com/thoas/go-funk v0.9.3 @@ -81,7 +81,7 @@ require ( github.com/eapache/channels v1.1.0 // indirect github.com/eapache/queue v1.1.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect - github.com/go-jose/go-jose/v3 v3.0.3 // indirect + github.com/go-jose/go-jose/v4 v4.0.3 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/cel-go v0.20.1 // indirect @@ -134,8 +134,8 @@ require ( golang.org/x/sys v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect golang.org/x/tools v0.23.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/grpc v1.64.1 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d // indirect + google.golang.org/grpc v1.65.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index e2cc22cdb..f85079a50 100644 --- a/go.sum +++ b/go.sum @@ -80,8 +80,8 @@ github.com/gammazero/deque v0.2.1 h1:qSdsbG6pgp6nL7A0+K/B7s12mcCY/5l5SIUpMOl+dC0 github.com/gammazero/deque v0.2.1/go.mod h1:LFroj8x4cMYCukHJDbxFCkT+r9AndaJnFMuZDV34tuU= github.com/gammazero/workerpool v1.1.3 h1:WixN4xzukFoN0XSeXF6puqEqFTl2mECI9S6W44HWy9Q= github.com/gammazero/workerpool v1.1.3/go.mod h1:wPjyBLDbyKnUn2XwwyD3EEwo9dHutia9/fwNmSHWACc= -github.com/go-jose/go-jose/v3 v3.0.3 h1:fFKWeig/irsp7XD2zBxvnmA/XaRWp5V3CBsZXJF7G7k= -github.com/go-jose/go-jose/v3 v3.0.3/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ= +github.com/go-jose/go-jose/v4 v4.0.3 h1:o8aphO8Hv6RPmH+GfzVuyf7YXSBibp+8YyHdOoDESGo= +github.com/go-jose/go-jose/v4 v4.0.3/go.mod h1:NKb5HO1EZccyMpiZNbdUw/14tiXNyUJh188dfnMCAfc= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfCHuOE= @@ -99,7 +99,6 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= -github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= @@ -167,8 +166,8 @@ github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 h1:jm09419p0lqTkD github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1/go.mod h1:Rs3MhFwutWhGwmY1VQsygw28z5bWcnEYmS1OG9OxjOQ= github.com/livekit/mediatransportutil v0.0.0-20240625074155-301bb4a816b7 h1:F1L8inJoynwIAYpZENNYS+1xHJMF5RFRorsnAlcxfSY= github.com/livekit/mediatransportutil v0.0.0-20240625074155-301bb4a816b7/go.mod h1:jwKUCmObuiEDH0iiuJHaGMXwRs3RjrB4G6qqgkr/5oE= -github.com/livekit/protocol v1.19.2-0.20240708223954-14b61d08b08f h1:LRbONV5dyxcf+YWDEhsyxIMCXZQe/xW/Bc2mhU9TnS0= -github.com/livekit/protocol v1.19.2-0.20240708223954-14b61d08b08f/go.mod h1:bNjJi+8frdvC84xG0CJ/7VfVvqerLg2MzjOks0ucyC4= +github.com/livekit/protocol v1.19.2-0.20240711102651-18c869b1089e h1:Rl45RfvzP3FtPyrLU/rsLQNTnwEi5L/OKD5xGVxHbl0= +github.com/livekit/protocol v1.19.2-0.20240711102651-18c869b1089e/go.mod h1:Ca0jA/Tk9sHIIFRQhwIjOPMOM0Foh0psYmDBjgIQa4g= github.com/livekit/psrpc v0.5.3-0.20240616012458-ac39c8549a0a h1:EQAHmcYEGlc6V517cQ3Iy0+jHgP6+tM/B4l2vGuLpQo= github.com/livekit/psrpc v0.5.3-0.20240616012458-ac39c8549a0a/go.mod h1:CQUBSPfYYAaevg1TNCc6/aYsa8DJH4jSRFdCeSZk5u0= github.com/mackerelio/go-osstat v0.2.5 h1:+MqTbZUhoIt4m8qzkVoXUJg1EuifwlAJSk4Yl2GXh+o= @@ -288,8 +287,8 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/puzpuzpuz/xsync/v3 v3.1.0 h1:EewKT7/LNac5SLiEblJeUu8z5eERHrmRLnMQL2d7qX4= github.com/puzpuzpuz/xsync/v3 v3.1.0/go.mod h1:VjzYrABPabuM4KyBh1Ftq6u8nhwY5tBPKP9jpmh0nnA= -github.com/redis/go-redis/v9 v9.5.3 h1:fOAp1/uJG+ZtcITgZOfYFmTKPE7n4Vclj1wZFgRciUU= -github.com/redis/go-redis/v9 v9.5.3/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M= +github.com/redis/go-redis/v9 v9.5.4 h1:vOFYDKKVgrI5u++QvnMT7DksSMYg7Aw/Np4vLJLKLwY= +github.com/redis/go-redis/v9 v9.5.4/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= @@ -362,7 +361,6 @@ golang.org/x/crypto v0.11.0/go.mod h1:xgJhtzW8F9jGdVFWZESrid1U1bjeNy4zgy5cRr/CIi golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw= golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg= -golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= golang.org/x/exp v0.0.0-20240707233637-46b078467d37 h1:uLDX+AfeFCct3a2C7uIWBKMJIR3CJMhcgfrUAqjRK6w= @@ -451,7 +449,6 @@ golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -464,7 +461,6 @@ golang.org/x/term v0.10.0/go.mod h1:lpqdcUyK/oCiQxvxVrppt5ggO2KCZ5QblwqPnfZ6d5o= golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU= golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -492,12 +488,12 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda h1:b6F6WIV4xHHD0FA4oIyzU6mHWg2WI2X1RBehwa5QN38= -google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda/go.mod h1:AHcE/gZH76Bk/ROZhQphlRoWo5xKDEtz3eVEO1LfA8c= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda h1:LI5DOvAxUPMv/50agcLLoo+AdWc1irS9Rzz4vPuD1V4= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY= -google.golang.org/grpc v1.64.1 h1:LKtvyfbX3UGVPFcGqJ9ItpVWW6oN/2XqTxfAnwRRXiA= -google.golang.org/grpc v1.64.1/go.mod h1:hiQF4LFZelK2WKaP6W0L92zGHtiQdZxk8CrSdvyjeP0= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 h1:7whR9kGa5LUwFtpLm2ArCEejtnxlGeLbAyjFY8sGNFw= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d h1:JU0iKnSg02Gmb5ZdV8nYsKEKsP6o/FGVWTrw4i1DA9A= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= +google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc= +google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= diff --git a/pkg/config/config.go b/pkg/config/config.go index 3f9dd31aa..c420d2685 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -332,6 +332,31 @@ type LimitConfig struct { MaxAttributesSize uint32 `yaml:"max_attributes_size,omitempty"` MaxRoomNameLength int `yaml:"max_room_name_length,omitempty"` MaxParticipantIdentityLength int `yaml:"max_participant_identity_length,omitempty"` + MaxParticipantNameLength int `yaml:"max_participant_name_length,omitempty"` +} + +func (l LimitConfig) CheckRoomNameLength(name string) bool { + return l.MaxRoomNameLength == 0 || len(name) <= l.MaxRoomNameLength +} + +func (l LimitConfig) CheckParticipantNameLength(name string) bool { + return l.MaxParticipantNameLength == 0 || len(name) <= l.MaxParticipantNameLength +} + +func (l LimitConfig) CheckMetadataSize(metadata string) bool { + return l.MaxMetadataSize == 0 || uint32(len(metadata)) <= l.MaxMetadataSize +} + +func (l LimitConfig) CheckAttributesSize(attributes map[string]string) bool { + if l.MaxAttributesSize == 0 { + return true + } + + total := 0 + for k, v := range attributes { + total += len(k) + len(v) + } + return uint32(total) <= l.MaxAttributesSize } type IngressConfig struct { @@ -540,6 +565,7 @@ var DefaultConfig = Config{ MaxAttributesSize: 64000, MaxRoomNameLength: 256, MaxParticipantIdentityLength: 256, + MaxParticipantNameLength: 256, }, Logging: LoggingConfig{ PionLevel: "error", diff --git a/pkg/rtc/errors.go b/pkg/rtc/errors.go index 08e8ef33e..27bf936c3 100644 --- a/pkg/rtc/errors.go +++ b/pkg/rtc/errors.go @@ -31,7 +31,9 @@ var ( ErrEmptyParticipantID = errors.New("participant ID cannot be empty") ErrMissingGrants = errors.New("VideoGrant is missing") ErrInternalError = errors.New("internal error") - ErrAttributeExceedsLimits = errors.New("attribute size exceeds limits") + ErrNameExceedsLimits = errors.New("name length exceeds limits") + ErrMetadataExceedsLimits = errors.New("metadata size exceeds limits") + ErrAttributesExceedsLimits = errors.New("attributes size exceeds limits") // Track subscription related ErrNoTrackPermission = errors.New("participant is not allowed to subscribe to this track") diff --git a/pkg/rtc/participant.go b/pkg/rtc/participant.go index b1d8a1965..e8248d0b0 100644 --- a/pkg/rtc/participant.go +++ b/pkg/rtc/participant.go @@ -104,6 +104,7 @@ type ParticipantParams struct { Sink routing.MessageSink AudioConfig config.AudioConfig VideoConfig config.VideoConfig + LimitConfig config.LimitConfig ProtocolVersion types.ProtocolVersion SessionStartTime time.Time Telemetry telemetry.TelemetryService @@ -136,7 +137,6 @@ type ParticipantParams struct { VersionGenerator utils.TimedVersionGenerator TrackResolver types.MediaTrackResolver DisableDynacast bool - MaxAttributesSize uint32 SubscriberAllowPause bool SubscriptionLimitAudio int32 SubscriptionLimitVideo int32 @@ -407,6 +407,27 @@ func (p *ParticipantImpl) GetBufferFactory() *buffer.Factory { return p.params.Config.BufferFactory } +// CheckMetadataLimits check if name/metadata/attributes of a participant is within configured limits +func (p *ParticipantImpl) CheckMetadataLimits( + name string, + metadata string, + attributes map[string]string, +) error { + if !p.params.LimitConfig.CheckParticipantNameLength(name) { + return ErrNameExceedsLimits + } + + if !p.params.LimitConfig.CheckMetadataSize(metadata) { + return ErrMetadataExceedsLimits + } + + if !p.params.LimitConfig.CheckAttributesSize(attributes) { + return ErrAttributesExceedsLimits + } + + return nil +} + // SetName attaches name to the participant func (p *ParticipantImpl) SetName(name string) { p.lock.Lock() @@ -460,9 +481,9 @@ func (p *ParticipantImpl) SetMetadata(metadata string) { } } -func (p *ParticipantImpl) SetAttributes(attrs map[string]string) error { +func (p *ParticipantImpl) SetAttributes(attrs map[string]string) { if len(attrs) == 0 { - return nil + return } p.lock.Lock() grants := p.grants.Load().Clone() @@ -481,18 +502,6 @@ func (p *ParticipantImpl) SetAttributes(attrs map[string]string) error { delete(grants.Attributes, k) } - maxAttributesSize := p.params.MaxAttributesSize - if maxAttributesSize > 0 { - total := 0 - for k, v := range grants.Attributes { - total += len(k) + len(v) - } - if uint32(total) > maxAttributesSize { - p.lock.Unlock() - return ErrAttributeExceedsLimits - } - } - p.grants.Store(grants) p.requireBroadcast = true // already checked above p.dirty.Store(true) @@ -507,7 +516,6 @@ func (p *ParticipantImpl) SetAttributes(attrs map[string]string) error { if onClaimsChanged != nil { onClaimsChanged(p) } - return nil } func (p *ParticipantImpl) ClaimGrants() *auth.ClaimGrants { diff --git a/pkg/rtc/room.go b/pkg/rtc/room.go index 27c9a3856..3671f1b0a 100644 --- a/pkg/rtc/room.go +++ b/pkg/rtc/room.go @@ -819,26 +819,6 @@ func (r *Room) SetMetadata(metadata string) <-chan struct{} { return r.protoProxy.MarkDirty(true) } -func (r *Room) UpdateParticipantMetadata( - participant types.LocalParticipant, - name string, - metadata string, - attributes map[string]string, -) error { - if attributes != nil && len(attributes) > 0 { - if err := participant.SetAttributes(attributes); err != nil { - return err - } - } - if metadata != "" { - participant.SetMetadata(metadata) - } - if name != "" { - participant.SetName(name) - } - return nil -} - func (r *Room) sendRoomUpdate() { roomInfo := r.ToProto() // Send update to participants diff --git a/pkg/rtc/signalhandler.go b/pkg/rtc/signalhandler.go index bac5181e6..dd808ef9a 100644 --- a/pkg/rtc/signalhandler.go +++ b/pkg/rtc/signalhandler.go @@ -94,19 +94,37 @@ func HandleParticipantSignal(room types.Room, participant types.LocalParticipant case *livekit.SignalRequest_UpdateMetadata: var errorResponse *livekit.ErrorResponse if participant.ClaimGrants().Video.GetCanUpdateOwnMetadata() { - err := room.UpdateParticipantMetadata( - participant, + if err := participant.CheckMetadataLimits( msg.UpdateMetadata.Name, msg.UpdateMetadata.Metadata, msg.UpdateMetadata.Attributes, - ) - if err != nil { + ); err == nil { + if msg.UpdateMetadata.Name != "" { + participant.SetName(msg.UpdateMetadata.Name) + } + if msg.UpdateMetadata.Metadata != "" { + participant.SetMetadata(msg.UpdateMetadata.Metadata) + } + if msg.UpdateMetadata.Attributes != nil { + participant.SetAttributes(msg.UpdateMetadata.Attributes) + } + } else { pLogger.Warnw("could not update metadata", err) switch err { - case ErrAttributeExceedsLimits: + case ErrNameExceedsLimits: errorResponse = &livekit.ErrorResponse{ - Reason: livekit.ErrorResponse_INVALID_ARGUMENT, + Reason: livekit.ErrorResponse_LIMIT_EXCEEDED, + Message: "exceeds name length limit", + } + case ErrMetadataExceedsLimits: + errorResponse = &livekit.ErrorResponse{ + Reason: livekit.ErrorResponse_LIMIT_EXCEEDED, + Message: "exceeds metadata size limit", + } + case ErrAttributesExceedsLimits: + errorResponse = &livekit.ErrorResponse{ + Reason: livekit.ErrorResponse_LIMIT_EXCEEDED, Message: "exceeds attributes size limit", } } diff --git a/pkg/rtc/types/interfaces.go b/pkg/rtc/types/interfaces.go index 73e656a99..04568f1e0 100644 --- a/pkg/rtc/types/interfaces.go +++ b/pkg/rtc/types/interfaces.go @@ -328,9 +328,10 @@ type LocalParticipant interface { HandleSignalSourceClose() // updates + CheckMetadataLimits(name string, metadata string, attributes map[string]string) error SetName(name string) SetMetadata(metadata string) - SetAttributes(attributes map[string]string) error + SetAttributes(attributes map[string]string) // permissions ClaimGrants() *auth.ClaimGrants @@ -441,7 +442,6 @@ type Room interface { SimulateScenario(participant LocalParticipant, scenario *livekit.SimulateScenario) error ResolveMediaTrackForSubscriber(subIdentity livekit.ParticipantIdentity, trackID livekit.TrackID) MediaResolverResult GetLocalParticipants() []LocalParticipant - UpdateParticipantMetadata(participant LocalParticipant, name string, metadata string, attributes map[string]string) error } // MediaTrack represents a media track diff --git a/pkg/rtc/types/typesfakes/fake_local_participant.go b/pkg/rtc/types/typesfakes/fake_local_participant.go index 30faa1122..233c013e3 100644 --- a/pkg/rtc/types/typesfakes/fake_local_participant.go +++ b/pkg/rtc/types/typesfakes/fake_local_participant.go @@ -110,6 +110,19 @@ type FakeLocalParticipant struct { canSubscribeReturnsOnCall map[int]struct { result1 bool } + CheckMetadataLimitsStub func(string, string, map[string]string) error + checkMetadataLimitsMutex sync.RWMutex + checkMetadataLimitsArgsForCall []struct { + arg1 string + arg2 string + arg3 map[string]string + } + checkMetadataLimitsReturns struct { + result1 error + } + checkMetadataLimitsReturnsOnCall map[int]struct { + result1 error + } ClaimGrantsStub func() *auth.ClaimGrants claimGrantsMutex sync.RWMutex claimGrantsArgsForCall []struct { @@ -744,17 +757,11 @@ type FakeLocalParticipant struct { sendSpeakerUpdateReturnsOnCall map[int]struct { result1 error } - SetAttributesStub func(map[string]string) error + SetAttributesStub func(map[string]string) setAttributesMutex sync.RWMutex setAttributesArgsForCall []struct { arg1 map[string]string } - setAttributesReturns struct { - result1 error - } - setAttributesReturnsOnCall map[int]struct { - result1 error - } SetICEConfigStub func(*livekit.ICEConfig) setICEConfigMutex sync.RWMutex setICEConfigArgsForCall []struct { @@ -1491,6 +1498,69 @@ func (fake *FakeLocalParticipant) CanSubscribeReturnsOnCall(i int, result1 bool) }{result1} } +func (fake *FakeLocalParticipant) CheckMetadataLimits(arg1 string, arg2 string, arg3 map[string]string) error { + fake.checkMetadataLimitsMutex.Lock() + ret, specificReturn := fake.checkMetadataLimitsReturnsOnCall[len(fake.checkMetadataLimitsArgsForCall)] + fake.checkMetadataLimitsArgsForCall = append(fake.checkMetadataLimitsArgsForCall, struct { + arg1 string + arg2 string + arg3 map[string]string + }{arg1, arg2, arg3}) + stub := fake.CheckMetadataLimitsStub + fakeReturns := fake.checkMetadataLimitsReturns + fake.recordInvocation("CheckMetadataLimits", []interface{}{arg1, arg2, arg3}) + fake.checkMetadataLimitsMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeLocalParticipant) CheckMetadataLimitsCallCount() int { + fake.checkMetadataLimitsMutex.RLock() + defer fake.checkMetadataLimitsMutex.RUnlock() + return len(fake.checkMetadataLimitsArgsForCall) +} + +func (fake *FakeLocalParticipant) CheckMetadataLimitsCalls(stub func(string, string, map[string]string) error) { + fake.checkMetadataLimitsMutex.Lock() + defer fake.checkMetadataLimitsMutex.Unlock() + fake.CheckMetadataLimitsStub = stub +} + +func (fake *FakeLocalParticipant) CheckMetadataLimitsArgsForCall(i int) (string, string, map[string]string) { + fake.checkMetadataLimitsMutex.RLock() + defer fake.checkMetadataLimitsMutex.RUnlock() + argsForCall := fake.checkMetadataLimitsArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeLocalParticipant) CheckMetadataLimitsReturns(result1 error) { + fake.checkMetadataLimitsMutex.Lock() + defer fake.checkMetadataLimitsMutex.Unlock() + fake.CheckMetadataLimitsStub = nil + fake.checkMetadataLimitsReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeLocalParticipant) CheckMetadataLimitsReturnsOnCall(i int, result1 error) { + fake.checkMetadataLimitsMutex.Lock() + defer fake.checkMetadataLimitsMutex.Unlock() + fake.CheckMetadataLimitsStub = nil + if fake.checkMetadataLimitsReturnsOnCall == nil { + fake.checkMetadataLimitsReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.checkMetadataLimitsReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeLocalParticipant) ClaimGrants() *auth.ClaimGrants { fake.claimGrantsMutex.Lock() ret, specificReturn := fake.claimGrantsReturnsOnCall[len(fake.claimGrantsArgsForCall)] @@ -4964,23 +5034,17 @@ func (fake *FakeLocalParticipant) SendSpeakerUpdateReturnsOnCall(i int, result1 }{result1} } -func (fake *FakeLocalParticipant) SetAttributes(arg1 map[string]string) error { +func (fake *FakeLocalParticipant) SetAttributes(arg1 map[string]string) { fake.setAttributesMutex.Lock() - ret, specificReturn := fake.setAttributesReturnsOnCall[len(fake.setAttributesArgsForCall)] fake.setAttributesArgsForCall = append(fake.setAttributesArgsForCall, struct { arg1 map[string]string }{arg1}) stub := fake.SetAttributesStub - fakeReturns := fake.setAttributesReturns fake.recordInvocation("SetAttributes", []interface{}{arg1}) fake.setAttributesMutex.Unlock() if stub != nil { - return stub(arg1) + fake.SetAttributesStub(arg1) } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 } func (fake *FakeLocalParticipant) SetAttributesCallCount() int { @@ -4989,7 +5053,7 @@ func (fake *FakeLocalParticipant) SetAttributesCallCount() int { return len(fake.setAttributesArgsForCall) } -func (fake *FakeLocalParticipant) SetAttributesCalls(stub func(map[string]string) error) { +func (fake *FakeLocalParticipant) SetAttributesCalls(stub func(map[string]string)) { fake.setAttributesMutex.Lock() defer fake.setAttributesMutex.Unlock() fake.SetAttributesStub = stub @@ -5002,29 +5066,6 @@ func (fake *FakeLocalParticipant) SetAttributesArgsForCall(i int) map[string]str return argsForCall.arg1 } -func (fake *FakeLocalParticipant) SetAttributesReturns(result1 error) { - fake.setAttributesMutex.Lock() - defer fake.setAttributesMutex.Unlock() - fake.SetAttributesStub = nil - fake.setAttributesReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeLocalParticipant) SetAttributesReturnsOnCall(i int, result1 error) { - fake.setAttributesMutex.Lock() - defer fake.setAttributesMutex.Unlock() - fake.SetAttributesStub = nil - if fake.setAttributesReturnsOnCall == nil { - fake.setAttributesReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.setAttributesReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeLocalParticipant) SetICEConfig(arg1 *livekit.ICEConfig) { fake.setICEConfigMutex.Lock() fake.setICEConfigArgsForCall = append(fake.setICEConfigArgsForCall, struct { @@ -6575,6 +6616,8 @@ func (fake *FakeLocalParticipant) Invocations() map[string][][]interface{} { defer fake.canSkipBroadcastMutex.RUnlock() fake.canSubscribeMutex.RLock() defer fake.canSubscribeMutex.RUnlock() + fake.checkMetadataLimitsMutex.RLock() + defer fake.checkMetadataLimitsMutex.RUnlock() fake.claimGrantsMutex.RLock() defer fake.claimGrantsMutex.RUnlock() fake.closeMutex.RLock() diff --git a/pkg/rtc/types/typesfakes/fake_room.go b/pkg/rtc/types/typesfakes/fake_room.go index 503bf73ad..576e8ddcf 100644 --- a/pkg/rtc/types/typesfakes/fake_room.go +++ b/pkg/rtc/types/typesfakes/fake_room.go @@ -82,20 +82,6 @@ type FakeRoom struct { syncStateReturnsOnCall map[int]struct { result1 error } - UpdateParticipantMetadataStub func(types.LocalParticipant, string, string, map[string]string) error - updateParticipantMetadataMutex sync.RWMutex - updateParticipantMetadataArgsForCall []struct { - arg1 types.LocalParticipant - arg2 string - arg3 string - arg4 map[string]string - } - updateParticipantMetadataReturns struct { - result1 error - } - updateParticipantMetadataReturnsOnCall map[int]struct { - result1 error - } UpdateSubscriptionPermissionStub func(types.LocalParticipant, *livekit.SubscriptionPermission) error updateSubscriptionPermissionMutex sync.RWMutex updateSubscriptionPermissionArgsForCall []struct { @@ -499,70 +485,6 @@ func (fake *FakeRoom) SyncStateReturnsOnCall(i int, result1 error) { }{result1} } -func (fake *FakeRoom) UpdateParticipantMetadata(arg1 types.LocalParticipant, arg2 string, arg3 string, arg4 map[string]string) error { - fake.updateParticipantMetadataMutex.Lock() - ret, specificReturn := fake.updateParticipantMetadataReturnsOnCall[len(fake.updateParticipantMetadataArgsForCall)] - fake.updateParticipantMetadataArgsForCall = append(fake.updateParticipantMetadataArgsForCall, struct { - arg1 types.LocalParticipant - arg2 string - arg3 string - arg4 map[string]string - }{arg1, arg2, arg3, arg4}) - stub := fake.UpdateParticipantMetadataStub - fakeReturns := fake.updateParticipantMetadataReturns - fake.recordInvocation("UpdateParticipantMetadata", []interface{}{arg1, arg2, arg3, arg4}) - fake.updateParticipantMetadataMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3, arg4) - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeRoom) UpdateParticipantMetadataCallCount() int { - fake.updateParticipantMetadataMutex.RLock() - defer fake.updateParticipantMetadataMutex.RUnlock() - return len(fake.updateParticipantMetadataArgsForCall) -} - -func (fake *FakeRoom) UpdateParticipantMetadataCalls(stub func(types.LocalParticipant, string, string, map[string]string) error) { - fake.updateParticipantMetadataMutex.Lock() - defer fake.updateParticipantMetadataMutex.Unlock() - fake.UpdateParticipantMetadataStub = stub -} - -func (fake *FakeRoom) UpdateParticipantMetadataArgsForCall(i int) (types.LocalParticipant, string, string, map[string]string) { - fake.updateParticipantMetadataMutex.RLock() - defer fake.updateParticipantMetadataMutex.RUnlock() - argsForCall := fake.updateParticipantMetadataArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 -} - -func (fake *FakeRoom) UpdateParticipantMetadataReturns(result1 error) { - fake.updateParticipantMetadataMutex.Lock() - defer fake.updateParticipantMetadataMutex.Unlock() - fake.UpdateParticipantMetadataStub = nil - fake.updateParticipantMetadataReturns = struct { - result1 error - }{result1} -} - -func (fake *FakeRoom) UpdateParticipantMetadataReturnsOnCall(i int, result1 error) { - fake.updateParticipantMetadataMutex.Lock() - defer fake.updateParticipantMetadataMutex.Unlock() - fake.UpdateParticipantMetadataStub = nil - if fake.updateParticipantMetadataReturnsOnCall == nil { - fake.updateParticipantMetadataReturnsOnCall = make(map[int]struct { - result1 error - }) - } - fake.updateParticipantMetadataReturnsOnCall[i] = struct { - result1 error - }{result1} -} - func (fake *FakeRoom) UpdateSubscriptionPermission(arg1 types.LocalParticipant, arg2 *livekit.SubscriptionPermission) error { fake.updateSubscriptionPermissionMutex.Lock() ret, specificReturn := fake.updateSubscriptionPermissionReturnsOnCall[len(fake.updateSubscriptionPermissionArgsForCall)] @@ -687,8 +609,6 @@ func (fake *FakeRoom) Invocations() map[string][][]interface{} { defer fake.simulateScenarioMutex.RUnlock() fake.syncStateMutex.RLock() defer fake.syncStateMutex.RUnlock() - fake.updateParticipantMetadataMutex.RLock() - defer fake.updateParticipantMetadataMutex.RUnlock() fake.updateSubscriptionPermissionMutex.RLock() defer fake.updateSubscriptionPermissionMutex.RUnlock() fake.updateSubscriptionsMutex.RLock() diff --git a/pkg/service/auth_test.go b/pkg/service/auth_test.go index 8a70d6a18..00b89bc48 100644 --- a/pkg/service/auth_test.go +++ b/pkg/service/auth_test.go @@ -29,7 +29,7 @@ import ( func TestAuthMiddleware(t *testing.T) { api := "APIabcdefg" - secret := "somesecretencodedinbase62" + secret := "somesecretencodedinbase62extendto32bytes" provider := &authfakes.FakeKeyProvider{} provider.GetSecretReturns(secret) diff --git a/pkg/service/errors.go b/pkg/service/errors.go index 4a5f4d37d..d208e434c 100644 --- a/pkg/service/errors.go +++ b/pkg/service/errors.go @@ -25,6 +25,7 @@ var ( ErrIngressNotConnected = psrpc.NewErrorf(psrpc.Internal, "ingress not connected (redis required)") ErrIngressNotFound = psrpc.NewErrorf(psrpc.NotFound, "ingress does not exist") ErrIngressNonReusable = psrpc.NewErrorf(psrpc.InvalidArgument, "ingress is not reusable and cannot be modified") + ErrNameExceedsLimits = psrpc.NewErrorf(psrpc.InvalidArgument, "name length exceeds limits") ErrMetadataExceedsLimits = psrpc.NewErrorf(psrpc.InvalidArgument, "metadata size exceeds limits") ErrAttributeExceedsLimits = psrpc.NewErrorf(psrpc.InvalidArgument, "attribute size exceeds limits") ErrRoomNameExceedsLimits = psrpc.NewErrorf(psrpc.InvalidArgument, "room name length exceeds limits") diff --git a/pkg/service/roommanager.go b/pkg/service/roommanager.go index 3ac7520de..5d0ebc025 100644 --- a/pkg/service/roommanager.go +++ b/pkg/service/roommanager.go @@ -417,6 +417,7 @@ func (r *RoomManager) StartSession( Sink: responseSink, AudioConfig: r.config.Audio, VideoConfig: r.config.Video, + LimitConfig: r.config.Limit, ProtocolVersion: pv, SessionStartTime: sessionStartTime, Telemetry: r.telemetry, @@ -433,7 +434,6 @@ func (r *RoomManager) StartSession( AdaptiveStream: pi.AdaptiveStream, AllowTCPFallback: allowFallback, TURNSEnabled: r.config.IsTURNSEnabled(), - MaxAttributesSize: r.config.Limit.MaxAttributesSize, GetParticipantInfo: func(pID livekit.ParticipantID) *livekit.ParticipantInfo { if p := room.GetParticipantByID(pID); p != nil { return p.ToProto() @@ -703,7 +703,7 @@ func (r *RoomManager) MutePublishedTrack(ctx context.Context, req *livekit.MuteR } func (r *RoomManager) UpdateParticipant(ctx context.Context, req *livekit.UpdateParticipantRequest) (*livekit.ParticipantInfo, error) { - room, participant, err := r.roomAndParticipantForReq(ctx, req) + _, participant, err := r.roomAndParticipantForReq(ctx, req) if err != nil { return nil, err } @@ -713,10 +713,20 @@ func (r *RoomManager) UpdateParticipant(ctx context.Context, req *livekit.Update "permission", req.Permission, "attributes", req.Attributes, ) - err = room.UpdateParticipantMetadata(participant, req.Name, req.Metadata, req.Attributes) - if err != nil { + if err = participant.CheckMetadataLimits(req.Name, req.Metadata, req.Attributes); err != nil { return nil, err } + + if req.Name != "" { + participant.SetName(req.Name) + } + if req.Metadata != "" { + participant.SetMetadata(req.Metadata) + } + if req.Attributes != nil { + participant.SetAttributes(req.Attributes) + } + if req.Permission != nil { participant.SetPermission(req.Permission) } diff --git a/pkg/service/roomservice.go b/pkg/service/roomservice.go index d7b7b9d7e..9aacef06f 100644 --- a/pkg/service/roomservice.go +++ b/pkg/service/roomservice.go @@ -86,8 +86,8 @@ func (s *RoomService) CreateRoom(ctx context.Context, req *livekit.CreateRoomReq return nil, ErrEgressNotConnected } - if limit := s.limitConf.MaxRoomNameLength; limit > 0 && len(req.Name) > limit { - return nil, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, limit) + if !s.limitConf.CheckRoomNameLength(req.Name) { + return nil, fmt.Errorf("%w: max length %d", ErrRoomNameExceedsLimits, s.limitConf.MaxRoomNameLength) } rm, created, err := s.roomAllocator.CreateRoom(ctx, req) @@ -248,19 +248,17 @@ func (s *RoomService) MutePublishedTrack(ctx context.Context, req *livekit.MuteR func (s *RoomService) UpdateParticipant(ctx context.Context, req *livekit.UpdateParticipantRequest) (*livekit.ParticipantInfo, error) { AppendLogFields(ctx, "room", req.Room, "participant", req.Identity) - maxMetadataSize := int(s.limitConf.MaxMetadataSize) - if maxMetadataSize > 0 && len(req.Metadata) > maxMetadataSize { - return nil, twirp.InvalidArgumentError(ErrMetadataExceedsLimits.Error(), strconv.Itoa(maxMetadataSize)) + + if !s.limitConf.CheckParticipantNameLength(req.Name) { + return nil, twirp.InvalidArgumentError(ErrNameExceedsLimits.Error(), strconv.Itoa(s.limitConf.MaxParticipantNameLength)) } - maxAttributeSize := int(s.limitConf.MaxAttributesSize) - if maxAttributeSize > 0 { - total := 0 - for key, val := range req.Attributes { - total += len(key) + len(val) - } - if total > maxAttributeSize { - return nil, twirp.InvalidArgumentError(ErrAttributeExceedsLimits.Error(), strconv.Itoa(maxAttributeSize)) - } + + if !s.limitConf.CheckMetadataSize(req.Metadata) { + return nil, twirp.InvalidArgumentError(ErrMetadataExceedsLimits.Error(), strconv.Itoa(int(s.limitConf.MaxMetadataSize))) + } + + if !s.limitConf.CheckAttributesSize(req.Attributes) { + return nil, twirp.InvalidArgumentError(ErrAttributeExceedsLimits.Error(), strconv.Itoa(int(s.limitConf.MaxAttributesSize))) } if err := EnsureAdminPermission(ctx, livekit.RoomName(req.Room)); err != nil { diff --git a/test/integration_helpers.go b/test/integration_helpers.go index 69ea68291..088a31e4b 100644 --- a/test/integration_helpers.go +++ b/test/integration_helpers.go @@ -40,7 +40,7 @@ import ( const ( testApiKey = "apikey" - testApiSecret = "apiSecret" + testApiSecret = "apiSecretExtendTo32BytesAsThatIsMinimum" testRoom = "mytestroom" defaultServerPort = 7880 secondServerPort = 8880