fix(bpf): verify XDP program on 6.x kernels + cross-kernel CI#426
Merged
Conversation
The XDP program's TCP-option parse loops walk PTR_TO_PACKET with a variable stride, so the verifier re-explores the loop body per packet offset and total processed instructions exceed the 1,000,000 ceiling (-E2BIG) on every 6.x verifier tested (6.1..6.19, x86 and arm64). Where it cannot load, capture falls back to AF_PACKET. Parse TCP options via bpf_loop() over a per-CPU scratch copy so the body is verified once (1,000,001 -> 168,876 processed insns), and move the 96-byte tcp_fingerprint_data off the BPF stack into a per-CPU scratch map value so the added callback frame stays within the 512-byte stack limit. Behavior-preserving: same option fidelity, map keys and values.
![github-advanced-security[bot]](https://avatars.github.com/in/57789?s=60&v=4){kind=small}
pigri
force-pushed
the
fix/xdp-verifier-6x
branch
from
b8dd97a to
14c3f8c
Compare
Add tests/e2e-xdp: for each kernel version, dump its BTF, recompile xdp.bpf.c against it (CO-RE-off, so a failure is a real verifier rejection rather than an offset mismatch), and load it in QEMU with the JIT forced on. Guards against XDP verifier complexity / stack-depth regressions across kernel generations. The workflows run a matrix (6.6.125 / 6.12.94 / 6.19.14) via a reusable per-kernel job; test kernels are built with BPF_JIT_ALWAYS_ON + BTF and cached. Requires the CARGO_REGISTRIES_GEN0SEC_TOKEN secret to resolve dendrite-core BPF headers.
pigri
force-pushed
the
fix/xdp-verifier-6x
branch
from
14c3f8c to
9815922
Compare
Make managed IDS work with just `ids.enabled: true`, driven by the platform's rule-class selection (ids_rules.classes), without extra local wiring: - managed_rules_base_url falls back to the platform API URL (platform.base_url) when unset. - managed_rules_path defaults to /var/lib/synapse/managed.rules. - egress_ifaces defaults to ["auto"] (the XDP-attached NICs); "auto" entries are a no-op marker, not a literal interface name. - config.yaml: document the above as commented defaults.
Both are Vec<String>, but a single value is the common case and `egress_ifaces: auto` (a scalar) is the natural way to write it — which failed with "invalid type: string "auto", expected a sequence". Accept either a bare string or a sequence via a string-or-vec deserializer.
- Fingerprint prober logs (target synapse_utils::fingerprint::*, e.g. the JA4X prober) were landing in app.log via the root logger. Route them to a dedicated, size-rotated fingerprint.log with additive(false) so they stay out of app.log. - eventbridge.log used a plain append writer with no rotation, unlike the log4rs-managed app/error/access logs. Add size-based rotation (max_log_size / log_file_count) so every log file rotates under the same policy.
blocked.log (synapse-blocking-log) used a plain mutex-guarded append writer with no rotation, unlike the log4rs-managed app/error/access logs. Add size-based rotation driven by the same logging.max_log_size / log_file_count config so blocked.log rotates under the same policy as every other log file.
telemetry.log (synapse-telemetry file sink) used a plain Mutex<File> append writer with no rotation. Add size-based rotation via new Config.max_log_size / log_file_count, wired from logging.max_log_size / log_file_count — completing rotation coverage across every synapse log file (log4rs app/error/access/terminal/fingerprint plus the custom-writer blocked/eventbridge/telemetry logs).
- init_ids_runtime bailed when rule_paths was empty even with managed_rules_path set (contradicting the field's own doc). Now it starts when either is configured, and seeds an empty managed_rules_path file so the engine starts at 0 rules and hot-reloads when the bundle lands. - The ids-rules sync only re-ran on its interval or a dedicated ids-rules-updated event, so a config-updated that changed ids_rules (and the startup race where the first sync precedes the initial config apply) left managed rules empty until the next interval. set_global_config now fires an on_ids_rules_change callback — config is stored before the callback, so the sync reads the fresh selection with no race.
synapse built thalamus RuleVars straight from the (often empty) config, bypassing thalamus's own RuleVars::default() — so managed rules referencing $HTTP_PORTS / $ORACLE_PORTS / $SSH_PORTS / etc. fell back to `any` (over-broad matching + potential false positives) and flooded a per-rule "Unknown port/address variable" WARN. Add build_rule_vars(): thalamus defaults, then the standard Suricata port/ address vars it omits (inserted only when absent), then operator config on top. Used by all four rule-loading paths (XDP, AF_PACKET, L7 ingest, Windows).
When geoip config leaves asn/city url+path empty (e.g. IPinfo Lite's
combined DB already carries ASN), asn_path/city_path resolve to None but
the manager still fell back to GeoLite2-{ASN,City}.mmdb and refresh_all
warned every cycle about the missing file. Track whether each optional DB
is configured and skip its refresh (and warning) when it isn't. Country
is unchanged.
- rustfmt the recent logging/config/ids changes (fmt job was failing). - Update test/example call sites for the changed signatures that `clippy --all-targets` compiles: the eventbridge_log::start(path, max, count) test call, and synapse_telemetry::Config's new max_log_size / log_file_count fields in the e2e_emit example.
to_log_line() emitted layer/action/src/dst/rule/msg/hits/fingerprints but dropped `source` (primary driving signal) and `signals` (all contributing detectors) — which the OTEL block_event already carries from the same BlockEvent. Add both, plus a canonical BlockSource::as_str() so the local line uses identical names to the remote record (e.g. source=threat_intel, signals=fingerprint_rule,ids).
… blocks The platform derives the geo columns (country, ASN) server-side, so the local blocked.log line lacked them. The smart-firewall already resolves source-IP geo for the wirefilter ip.src.country/asn fields, so carry it onto the BlockEvent via a new Builder::geo() and emit it in to_log_line. Wired at the five external-facing smart-firewall emit sites (kernel_pump XDP + afpacket); the east-west microseg path is left out (internal, geo N/A). Fields are optional and skipped when unset, so other layers' lines are unchanged.
blocked.log was human-readable text only. Add logging.blocked_log_format (text|json, default text): in json mode the file sink writes one serde BlockEvent object per line — the same JSON shape already used on the log::info fallthrough — carrying every field (source/signals/geo/ fingerprints/eve). init() gains a `json` flag; unknown format values fall back to text. Text remains the default, so existing deploys are unchanged.
The smart-firewall block message was a static "smart_firewall_rules wirefilter match (kernel_pump)" — internal jargon (kernel_pump is a module name) that says nothing about why the block fired. Carry the matched rule's wirefilter expression through EvalMatch (via a name->expr map kept alongside the ruleset, since CompiledRule doesn't expose its source text) and emit it as rule_msg, so the log shows the actual condition, e.g. `threat.score ge 90 and threat.advice eq "block"`. The identity edge-drop path keeps an "identity edge-drop —" prefix since that enforcement detail (edge drop vs IP ban) isn't in the expression.
The earlier guard (7fc9ddc) used asn_path.is_some(), but the config push emits an empty-string path for an unconfigured DB, which serde deserializes to Some(PathBuf("")) — is_some() is true, so refresh_all still tried ASN/City, MaxMindManager fell back to its default GeoLite2-*.mmdb filename, and the "Failed to open MMDB file" warning kept firing every refresh. Require a non-empty path for asn_configured / city_configured.
pigri
force-pushed
the
fix/xdp-verifier-6x
branch
from
2a0d3f7 to
e1a425e
Compare
Add `ml: { fingerprint_classifier, traffic_classifier, penalty }` as the
canonical home for the ML model configs. `classifier` was an over-generic
name for the JA4+ fingerprint model; it becomes `ml.fingerprint_classifier`,
parallel to `ml.traffic_classifier` / `ml.penalty`.
The deprecated top-level keys (`classifier`/`traffic_classifier`/`penalty`)
are still accepted and migrated into the flat runtime fields at load with a
deprecation warning, so existing platform-pushed configs keep working while
the config-generator transitions to emitting `ml:`. The `ml` section and the
migration are feature-gated behind `classifier`, like the fields they group.
Move the JA4+ classifier example under `ml.fingerprint_classifier` and document `ml.traffic_classifier` / `ml.penalty` alongside it, matching the new schema. The Windows example is updated in kind (still commented — ML is Linux-only).
…restart
The worker tracked the applied ruleset version only in memory (RwLock, None on
a fresh process), so every restart re-downloaded the full managed ruleset
(~49k rules) even when managed.rules on disk was already the wanted version —
the version-equality skip at reconcile could never fire at startup.
Write a `{managed_rules_path}.version` sidecar (AppliedState JSON) on each
successful sync and seed `applied` from it in `new()`, so a restart with a
current on-disk ruleset skips the redundant re-download. The sidecar is only
trusted when managed.rules itself exists; a missing/unparseable sidecar falls
back to a normal sync. Mirrors the traffic/penalty model `.version` sidecar.
A model with enabled:true but no model_path/manifest_url silently never registered its worker — nothing loaded, nothing downloaded, and the log stayed empty, leaving operators to guess why. Emit a per-model startup warning naming the missing field(s), for fingerprint_classifier / traffic_classifier / penalty (the download models also flag a missing manifest_url with no local file to fall back on).
An enabled traffic/penalty model previously needed model_path + manifest_url + refresh_secs all set explicitly or it silently did nothing. Now `enabled: true` alone works: model_path → <working dir>/<name>.onnx (service WorkingDirectory=/var/lib/synapse) manifest_url → <platform.base_url>/models/<name>/manifest refresh_secs → 3600 fingerprint_classifier (local, no manifest) gets the model_path default only. Any field set explicitly still overrides its default. Example config updated.
Add default_data_dir() = process cwd (the systemd units pin WorkingDirectory=
/var/lib/synapse), falling back to /var/lib/synapse when cwd is the filesystem
root or unavailable. Route the data-file path defaults through it so they
follow one working directory instead of scattered hardcoded paths:
- managed_rules_path: /var/lib/synapse/managed.rules → {data_dir}/managed.rules
- acme storage: /tmp/synapse-acme → {data_dir}/acme (was volatile — certs
lost on reboot, risking Let's Encrypt rate limits)
- ml.*.model_path already derived from cwd; now shares the helper
Log paths (/var/log), pid files and sockets (/run) are intentionally left
alone — those don't belong under the working dir.
Add a `logging.module_levels` map (target → level) so one component can be turned up without flooding the whole log — e.g. `synapse_idp: debug` for Thalamus (IDS) debug only, while everything else stays at `logging.level`. Each entry becomes a log4rs per-target Logger (additive → routes to the same app.log/console appenders); log4rs raises the global max level so the finer records actually flow. Invalid levels are warned and skipped.
The rule-expression map static (added for rule_msg = matched expression) was left ungated while OnceLock/RwLock and the sibling GLOBAL_RULESET / GLOBAL_SOURCES statics are cfg(feature = "amygdala-reactor") only. Non-reactor builds — the Windows MSI job (amygdala-ndis) — then failed to compile with "cannot find type OnceLock/RwLock in this scope". Gate GLOBAL_EXPRESSIONS the same way; everything that reads it already lives in reactor-gated functions.
`evaluate()` used RuleSet::first_match, so the first matching rule — even a staged `log` rule — stopped evaluation and shadowed any enforcing rule behind it. Change it to walk all rules (RuleSet::iter + CompiledRule::matches): `log` matches are collected and evaluation continues; the first non-log match (drop/allow) terminates the chain. Returns Vec<EvalMatch> (log entries then one optional enforcing entry). All three call sites (kernel_pump north-south, afpacket, east-west microseg) updated to emit a Notice per log match and act on the enforcing one. Non-reactor build returns an empty vec.
Firewall — per-(src,dst) north-south edge-drop across three backends: XDP (banned_edges_v4/v6 maps), nftables (saddr.daddr timeout set + forward chain) and iptables (FORWARD chain), with a userspace TTL sweeper. IDS/SNI — extract tls.sni on one-sided / NAT'd captures and route it over the eventbridge inspect broadcast so the smart-firewall tls.sni field populates. Consumes thalamus 0.0.12 (drops the local [patch]; bumps the pin across all consumers). Security-event log — rename blocked.log -> events.log and the BlockEvent schema -> SecurityEvent (v2). Config keys events_log_file/events_log_format (blocked_log_* kept as serde aliases). Add sni, traffic_class/confidence, classifier_score, penalty_severity/bucket, ids_sids, geo and eve to the record, JSON schema and OTel attributes. Split the smart-firewall log-rule action into a distinct 'log' (notice stays for detector detect-only). Stores — ids_signals retains contributing SIDs; new penalty_signals holds per-source penalty-model decisions. Also fixes a stale dns_fqdn_repro test against the Vec-returning evaluate API.
Collapse the eprintln! call that nightly rustfmt fits on one line (the wellness-check Formatting gate uses nightly).
The access-rules EdgeFallback dispatch calls ban_edge/unban_edge on NftablesFirewall/IptablesFirewall, but on not(all(unix, feature="bpf")) targets — Windows (not unix) and no-bpf legacy builds — those resolve to the firewall_noop stubs, which lacked the methods, so the build failed. Add no-op edge methods (+ the IpAddr import) so the dispatch compiles on every target; real nftables/iptables edge-drop stays a Linux+bpf feature.
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
2 participants
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.
fix(synapse-security/bpf): XDP program exceeds the BPF verifier limit on 6.x —
bpf_loop+ off-stack fingerprint data; add cross-kernel CISummary
The XDP firewall/fingerprint program
(
crates/synapse-security/src/firewall/bpf/xdp.bpf.c) fails to load on stock 6.xkernels: the verifier rejects it with
-E2BIG(
BPF program is too large. Processed 1000001 insns). Where it can't load,synapse silently drops to the AF_PACKET capture fallback — functional, but no XDP
fast path and no in-XDP drop.
This PR makes the program verify across kernel generations and adds a CI job that
proves it.
Root cause
The TCP-option parse loops in
lib/tcp_fingerprinting.h(
parse_tcp_mss_wscale,fill_ja4t_option_kinds) walkPTR_TO_PACKETwith avariable stride. The verifier re-explores the loop body for every reachable
packet offset; the resulting state explosion pushes total processed instructions
just past the 1,000,000 ceiling.
It is not kernel-version-specific — measured identically (1,000,001 insns) on
x86 6.1/6.6/6.12/6.18/6.19 and on arm64 6.6. Newer verifiers do not prune it
under the limit, and compiler flags (
-O2/-O3/unroll) don't change it. The fixhas to reduce the verifier's work.
Fix (
lib/tcp_fingerprinting.h, behavior-preserving)bpf_loop()for option parsing. Each option loop now copies the ≤40option bytes once (constant-size ladder) into a per-CPU scratch buffer and
walks it via
bpf_loop(), so the verifier checks the body a single time.→ 1,000,001 → 168,876 processed instructions.
bpf_loop()adds a callback frame;combined with the 96-byte
struct tcp_fingerprint_dataheld on the stack ofthe reachable subprograms, the call chain then broke the 512-byte BPF stack
limit (
combined stack size … 544. Too large). Moving that struct into aper-CPU scratch map value clears it.
Option fidelity and all map keys/values are unchanged. Per-CPU scratch is
race-free under XDP (preemption disabled).
Before / after
Real object, recompiled against each kernel's own BTF (CO-RE-off), JIT on:
CI (
.github/workflows/xdp-e2e*.yml,tests/e2e-xdp/)Adds a cross-kernel verification gate (matrix 6.6.125 / 6.12.94 / 6.19.14),
modeled on the k8spacket reusable-workflow pattern. Each leg dumps the target
kernel's BTF → recompiles
xdp.bpf.cagainst it (CO-RE-off, so a FAIL is a realverifier rejection, not an offset mismatch) → loads it in QEMU with
bpf_jit_enable=1→ asserts the program verifies. Test kernels are built withBPF_JIT_ALWAYS_ON + DEBUG_INFO_BTFand cached; x86 uses KVM when the runnerprovides it, TCG otherwise.
Requires a
CARGO_REGISTRIES_GEN0SEC_TOKENrepo secret (to resolvedendrite-coreBPF headers viacargo metadata).Testing
make -C tests/e2e-xdp e2e KERNEL=6.6.125).Notes
targets.