From 094eca32b5b2ae65e6e40b4c0692068015f8e34c Mon Sep 17 00:00:00 2001 From: Denys Smirnov Date: Mon, 25 Nov 2024 12:29:37 +0200 Subject: [PATCH] Require number, CIDR or auth for SIP inbound. --- .changeset/violet-sheep-bathe.md | 5 ++ livekit/sip.go | 17 +++- livekit/sip_test.go | 141 ++++++++++++++++++++++++++++++- 3 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 .changeset/violet-sheep-bathe.md diff --git a/.changeset/violet-sheep-bathe.md b/.changeset/violet-sheep-bathe.md new file mode 100644 index 00000000..6eaccdd7 --- /dev/null +++ b/.changeset/violet-sheep-bathe.md @@ -0,0 +1,5 @@ +--- +"github.com/livekit/protocol": patch +--- + +Require number, CIDR or auth for SIP inbound. diff --git a/livekit/sip.go b/livekit/sip.go index e91db6bb..a479ad63 100644 --- a/livekit/sip.go +++ b/livekit/sip.go @@ -122,6 +122,9 @@ func (p *CreateSIPOutboundTrunkRequest) Validate() error { if p.Trunk == nil { return errors.New("missing trunk") } + if p.Trunk.SipTrunkId != "" { + return errors.New("trunk id must not be set") + } if err := p.Trunk.Validate(); err != nil { return err } @@ -132,6 +135,9 @@ func (p *CreateSIPInboundTrunkRequest) Validate() error { if p.Trunk == nil { return errors.New("missing trunk") } + if p.Trunk.SipTrunkId != "" { + return errors.New("trunk id must not be set") + } if err := p.Trunk.Validate(); err != nil { return err } @@ -139,8 +145,11 @@ func (p *CreateSIPInboundTrunkRequest) Validate() error { } func (p *SIPInboundTrunkInfo) Validate() error { - if len(p.Numbers) == 0 { - return errors.New("no trunk numbers specified") + hasAuth := p.AuthUsername != "" || p.AuthPassword != "" + hasCIDR := len(p.AllowedAddresses) != 0 + hasNumbers := len(p.Numbers) != 0 // TODO: remove this condition, it doesn't really help with security + if !hasAuth && !hasCIDR && !hasNumbers { + return errors.New("for security, one of the fields must be set: AuthUsername+AuthPassword, AllowedAddresses or Numbers") } if err := validateHeaders(p.Headers); err != nil { return err @@ -157,7 +166,9 @@ func (p *SIPOutboundTrunkInfo) Validate() error { } if p.Address == "" { return errors.New("no outbound address specified") - } else if strings.Contains(p.Address, "@") { + } else if strings.Contains(p.Address, "transport=") { + return errors.New("trunk transport should be set as a field, not a URI parameter") + } else if strings.ContainsAny(p.Address, "@;") || strings.HasPrefix(p.Address, "sip:") || strings.HasPrefix(p.Address, "sips:") { return errors.New("trunk address should be a hostname or IP, not SIP URI") } if err := validateHeaders(p.Headers); err != nil { diff --git a/livekit/sip_test.go b/livekit/sip_test.go index c8e05308..2afa9b7b 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -1,8 +1,9 @@ package livekit import ( - "github.com/stretchr/testify/require" "testing" + + "github.com/stretchr/testify/require" ) func TestSIPTrunkAs(t *testing.T) { @@ -35,3 +36,141 @@ func TestSIPTrunkAs(t *testing.T) { require.Equal(t, out, got) }) } + +func TestSIPValidate(t *testing.T) { + cases := []struct { + name string + req interface { + Validate() error + } + exp bool + }{ + { + name: "inbound empty", + req: &SIPInboundTrunkInfo{}, + exp: false, + }, + { + name: "inbound numbers", + req: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + }, + exp: true, + }, + { + name: "inbound ips", + req: &SIPInboundTrunkInfo{ + AllowedAddresses: []string{"1.1.1.1"}, + }, + exp: true, + }, + { + name: "inbound auth", + req: &SIPInboundTrunkInfo{ + AuthUsername: "user", + AuthPassword: "pass", + }, + exp: true, + }, + { + name: "inbound x-header", + req: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + HeadersToAttributes: map[string]string{ + "X-Test": "test", + }, + }, + exp: true, + }, + { + name: "inbound other header", + req: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + HeadersToAttributes: map[string]string{ + "From": "from", + }, + }, + exp: false, + }, + { + name: "outbound empty", + req: &SIPOutboundTrunkInfo{}, + exp: false, + }, + { + name: "outbound no numbers", + req: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + }, + exp: false, + }, + { + name: "outbound with numbers", + req: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + Numbers: []string{"+2222"}, + }, + exp: true, + }, + { + name: "outbound with user", + req: &SIPOutboundTrunkInfo{ + Address: "user@sip.example.com", + Numbers: []string{"+2222"}, + }, + exp: false, + }, + { + name: "outbound with transport", + req: &SIPOutboundTrunkInfo{ + Address: "sip.example.com;transport=tcp", + Numbers: []string{"+2222"}, + }, + exp: false, + }, + { + name: "outbound with schema", + req: &SIPOutboundTrunkInfo{ + Address: "sip:example.com", + Numbers: []string{"+2222"}, + }, + exp: false, + }, + { + name: "outbound with schema (tls)", + req: &SIPOutboundTrunkInfo{ + Address: "sips:example.com", + Numbers: []string{"+2222"}, + }, + exp: false, + }, + { + name: "outbound x-header", + req: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + Numbers: []string{"+2222"}, + HeadersToAttributes: map[string]string{ + "X-Test": "test", + }, + }, + exp: true, + }, + { + name: "outbound other header", + req: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + Numbers: []string{"+2222"}, + HeadersToAttributes: map[string]string{ + "From": "from", + }, + }, + exp: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := c.req.Validate() + require.Equal(t, c.exp, err == nil, "error: %v", err) + }) + } +}