Repository navigation
feat(rust-plugins): SNMPv3 with the User-based Security Model - #6421
Open
julienmathis wants to merge 8 commits into
Open
julienmathis wants to merge 8 commits into
julienmathis wants to merge 8 commits into
Conversation
…, bound walks, timeouts & retries The engine blindly trusted the agent it queried, which is precisely the untrusted party of the dialogue: - Agent errors are now detected: a response with error-status != 0 produces a typed error naming the RFC 3416 status (e.g. "noSuchName"). - Request/response correlation: every request carries a fresh request-id, and received datagrams are validated (request-id, community echo, PDU type) before being accepted — unrelated datagrams (e.g. a late retransmission of a previous request) are discarded instead of being consumed as the current answer. - Bounded walks: OIDs must be strictly increasing during a walk (the classic snmpwalk "OID not increasing" guard) and a single walk may collect at most 100 000 values — both protect against a buggy or malicious agent looping the walk or streaming endless data. - Configurable timeouts and retries via three new CLI options: --timeout (per-attempt receive timeout, default 1s), --snmp-retries (default 2), and --collect-timeout (global budget for the whole collection, default 50s, so the plugin exits with a clean UNKNOWN before centengine's own kill timeout). - The UDP receive buffer was 1024 bytes, silently truncating large bulk responses; raised to 65535. Connection parameters (target, community, timeouts, retries) are now threaded through a single SnmpConfig instead of loose string arguments, and the two walk loops share one implementation. Test plan: - cargo build --release succeeds - cargo test: 75 passed, 0 failed (10 new: RFC 3416 names, response validation, non-increasing OID, varbind cap, expired deadline) - Manual check against an unroutable address: default settings exit UNKNOWN after retries*timeout with the attempt count in the message; --collect-timeout correctly preempts a longer per-attempt timeout
…-failure message The protocol-hardening commit changed the no-connection error message from "Could not connect to X is the hostname..." to "No valid SNMP response from X after N attempts (timeout Ts per attempt)" (clearer: it names the retry budget actually exhausted), but never updated the Robot fixture that pins it. Both cgs-no-connection cases have been failing CI since this branch was opened.
julienmathis
force-pushed
the
feat/snmpv3-usm-official
branch
from
September 8, 2026 12:16
25b80be to
e098d48
Compare
snmp_bulk_get built a GetBulkRequest PDU for every "get" query, relying on a workaround that stripped a trailing .0 and depended on GetNext landing exactly one leaf ahead. GetBulk (even with max-repetitions=1) can never perform an exact match, so this silently returned the wrong value for any non-.0-suffixed OID, such as a specific row of a multi-row table. Every currently shipped definition only queries .0-suffixed scalars, so the bug was latent. Switch to a real GetRequest and drop the trailing-zero workaround, which is no longer needed.
…ips on table walks Every change here is backed by a measurement (synthetic 10 000-row ifTable, 50 000 varbinds, snmpsim), not assumed: - GetBulk max-repetitions raised from a hardcoded 10 to a default of 50 (Perl parity), configurable globally via --maxrepetitions and per-query via "max-repetitions" in the JSON collect entry. On the synthetic table this cuts requests from 5001 to 1001 (a walk of N rows now costs ceil(N/max_repetitions) round-trips instead of ceil(N/10)). - Nagios thresholds are now parsed once per metric (parse_threshold) instead of once per vector element — a 10 000-row table with warning+critical thresholds no longer reparses the same two strings 10 000 times, and threshold syntax errors now name the metric and field. - The UDP socket and receive buffer are allocated once per walk instead of once per request (open_socket / shared buffer threaded through send_request), removing a bind+connect+alloc per round trip on large walks. - eval_str's macro-matching regex is compiled once (OnceLock) instead of on every template evaluation. Test plan: - cargo build --release succeeds - cargo test: 75 passed, 0 failed - Manual check against an unroutable address: retry/timeout behavior unchanged (UNKNOWN + exit 3 after the configured attempts)
…Perfetto export
Replaces log/env_logger with tracing/tracing-subscriber across the
crate, in three activation modes:
- default (production): silent on stderr, zero measurable overhead.
- PLUGIN_LOG=debug (or any level, mirroring the historical env var):
structured logs plus **span durations on close**
(`walk{oid=...} close time.busy=5.65ms`) — which stage was slow on
this host, from one env var. PLUGIN_LOG's default moves from `info`
to `warn`: a plugin must be silent on stderr nominally, and per-value
info-level formatting was hot-path waste (deliberate behavior change).
- --trace-file <path>: full Chrome-trace recording, loadable in
Perfetto — validated manually (JSON parses, span hierarchy present).
Spans: check -> collect -> walk{oid}/get -> request{id} per attempt,
metric{name}, aggregation{name}, output.
No OTLP/collector export: a process running at high check-per-minute
rates must not ship spans over the network; out of scope by design.
snmp_plugin() now returns the exit code instead of calling
process::exit mid-flight: destructors run on every path, so the trace
flush guard (held for the whole function) always writes the file
before the process exits. main() calls process::exit(code) once, at
the top level. The CLI argument loop is flattened to
`while let Some(arg) = parser.next()? { ... }`, so a lexopt parse
error now goes through the same UNKNOWN + exit(3) path as every other
error instead of a bare `Error: {err}` + exit(1).
Test plan:
- cargo build --release / cargo test: 75 passed, 0 failed
- Manual: nominal run has 0 bytes on stderr; PLUGIN_LOG=debug shows
nested check/metric/aggregation spans with time.busy/time.idle on
close; --trace-file produces a valid Chrome-trace JSON; unknown-flag,
missing-JSON, --help and unroutable-target retry behavior unchanged
SNMP counters (ifInOctets, ...) are monotonically increasing values;
turning them into per-second rates requires the value and timestamp of
the previous run. Adds a "rate": true field on a collect entry:
{ "name": "if", "oid": "1.3.6.1.2.1.2.2.1", "query": "Walk",
"labels": { "1.2": "descr", "1.10": "in", "1.16": "out" },
"rate": true }
Every numeric value of the entry becomes a per-second rate computed
against the previous run; string columns (labels) are left untouched
and vectors stay aligned.
Design:
- Identity = full OID, never the table position — a new interface
appearing between runs cannot shift its neighbors' rates.
- State files (new src/state.rs module): mode 0600 + atomic write
(temp file + rename + fsync) — never world-readable, never
half-written. A read failure (missing/corrupt file) is never fatal
(fresh start); a write failure IS fatal (a stale reference would
silently produce wrong rates forever).
- 32-bit counter wraparound corrected; a 64-bit decrease or a missing
previous instance is treated as a reset and yields one aligned 0.0
sample rather than desynchronizing the vector.
- First run: "OK: Buffer creation", exit 0 (Perl parity).
- New --statefile-dir CLI option (Perl parity, default
/var/lib/centreon/centplugins).
- Instrumented with state_read/state_write spans (visible via
PLUGIN_LOG=debug or --trace-file, see the tracing PR).
SnmpResult gains a `samples: Vec<(item_key, full_oid, value)>` field,
populated only when a collect entry asks for rates (capture_samples
threaded through the walk/get functions and process_response), so
apply_rate can rebuild each numeric vector in original push order
while keying state persistence by OID.
Test plan:
- cargo build --release / cargo test: 84 passed, 0 failed (9 new:
wraparound, reset, dt<=0, state roundtrip + 0600 assert + corrupt-file
recovery, two-run rate integration, new-instance alignment, sample
capture)
- --check-format validates the new example (examples/new-traffic-rate.json)
- Manual: state files created under a temp statefile-dir are mode 0600
and survive a corrupt-content injection without crashing (exercised
by the state.rs unit tests directly against the real filesystem)
Positional alignment silently pairs values of DIFFERENT instances when a table has holes or two tables have different instance sets — a correctness bug with no crash to reveal it. - Index alignment is now the default (not optional) semantics for labeled walks: row indices (last OID arc) are captured per column, and the columns of one entry keep only the rows present in ALL of them, in ascending walk order; drops are counted in a warning. - New optional "join" field on a collect entry: entries sharing a group name are aligned ACROSS entries (the ifTable + ifXTable case). - Rate integration: alignment runs before apply_rate and filters the captured samples with the same mask, so state and vectors stay parallel. - Prefix indexing guarded: a prefix vector shorter than the values now falls back to the numeric counter with a warning instead of panicking out of bounds. SnmpResult gains an `indices: HashMap<String, Vec<u32>>` field, populated only for labeled walks (an invariant of using labels, not a caller-configurable option) — the join key align_by_index intersects across the listed columns. Test plan: - cargo build --release / cargo test: 88 passed, 0 failed (4 new: holes dropped consistently across numeric+string columns, disjoint sets -> empty vectors, rate samples follow the alignment mask, cross-entry group join) - --check-format validates existing collections and the new examples/new-traffic-rate.json unchanged
rasn-snmp provides the v3 ASN.1 structures; everything that makes them *secure* is implemented in the new src/snmp/usm.rs: - Key derivation: password-to-key over the 1 MB expansion (RFC 3414 A.2) + localization to the authoritative engine. - Authentication: truncated HMAC over the whole message — MD5, SHA-1 (RFC 3414) and SHA-224/256/384/512 (RFC 7860). The RFC 7860 tag lengths are encoded in the protocol names themselves (usmHMAC128SHA224 = 128 bits, i.e. 16 bytes, not the 12 bytes MD5/ SHA-1 use) and implemented as such here. - Privacy: CBC-DES (RFC 3414 §8) and CFB128-AES-128 (RFC 3826), unique salt per message. - Session: engine discovery + time synchronization (RFC 3414 §4), done once per collection (not per request). - Context: --contextname / --contextengineid (Perl parity). CLI mirrors the Perl options: --snmp-username, --authprotocol, --authpassphrase, --privprotocol, --privpassphrase. Misconfigurations fail fast: privacy without authentication, credentials without --snmp-version=3, half-specified protocol/passphrase pairs. An agent refusing credentials answers with an unauthenticated Report (it cannot sign with a key it does not share). That report is read before the digest check and its usmStats counter translated into a human cause instead of a cryptic "malformed digest". Deliberate scope limit: AES-192/256 are rejected with an explicit message rather than implemented — they exist only as expired drafts with two incompatible key-extension variants (Blumenthal, Reeder); picking the wrong one fails to interoperate silently. Test plan: - cargo build --release / cargo test: 99 passed, 0 failed (11 new), including the RFC 3414 §A.3 key-localization test vectors for MD5 and SHA-1 pinned as unit tests (byte-exact match against the RFC's own worked example), plus DES/AES roundtrips and salt uniqueness - Manual: --help lists the new flags; --snmp-version=3 without --snmp-username, half-specified auth/priv pairs, priv-without-auth, and --privprotocol=AES256 all fail fast with a clear UNKNOWN message and exit 3; a discovery attempt against an unroutable address times out cleanly through the existing retry/collect-timeout machinery (no panic, no hang) IMPORTANT — not yet interop-tested: unlike the rest of this series, this PR has NOT been validated against a real SNMPv3 agent (no v3 device/simulator available in this environment). The RFC 3414 test vectors confirm the key-derivation math is byte-correct, and the DES/AES roundtrip tests confirm internal self-consistency, but neither proves wire compatibility with a real agent (e.g. an oracle like net-snmp) — that requires a dedicated SNMPv3 test setup and should happen before merging this one.
julienmathis
force-pushed
the
feat/snmpv3-usm-official
branch
from
September 22, 2026 20:46
e098d48 to
c806c27
Compare
github-actions
Bot
added
skip-workflow-Generic Plugins
and removed
skip-workflow-Generic Plugins
labels
Contributor
Author
|
The `plugins` / `check-status` checks are failing on `test-plugins jammy amd64` and `test-plugins noble amd64`, both with `RuntimeError: Connector is not running.` from `tests/resources/connector.py` (the Perl connector daemon binary is absent in these two runner environments, so every robot test that goes through `ctn_send_to_connector` fails outright). This is unrelated to this PR's changes (SNMPv3/USM support, plus a GetRequest-vs-GetBulk protocol fix inherited from the rebase) — those only touch `rust-plugins/src/*.rs`, never the Perl connector or the robot test harness. It's also not new: this same `test-plugins` job has failed on every CI run of this branch since it was created, each time on a different distro/arch (`bullseye arm64` on 2026-09-04 and 2026-09-08, now `jammy`/`noble amd64`), always with the same connector-not-found root cause. Confirmed reproducible after two reruns. Flagging as a known pre-existing CI flake in the connector test harness rather than something this PR introduced. |
Contributor
|
please update with the base, a key delivery fix has been merged in develop to ensure cpan libs for tests do exist in unstable on packages.centreon.com |
This branch has not been deployed
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.
Unlike the rest of this series, this PR has not been validated against a real SNMPv3 agent — no v3 device/simulator was available in the environment this was written in. The RFC 3414 §A.3 test vectors confirm the key-derivation math is byte-correct, and the DES/AES roundtrip tests confirm internal self-consistency, but neither proves wire compatibility with a real agent (e.g. against net-snmp as an independent oracle). That requires a dedicated SNMPv3 test setup and should happen before merging this one — please treat this as a draft for review/testing help rather than merge-ready.
Summary
rasn-snmpprovides the v3 ASN.1 structures; everything that makes them secure is implemented in the newsrc/snmp/usm.rs:--contextname/--contextengineid(Perl parity)CLI mirrors the Perl options:
--snmp-username,--authprotocol,--authpassphrase,--privprotocol,--privpassphrase. Misconfigurations fail fast: privacy without authentication, credentials without--snmp-version=3, half-specified protocol/passphrase pairs.Note the RFC 7860 tag length for SHA-224 is 16 bytes, not 12: the protocol names encode the truncation (
usmHMAC128SHA224= 128 bits) — implemented as such here, worth double-checking in interop testing since it's an easy spot to get wrong.An agent refusing credentials answers with an unauthenticated Report (it cannot sign with a key it does not share). That report is read before the digest check and its
usmStatscounter translated into a human cause — instead of a cryptic "malformed digest".Deliberate scope limit
AES-192/256 are rejected with an explicit message rather than implemented: they exist only as expired drafts with two incompatible key-extension variants (Blumenthal, Reeder); picking the wrong one fails to interoperate silently. Better a clear error than a mystery.
Test plan
cargo build --release/cargo test: 99 passed, 0 failed (11 new), including the RFC 3414 §A.3 key-localization vectors for MD5 and SHA-1 pinned as unit tests (byte-exact match against the RFC's own worked example), plus DES/AES roundtrips and salt uniqueness--helplists the new flags;--snmp-version=3without--snmp-username, half-specified auth/priv pairs, priv-without-auth, and--privprotocol=AES256all fail fast with a clearUNKNOWNmessage and exit 3; a discovery attempt against an unroutable address times out cleanly through the existing retry/collect-timeout machinery (no panic, no hang)