Skip to content

Docker update - #74

Open
nilsbehlen wants to merge 4 commits into
masterfrom
docker-update
Open

Docker update#74
nilsbehlen wants to merge 4 commits into
masterfrom
docker-update

Conversation

@nilsbehlen

@nilsbehlen nilsbehlen commented Aug 3, 2026

Copy link
Copy Markdown
Member

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)

  • Opt-in POLL (default off; per-Auth-Type overridable), with POLL_TIMEOUT
    / POLL_INTERVAL. When privacyIDEA returns a client_mode = "poll" push
    challenge, the module polls /validate/polltransaction for confirmation
    (accept → finalize → OK; declined → reject; timeout → fall back to an
    Access-Challenge) instead of returning an empty-input challenge.
  • Interactive push (push_code_to_phone) is untouched and still uses the normal
    challenge-response path.

Robustness / correctness (from an xhigh code review — 15 findings fixed)

  • Poll finalize keeps realm/resConf (a confirmed push in a non-default realm
    no longer wrongly rejects) and soft-fails (FAIL, not REJECT) on transport/
    parse errors after a confirmed push.
  • Robust poll-URL derivation (poll_url(): tolerates a trailing slash; warns if
    the /validate/check suffix is missing).
  • Wall-clock poll budget (counts HTTP time, not just sleeps), no trailing sleep,
    numeric guard for a blank POLL_TIMEOUT, and fractional POLL_INTERVAL no
    longer busy-loops (Time::HiRes sleep).
  • transaction_id URL-encoded in the poll GET; enrollment-via-multichallenge
    push 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)

  • FreeRADIUS 3.2 (Debian) image with the module baked in, configured via env vars
    rendered into config at start (entrypoint.sh + envsubst): PI_*,
    RADIUS_CLIENT_*, RADIUS_REQUIRE_MSG_AUTH, RADIUS_MAX_SERVERS.
  • Secure defaults: require_message_authenticator=yes (BlastRADIUS /
    CVE-2024-3596) on by default; client secret rendered single-quoted so special
    chars are literal.
  • Multiple NAS devices via a mounted clients.d/; documented bind-mount escape
    hatch 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 (pinned
    container image), trivy image scan.
  • Workflows hardened: least-privilege permissions, SHA-pinned actions,
    persist-credentials: false; added .perlcriticrc, .hadolint.yaml.

Docsdocker/README.md (env-var reference, multi-client, escape hatch,
push caveat), self-documenting docker-compose.yml, and the design rationale
below 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.

Approach privacyIDEA (Apache/wsgi) workers FreeRADIUS worker UX Notes
push_wait (previous) blocked for the whole wait blocked (waiting on the one long HTTP call) good (single prompt) Apache worker pool is scarce; exhaustion takes the whole IdP down under push load
Polling (this PR) free between short polls blocked for the wait good (single prompt) Eliminates the IdP-fatal block; FR occupancy was already present under push_wait
Challenge-response free free bad (empty input field the user can't fill) Frees both pools but degrades UX
Async proxy between FR↔PI free still blocked (FR makes a sync call to the proxy) good Same PI relief as polling, but adds a middlebox and does not free the FR worker
FR4 async free free good Frees both, no middlebox — but FR4 is unreleased, not shippable for a vendor

Net: polling eliminates the scarce Apache/wsgi worker block (the one that
caused unresponsiveness); the FreeRADIUS-worker occupancy already existed under
push_wait and is unchanged; and UX stays the same as push_wait (no empty
field). 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 timeout
accordingly. POLL is default-off so existing deployments are unchanged on
upgrade. Requires Perl built with ithreads (Debian/Ubuntu default) for parallel
Perl execution.

Testing

  • Unit: tests/poll.t (pending→accept→finalize, declined, timeout→challenge,
    finalize FAIL-on-transport, poll_url derivation).
  • Integration: real radiusd + radclient + mock privacyIDEA — simple accept,
    push-poll, reject, and push_code_to_phone (interactive) challenge+code.
  • SAST/lint: zizmor, perlcritic, hadolint, shellcheck, trivy — all green.

Follow-ups (not in this PR)

  • Upstream the polling change into the canonical plugin.
  • Decide whether to commit to maintaining the container long-term (FR version /
    base-image CVE treadmill).

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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant