Diagnose failures on developer/* branches as well as master
workflow_run jobs only run if the if: condition holds, and previously
that required head_branch == 'master'. Failures on personal
developer/<user> branches were silently skipped even though that's
exactly the place where seeing a diagnosis comment is most useful
(you're iterating on a fix and want the prior failure understood).
Loosen the gate to accept any branch whose name is master or starts
with developer/. Push event + failure conclusion gates stay; PR-event
failures still skip (they get covered by the separate Claude Code
Review path).
New subpackage containing the three JSON conversion utilities
(radconf2json, radjson2conf, radmod2json) for both Debian and RPM.
Split out from the main freeradius-utils package because these
binaries hard-link against libfreeradius-json.so and pull in json-c
as a runtime dep - clients that only want radclient and friends
shouldn't be forced to install json-c.
New public helpers that return the C source-identifier form of a value
(matching the `fr_*_to_enum_str` family added earlier):
- fr_token_from_quote_enum_str() (util/token.c)
Inverse of fr_token_to_enum_str for the five quote-typed tokens.
Used by radjson2conf when rebuilding a CF tree from JSON.
The three CONF_FLAG_* / CALL_ENV_FLAG_* / call_env_*_type lookup tables
that lived in radmod2json.c move into the libraries they belong to so
other consumers (introspection, debug logging, future tooling) can use
them.
`FR_TABLE_INDEXED_BIT_POS_ENTRY(_v)` in table.h is the bit-pos analogue
of FR_TABLE_INDEXED_ENTRY - expands to
`[__builtin_ctz(_v) + 1] = { L("_v"), _v }`, matching what
fr_table_indexed_str_by_bit_field expects.
table.h: fix two latent bugs in the value-by-index lookup macros.
TABLE_TYPE_VALUE_INDEX_FUNC / TABLE_TYPE_VALUE_INDEX_BIT_FIELD_FUNC
returned `table[value].name.str` without checking whether that slot was
populated. For sparse tables (designated-initialiser entries with
gaps - which is exactly the shape FR_TABLE_INDEXED_ENTRY produces) the
zero-initialised gap returned a NULL pointer instead of the caller's
default, crashing strlen() in any caller that passes the result to a
string API. Both macros now fall through to `def` when name.str is
NULL.
Drop the local symbolic-name lookups that lived in each utility; call
into the new fr_token_to_enum_str() (src/lib/util/token.c) and
fr_type_to_enum_str() (src/lib/util/types.c) directly. Removes ~80
lines of duplicated enum tables across the two binaries.
The remaining bit-pos flag tables (CONF_FLAG_*, CALL_ENV_FLAG_*) and
the call_env enum tables stay in radmod2json.c for now - those are
candidates for their own move into cf_parse.c / call_env.c when the
public APIs settle.
Two new public helpers, each returning the C source-identifier form of
an enum value (FR_TYPE_STRING / T_BARE_WORD / T_OP_SET / ...). Mirror
the existing fr_type_to_str / fr_tokens[] which return the short
config-file form ("string", "<BARE-WORD>", ":=") - tools that want to
grep the v4 tree by enum identifier (radmod2json / radconf2json) use
these instead.
Both look up through v4's standard table machinery:
fr_table_str_by_value() with fr_table_num_indexed_t tables (key = enum
value, value = symbolic name).
`FR_TABLE_INDEXED_ENTRY(_v)` in table.h expands to
`[_v] = { L("_v"), _v }` so the identifier doesn't have to be written
three times when populating one of these tables. Used in types.c and
token.c.
Add radconf2json, radmod2json and radjson2conf utilities
Three JSON-bridge utilities used by the v3-to-v4 converter pipeline:
* radconf2json: dump a parsed server config tree as JSON. Opt-in
comment preservation lets `# ...` lines round-trip through the
JSON so downstream tooling can edit and re-emit the source.
* radmod2json: dump module conf_parser_t + call_env_parser_t
schemas as JSON via dl_module_alloc (proper module
loader, magic verified, init callbacks honoured).
Each dlopen+dump is fork-sandboxed so a misbehaving
module can't take the whole run down. Function
pointers are resolved to source symbols via
dladdr.
* radjson2conf: parse the JSON back into a CONF_SECTION tree and
write it out as a v4 .conf file using
cf_section_write / cf_section_write_children. -r
emits each child of the synthetic root at file
scope so a fragment can be rendered without an
outer wrapper.
JSON shape mirrors the C struct field names verbatim (FR_TYPE_STRING,
T_BARE_WORD, CONF_FLAG_REQUIRED, ...) so converter rule files can
grep against the source tree.
Symbolic-name <-> token lookups use v4's existing table machinery
(fr_tokens / fr_tokens_table for operators, fr_table_num_indexed_bit_pos_t
for CONF_FLAG_* / CALL_ENV_FLAG_* masks, fr_table_ptr_sorted_t for the
JSON `type` -> builder dispatch); no hand-rolled switch cascades.
Build-out of the CF parser to support tooling that wants to round-trip
the source through the tree without losing structure (radconf2json and
friends in the v3-to-v4 converter pipeline). No behaviour change for
the runtime server parser - it leaves the new flag off and continues
to drop comments as it always has.
* New CONF_ITEM_COMMENT item type: a `# ...` line captured verbatim
and parented on the current section. Off by default; utilities
flip a static flag via `cf_preserve_comments_set(true)` before
cf_file_read(). Private getter `_cf_preserve_comments()` lives
in cf_priv.h and gates the capture in cf_section_read.
* cf_section_write exposed in cf_file.h so external utilities can
serialise a tree back to a .conf file without rebuilding the
output by hand. CONF_COMMENT items render at section depth.
* Defensive NULL guards on the rbtree look-up in cf_find() so a
section that's only ever held CONF_COMMENT children (and so
didn't grow an ident2 tree) doesn't crash on look-up.
Re-add Hub login inside dind for the three selfhosted dind workflows
self-hosted-docker-dind has registry-mirrors -> docker.internal.networkradius.com
baked in via /etc/docker/daemon.json. The assumption was that the internal
registry would proxy Docker Hub pulls (pull-through cache). It doesn't -
when dind tries to pull a public image (kafka, ubuntu:24.04, debian:bookworm,
etc.), the mirror returns 404 and dockerd falls back to docker.io
anonymously. The fallback hits Docker Hub's 100/6h anonymous limit and
fails with "toomanyrequests".
Re-add the docker/login-action@v4 step in multi-server-tests, docker.yml-
selfhosted, and crossbuild.yml-selfhosted so dind authenticates against
Hub before any FROM-pull. Authenticated rate limit is 5000/day.
Real fix for the future is configuring docker.internal.networkradius.com
as an actual Docker Hub pull-through cache (Harbor/Nexus side), at which
point this Hub-login can come back out.
Drop fork-side credentials, replace fkirc with native concurrency
Two unrelated CI fixes batched:
1. The docker-public and crossbuild-public variants had
credentials: blocks referencing org-level vars/secrets that aren't
available on fork CI. Empty values fail workflow template
validation ("Unexpected value ''"). Drop the credentials block
from the public variants - they fall back to anonymous Docker Hub
pulls, which is fine for fork CI on GH-hosted runners (many egress
IPs, per-IP limit rarely binds).
2. Replace fkirc/skip-duplicate-actions@v5.3.1 with workflow-level
concurrency: groups in ci.yml, ci-macos.yml, and ci-sanitizers.yml.
Native GH Actions feature, runner-builtin, no Node 20 deprecation
warning, no third-party action to chase. Slightly different
semantics (cancels older queued/in-flight runs of the same workflow
on the same ref, vs fkirc's diff-based skip) - same practical
effect for the FreeRADIUS use case.
ci.yml and ci-sanitizers.yml's pre-ci job still exists because it
exposes selfhosted/docker_prefix outputs the matrix consumes; its
only step is now a no-op. ci-macos.yml's pre-ci had no other
purpose so it's gone entirely.
Per request - keep the fkirc/skip-duplicate-actions pin to v5.3.1 from
the previous commit, but don't paper over the Node.js 20 deprecation
with the env-var override. The warning will keep appearing until
upstream actions ship Node 24 releases (or we replace them) - which
is the intended signal.
Silence Node.js 20 deprecation warnings in CI/macOS/sanitizers
Two prongs - both needed because the warning's worst offenders
(fkirc/skip-duplicate-actions, mxschmitt/action-tmate) haven't shipped
Node 24 releases:
- Pin fkirc/skip-duplicate-actions from @master to @v5.3.1 (the
current latest release). Doesn't fix the Node 20 issue but stops
us tracking a moving target.
- Set FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true at workflow env level
on ci.yml and ci-macos.yml so the Node 24 runtime kicks in for all
Node 20 actions immediately. Removable per-action as upstream
catches up.
Refresh stale selfhosted-variant header comments in docker.yml/crossbuild.yml
The header comments still claimed the selfhosted variant "mounts the
internal CA into both dind and the job container so internal registry
pulls work" - but the dind-side mount went away when those workflows
switched to self-hosted-docker-dind, which has the CA baked into the
image. Update the headers to describe the actual current shape.
Bump self-hosted-docker-dind base from docker:24-dind to docker:27-dind
The docker CLI in self-hosted-docker-cli (apt-installed on ubuntu:24.04)
requires daemon API version >= 1.44, but docker:24-dind ships dockerd
24.x which speaks API 1.43. Result: dind-based jobs fail with
"API version 1.43 is not supported by this client: the minimum
supported API version is 1.44" the moment the CLI tries to talk to
dind. Bumping to docker:27-dind (engine 27.x, API 1.46) restores
compatibility.
Wire dind workflows up to use the new self-hosted-docker-* images
Now that docker-refresh.yml has published self-hosted-docker-cli and
self-hosted-docker-dind to the internal registry, switch the dind-using
workflows to reference them and shed most of the per-job setup:
- ci-multi-server-tests.yml: container -> self-hosted-docker-cli,
services.dind -> self-hosted-docker-dind, dropped the credentials
block on the dind service (internal registry, runner pre-auth'd),
dropped the CA cert mount on dind (baked into the image), dropped
the inline "Docker Hub login (via dind)" step (registry mirror in
the dind image now handles all public-image pulls).
- docker.yml docker-selfhosted: same swaps as multi-server. Also
dropped the pre-checkout "Ensure git is available" step since the
new image has git baked in. setup-dind invocation no longer needs
`packages: m4 make` since both are pre-installed.
- crossbuild.yml crossbuild-selfhosted: same as docker-selfhosted.
- The public variants of docker.yml/crossbuild.yml are unchanged -
they still use docker:dind + ubuntu:24.04 from Hub with credentials
on the services.dind block, since fork CI can't reach the internal
registry.
For the workflows that still pull stock images from Docker Hub
(ci-deb, ci-rpm, the public docker/crossbuild variants), add
credentials: blocks so the host docker daemon's job-container pull
authenticates via DOCKERHUB_READ_USER (org var) + DOCKERHUB_READ_KEY
(org secret), avoiding the anonymous 100/6h rate limit.
Make setup-dind composite action smart-skip the apt install of docker
tooling when docker is already on PATH (i.e. when running inside the
new self-hosted-docker-cli image), so it's still useful for the public
variants on plain ubuntu:24.04 but adds zero overhead on selfhosted.
v3 is a Node.js 20 action and triggers GitHub's deprecation warning
(Node 20 forced to Node 24 starting June 2026, removed September 2026).
v4 is the Node 24 release.
docker-refresh: split derived images into a second job + add retries
Two changes:
1. Move self-hosted-docker-cli out of the ubuntu:24.04 matrix entry
into its own job (process-derived-images) that needs:process-images.
The previous arrangement did two back-to-back `docker build --no-cache`
calls in the same job; the second's export raced with containerd GC
from the first build's push and the export-phase lock failure killed
the run. Splitting jobs removes the race entirely - process-images
pushes finish completely before process-derived-images starts.
The derived job pulls the parent image from the internal registry
since it almost always lands on a different self-hosted runner.
Easy to add more derived images later (more entries in the matrix).
2. Wrap docker build / push / pull in `for i in 1 2 3 ; ... ; sleep 10`
retry loops so transient containerd snapshot-store lock contention
or registry hiccups self-heal. Three attempts is enough for the ms-
scale lock window we've been hitting; longer-lived issues still fail.
docker-refresh: read Hub username from vars (not secrets)
DOCKERHUB_READ_USER is a non-secret org variable; only the PAT itself
is a secret. Resolving it via secrets.* returned an empty string and
the login step failed with "Username required". Switch to vars.*.
docker-refresh: rename Hub secrets to DOCKERHUB_READ_*
The org secrets were created as DOCKERHUB_READ_USER / DOCKERHUB_READ_KEY
(no underscore between DOCKER and HUB). Match the actual names so the
docker/login-action sees non-empty values.
docker-refresh: authenticate to Docker Hub before pulls
The previous run failed mid-matrix with "toomanyrequests: You have
reached your unauthenticated pull rate limit." 31 self-hosted runners
share egress IPs and exhaust Hub's anonymous 100/6h limit easily when
refreshing 6+ images in parallel.
Add a docker/login-action step against Docker Hub up-front using the
new org-level DOCKER_HUB_READ_USER / DOCKER_HUB_READ_KEY secrets so
all subsequent docker pull and FROM-pulls authenticate. The existing
internal-registry login (DOCKER_REPO_UPDATE_*) at the bottom of the
job stays as-is; the two are scoped to different registries so they
co-exist in ~/.docker/config.json without conflict.
Rename custom CI images to consistent self-hosted-docker-* family
The dind sidecar and the dind-using job container were named
inconsistently with the existing self-hosted family. Rename so all
three images and their Dockerfiles share the same convention:
self-hosted -> base CI image (unchanged)
self-hosted-docker-cli -> base + docker CLI/buildx/m4 (was self-hosted-builder)
self-hosted-docker-dind -> docker:dind + registry-mirror + internal CA (was fr-dind)
Inside an if: clause, the entire value is already an implicit
expression context, so an embedded ${{ ... }} is treated as literal
text and triggers "Conditional expression contains literal text outside
replacement tokens. This will cause the expression to always evaluate
to truthy."
Replace endsWith(github.ref, "/${{ github.actor }}") with
endsWith(github.ref, format('/{0}', github.actor)) - same semantics,
expressed inside the implicit if-expression context.
The previous commit baked docker.io, docker-buildx and m4 directly into
scripts/ci/Dockerfile (the self-hosted CI base image). That bloats the
image for ci.yml and ci-sanitizers.yml jobs which use that base but
never call docker.
Restore the base image to its pre-bloat package set and add a separate
scripts/ci/Dockerfile.builder that FROMs self-hosted and adds just the
docker CLI / buildx / m4 needed for dind-based jobs (docker.yml,
crossbuild.yml, multi-server-tests). Published as
docker.internal.networkradius.com/self-hosted-builder.
Builds are wired into docker-refresh.yml's existing self-hosted matrix
entry as a follow-on step (builder_dockerfile / builder_image_name),
not a separate matrix entry, so the FROM finds self-hosted in the local
docker cache and we don't have to invent inter-matrix-entry ordering.
The system-wide git safe.directory='*' added in the previous commit
stays in the base - it's harmless for non-dind users and prevents the
"dubious ownership" failure in any future workflow that does git ops
on the bind-mounted workspace.
Bake CI tooling and registry-mirror config into custom images
Two additions to docker-refresh.yml's matrix:
- Extend the existing self-hosted-ubuntu24 image (scripts/ci/Dockerfile)
to install docker.io, docker-buildx and m4, and pre-set git's
safe.directory='*' system config. These are the per-job knobs
setup-dind currently apt-installs and git-configures on every CI
run; baking them in saves ~20-30s per dind-based job.
- New scripts/ci/Dockerfile.dind, published as
docker.internal.networkradius.com/fr-dind. It's docker:24-dind
with /etc/docker/daemon.json baked in pointing at the internal
NetworkRADIUS registry as a Docker Hub pull-through mirror, and
the internal CA pre-trusted. This is what the services: dind:
block in our dind-based workflows should reference once the image
is built and pushed - it removes the per-job anonymous Docker Hub
pulls that have been hitting the 100/6h rate limit.
The dind image needs the internal CA cert which lives on the runner
host (not in the repo); a new "Stage internal CA into build context"
step copies it into the build context just before docker build for
matrix entries that set needs_internal_ca: true.
Once docker-refresh is run on master, the dind-based workflows can be
simplified to reference the new images and drop most of setup-dind.
The dind-based jobs run as root inside the job container, but the
bind-mounted workspace files are owned by the runner user on the host.
Modern git refuses to operate on a repo whose ownership doesn't match
the current uid ("fatal: detected dubious ownership"), which blew up
crossbuild.mk's `git rev-parse --git-dir` the moment it ran.
Add a setup-dind step that does `git config --global --add safe.directory '*'`
so git inside the container trusts any workspace path. setup-dind
already installed git tools by the time this runs.
Install git before checkout in dind-based docker/crossbuild jobs
actions/checkout silently falls back to a REST API tarball download
when git isn't on PATH inside the job container, and the tarball does
not include the .git directory. The dind-based docker.yml and
crossbuild.yml jobs run inside a plain ubuntu:24.04 (public variant)
or self-hosted CI image, neither of which we can rely on having git
preinstalled. Without .git the Dockerfile's `git clean -fdxx` step
fails immediately with "fatal: not a git repository", killing the
build.
Add a small pre-checkout step that does `apt-get install git` so
checkout sees a real git binary and writes a real .git directory into
the workspace. The `apt-get update` is a few-second cost shared with
the later setup-dind run.
The previous workflow ran on ubuntu-latest directly (no container) and
the GitHub-hosted runner image already had git, which is why this
issue only appeared after the dind restructure.
Make Collect DEBs/RPMs steps idempotent for self-hosted reuse
The container directive gives ci-deb and ci-rpm a fresh container per
job, but the workspace directory is bind-mounted from the host. On
GitHub-hosted runners the host VM is fresh too, so the workspace appears
empty; on the self-hosted Proxmox fleet the workspace persists across
runs and last run's debs/ or rpms/ directory is still there, so a bare
mkdir explodes with "File exists".
Add an rm -rf before each mkdir so the collect step starts clean
regardless of what the previous run left behind. This also avoids the
subtler bug where stale .debs from a previous build could end up in the
artifact alongside the new ones.
Split crossbuild.yml into self-hosted and public variants
Mirror the docker.yml change: keep the matrix-generating gen-matrix job
on ubuntu-latest, but route the actual crossbuild work to the FreeRADIUS
Proxmox fleet on org pushes (using the internal CI base image + dind
sidecar with the internal CA mounted), and fall back to ubuntu:24.04 +
public docker:dind on fork pushes.
The workflow_run trigger watches by name so the Claude failure diagnosis
workflow keeps catching failures unchanged.
crossbuild's make target does `docker run --mount=type=bind,source=...`
into the source tree, so both containers share the workspace at
/workspace via the same dind volume mount used by docker.yml and
multi-server-tests. The inner --privileged --cap-add=ALL nests fine
inside the already-privileged dind sidecar.
Revert self-delete trick in CI failure diagnosis workflow
The Gate-or-delete step was supposed to keep the Actions tab tidy by
deleting its own workflow_run record on non-failures, but the GitHub API
refuses DELETE on in-progress runs with 403, and self-deletion is by
definition always in-progress. Restore the original job-level if: that
gates on conclusion/branch/event and accept the Skipped entries.
Three comment blocks went missing when the workflow was rewritten:
- the workspace volume-mount rationale (why /workspace as a fixed
in-container path, what github.workspace actually points at)
- the rationale for the CA cert mount on dind (internal registry trust)
- the ci-debug branch / tmate session note above the tmate step
Add them back to both jobs.
Split docker.yml into self-hosted and public variants
Restore the NetworkRADIUS-internal config for self-hosted runs while
keeping fork (GitHub-hosted) builds on the public ubuntu:24.04 + dind
shape:
docker-selfhosted (if owner == FreeRADIUS): runs on self-hosted with
docker.internal.networkradius.com/self-hosted as the job container,
internal CA cert mounted into both dind and the job container so
registry pulls and HTTPS to internal hosts work, NO_PROXY set on both.
docker-public (else): runs on ubuntu-latest with the public docker:dind
sidecar and a plain ubuntu:24.04 job container - no internal bits.
The two jobs are mutually exclusive via if:, so each push fires exactly
one. The matrix and step list are short enough that duplicating beats a
third composite action.
Make docker.yml dual-environment: dind on self-hosted and fork runners
Drop the FreeRADIUS-org-only gate on the Build Dockerfiles job and the
internal-image / internal-CA wiring. The build only pulls public base
images, so the same shape works on both fleets:
- runs-on toggles to self-hosted on FreeRADIUS pushes, falls back to
ubuntu-latest on forks (matches ci.yml/ci-sanitizers.yml pattern).
- Container is public ubuntu:24.04 instead of the internal
self-hosted image; dind sidecar stays as the public docker:dind.
- Workspace mount is the only volume the job needs.
Reorder the setup-dind composite so it installs ca-certificates before
running update-ca-certificates - the previous order assumed the base
image already had ca-certificates installed, which the internal
self-hosted image did but plain ubuntu:24.04 doesn't. Functionally
equivalent on multi-server-tests, lets docker.yml use the same action.
Add make to the package list for docker.yml since ubuntu:24.04 doesn't
include it by default (the self-hosted CI image did).
Run Build Dockerfiles on the self-hosted fleet in DinD; share setup as composite action
Switch the Build Dockerfiles workflow to the same job shape as Multi-
Server CI Tests: runs-on: self-hosted, gated to FreeRADIUS-owned repos,
job runs inside docker.internal.networkradius.com/self-hosted with a
docker:dind sidecar. The build talks to the sidecar over DOCKER_HOST so
the runner's host docker daemon stays untouched, and the sidecar tears
down with the job so no image-cache or dangling-layer cleanup is needed.
Extract the in-job DinD setup (trust the internal CA, install docker.io
+ docker-buildx + extras, wait for the daemon) into a composite action
at .github/actions/setup-dind so both workflows reference the same
sequence. The services:/container: blocks themselves stay duplicated -
those are job-level concerns that composite actions can't capture.
Route deb/rpm CI builds to self-hosted runners when on the org repo
ci-deb and ci-rpm previously pinned runs-on to ubuntu-latest, which
forced them onto GitHub-hosted runners even when running on the org
repo where reliable connectivity to packages.networkradius.com lives on
the self-hosted Proxmox fleet. Match the pattern already used by ci.yml
and ci-sanitizers.yml: thread a selfhosted flag through set-matrix and
flip runs-on to self-hosted when the repository owner is FreeRADIUS,
falling back to ubuntu-latest for forks.
Both jobs already run inside a container: directive, so host-state
hygiene on the self-hosted runners is unaffected.
CI workflow housekeeping: self-deleting diagnosis runs, package-repo retries
Self-destruct the Claude CI failure-diagnosis workflow when there's
nothing to triage. GitHub doesn't expose an event-level conclusion
filter for workflow_run, so the job starts unconditionally and deletes
its own run record unless the upstream conclusion is a master-push
failure - this avoids cluttering the Actions tab with "Skipped" entries.
Retry transient package-repo failures inside the docker image build.
apt defaults to zero retries and dnf has a 30s connect timeout that
trips before the retry kicks in. Configure apt and dnf for 3 retries
with a 15s per-request timeout, and add --retry/--retry-connrefused to
the curl that fetches the NetworkRADIUS signing key. dnf stall detection
(minrate) stays on so a hung in-flight download still gets killed
quickly. Regenerated all committed Dockerfiles from the m4 templates.
Gate automatic Claude PR review on prior contributions
Only auto-trigger Claude reviews for authors with write access or a
prior accepted contribution (OWNER / MEMBER / COLLABORATOR /
CONTRIBUTOR). For first-time contributors and drive-by PRs, a
maintainer must add the 'claude-review' label to opt the PR in --
avoids spending API tokens on spam PRs.
Drop backslash assertion from sql_postgresql escape test
The ALTER DATABASE pin in postgresql-setup.sh ran fine but didn't
shift libpq's conn->std_strings flag for the radius user's
connection, so PQescapeStringConn kept doubling backslashes and the
test still failed. Rather than chase the precedence, drop the
backslash assertion - single quote doubling, ASCII pass-through and
UTF-8 pass-through are deterministic across all server configs and
that's what the test now covers. Reverts the setup-script change
from df13dfd054.
Pin standard_conforming_strings on the PostgreSQL CI test database
PQescapeStringConn doubles backslashes when the connection's
std_strings flag is false, which trips the backslash assertion in
src/tests/modules/sql_postgresql/escape.unlang on test servers where
the parameter isn't being reported as on. Pin it at the database
level in postgresql-setup.sh so every connection inherits it, and
restore the backslash assertion in the test.
Alan T. DeKok [Thu, 7 May 2026 21:12:20 +0000 (17:12 -0400)]
don't pass a free function to the hash tables
both the hash and das are parented by the same dict. so we can
just rely on the talloc ordering to do the cleanups correctly.
if there's a free function which frees the hash table entry, then
we need a corresponding destructor in the 'da', which removes the
da from the hash table.
Without that, we have accidental ordering, and potential for
problems if anything inside of talloc
libfreeradius-eap is meant to be protocol-agnostic, but it autoloaded
the RADIUS State attribute and reached into the request reply_pairs
from eap_fail() to delete it. State management on Access-Reject is
process_radius's job: RESUME(access_reject) already calls
fr_state_discard() to unlink the state-tree entry. Nothing on the
reject path actually adds a State pair to reply_pairs, so the strip
in eap_fail was guarding against a case that doesn't happen.
Remove the autoload entry and the strip; if a future caller needs to
enforce "no State on Access-Reject" at the wire level, that belongs in
the process module's send-reject path, not in the EAP library.
eap: fix talloc abort on EAP-NAK for unsupported method (#5846)
eap_session_discard() looked up the request_data_t via
request_data_reference(), which leaves the entry in place with its
opaque pointer set to the eap_session we then talloc_free. When the
session is frozen (request == NULL on the eap_session), the destructor's
own request_data_get() cleanup path is short-circuited, so the entry
survives with a dangling rd->opaque. When the request's
session_state_ctx is later freed, the rd destructor calls
talloc_free(rd->opaque) on the freed chunk and aborts with "Bad talloc
magic value".
Switch to request_data_get() so the entry is unlinked atomically as
the eap_session is pulled out, then free the session. No callers rely
on the entry surviving the discard.
Triggered by NAK-for-unsupported-method when ignore_unknown_eap_types
is no, but applies to any path that hits eap_failure() for a frozen
session.
Add src/tests/eapol_test/fail-aka-nak.conf as a regression test under
the new fail-<type>-* harness convention: server loads only EAP-AKA,
supplicant offers PEAP and NAKs the AKA challenge. Pre-fix this aborts
in request_slab_deinit; post-fix the daemon stays up and the request is
cleanly rejected.
tests: support fail-<type>-* eapol_test cases that should reject cleanly
A conf file whose basename starts with `fail-` is now treated by the
harness as a negative scenario: eapol_test is expected to NOT complete
authentication, and the recipe inverts the exit-code check accordingly.
The server still has to be alive at the end (radiusd_stop checks the
PID), which is what catches an actual server crash.
network: detect reservation aliasing via data_size before reset
When app_io->read() calls fr_network_listen_send_packet() internally
(e.g. ldap_sync for AD notifications), that function calls
fr_message_and_data_alloc() on the same message set while our
reservation is outstanding.
message_reserve() uses fr_ring_buffer_reserve(), which does NOT advance
write_offset. The subsequent alloc therefore lands at the same message
ring slot as the existing reservation. The memset inside message_reserve
zeroes our struct (clearing data, rb, etc.), then the new message commits
into that slot and fills in data_size with the actual packet size.
After app_io->read() returns 0, the previous fix unconditionally called
fr_message_and_data_reset() on cd. In the aliased case cd now points to
the already-committed, already-dispatched message; resetting it sets
data_size and data to NULL, causing the worker to decode zero bytes and
fail to find attr_packet_type.
Fix: before resetting, check cd->m.data_size. A non-zero value means
an alloc claimed the slot while we held the reservation. In that case
skip the reset entirely and set s->cd = NULL so the next call gets a
fresh reservation from the now-advanced write_offset.
network: fix uncommitted reservation aliasing when app_io->read returns no data
fr_message_and_data_reserve() uses fr_ring_buffer_reserve() which does not
advance write_offset. If app_io->read() returns 0 (e.g. ldap_sync, which
does its own reads internally) and the cached cd is held in s->cd, any
subsequent allocation on the same message set returns the same ring-buffer
slot and zeroes it, corrupting cd->m.data.
Fix: only cache s->cd when s->leftover > 0 (partial stream data to preserve).
With no leftover, cancel the reservation explicitly via fr_message_and_data_reset()
which clears the message fields and marks the slot FR_MESSAGE_FREE so the next
reserve can reclaim it cleanly.
Also fix the B2 commit size in fr_network_read(): data_size returned by TCP
app_io already includes the leftover bytes already in the buffer, so the old
cd->m.data_size + data_size was double-counting.
Alan T. DeKok [Thu, 7 May 2026 01:41:49 +0000 (21:41 -0400)]
free children entries after they are used
i.e. after vasprintf() is called. otherwise the children are
freed, adn then the pointer which is passed to vasprintf is then
pointing to unused memory
Set the OS thread name for network and worker threads via
pthread_setname_np so they appear in ps/top/htop output.
Adds a configure check for pthread_setname_np (from pthread.h).
Linux and macOS have different signatures (2-arg vs 1-arg), handled
with an __APPLE__ guard matching the existing pattern in thread.c.
Alan T. DeKok [Tue, 5 May 2026 16:29:36 +0000 (12:29 -0400)]
update RB tree freeing process
There are conflicts between the behavior of "free the tree", and
the talloc destructors for a node. For simplicity / laziness,
the "free tree walker" just walks over the tree, freeing the
node data. It expects that the tree nodes themselves remain
active during this walk, as the tree is not rebalanced.
The free walker will free the node data, which should NOT free
the individual node. The node may, in fact, be inside of the
block which is being freed!
We therefore free the talloc children before mangling the tree
structure, so that any talloc destructors can look at the "clean"
tree structure.
We update the various destructors for tree data to check if the
tree is being freed, and then don't try to find / remove the entry
in the tree.
We also update the allocations so that the nodes in the tree are
always parented from the tree. That way they are cleaned up before
the tree is cleaned up.
If (as before) the tree and nodes are both parented from the same
parent, then the nodes / tree are freed in essentially random order,
and the nodes might stick around after the tree is freed
LDAP_DN_CALL_ENV_ESCAPE and LDAP_FILTER_CALL_ENV_ESCAPE were using
fr_ldap_dn_box_escape and fr_ldap_filter_box_escape as their safe_for
tokens, but %ldap.dn.safe and %ldap.filter.safe mark values with
LDAP_DN_SAFE_FOR (fr_ldap_dn_escape_func) and LDAP_FILTER_SAFE_FOR
(fr_ldap_filter_escape_func) respectively.
The mismatched tokens meant pre-marked-safe values (e.g. dc=example,dc=com
passed through %ldap.dn.safe) were not recognised as safe by the call_env
escape check and got re-escaped to dc\3dexample\2cdc\3dcom, producing an
invalid DN syntax error.
Fix: move LDAP_DN_SAFE_FOR / LDAP_FILTER_SAFE_FOR before the call_env
macros and use them consistently in .safe_for and .literals_safe_for.
Also add missing radprofile attributes to profile_injection.attrs: the
filter listed only Idle-Timeout but radprofile for user "john" also sets
Session-Timeout, Acct-Interim-Interval and Framed-IP-Netmask.
ldap xlat_profile test: drop bogus notfound rcode check
%ldap.profile() is an xlat, not a module call. It returns a bool but
does not write the unlang rcode, so checking (!notfound) after the xlat
always sees whatever rcode was current from a prior statement. The bool
check immediately above already verifies the injection payload does not
match any profile; the rcode check was both wrong and redundant.
rlm_ldap: Add examples to filter.safe, filter.unescape, and uri.* xlat doc sections
Split the combined ldap.uri.escape/safe/unescape alias blurb into three
separate sections, each with a concrete example. Add examples to the
previously bare ldap.filter.safe and ldap.filter.unescape sections.
LDAP requires _two_ safety schemes, one for DNs one for filters
- The DN safety scheme would escape '+', which is the RDN separator char. This would break instances where usernames were extracted directly from certificates, as '+' would become \2c and would not correctly be broken into its constituent RDN values.
- The existing filter schemes were not correctly applied in a number of places, meaning that if the administrator did not escape values with %ldap.uri.escape(), content from unsafe attributes could become structural elements of filters or DNs.
Alan T. DeKok [Tue, 5 May 2026 01:20:39 +0000 (21:20 -0400)]
clean up cf_data_add() usages
* client / unlang code marked the data as "to free", even though
the data was already parented by a talloc'd chunk. So any call
to decrease_ref_count() would result in a use after free
* update cf_data_free() set / destructor so that the destructor
is set only when the data needs to be freed. which means that
the destructor doesn't need to check the "do_free" flag
ci: switch from luajit to luajit2 (OpenResty fork) for CI dependencies
libnginx-mod-http-lua (the nginx Lua module used for rlm_rest testing)
depends on libluajit2-5.1-2 from the OpenResty luajit2 fork. This package
conflicts with libluajit-5.1-2 from the canonical luajit source, so both
package families cannot be installed simultaneously.
The CI Docker image was built with libnginx-mod-http-lua installed, so
libluajit2-5.1-2 is already present and libluajit-5.1-2 / luajit are
absent. mk-build-deps for extra-packages.debian.control was failing because
installing luajit would require removing libluajit2-5.1-2 (and thus
libnginx-mod-http-lua).
Switch extra-packages.debian.control to request libluajit2-5.1-dev and
luajit2 instead. They coexist with the already-installed libluajit2-5.1-2,
provide the same headers and soname for rlm_lua compilation, and the luajit2
interpreter binary is equivalent for any runtime use.
Remove the now-irrelevant apt preferences pin from freeradius-deps/action.yml:
the libluajit-5.1-* packages are not installed in the Docker image at all,
so there was nothing to hold back.
The first pin only covered libluajit-5.1-2, but the Ubuntu security
rebuild produced build1 versions of all four binary packages from the
luajit source (libluajit-5.1-2, libluajit-5.1-common, libluajit-5.1-dev,
luajit). luajit and libluajit-5.1-dev carry strict (= dfsg-1) deps on
BOTH libluajit-5.1-2 and libluajit-5.1-common, so full-upgrade upgrading
libluajit-5.1-common to build1 still removed luajit as a casualty.
Extend the apt preferences pin to cover libluajit-5.1-* (glob) and luajit
explicitly, keeping the entire package set at dfsg-1 until Ubuntu ships a
coherent build1 rebuild of all four.
ci: Add Ubuntu archive mirrors and switch from OpenResty to stock nginx+lua
Add ubuntu-mirrors-setup.sh which rewrites /etc/apt/sources.list.d/ubuntu.sources
to use mirror+file: lists for the main archive, ports (arm64), and security suites.
Mirrors are ordered by proximity to Ottawa with the canonical servers as last-resort
fallbacks. Also drops the apt connect timeout to 5s so dead hosts fail over quickly
rather than stalling for the 120s default.
Remove the ubuntu-toolchain-r/test PPA: gcc-13 and gcc-14 are both in Ubuntu 24.04
universe, so the PPA adds nothing for those versions and has no public mirrors.
Remove the OpenResty apt repo and replace the openresty package with nginx +
libnginx-mod-http-lua + lua-cjson, all from Ubuntu's own repos. All Lua primitives
used by the rlm_rest test API are standard ngx_lua. Update openresty-setup.sh to
detect OpenResty vs stock nginx at runtime (macOS dev vs CI), inject the lua
load_module directive when needed, and replace the OpenResty-specific
ngx.ctx.openresty_request_time_us with ngx.now() elapsed timing.
projects / thirdparty / freeradius-server.git / log
