Skip to content

Optionally add Record-Route for inbound, better errors #315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Optionally add Record-Route for inbound, better errors #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf8aac7
Better error messages for media.
dennwc Mar 17, 2025
3f8478d
Add Record-Route header for inbound.
dennwc Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ type Config struct {
// Setting it to true makes SIP server silently drop INVITE requests if it gets a negative Auth or Dispatch response.
// Doing so hides our SIP endpoint from (a low effort) port scanners.
HideInboundPort bool `yaml:"hide_inbound_port"`
// AddRecordRoute forces SIP to add Record-Route headers to the responses.
AddRecordRoute bool `yaml:"add_record_route"`

// AudioDTMF forces SIP to generate audio DTMF tones in addition to digital.
AudioDTMF bool `yaml:"audio_dtmf"`
Expand Down
32 changes: 24 additions & 8 deletions pkg/sip/inbound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ import (
"github.com/livekit/protocol/tracer"
"github.com/livekit/psrpc"
lksdk "github.com/livekit/server-sdk-go/v2"
"github.com/livekit/sip/pkg/media/sdp"
"github.com/livekit/sipgo/sip"

"github.com/livekit/sip/pkg/config"
"github.com/livekit/sip/pkg/media"
"github.com/livekit/sip/pkg/media/dtmf"
"github.com/livekit/sip/pkg/media/rtp"
"github.com/livekit/sip/pkg/media/sdp"
"github.com/livekit/sip/pkg/media/tones"
"github.com/livekit/sip/pkg/stats"
"github.com/livekit/sip/res"
Expand Down Expand Up @@ -586,19 +586,19 @@ func (c *inboundCall) runMediaConn(offerData []byte, enc livekit.SIPMediaEncrypt
EnableJitterBuffer: c.s.conf.EnableJitterBuffer,
}, RoomSampleRate)
if err != nil {
return nil, err
return nil, fmt.Errorf("cannot create media port: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use some error wrapping utility or psrpc.NewError here (and below) instead, to allow extracting the original error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's mostly for debugging for now. I'll need to annotate all errors this way if we want to propagate them.

}
c.media = mp
c.media.EnableTimeout(false) // enabled once we accept the call
c.media.SetDTMFAudio(conf.AudioDTMF)

answer, mconf, err := mp.SetOffer(offerData, e)
if err != nil {
return nil, err
return nil, fmt.Errorf("cannot set offer: %w", err)
}
answerData, err = answer.SDP.Marshal()
if err != nil {
return nil, err
return nil, fmt.Errorf("cannot marshal sdp: %w", err)
}
c.mon.SDPSize(len(answerData), false)
c.log.Debugw("SDP answer", "sdp", string(answerData))
Expand Down Expand Up @@ -1047,10 +1047,10 @@ func (c *sipInbound) respond(status sip.StatusCode, reason string) {
return
}

resp := sip.NewResponseFromRequest(c.invite, status, reason, nil)
resp.AppendHeader(sip.NewHeader("Allow", "INVITE, ACK, CANCEL, BYE, NOTIFY, REFER, MESSAGE, OPTIONS, INFO, SUBSCRIBE"))

_ = c.inviteTx.Respond(resp)
r := sip.NewResponseFromRequest(c.invite, status, reason, nil)
r.AppendHeader(sip.NewHeader("Allow", "INVITE, ACK, CANCEL, BYE, NOTIFY, REFER, MESSAGE, OPTIONS, INFO, SUBSCRIBE"))
c.addExtraHeaders(r)
_ = c.inviteTx.Respond(r)
}

func (c *sipInbound) RespondAndDrop(status sip.StatusCode, reason string) {
Expand Down Expand Up @@ -1176,6 +1176,20 @@ func (c *sipInbound) setDestFromVia(r *sip.Response) {
}
}

func (c *sipInbound) addExtraHeaders(r *sip.Response) {
if c.s.conf.AddRecordRoute {
// Other in-dialog requests should be sent to this instance as well.
recordRoute := c.contact.Address.Clone()
if recordRoute.UriParams == nil {
recordRoute.UriParams = sip.HeaderParams{}
}
recordRoute.UriParams.Add("lr", "")
r.PrependHeader(&sip.RecordRouteHeader{
Address: *recordRoute,
})
}
}

func (c *sipInbound) Accept(ctx context.Context, sdpData []byte, headers map[string]string) error {
ctx, span := tracer.Start(ctx, "sipInbound.Accept")
defer span.End()
Expand All @@ -1189,6 +1203,8 @@ func (c *sipInbound) Accept(ctx context.Context, sdpData []byte, headers map[str
// This will effectively redirect future SIP requests to this server instance (if host address is not LB).
r.AppendHeader(c.contact)

c.addExtraHeaders(r)

c.setDestFromVia(r)

r.AppendHeader(&contentTypeHeaderSDP)
Expand Down
3 changes: 2 additions & 1 deletion pkg/sip/media_port.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package sip

import (
"context"
"fmt"
"errors"
"io"
"net"
Expand Down Expand Up @@ -280,7 +281,7 @@ func (p *MediaPort) SetAnswer(offer *sdp.Offer, answerData []byte, enc sdp.Encry
func (p *MediaPort) SetOffer(offerData []byte, enc sdp.Encryption) (*sdp.Answer, *MediaConf, error) {
offer, err := sdp.ParseOffer(offerData)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("cannot parse sdp: %w", err)
}
answer, mc, err := offer.Answer(p.externalIP, p.Port(), enc)
if err != nil {
Expand Down
Loading