Update code to be tolerant of unknown authentication failure reasons. - #844
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| switch resp.ErrorCode { | ||
| case rpc.SIPTrunkAuthenticationError_SIP_TRUNK_AUTH_ERROR_QUOTA_EXCEEDED: | ||
| authResult = sip.AuthQuotaExceeded | ||
| case rpc.SIPTrunkAuthenticationError_SIP_TRUNK_AUTH_ERROR_NO_TRUNK_FOUND: |
There was a problem hiding this comment.
what's the difference between resp.ErrorCode with rpc.SIPTrunkAuthenticationError_SIP_TRUNK_AUTH_ERROR_NO_TRUNK_FOUND and psrpc.GetErrorCode(err) with psrpc.NotFound? both are not found 🤔
There was a problem hiding this comment.
The auth service might return errors in two different channels: (a) the RPC error itself and (b) the ErrorCode field in the response itself. The channels are mutually exclusive, and so if we receive an RPC error then we won't have a response and an ErrorCode, and if we have a response and an ErrorCode then we won't have an RPC error.
| return psrpc.NewErrorf(psrpc.PermissionDenied, "auth failure") | ||
| case AuthRejectedAsError: | ||
| // Own metric reason so auth-service contract violations show up as their own series. | ||
| cmon.InviteErrorShort(stats.ClientError("auth-rejected-as-error")) |
There was a problem hiding this comment.
do we know what kind of other errors could there be? are they all client errors?
There was a problem hiding this comment.
Since you've last viewed the PR, I've added handling for a new kind of auth failure (ROUTE_NOT_ALLOWED, which is returned when anycast isn't enabled).
The other sorts of auth failure are related to trunk configuration issues (i.e. client errors).
07f6180 to
5e22438
Compare
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| case AuthRouteNotAllowed: | ||
| cmon.InviteErrorShort(stats.ClientError("route-not-allowed")) | ||
| log.Warnw("Rejecting inbound, route not allowed", nil) | ||
| cc.RespondAndDrop(sip.StatusServiceUnavailable, "Service temporarily unavailable") |
There was a problem hiding this comment.
🟡 Cached 503 blocks route failover
When AuthRouteNotAllowed causes same-server failover with the same Call-ID and From-tag, rejectedInvites replays the 503. The alternate origination URI receives no authentication check until callCacheTTL expires.
Learn more
RespondAndDrop caches every final response at or above 300 using only the SIP Call-ID and From-tag as its key. A carrier can reuse both values when retrying a 503 against another origination URI. The replay path runs before authentication and ignores the new request URI, so the second route inherits the first route's rejection. The entry remains active for one minute.
Example: A carrier sends Call-ID abc with From-tag t1 to sip-a.example.com and receives 503 for ROUTE_NOT_ALLOWED. It retries the same dialog identifiers against sip-b.example.com, which reaches the same SIP server. The server replays 503 instead of authorizing sip-b.example.com, although that route can be allowed.
Recommended fix: Do not add retryable 5xx responses to rejectedInvites, or include the destination route in the cache key. Preserve deduplication for terminal rejections while allowing a changed origination URI to run GetAuthCredentials again.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Treat unrecognized
rpc.SIPTrunkAuthenticationErrorvalues as an authentication failure instead of accepting the call. Also, map certain authentication error codes toClientErrorrather thanServerError.Today the adapter in pkg/service/psrpc.go handles only
QUOTA_EXCEEDEDandNO_TRUNK_FOUND. Any other non-zero code falls through the drop and password checks and is returned asAuthAcceptwith an empty trunk ID. This PR adds an AuthFailureUnknown result for that case. The INVITE is answered with 401 and recorded asClientError("auth-unknown").Also, update the switch statements to handle a new kind of authentication failure:
ROUTE_NOT_ALLOWED.Motivation: The auth service will start returning new enum values for rejections it currently reports as errors. Those errors make SIP end the call as
ServerError("auth-error")with a 503, which should be reserved for availability failures. This change has to roll out before the auth service does, so a newer auth service talking to an older SIP rejects rather than admits.NOTE: This depends on livekit/protocol#1798.