Skip to content

TEL-575: do not transition state for 407 #680

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

Merged
hechen-eng merged 3 commits into from
May 9, 2026
+55 −12
Merged

TEL-575: do not transition state for 407 #680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04233cf
TEL-575: do not transition state for 407
hechen-eng May 8, 2026
21b3575
update call status with SCS_ERROR if second invite is not received in…
hechen-eng May 8, 2026
87acc80
update
hechen-eng May 9, 2026
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
61 changes: 51 additions & 10 deletions pkg/sip/inbound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const (
inviteOKRetryAttempts = 5
inviteOKRetryAttemptsNoACK = 2
inviteOkAckLateTimeout = inviteOkRetryIntervalMax
authChallengeTimeout = 30 * time.Second
)

var allowHeader = sip.NewHeader("Allow", "INVITE, ACK, CANCEL, BYE, NOTIFY, REFER, MESSAGE, OPTIONS, INFO, SUBSCRIBE")
Expand Down Expand Up @@ -158,7 +159,33 @@ func (s *Server) getInvite(sipCallID string) *inProgressInvite {
return is
}

func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Request, tx sip.ServerTransaction, from, username, password string) (ok bool) {
// scheduleAuthChallengeTimeout finalizes st as SCS_ERROR after authChallengeTimeout
// unless authResolved is set to true (by a follow-up INVITE) before the timer fires.
func (i *inProgressInvite) scheduleAuthChallengeTimeout(st *CallState, log logger.Logger) {
i.authResolved.Store(false)
challengedAt := time.Now()
time.AfterFunc(authChallengeTimeout, func() {
if !i.authResolved.CompareAndSwap(false, true) {
return
}
log.Infow("auth challenge timed out without authenticated retry; finalizing call as error",
"sipCallID", i.sipCallID, "timeout", authChallengeTimeout)
st.Update(context.Background(), func(info *livekit.SIPCallInfo) {
info.CallStatus = livekit.SIPCallStatus_SCS_ERROR
info.Error = "auth challenge issued, no authenticated retry received"
// EndedAtNs reflects when the call effectively ended.
info.EndedAtNs = challengedAt.UnixNano()
})
})
}

// handleInviteAuth performs SIP digest authentication on an inbound INVITE.
// The challenge return value distinguishes the normal digest handshake (where we
// just sent the initial 407 with no credentials yet provided and expect the
// client to retry) from a hard auth failure. Callers should treat
// (ok=false, challenge=true) as non-terminal so it doesn't end up recorded as
// a finalized error state.
func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Request, tx sip.ServerTransaction, from, username, password string) (ok bool, challenge bool) {
log = log.WithValues(
"username", username,
"passwordHash", hashPassword(password),
Expand All @@ -170,7 +197,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re

if username == "" || password == "" {
log.Debugw("Skipping authentication - no credentials provided")
return true
return true, false
}

if s.conf.HideInboundPort {
Expand Down Expand Up @@ -205,7 +232,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
res.AppendHeader(sip.NewHeader("Proxy-Authenticate", inviteState.challenge.String()))
_ = tx.Respond(res)
log.Infow("No Proxy header found. Sending 407 Unauthorized response with Proxy-Authenticate header")
return false
return false, true
}

log.Debugw("Found Proxy-Authorization header, parsing credentials")
Expand All @@ -215,7 +242,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
"headerValue", h.Value(),
)
_ = tx.Respond(sip.NewResponseFromRequest(req, 401, "Bad credentials", nil))
return false
return false, false
}

// Set credURI and credUsername in logger early to avoid repetitive logging
Expand All @@ -230,7 +257,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
"receivedUsername", cred.Username,
)
_ = tx.Respond(sip.NewResponseFromRequest(req, 401, "Unauthorized", nil))
return false
return false, false
}

// Check if we have a valid challenge state
Expand All @@ -240,7 +267,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
"expectedRealm", UserAgent,
)
_ = tx.Respond(sip.NewResponseFromRequest(req, 401, "Bad credentials", nil))
return false
return false, false
}

log.Debugw("Computing digest response",
Expand All @@ -259,7 +286,7 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
if err != nil {
log.Warnw("Failed to compute digest response", err)
_ = tx.Respond(sip.NewResponseFromRequest(req, 401, "Bad credentials", nil))
return false
return false, false
}

log.Debugw("Digest computation completed",
Expand All @@ -274,11 +301,11 @@ func (s *Server) handleInviteAuth(tid traceid.ID, log logger.Logger, req *sip.Re
"receivedResponse", cred.Response,
)
_ = tx.Respond(sip.NewResponseFromRequest(req, 401, "Unauthorized", nil))
return false
return false, false
}

log.Infow("SIP invite authentication successful")
return true
return true, false
}

func (s *Server) onInvite(log *slog.Logger, req *sip.Request, tx sip.ServerTransaction) {
Expand Down Expand Up @@ -456,13 +483,27 @@ func (s *Server) processInvite(req *sip.Request, tx sip.ServerTransaction) (retE
cc.Processing()
tryingTime = time.Now()
}
sipCallID := ""
if h := req.CallID(); h != nil {
sipCallID = h.Value()
}
inviteState := s.getInvite(sipCallID)
// New INVITE supersedes any pending 407-challenge timer for this Call-ID.
inviteState.authResolved.Store(true)

s.getCallInfo(cc.ID()).countInvite(log, req)
if !s.handleInviteAuth(tid, log, req, tx, from.User, r.Username, r.Password) {
if ok, challenge := s.handleInviteAuth(tid, log, req, tx, from.User, r.Username, r.Password); !ok {
// Store (call-ID + from tag) to (to tag) mapping
s.cmu.Lock()
s.provisionalInvites.Add([2]string{cc.SIPCallID(), string(cc.Tag())}, cc.ID())
s.cmu.Unlock()
cmon.InviteErrorShort(stats.ClientError("unauthorized"))
if challenge {
// 407 sent: defer finalization to the timer or the next INVITE,
// not the deferred handler at the top of processInvite.
inviteState.scheduleAuthChallengeTimeout(state, log)
state = nil
Copy link
Copy Markdown
Contributor

@nishadmusthafa nishadmusthafa May 8, 2026

Choose a reason for hiding this comment

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

I think this is fine for now. But if we don't get the second invite, as its designed currently, we won't be logging the auth failure or showing it in the dashboard.. right ? Any way around that ?

Copy link
Copy Markdown
Contributor Author

@hechen-eng hechen-eng May 8, 2026

Choose a reason for hiding this comment

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

hmm, I thought that's part of the long-term plan? We don't need to log since sipfe will log the 407 response.

With current short-term fix, if there is a second invite, we are all good. If not, we'll have a hanging INCOMING record. As far as sip, 407 is a final response, the SIP transaction is complete from the server's perspective. Are we okay with this for now?

Copy link
Copy Markdown
Contributor

@nishadmusthafa nishadmusthafa May 8, 2026

Choose a reason for hiding this comment

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

The long term was more about logging an event for the first auth challenge. We still need to handle the case of a second invite not coming. If 407 is the final response in that case, how will billing know to end the call ?

Copy link
Copy Markdown
Contributor Author

@hechen-eng hechen-eng May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

from billing's perspective, the call never started since the call is never active, so we should be fine in terms of billing.

It's bad user experience though (a hanging INCOMING call status). Maybe we can use a timeout to guard this 🤔

Copy link
Copy Markdown
Contributor

@nishadmusthafa nishadmusthafa May 8, 2026

Choose a reason for hiding this comment

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

So we send them the UpdateSIPCallStateRequest with callStatus set to SCS_INCOMING before all this happens don't we ? If that's not the case, then we are good.

Copy link
Copy Markdown
Contributor

@nishadmusthafa nishadmusthafa May 8, 2026

Choose a reason for hiding this comment

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

If we are sending SCS_INCOMING, we should also send either SCS_DISCONNECT or SCS_ERROR. As you suggested a timeout for the second invite might be a good approach then

}
// handleInviteAuth will generate the SIP Response as needed
return psrpc.NewErrorf(psrpc.PermissionDenied, "invalid credentials were provided")
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/sip/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net"
"net/netip"
"sync"
"sync/atomic"
"time"

"github.com/frostbyte73/core"
Expand Down Expand Up @@ -167,8 +168,9 @@ type Server struct {
}

type inProgressInvite struct {
sipCallID string
challenge digest.Challenge
sipCallID string
challenge digest.Challenge
authResolved atomic.Bool
}

type ServerOption func(s *Server)
Expand Down
Loading