Docker update - #74
Open
nilsbehlen wants to merge 4 commits into
Open
Conversation
Module (privacyidea_radius.pm): - Add opt-in server-side push polling via /validate/polltransaction. When enabled (POLL, default off) and privacyIDEA returns a "poll" client_mode challenge, poll for confirmation instead of returning an Access-Challenge with an empty input field; fall back to a challenge on timeout. accept -> finalize -> OK, declined -> reject. Interactive challenges (e.g. push code_to_phone) are unaffected and still use the existing challenge-response path. - New config: POLL / POLL_TIMEOUT / POLL_INTERVAL (Default section and per-Auth-Type overrides), documented in rlm_perl.ini. Container (docker/): - FreeRADIUS 3.2 image (Debian) with the module baked in, configured via PI_* / RADIUS_CLIENT_* env vars rendered into rlm_perl.ini and clients.conf at start (entrypoint + envsubst). - Minimal validated raddb, mock privacyIDEA for tests, docker-compose stack. CI: - perl-test.yml: add unit tests (tests/poll.t) alongside the compile check. - docker.yml: build -> freeradius -XC -> integration test (tests/integration.sh, radclient vs mock: accept / push-poll / reject / code_to_phone). - sast.yml: zizmor, perlcritic, hadolint, shellcheck, trivy. - Harden workflows: least-privilege permissions, SHA-pinned actions, persist-credentials: false. Add .perlcriticrc and .hadolint.yaml.
Code-review fixes (privacyidea_radius.pm): - Push finalize now reuses the request params so realm/resConf are kept (a confirmed push in a non-default realm no longer wrongly rejects). - Finalize soft-fails (RLM_MODULE_FAIL) on transport/parse errors instead of hard-denying an already-confirmed user. - Robust poll-URL derivation (poll_url(): tolerates a trailing slash; warns if the /validate/check suffix is absent). - Poll budget is wall-clock (includes HTTP time), with no trailing sleep and a numeric guard for a blank POLL_TIMEOUT. - Fractional POLL_INTERVAL no longer busy-loops (import Time::HiRes sleep). - transaction_id is URL-encoded in the poll GET. - Enrollment-via-multichallenge push challenges are excluded from polling so their QR/link still reach the client. - Timeout fallback now runs mapResponse() like the normal challenge path. Container config (docker/): - require_message_authenticator is env-driven (RADIUS_REQUIRE_MSG_AUTH, default "yes") — BlastRADIUS/CVE-2024-3596 mitigation on by default. - Client secret rendered single-quoted so special chars are literal. - RADIUS_MAX_SERVERS exposes the thread-pool cap (radiusd.conf is now a template). - clients.d/ include for additional NAS devices; documented mount escape hatch for anything not env-driven; entrypoint tolerates a read-only mounted target. Tests / gates: - integration.sh no longer false-greens a reject on a no-reply server; sends Message-Authenticator so it works with the secure default. - poll.t exercises the real poll_url() and finalize FAIL-on-transport path. - perlcritic raised to severity 4 with documented legacy-only exclusions.
…rsion The GitHub runner's shellcheck flagged SC2317 (unreachable) on the trap-invoked cleanup(); newer versions use SC2329. Cover both in the disable directive, and run shellcheck from a pinned container image so CI results are deterministic (no longer depend on the runner's drifting preinstalled version).
Clarify in rlm_perl.ini, docker/README.md and the module that polling *eliminates* the scarce privacyIDEA (Apache/wsgi) worker block that push_wait caused (the IdP-fatal one); the FreeRADIUS-worker occupancy already existed under push_wait and is unchanged, and the user experience is the same (single prompt, no empty field).
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.
Adds opt-in push polling to the privacyIDEA FreeRADIUS module, packages the
whole thing as a configurable FreeRADIUS 3.2 container, and adds CI + SAST.
4 commits, 21 files (+1011 / -15).
Summary of everything in this branch
Module — push polling (
privacyidea_radius.pm,rlm_perl.ini)POLL(default off; per-Auth-Type overridable), withPOLL_TIMEOUT/
POLL_INTERVAL. When privacyIDEA returns aclient_mode = "poll"pushchallenge, the module polls
/validate/polltransactionfor confirmation(accept → finalize → OK; declined → reject; timeout → fall back to an
Access-Challenge) instead of returning an empty-input challenge.
push_code_to_phone) is untouched and still uses the normalchallenge-response path.
Robustness / correctness (from an xhigh code review — 15 findings fixed)
realm/resConf(a confirmed push in a non-default realmno longer wrongly rejects) and soft-fails (
FAIL, notREJECT) on transport/parse errors after a confirmed push.
poll_url(): tolerates a trailing slash; warns ifthe
/validate/checksuffix is missing).numeric guard for a blank
POLL_TIMEOUT, and fractionalPOLL_INTERVALnolonger busy-loops (
Time::HiRes sleep).transaction_idURL-encoded in the poll GET; enrollment-via-multichallengepush challenges excluded from polling so their QR/link reach the client; the
timeout fallback runs
mapResponse()like the normal path.Container (
docker/,docker-compose.yml)rendered into config at start (
entrypoint.sh+ envsubst):PI_*,RADIUS_CLIENT_*,RADIUS_REQUIRE_MSG_AUTH,RADIUS_MAX_SERVERS.require_message_authenticator=yes(BlastRADIUS /CVE-2024-3596) on by default; client secret rendered single-quoted so special
chars are literal.
clients.d/; documented bind-mount escapehatch for anything not env-driven (entrypoint tolerates a read-only mounted
target); minimal validated
raddb; mock privacyIDEA for tests.CI / SAST (
.github/workflows/)perl-test.yml:perl -cw+ unit tests (tests/poll.t) across Perl 5.16–5.34.docker.yml: build →freeradius -XC→ integration test (tests/integration.sh,radclient vs the mock).
sast.yml: zizmor, perlcritic (severity 4), hadolint, shellcheck (pinnedcontainer image), trivy image scan.
permissions, SHA-pinned actions,persist-credentials: false; added.perlcriticrc,.hadolint.yaml.Docs —
docker/README.md(env-var reference, multi-client, escape hatch,push caveat), self-documenting
docker-compose.yml, and the design rationalebelow captured at each feature switch.
Design rationale — why server-side polling?
privacyIDEA (wsgi under Apache) is synchronous and cannot push a notification, so
something must sit through the 30–60s until the phone confirms. The only
question is which thread pool eats that wait, and whether it blocks or polls.
push_wait(previous)push_waitNet: polling eliminates the scarce Apache/wsgi worker block (the one that
caused unresponsiveness); the FreeRADIUS-worker occupancy already existed under
push_waitand is unchanged; and UX stays the same aspush_wait(no emptyfield). An async proxy is not better — it duplicates the PI-side relief while
leaving the FR worker blocked and adding a component in the auth path. The only
approach that also frees the FR worker is async RADIUS termination (FR4 / a
bespoke async front-end), deliberately out of scope for a FR3 vendor product.
Operator cost: a polling push holds one FR worker for the wait, so size
thread pool { max_servers }(RADIUS_MAX_SERVERS) and the NAS request timeoutaccordingly.
POLLis default-off so existing deployments are unchanged onupgrade. Requires Perl built with ithreads (Debian/Ubuntu default) for parallel
Perl execution.
Testing
tests/poll.t(pending→accept→finalize, declined, timeout→challenge,finalize FAIL-on-transport,
poll_urlderivation).radiusd+radclient+ mock privacyIDEA — simple accept,push-poll, reject, and
push_code_to_phone(interactive) challenge+code.Follow-ups (not in this PR)
base-image CVE treadmill).