fix: recover from a stuck legacy (cert-based) auth/refresh request - #1086
Open
bjornmage wants to merge 1 commit into
Open
fix: recover from a stuck legacy (cert-based) auth/refresh request#1086bjornmage wants to merge 1 commit into
bjornmage wants to merge 1 commit into
Conversation
tlsuv_http exposes a connect timeout but no idle/response timeout, so a request that completes its TCP handshake and then never receives a response (e.g. a controller instance that disappears mid-request during a rolling restart) is never delivered to legacy_session_cb(). Without a callback, `auth->refreshing` stays latched true forever and the auth state machine never retries -- the identity is stuck in UNAUTHORIZED until the process is restarted. Add a bounded watchdog timer (AUTH_REQUEST_TIMEOUT_SECONDS, 30s) around each outstanding legacy refresh/auth request. On expiry it cancels the request via tlsuv_http_cancel_all(), which routes it back through legacy_session_cb() with UV_ECANCELED, clearing `refreshing` and re-arming the existing retry/backoff path unchanged. This mirrors the recovery semantics PR openziti#1025 added for the OIDC auth path, applied to the legacy/cert-based path which openziti#1025 did not cover. No behavior change for healthy sessions: the watchdog only fires when a request has gone unanswered past AUTH_REQUEST_TIMEOUT_SECONDS.
|
Thank you for your submission! Please read and sign our Contributor License Agreement before we can accept your contribution. I have read the CLA Document and I hereby sign the CLA Björn Mage seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
Member
|
recheck cla |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
A cert-based (non-OIDC) identity lost its API session and never recovered on
its own -- it looped on
UNAUTHORIZED/no api session token set for ziti_controllercontinuously for hours, well past the point the underlyingdisruption had cleared, until the process was manually restarted. This is the
same defect class as #990 ("OIDC token refresh never recovers from network
disruption, permanent UNAUTHORIZED state requires restart"), but #990's fix
(#1025) is scoped entirely to
oidc.c/oidc.h/credentials.c-- the OIDCtoken-refresh path. The identity in this report uses cert-based auth
(
--identity <file>.json, not OIDC enrollment), so #1025's fix does notcover this code path. #572 (2023, general controller-API "stuck, never
reconnects" pattern, not OIDC-specific, closed without an evident code fix)
looks like the same still-open gap in the non-OIDC path.
Environment:
ziti-edge-tunnel/ziti-sdk-c1.18.1,tlsuvv0.41.4, 3-nodeRaft HA controller cluster, 30-minute API session timeout. Client process
never crashed or exited (
systemctlshowedactive (running)throughout),so nothing triggered
Restart=always; only an explicit process restartrecovered it, and the fresh process authenticated immediately.
Root cause
tlsuv_httpexposes a connect timeout (tlsuv_http_connect_timeout()) butno idle/response timeout. In
legacy_auth.c, once a refresh/auth requestcompletes its TCP handshake, there is no bound on how long it can wait for a
response. If the peer accepts the connection and then never responds (e.g. a
controller instance that disappears mid-request during a rolling restart,
which is what coincided with the session invalidation in this report),
legacy_session_cb()is simply never invoked.auth->refreshingstayslatched
trueforever, and the existing retry/backoff logic inlegacy_session_cb()-- which does work correctly for every other failuremode -- never gets a chance to run, because it's never reached.
Fix
Add a bounded watchdog timer (
AUTH_REQUEST_TIMEOUT_SECONDS, 30s) aroundeach outstanding legacy refresh/auth request (
library/legacy_auth.c). Onexpiry it calls
tlsuv_http_cancel_all()on the auth context's HTTP client,which routes the stuck request back through
legacy_session_cb()withUV_ECANCELED-- the same transport-failure path already exercised forother network errors, and it re-arms the existing retry/backoff calculation
unchanged. The watchdog timer is started alongside every outstanding
request and stopped as soon as a response (successful or not) arrives, so it
never fires for healthy sessions.
This mirrors the recovery semantics #1025 added for the OIDC auth path,
applied to the legacy/cert-based path that #1025 did not cover.
Handle lifecycle:
req_timeris closed and its close callback chains intoclosing the existing
timerhandle (whose close callback freesauth), soauthis only freed after both embeddeduv_timer_thandles have finishedclosing.
No behavior change for healthy sessions: the watchdog only fires when a
request has gone unanswered past
AUTH_REQUEST_TIMEOUT_SECONDS; theexisting delay/backoff calculations (
refresh_delay(),next_backoff())are untouched.
Test evidence
ctest, Catch2, 83 tests / ~100kassertions) passes unchanged against the patched build -- no regressions.
verification) that opens a mock controller which accepts a connection and
then goes silent, confirming: the watchdog fires at ~30s, cancels the
stuck request, the existing backoff path schedules a retry, the retry
opens a fresh connection, and a subsequent valid response drives the auth
state machine to
ZitiAuthStateFullyAuthenticated.reproducing the exact rolling-restart timing from the original incident
(
tests/integrequires a live quickstart Ziti network and was out ofscope for this change's local verification).
Related
Fix/OIDC reconnect refresh #1025, but scoped to OIDC auth only.
handle never reconnects" pattern via the general controller-API path,
likely the same defect class as OIDC token refresh never recovers from network disruption, permanent UNAUTHORIZED state requires restart #990 in the non-OIDC path.