From 51a5ed26ad2ed11d4308aa53b21a493236f96db4 Mon Sep 17 00:00:00 2001 From: utarafdar Date: Wed, 15 Jul 2026 15:49:45 +0530 Subject: [PATCH] Status-Server liveness, multi-server comparison, --bind source interface --- README.md | 70 ++++++++++- cmd/authhound-probe/leak_test.go | 1 + cmd/authhound-probe/main.go | 186 ++++++++++++++++++++++++---- cmd/authhound-probe/main_test.go | 58 +++++++++ docs/json-schema.md | 47 ++++++- internal/check/blastradius.go | 2 +- internal/check/check.go | 7 ++ internal/check/compare.go | 158 +++++++++++++++++++++++ internal/check/compare_test.go | 88 +++++++++++++ internal/check/eaptls.go | 11 +- internal/check/eaptls_auth.go | 11 +- internal/check/mtu.go | 4 +- internal/check/pap.go | 2 +- internal/check/peap.go | 11 +- internal/check/reachability.go | 2 +- internal/check/secret.go | 2 +- internal/check/statusserver.go | 63 ++++++++++ internal/check/statusserver_test.go | 74 +++++++++++ internal/check/ttls.go | 11 +- internal/radius/client.go | 10 +- internal/radius/client_test.go | 2 +- internal/radius/eapsession.go | 13 +- internal/radius/mtu.go | 5 +- internal/radius/packet.go | 16 +++ internal/report/compare.go | 20 +++ internal/report/json.go | 66 ++++++++-- test/freeradius-smoke.sh | 71 ++++++++++- test/lab/docker-compose.yml | 19 +++ 28 files changed, 959 insertions(+), 71 deletions(-) create mode 100644 internal/check/compare.go create mode 100644 internal/check/compare_test.go create mode 100644 internal/check/statusserver.go create mode 100644 internal/check/statusserver_test.go create mode 100644 internal/report/compare.go diff --git a/README.md b/README.md index 757bf85..d454d4a 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ Skipped this step? The probe notices: on a first-run timeout it prints this exac | Check | What it proves | |---|---| +| **Status-Server** | An [RFC 5997](https://www.rfc-editor.org/rfc/rfc5997) liveness ping that runs first and **consumes no authentication attempt** — nothing shows up in the server's auth log. PASS if the server answers; a neutral INFO (never a failure) if it doesn't, since many servers leave it off. See [Liveness & multi-server](#liveness--comparing-servers). | | **Reachability** | The server answers on UDP/1812 — and how fast. A timeout means unreachable, not listening, **or the probe isn't whitelisted / the secret is wrong** (servers silently drop unverifiable requests). | | **Shared secret** | Cryptographically verifies the server's reply signature. A pass *proves* the secret matches — no more guessing whether "everyone's getting rejected" is a secret problem or something else. | | **BlastRADIUS posture** | Observes whether the server signs its replies with a **Message-Authenticator** — the mitigation for the RADIUS/UDP reply-forgery flaw [CVE-2024-3596](https://blastradius.fail) ("BlastRADIUS"). PASS if it does; WARN, with config pointers, if it accepts the probe's (signed) request but replies unsigned. Observation only — see below. | @@ -193,7 +194,7 @@ $ authhound-probe radsec test --server radius.corp.com \ | Flag | Purpose | |---|---| -| `--server HOST[:port]` | RADIUS server (default port 1812). **Required.** | +| `--server HOST[:port]` | RADIUS server (default port 1812). **Required.** Comma-separate several to compare them — see [Comparing servers](#liveness--comparing-servers). | | `--secret SECRET` | Shared secret (**required**, but prefer `AUTHHOUND_SECRET` / `--secret-file` / `--secret-stdin` — see [below](#where-credentials-come-from)). | | `--secret-file FILE` | Read the shared secret from a file (must not be world-readable on unix). | | `--secret-stdin` | Read the shared secret from standard input (one line). | @@ -211,6 +212,7 @@ $ authhound-probe radsec test --server radius.corp.com \ | `--server-name NAME` | Expected server-certificate name (TLS SNI). | | `--nas-id NAME` | NAS-Identifier to send (default `authhound-probe`). | | `--timeout DURATION` | Per-request timeout (default `5s`). | +| `--bind IP[:port]` | Source IP to send from, for pinning the outgoing interface on a multi-homed host — see [Binding a source interface](#binding-a-source-interface---bind). | | `--json` | Machine-readable output for scripts / RMM ([schema](docs/json-schema.md)). | | `--strict` | Exit non-zero on **warnings** too (e.g. a soon-to-expire cert), for scheduled monitoring. | | `--no-color` | Force plain output. Colour is auto-detected otherwise — see [Colour](#colour-and-windows-terminals). | @@ -328,6 +330,72 @@ With `--json`, each iteration's results plus per-check aggregate statistics (success counts, timeouts, latency min/median/p95/max) appear in an additive `repeat` block — see the [schema](docs/json-schema.md). +### Liveness & comparing servers + +**Status-Server** runs first: an [RFC 5997](https://www.rfc-editor.org/rfc/rfc5997) +liveness ping that a server answers *without* logging an authentication attempt. +It's the polite way to ask "are you alive?" — nothing shows up in the auth log. +Many servers leave it off, so silence here is a neutral **INFO**, never a +failure; the reachability check below is the authoritative one. To turn it on in +FreeRADIUS, set `status_server = yes` in `radiusd.conf`. + +**Comparing servers.** Almost every site runs a primary and a secondary RADIUS +server, and clients reach them through DNS round-robin or a shared VIP. When one +of the pair is quietly broken — down, unregistered, or drifted out of config — +roughly half of authentications fail *depending on which server the client +happened to hit*. That is the single most common hidden cause of "it works +sometimes" tickets, and a single-server test can't see it. Comma-separate the +servers and the probe tests each, then prints a comparison: + +```console +$ export AUTHHOUND_SECRET='shared-secret' +$ authhound-probe radius test --server radius1.corp.com,radius2.corp.com --pap alice + +=== Server 1/2: radius1.corp.com:1812 === +... per-check results ... +Verdict: 4 passed, 0 failed, 0 warnings, 5 skipped + +=== Server 2/2: radius2.corp.com:1812 === +... per-check results ... +Verdict: 0 passed, 1 failed, 0 warnings, 8 skipped + +Comparison across servers: + radius1.corp.com:1812 is responding, but radius2.corp.com:1812 is NOT. If + clients reach these servers via DNS round-robin or a shared VIP, roughly 50% + of authentications would fail intermittently depending on which server they + land on — the classic 'it works sometimes' ticket. Take the unresponsive + server(s) out of rotation or bring them back. +``` + +The verdict also covers the subtler case where both servers answer but +**disagree** — one accepts a login the other rejects, or assigns a different +VLAN — which points at config or replication drift between them. + +The exit code follows the combined result: a FAIL on *any* server fails the run +(and under `--strict`, a WARN does too). Each server is still bounded by the same +hard-coded rate ceiling; comparing servers never raises the load on any one of +them. `--count` and multiple `--server` are mutually exclusive — chase +intermittency on one server, compare across servers separately. With `--json`, +per-server blocks and the verdict appear in additive `servers` / `comparison` +fields — see the [schema](docs/json-schema.md#servers--comparison-multiple---server). + +### Binding a source interface (`--bind`) + +Jump boxes and monitoring servers are usually multi-homed. `--bind IP[:port]` +pins the source address the probe sends from, so RADIUS leaves the interface you +intend (and reaches a server whose firewall only permits that address): + +```console +$ authhound-probe radius test --server radius.corp.com --bind 10.20.0.5 +``` + +The source must be a local IP literal on this host (not a hostname). This is also +the address the server sees, so it's the one to register as a RADIUS client — and +the [Step 0 registration snippet](#step-0--register-the-probe-on-your-server-one-time) +the probe prints on a timeout automatically reflects the bound IP, so you can +paste it as-is. (If NAT sits between this host and the server, the server still +sees the post-NAT address — register that instead.) + ### Where credentials come from This tool is meant to run on shared jump boxes, so it never *requires* a secret or diff --git a/cmd/authhound-probe/leak_test.go b/cmd/authhound-probe/leak_test.go index bdc6bca..f580331 100644 --- a/cmd/authhound-probe/leak_test.go +++ b/cmd/authhound-probe/leak_test.go @@ -32,6 +32,7 @@ func TestNoCredentialLeak(t *testing.T) { NASIdentifier: "authhound-probe", } checks := []check.Check{ + check.StatusServer{}, check.Reachability{}, check.SharedSecret{}, check.PAP{User: "alice", Pass: sentinelPass}, diff --git a/cmd/authhound-probe/main.go b/cmd/authhound-probe/main.go index a5e53b6..ba6779f 100644 --- a/cmd/authhound-probe/main.go +++ b/cmd/authhound-probe/main.go @@ -13,8 +13,10 @@ package main import ( "context" + "errors" "flag" "fmt" + "net" "os" "os/signal" "runtime/debug" @@ -173,7 +175,7 @@ func cmdRadsecTest(args []string) int { func cmdRadiusTest(args []string) int { fs := flag.NewFlagSet("radius test", flag.ExitOnError) - server := fs.String("server", "", "RADIUS server host or host:port (default port 1812)") + server := fs.String("server", "", "RADIUS server host or host:port (default port 1812); comma-separate several to compare them (e.g. primary,secondary)") secret := fs.String("secret", "", "shared secret (leaks into shell history/ps — prefer AUTHHOUND_SECRET, --secret-file, or the prompt)") secretFile := fs.String("secret-file", "", "read the shared secret from this file (must not be world-readable on unix)") secretStdin := fs.Bool("secret-stdin", false, "read the shared secret from standard input (one line)") @@ -193,6 +195,7 @@ func cmdRadiusTest(args []string) int { count := fs.Int("count", 1, "run the checks N times (2..50) and report aggregate statistics — for chasing intermittent failures") interval := fs.Duration("interval", 2*time.Second, "pause between --count iterations (a hard-coded safety floor applies)") timeout := fs.Duration("timeout", 5*time.Second, "per-request timeout") + bind := fs.String("bind", "", "source IP[:port] to send from, for pinning the outgoing interface on a multi-homed host") jsonOut := fs.Bool("json", false, "emit results as JSON instead of text") noColor := fs.Bool("no-color", false, "disable ANSI colour") strict := fs.Bool("strict", false, "exit non-zero on warnings too (for scheduled monitoring)") @@ -211,9 +214,20 @@ func cmdRadiusTest(args []string) int { } provided := map[string]bool{} fs.Visit(func(f *flag.Flag) { provided[f.Name] = true }) - addr := *server - if !strings.Contains(addr, ":") { - addr += ":1812" + + servers, err := parseServers(*server) + if err != nil { + fmt.Fprintln(os.Stderr, "error:", err) + return 2 + } + + // --bind pins the outgoing interface on a multi-homed host. The resolved + // source address flows into every socket the run opens, so the detected + // source IP (and thus the registration hint) reflects the bind. + localAddr, err := resolveBindAddr(*bind) + if err != nil { + fmt.Fprintln(os.Stderr, "error:", err) + return 2 } portType, ok := nasPortTypes[*nasPortType] @@ -233,6 +247,13 @@ func cmdRadiusTest(args []string) int { fmt.Fprintln(os.Stderr, "error: --interval only makes sense with --count") return 2 } + // --count multiplies requests against ONE server; comparing several servers + // is a different job. Keeping them separate keeps the output legible and the + // per-server load obviously bounded. Test one server at a time with --count. + if len(servers) > 1 && *count != 1 { + fmt.Fprintln(os.Stderr, "error: --count tests a single server; drop it to compare multiple --server entries, or test one server at a time") + return 2 + } prompter := credential.Default() secretValue, err := prompter.Resolve(credential.Spec{ @@ -250,12 +271,14 @@ func cmdRadiusTest(args []string) int { return 2 } + // Address is set per server below; everything else is shared across a + // multi-server comparison. target := check.Target{ - Address: addr, Secret: secretValue, Timeout: *timeout, NASIdentifier: *nasID, NASPortType: portType, + LocalAddr: localAddr, } papUser, papPass, err := resolveCreds(prompter, *pap, "--pap", *passwordFile) @@ -291,30 +314,38 @@ func cmdRadiusTest(args []string) int { } target.Expect = expect + // Status-Server runs first: an RFC 5997 liveness ping that doesn't consume an + // auth attempt. The rest follow in dependency order (reachability/secret before + // the auth methods that rely on them). + checks := []check.Check{ + check.StatusServer{}, + check.Reachability{}, + check.SharedSecret{}, + check.BlastRADIUS{}, + check.PAP{User: papUser, Pass: papPass}, + check.PEAPMSCHAPv2{User: peapUser, Pass: peapPass, ServerName: *serverName}, + check.EAPTTLS{User: ttlsUser, Pass: ttlsPass, ServerName: *serverName}, + check.EAPTLS{CertFile: *clientCert, KeyFile: *clientKey, ServerName: *serverName}, + check.ServerCert{ServerName: *serverName}, + check.MTUProbe{Enabled: *mtu}, + } + + if len(servers) > 1 { + return runMultiServer(target, checks, servers, *jsonOut, *noColor, *strict) + } + + target.Address = servers[0] + plan := check.Plan{Target: target, Checks: checks} + // Sink: JSON for scripting, text for humans. var sink resultSink if *jsonOut { sink = report.NewJSONSink(os.Stdout) } else { - fmt.Printf("Testing RADIUS server %s (as NAS %q)\n\n", addr, *nasID) + fmt.Printf("Testing RADIUS server %s (as NAS %q)\n\n", servers[0], *nasID) sink = report.NewTextSink(os.Stdout, report.UseColor(os.Stdout, *noColor)) } - plan := check.Plan{ - Target: target, - Checks: []check.Check{ - check.Reachability{}, - check.SharedSecret{}, - check.BlastRADIUS{}, - check.PAP{User: papUser, Pass: papPass}, - check.PEAPMSCHAPv2{User: peapUser, Pass: peapPass, ServerName: *serverName}, - check.EAPTTLS{User: ttlsUser, Pass: ttlsPass, ServerName: *serverName}, - check.EAPTLS{CertFile: *clientCert, KeyFile: *clientKey, ServerName: *serverName}, - check.ServerCert{ServerName: *serverName}, - check.MTUProbe{Enabled: *mtu}, - }, - } - if *count != 1 { return runRepeat(plan, sink, *count, *interval, *strict, *jsonOut) } @@ -326,6 +357,119 @@ func cmdRadiusTest(args []string) int { return exitCode(sink, *strict) } +// parseServers splits the --server value on commas into normalized host:port +// entries (default port 1812), dropping empties (a trailing comma) and +// duplicates while preserving order. At least one server must remain. +func parseServers(raw string) ([]string, error) { + seen := map[string]bool{} + var out []string + for _, part := range strings.Split(raw, ",") { + s := strings.TrimSpace(part) + if s == "" { + continue + } + if !strings.Contains(s, ":") { + s += ":1812" + } + if seen[s] { + continue + } + seen[s] = true + out = append(out, s) + } + if len(out) == 0 { + return nil, errors.New("--server is required") + } + return out, nil +} + +// resolveBindAddr turns a --bind IP[:port] value into the source address sockets +// bind to. Empty means "let the OS choose". The source must be an IP literal (a +// specific local address on this host), never a hostname — binding is about +// which interface leaves, not name resolution. Port defaults to 0 (ephemeral). +func resolveBindAddr(s string) (*net.UDPAddr, error) { + if s == "" { + return nil, nil + } + host, port, err := net.SplitHostPort(s) + if err != nil { + // Most commonly there's no port (a bare source IP); retry with port 0. + host, port = s, "0" + } + if net.ParseIP(host) == nil { + return nil, fmt.Errorf("--bind %q: source must be a local IP address, not a hostname", s) + } + addr, err := net.ResolveUDPAddr("udp", net.JoinHostPort(host, port)) + if err != nil { + return nil, fmt.Errorf("--bind %q is not a valid IP[:port]: %w", s, err) + } + return addr, nil +} + +// runMultiServer runs the full check plan against each server in turn, prints +// each server's block, then a comparison verdict — split-brain between RADIUS +// servers being a classic hidden cause of "intermittent" auth tickets. Each +// server is still bounded by the runner's rate ceiling; comparing servers never +// raises the load on any one of them. The exit code follows the combined result +// (a FAIL on any server fails the run; under --strict, a WARN does too). +func runMultiServer(base check.Target, checks []check.Check, servers []string, jsonOut, noColor, strict bool) int { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + defer stop() + + var jsink *report.JSONSink + if jsonOut { + jsink = report.NewJSONSink(os.Stdout) + } + + var runs []check.ServerRun + for i, srv := range servers { + if ctx.Err() != nil { + fmt.Fprintf(os.Stderr, "interrupted — tested %d of %d servers\n", i, len(servers)) + break + } + target := base + target.Address = srv + plan := check.Plan{Target: target, Checks: checks} + + var results []check.Result + if jsonOut { + runner := check.Runner{Sink: jsink} // accumulates combined results + summary + results = runner.Run(ctx, plan) + } else { + fmt.Printf("=== Server %d/%d: %s ===\n\n", i+1, len(servers), srv) + ts := report.NewTextSink(os.Stdout, report.UseColor(os.Stdout, noColor)) + runner := check.Runner{Sink: ts} + results = runner.Run(ctx, plan) + _ = ts.Close() + fmt.Println() + } + runs = append(runs, check.ServerRun{Server: srv, Results: results}) + } + + cmp := check.CompareServers(runs) + if jsonOut { + jsink.SetServers(runs, cmp) + _ = jsink.Close() + } else { + fmt.Print(report.ComparisonBlock(cmp)) + } + return multiExitCode(runs, strict) +} + +// multiExitCode maps the combined multi-server results to the process exit code, +// independent of the sink: 1 if any server had a FAIL, or — under --strict — a +// WARN; otherwise 0. +func multiExitCode(runs []check.ServerRun, strict bool) int { + for _, r := range runs { + for _, res := range r.Results { + if res.Status == check.StatusFail || (strict && res.Status == check.StatusWarn) { + return 1 + } + } + } + return 0 +} + // runRepeat drives --count: N sequential iterations, then the aggregate // verdicts through the normal sink, so text/JSON rendering and the exit-code // contract are identical to a single run. Ctrl-C mid-loop aggregates the diff --git a/cmd/authhound-probe/main_test.go b/cmd/authhound-probe/main_test.go index b18c82f..b482268 100644 --- a/cmd/authhound-probe/main_test.go +++ b/cmd/authhound-probe/main_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" "github.com/authhound/probe/internal/check" @@ -58,6 +59,63 @@ func TestCountFlagValidation(t *testing.T) { } } +func TestParseServers(t *testing.T) { + cases := []struct { + name string + in string + want []string + }{ + {"single bare host gets default port", "radius.corp.com", []string{"radius.corp.com:1812"}}, + {"single host:port kept", "10.0.0.1:1645", []string{"10.0.0.1:1645"}}, + {"comma list, mixed", "a,b:1812,c:1645", []string{"a:1812", "b:1812", "c:1645"}}, + {"whitespace and trailing comma dropped", " a , b , ", []string{"a:1812", "b:1812"}}, + {"duplicates de-duped, order kept", "a,a:1812,b", []string{"a:1812", "b:1812"}}, + } + for _, c := range cases { + got, err := parseServers(c.in) + if err != nil { + t.Errorf("%s: unexpected error %v", c.name, err) + continue + } + if strings.Join(got, "|") != strings.Join(c.want, "|") { + t.Errorf("%s: got %v, want %v", c.name, got, c.want) + } + } + if _, err := parseServers(" , , "); err == nil { + t.Error("empty server list should error") + } +} + +func TestResolveBindAddr(t *testing.T) { + if a, err := resolveBindAddr(""); err != nil || a != nil { + t.Errorf("empty bind: got (%v, %v), want (nil, nil)", a, err) + } + a, err := resolveBindAddr("127.0.0.1") + if err != nil { + t.Fatalf("bare IP: unexpected error %v", err) + } + if a.IP.String() != "127.0.0.1" || a.Port != 0 { + t.Errorf("bare IP: got %v, want 127.0.0.1:0", a) + } + if a, err := resolveBindAddr("127.0.0.1:5000"); err != nil || a.Port != 5000 { + t.Errorf("IP:port: got (%v, %v), want port 5000", a, err) + } + // A hostname is rejected — bind is about which interface leaves, not DNS. + if _, err := resolveBindAddr("jumpbox.corp.com"); err == nil { + t.Error("hostname bind should error") + } + if _, err := resolveBindAddr("not an ip"); err == nil { + t.Error("garbage bind should error") + } +} + +func TestMultiServerRejectsCount(t *testing.T) { + // --count against multiple servers is a usage error. + if got := cmdRadiusTest([]string{"--server", "192.0.2.1,192.0.2.2", "--secret", "x", "--count", "3"}); got != 2 { + t.Errorf("multi-server + --count: exit = %d, want 2", got) + } +} + func TestResolveVersion(t *testing.T) { orig := version t.Cleanup(func() { version = orig }) diff --git a/docs/json-schema.md b/docs/json-schema.md index 319f6d9..c14272d 100644 --- a/docs/json-schema.md +++ b/docs/json-schema.md @@ -45,6 +45,8 @@ is a deliberate, reviewed act rather than an accident. | `results` | array of result objects | always | One entry per check, in the order they ran. With `--count`, one **aggregate verdict** per check (see the `repeat` block below). | | `summary` | object | always | Per-status tally. **All five keys are always present**, including zeros — address `.summary.warn` without checking it exists first. | | `repeat` | object | only with `--count` | Additive: per-iteration results and aggregate statistics for a repeated run. Absent on single runs, whose documents are unchanged. | +| `servers` | array | only when comparing >1 `--server` | Additive: one block per server (`server`, `results`, `summary`). Absent for a single server, whose document is unchanged. See [Multi-server](#servers--comparison-multiple---server). | +| `comparison` | object | only when comparing >1 `--server` | Additive: the cross-server verdict (`verdict`, `reachable`, `unreachable`, `divergent`). Paired with `servers`. | ### `summary` object @@ -62,12 +64,12 @@ Each entry in `results`: | Field | Type | Presence | Notes | |---|---|---|---| -| `check` | string | always | Stable identifier for the check, e.g. `reachability`, `shared-secret`, `blastradius-posture`, `pap`, `peap-mschapv2`, `eap-ttls`, `eap-tls`, `server-cert`, `mtu`, `radsec-connect`, `radsec-tls`, `radsec-cert`, `radsec-radius`. | +| `check` | string | always | Stable identifier for the check, e.g. `status-server`, `reachability`, `shared-secret`, `blastradius-posture`, `pap`, `peap-mschapv2`, `eap-ttls`, `eap-tls`, `server-cert`, `mtu`, `radsec-connect`, `radsec-tls`, `radsec-cert`, `radsec-radius`. | | `status` | string | always | One of `pass`, `fail`, `warn`, `info`, `skip` (see below). | | `summary` | string | always | One plain-English line describing the outcome. | | `detail` | string | when present | Extra context. Omitted when empty. | | `hint` | string | when present | Multi-line, paste-ready remediation. Newline formatting is significant. Omitted when empty. Never contains secrets. | -| `fields` | object (string→string) | when present | Structured extras such as `rtt_ms`, `tls_version`, `not_after`, `subject`, `san`, `chain_len`, `source_ip`. `blastradius_posture` (on the `blastradius-posture` check) is `"signed"` or `"unsigned"` — whether the server signed its reply with a Message-Authenticator (see [BlastRADIUS posture](../README.md#blastradius--message-authenticator-posture)). `timeout: "true"` marks a request that got no reply at all (a *lost* request, as opposed to a processed rejection). Aggregate verdicts under `--count` add `success_rate`, `attempts`, `successes`, `timeouts`, and `latency_{min,median,p95,max}_ms`. Keys vary by check; values are always strings. Omitted when there are none. | +| `fields` | object (string→string) | when present | Structured extras such as `rtt_ms`, `tls_version`, `not_after`, `subject`, `san`, `chain_len`, `source_ip`. On the `status-server` check, `supported` is `"true"` (the server answered the RFC 5997 liveness query) or `"false"` (it didn't — which is fine; that check never fails). `blastradius_posture` (on the `blastradius-posture` check) is `"signed"` or `"unsigned"` — whether the server signed its reply with a Message-Authenticator (see [BlastRADIUS posture](../README.md#blastradius--message-authenticator-posture)). `timeout: "true"` marks a request that got no reply at all (a *lost* request, as opposed to a processed rejection). Aggregate verdicts under `--count` add `success_rate`, `attempts`, `successes`, `timeouts`, and `latency_{min,median,p95,max}_ms`. Keys vary by check; values are always strings. Omitted when there are none. | | `duration_ns` | integer | when present | How long the check took, in nanoseconds. Omitted when zero. | | `authorization` | object | when present | On an auth check that reached an Access-Accept, the authorization attributes the server returned (VLAN/Filter-Id/…) and the outcome of any `--expect-vlan`/`--expect-attr` assertions. See below. Omitted otherwise. | @@ -130,6 +132,43 @@ the raw material: | `iterations` | array | One entry per completed iteration, each with its `results` (same result-object shape as the top level). | | `aggregate` | array | Per-check tallies. `successes` = the server answered and processed the request (pass/warn/info); `timeouts` = the subset of `failures` where no reply arrived at all. `latency_ms` (nearest-rank percentiles over answered runs) is omitted when nothing was answered. | +## `servers` + `comparison` (multiple `--server`) + +Passing several comma-separated servers (`--server primary,secondary`) runs the +full plan against each and adds two top-level fields. **Single-server documents +are unaffected** — both fields are omitted and the output is byte-for-byte +unchanged. + +Top-level `results` and `summary` still describe the **combined** run across all +servers (so `.summary.fail` still drives the exit code — a FAIL on *any* server +fails the run). The per-server breakdown lives under `servers`: + +```json +"servers": [ + { "server": "primary:1812", "results": [ /* … */ ], "summary": { "pass": 4, "fail": 0, "warn": 0, "info": 1, "skip": 5 } }, + { "server": "secondary:1812", "results": [ /* … */ ], "summary": { "pass": 0, "fail": 1, "warn": 0, "info": 1, "skip": 8 } } +], +"comparison": { + "verdict": "primary:1812 is responding, but secondary:1812 is NOT. …", + "reachable": [ "primary:1812" ], + "unreachable": [ "secondary:1812" ], + "divergent": [ "pap: primary:1812=pass, secondary:1812=fail — servers disagree" ] +} +``` + +| Field | Type | Notes | +|---|---|---| +| `servers[].server` | string | The `host:port` tested. | +| `servers[].results` | array | That server's result objects (same shape as top-level `results`). | +| `servers[].summary` | object | That server's per-status tally. | +| `comparison.verdict` | string | One plain-English headline: all matching, one/some not responding (with the round-robin risk), or responding-but-disagreeing. | +| `comparison.reachable` | array of strings | Servers whose reachability check passed. | +| `comparison.unreachable` | array of strings | Servers that did not answer. | +| `comparison.divergent` | array of strings | Per-check disagreements among the servers that *did* answer (config/replication drift). Omitted when there are none. | + +`--count` and multiple `--server` are mutually exclusive (usage error) — chase +intermittency on one server, compare across servers separately. + ### `status` values | Value | Meaning | Effect on exit code | @@ -160,6 +199,10 @@ authhound-probe radius test --server r --json | jq '.summary.fail' # --count: which checks lost requests, from the aggregate block: ... --count 10 --json | jq -r '.repeat.aggregate[] | select(.timeouts > 0) | "\(.check): \(.timeouts) lost"' + +# Multiple --server: which servers aren't responding, and the verdict: +... --server a,b --json | jq -r '.comparison.unreachable[]' +... --server a,b --json | jq -r '.comparison.verdict' ``` ## Exit codes diff --git a/internal/check/blastradius.go b/internal/check/blastradius.go index 6ae4705..017e41f 100644 --- a/internal/check/blastradius.go +++ b/internal/check/blastradius.go @@ -38,7 +38,7 @@ func (BlastRADIUS) Run(ctx context.Context, t Target) Result { addCommon(p, t) reqAuth := p.Authenticator - _, raw, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout) + _, raw, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout, t.LocalAddr) if err != nil { if errors.Is(err, radius.ErrTimeout) { return markTimeout(Result{ diff --git a/internal/check/check.go b/internal/check/check.go index d6ddcf6..01ee7d6 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -11,6 +11,7 @@ package check import ( "context" + "net" "time" ) @@ -56,6 +57,12 @@ type Target struct { Timeout time.Duration NASIdentifier string + // LocalAddr is the source address to bind outgoing sockets to (--bind), for + // pinning the outgoing interface on a multi-homed host. nil = OS default. + // Whatever source IP results is what the server sees and what the + // registration hint reports. + LocalAddr net.Addr + // NAS attributes make the probe's request look like a real 802.1X client so // server network policies (which key on these) evaluate the same way. NASPortType int // radius.NASPort* value; 0 = omit diff --git a/internal/check/compare.go b/internal/check/compare.go new file mode 100644 index 0000000..add919f --- /dev/null +++ b/internal/check/compare.go @@ -0,0 +1,158 @@ +package check + +import ( + "fmt" + "sort" + "strings" +) + +// ServerRun is one server's outcome within a multi-server run: the address that +// was tested and the results it produced. It feeds CompareServers. +type ServerRun struct { + Server string + Results []Result +} + +// Comparison is the cross-server verdict printed after testing more than one +// --server. Split-brain between RADIUS servers — one healthy, one silently down, +// or two that disagree — is the classic hidden cause of "intermittent" auth +// tickets: whether a login works depends on which server the client's DNS +// round-robin or VIP happened to pick. +type Comparison struct { + Verdict string `json:"verdict"` // one plain-English headline + Reachable []string `json:"reachable"` // servers that answered the reachability probe + Unreachable []string `json:"unreachable"` // servers that did not answer + Divergent []string `json:"divergent,omitempty"` // per-check disagreements among the servers that DID answer +} + +// CompareServers classifies a set of per-server runs and produces the plain-English +// comparison verdict. Reachability is the split line: a server whose reachability +// check passed is "responding". Among the responding servers it then looks for +// checks that disagree (one accepts, another rejects) — config or replication drift. +func CompareServers(runs []ServerRun) Comparison { + var c Comparison + for _, r := range runs { + if serverResponded(r.Results) { + c.Reachable = append(c.Reachable, r.Server) + } else { + c.Unreachable = append(c.Unreachable, r.Server) + } + } + c.Divergent = divergentChecks(runs) + + total := len(runs) + switch { + case len(c.Unreachable) == total: + c.Verdict = "No server responded — none of the tested RADIUS servers answered. " + + "Fix the network path or client registration before reading the per-check results above." + case len(c.Unreachable) > 0: + pct := int(float64(len(c.Unreachable))/float64(total)*100 + 0.5) + c.Verdict = fmt.Sprintf( + "%s responding, but %s NOT. If clients reach these servers via DNS "+ + "round-robin or a shared VIP, roughly %d%% of authentications would fail "+ + "intermittently depending on which server they land on — the classic "+ + "'it works sometimes' ticket. Take the unresponsive server(s) out of rotation or bring them back.", + joinServers(c.Reachable), joinServers(c.Unreachable), pct) + case len(c.Divergent) > 0: + c.Verdict = fmt.Sprintf( + "All %d servers responded, but they DISAGREE on some checks (below) — likely "+ + "config or replication drift between them. Clients that land on the odd server "+ + "out will fail intermittently even though every server is 'up'.", total) + default: + c.Verdict = fmt.Sprintf( + "All %d servers responded and returned matching results — no split-brain between them.", total) + } + return c +} + +// serverResponded reports whether the reachability check passed for a run. +func serverResponded(results []Result) bool { + for _, r := range results { + if r.Check == "reachability" { + return r.Status == StatusPass + } + } + return false +} + +// divergentChecks finds checks that ran on the responding servers but ended in +// different states — the "one accepts, one rejects" drift. Servers that never +// responded are excluded (they fail everything, which is noise, not drift), as +// are checks that were skipped everywhere. +func divergentChecks(runs []ServerRun) []string { + // checkName -> server -> status, only over responding servers. + byCheck := map[string]map[string]Status{} + var order []string + for _, r := range runs { + if !serverResponded(r.Results) { + continue + } + for _, res := range r.Results { + if res.Status == StatusSkip { + continue + } + if _, ok := byCheck[res.Check]; !ok { + byCheck[res.Check] = map[string]Status{} + order = append(order, res.Check) + } + byCheck[res.Check][r.Server] = res.Status + } + } + + var out []string + for _, name := range order { + perServer := byCheck[name] + if len(perServer) < 2 { + continue // only one responding server ran it; nothing to compare + } + if !statusesAgree(perServer) { + out = append(out, fmt.Sprintf("%s: %s — servers disagree", name, describeStatuses(perServer))) + } + } + return out +} + +func statusesAgree(perServer map[string]Status) bool { + first := Status("") + for _, s := range perServer { + if first == "" { + first = s + continue + } + if s != first { + return false + } + } + return true +} + +// describeStatuses renders "server1=pass, server2=fail" with servers in a stable +// order, so the divergence line is deterministic. +func describeStatuses(perServer map[string]Status) string { + servers := make([]string, 0, len(perServer)) + for s := range perServer { + servers = append(servers, s) + } + sort.Strings(servers) + parts := make([]string, 0, len(servers)) + for _, s := range servers { + parts = append(parts, fmt.Sprintf("%s=%s", s, perServer[s])) + } + return strings.Join(parts, ", ") +} + +// joinServers renders a list of servers for the verdict headline, tolerating the +// empty case (every server down) without an awkward dangling phrase. +func joinServers(servers []string) string { + if len(servers) == 0 { + return "No server is" + } + return strings.Join(servers, ", ") + " " + plural(len(servers), "is", "are") +} + +func plural(n int, one, many string) string { + if n == 1 { + return one + } + return many +} diff --git a/internal/check/compare_test.go b/internal/check/compare_test.go new file mode 100644 index 0000000..ef370cc --- /dev/null +++ b/internal/check/compare_test.go @@ -0,0 +1,88 @@ +package check + +import ( + "strings" + "testing" +) + +// resultSet is a compact way to build a server's results for comparison tests: +// each entry is check name -> status. +func resultSet(pairs ...string) []Result { + var out []Result + for i := 0; i < len(pairs); i += 2 { + out = append(out, Result{Check: pairs[i], Status: Status(pairs[i+1])}) + } + return out +} + +func run(server string, results []Result) ServerRun { + return ServerRun{Server: server, Results: results} +} + +func TestCompareAllHealthyMatching(t *testing.T) { + c := CompareServers([]ServerRun{ + run("a:1812", resultSet("reachability", "pass", "pap", "pass")), + run("b:1812", resultSet("reachability", "pass", "pap", "pass")), + }) + if len(c.Unreachable) != 0 || len(c.Divergent) != 0 { + t.Fatalf("expected all healthy, matching; got unreachable=%v divergent=%v", c.Unreachable, c.Divergent) + } + if !strings.Contains(c.Verdict, "no split-brain") { + t.Errorf("verdict: %q", c.Verdict) + } +} + +func TestCompareOneDown(t *testing.T) { + c := CompareServers([]ServerRun{ + run("primary:1812", resultSet("reachability", "pass", "pap", "pass")), + run("secondary:1812", resultSet("reachability", "fail")), + }) + if len(c.Reachable) != 1 || c.Unreachable[0] != "secondary:1812" { + t.Fatalf("classification wrong: reachable=%v unreachable=%v", c.Reachable, c.Unreachable) + } + // 1 of 2 down -> ~50%, and the round-robin story. + if !strings.Contains(c.Verdict, "50%") || !strings.Contains(c.Verdict, "round-robin") { + t.Errorf("verdict should name the ~50%% round-robin risk: %q", c.Verdict) + } +} + +func TestCompareDivergent(t *testing.T) { + c := CompareServers([]ServerRun{ + run("a:1812", resultSet("reachability", "pass", "pap", "pass")), + run("b:1812", resultSet("reachability", "pass", "pap", "fail")), + }) + if len(c.Unreachable) != 0 { + t.Fatalf("both should be reachable, got unreachable=%v", c.Unreachable) + } + if len(c.Divergent) != 1 || !strings.Contains(c.Divergent[0], "pap") { + t.Fatalf("expected a pap divergence, got %v", c.Divergent) + } + if !strings.Contains(c.Divergent[0], "a:1812=pass") || !strings.Contains(c.Divergent[0], "b:1812=fail") { + t.Errorf("divergence should name each server's status: %q", c.Divergent[0]) + } + if !strings.Contains(c.Verdict, "DISAGREE") { + t.Errorf("verdict should flag disagreement: %q", c.Verdict) + } +} + +func TestCompareAllDown(t *testing.T) { + c := CompareServers([]ServerRun{ + run("a:1812", resultSet("reachability", "fail")), + run("b:1812", resultSet("reachability", "fail")), + }) + if !strings.Contains(c.Verdict, "No server responded") { + t.Errorf("verdict: %q", c.Verdict) + } +} + +// A check that only one responding server ran (skipped on the other) is not a +// divergence — there's nothing to compare. +func TestCompareSkipNotDivergent(t *testing.T) { + c := CompareServers([]ServerRun{ + run("a:1812", resultSet("reachability", "pass", "eap-tls", "pass")), + run("b:1812", resultSet("reachability", "pass", "eap-tls", "skip")), + }) + if len(c.Divergent) != 0 { + t.Errorf("skip on one server is not a divergence, got %v", c.Divergent) + } +} diff --git a/internal/check/eaptls.go b/internal/check/eaptls.go index 7c37cba..e11a4f3 100644 --- a/internal/check/eaptls.go +++ b/internal/check/eaptls.go @@ -23,11 +23,12 @@ func (ServerCert) Name() string { return "server-cert" } func (c ServerCert) Run(ctx context.Context, t Target) Result { sess := &radius.EAPSession{ - Addr: t.Address, - Secret: t.Secret, - Timeout: t.Timeout, - Identity: "authhound-probe", - Attrs: commonAttrs(t), + Addr: t.Address, + Secret: t.Secret, + Timeout: t.Timeout, + Identity: "authhound-probe", + Attrs: commonAttrs(t), + LocalAddr: t.LocalAddr, } captured, err := sess.InspectServerCert(ctx, c.ServerName) diff --git a/internal/check/eaptls_auth.go b/internal/check/eaptls_auth.go index 0475c3d..588332a 100644 --- a/internal/check/eaptls_auth.go +++ b/internal/check/eaptls_auth.go @@ -56,11 +56,12 @@ func (c EAPTLS) Run(ctx context.Context, t Target) Result { } sess := &radius.EAPSession{ - Addr: t.Address, - Secret: t.Secret, - Timeout: t.Timeout, - Identity: identity, - Attrs: commonAttrs(t), + Addr: t.Address, + Secret: t.Secret, + Timeout: t.Timeout, + Identity: identity, + Attrs: commonAttrs(t), + LocalAddr: t.LocalAddr, } res, err := sess.AuthEAPTLS(ctx, cert, c.ServerName) diff --git a/internal/check/mtu.go b/internal/check/mtu.go index 4f0a4a9..795aac7 100644 --- a/internal/check/mtu.go +++ b/internal/check/mtu.go @@ -36,7 +36,7 @@ func (c MTUProbe) Run(ctx context.Context, t Target) Result { attrs := commonAttrs(t) // Baseline: a small packet must round-trip, or there's nothing to measure. - ok, _, err := radius.MTUReachable(t.Address, t.Secret, mtuMin, attrs, t.Timeout) + ok, _, err := radius.MTUReachable(t.Address, t.Secret, mtuMin, attrs, t.Timeout, t.LocalAddr) if err != nil { return Result{Check: "path-mtu", Status: StatusSkip, Summary: "Could not reach the server for the MTU probe: " + err.Error()} } @@ -51,7 +51,7 @@ func (c MTUProbe) Run(ctx context.Context, t Target) Result { lo, hi, maxOK := mtuMin, mtuMax, mtuMin for hi-lo > mtuStep { mid := (lo + hi) / 2 - got, _, err := radius.MTUReachable(t.Address, t.Secret, mid, attrs, t.Timeout) + got, _, err := radius.MTUReachable(t.Address, t.Secret, mid, attrs, t.Timeout, t.LocalAddr) if err != nil { break } diff --git a/internal/check/pap.go b/internal/check/pap.go index 2d23beb..9fbe22e 100644 --- a/internal/check/pap.go +++ b/internal/check/pap.go @@ -38,7 +38,7 @@ func (c PAP) Run(ctx context.Context, t Target) Result { p.SetUserPassword(c.Pass, t.Secret) addCommon(p, t) - reply, _, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout) + reply, _, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout, t.LocalAddr) if err != nil { if errors.Is(err, radius.ErrTimeout) { return markTimeout(Result{Check: "pap-auth", Status: StatusSkip, Summary: "No reply — resolve reachability first"}) diff --git a/internal/check/peap.go b/internal/check/peap.go index e4b2702..8fc2102 100644 --- a/internal/check/peap.go +++ b/internal/check/peap.go @@ -33,11 +33,12 @@ func (c PEAPMSCHAPv2) Run(ctx context.Context, t Target) Result { } sess := &radius.EAPSession{ - Addr: t.Address, - Secret: t.Secret, - Timeout: t.Timeout, - Identity: c.User, - Attrs: commonAttrs(t), + Addr: t.Address, + Secret: t.Secret, + Timeout: t.Timeout, + Identity: c.User, + Attrs: commonAttrs(t), + LocalAddr: t.LocalAddr, } res, err := sess.AuthPEAPMSCHAPv2(ctx, c.User, c.Pass, c.ServerName) diff --git a/internal/check/reachability.go b/internal/check/reachability.go index 0cb4763..609d337 100644 --- a/internal/check/reachability.go +++ b/internal/check/reachability.go @@ -26,7 +26,7 @@ func (Reachability) Run(ctx context.Context, t Target) Result { p.AddString(radius.AttrUserName, "authhound-probe") addCommon(p, t) - _, _, rtt, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout) + _, _, rtt, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout, t.LocalAddr) fields := map[string]string{"rtt_ms": strconv.FormatInt(rtt.Milliseconds(), 10)} if err == nil { diff --git a/internal/check/secret.go b/internal/check/secret.go index 121a4d7..83a045e 100644 --- a/internal/check/secret.go +++ b/internal/check/secret.go @@ -32,7 +32,7 @@ func (SharedSecret) Run(ctx context.Context, t Target) Result { addCommon(p, t) reqAuth := p.Authenticator - _, raw, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout) + _, raw, _, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout, t.LocalAddr) if err != nil { if errors.Is(err, radius.ErrTimeout) { return markTimeout(Result{ diff --git a/internal/check/statusserver.go b/internal/check/statusserver.go new file mode 100644 index 0000000..e7b310f --- /dev/null +++ b/internal/check/statusserver.go @@ -0,0 +1,63 @@ +package check + +import ( + "context" + "errors" + "fmt" + "strconv" + + "github.com/authhound/probe/internal/radius" +) + +// StatusServer runs an RFC 5997 Status-Server liveness query. Unlike an +// Access-Request, it never consumes an authentication attempt on the server: a +// server that supports it answers with an Access-Accept and logs nothing about a +// user. It runs before the auth tests as a clean "is the server alive?" ping. +// +// It degrades gracefully. Many servers do not enable Status-Server, and — like +// any RADIUS request — an unregistered client is dropped silently, so a timeout +// here is ambiguous. It therefore never FAILs: a reply is a PASS, and silence is +// an INFO that defers to the reachability check below rather than guessing. +type StatusServer struct{} + +func (StatusServer) Name() string { return "status-server" } + +func (StatusServer) Run(ctx context.Context, t Target) Result { + p, err := radius.NewStatusServer(1) + if err != nil { + return Result{Check: "status-server", Status: StatusInfo, Summary: "internal error building Status-Server request: " + err.Error()} + } + // NAS-Identifier only — Status-Server carries no User-Name or password + // (RFC 5997). The Message-Authenticator it also requires is added by Exchange. + if t.NASIdentifier != "" { + p.AddString(radius.AttrNASIdentifier, t.NASIdentifier) + } + + _, _, rtt, err := radius.Exchange(t.Address, t.Secret, p, t.Timeout, t.LocalAddr) + if err == nil { + return Result{ + Check: "status-server", Status: StatusPass, + Summary: fmt.Sprintf("Server answered Status-Server in %dms (live; no auth attempt consumed)", rtt.Milliseconds()), + Fields: map[string]string{ + "rtt_ms": strconv.FormatInt(rtt.Milliseconds(), 10), + "supported": "true", + }, + } + } + // Anything short of a reply is reported as INFO, never FAIL: Status-Server is + // a bonus liveness signal, and the reachability check is the authoritative one. + fields := map[string]string{"supported": "false"} + if errors.Is(err, radius.ErrTimeout) { + fields[TimeoutField] = "true" + } + return Result{ + Check: "status-server", Status: StatusInfo, Fields: fields, + Summary: "No Status-Server reply — the server may not have it enabled (harmless)", + Detail: "Status-Server (RFC 5997) lets a monitor check liveness without " + + "consuming an auth attempt. Two reasons for silence here, and they look " + + "identical: the server doesn't have it turned on (in FreeRADIUS, set " + + "status_server = yes in radiusd.conf), or it isn't answering this probe at " + + "all — the reachability check below tells you which. Either way this is not " + + "a failure; the auth tests still verify the server end to end.", + } +} diff --git a/internal/check/statusserver_test.go b/internal/check/statusserver_test.go new file mode 100644 index 0000000..d1b5b7e --- /dev/null +++ b/internal/check/statusserver_test.go @@ -0,0 +1,74 @@ +package check + +import ( + "context" + "net" + "strings" + "testing" + "time" +) + +// TestStatusServerReply: a server that answers the Status-Server query is +// reported PASS, marked supported, with no auth attempt consumed. +func TestStatusServerReply(t *testing.T) { + addr := startFakeServer(t, &fakeServer{secret: "s3cret"}) + r := (StatusServer{}).Run(context.Background(), target(addr, "s3cret")) + if r.Status != StatusPass { + t.Fatalf("status-server reply: got %s (%s), want pass", r.Status, r.Summary) + } + if r.Fields["supported"] != "true" { + t.Errorf("supported field: got %q, want true", r.Fields["supported"]) + } + if strings.Contains(r.Summary+r.Detail, "s3cret") { + t.Error("secret leaked into status-server output") + } +} + +// TestStatusServerNoSupport: silence (server without Status-Server enabled, or +// an unregistered probe) must be INFO — never FAIL — and defer to reachability. +func TestStatusServerNoSupport(t *testing.T) { + addr := startFakeServer(t, &fakeServer{secret: "s3cret", silent: true}) + tgt := target(addr, "s3cret") + tgt.Timeout = 300 * time.Millisecond + r := (StatusServer{}).Run(context.Background(), tgt) + if r.Status != StatusInfo { + t.Fatalf("no Status-Server support: got %s, want info (never fail)", r.Status) + } + if r.Fields["supported"] != "false" { + t.Errorf("supported field: got %q, want false", r.Fields["supported"]) + } + if r.Fields[TimeoutField] != "true" { + t.Errorf("timeout field: got %q, want true", r.Fields[TimeoutField]) + } + if !strings.Contains(r.Detail, "status_server = yes") { + t.Error("detail should mention enabling status_server = yes") + } +} + +// TestBindLocalAddrHonored: a Target with LocalAddr set still reaches the server, +// and on timeout the reported source IP reflects the bind (so the WS-3 +// registration hint stays correct under --bind). +func TestBindLocalAddrHonored(t *testing.T) { + laddr := &net.UDPAddr{IP: net.ParseIP("127.0.0.1")} + + // Reaches a live server when bound to the loopback source. + addr := startFakeServer(t, &fakeServer{secret: "s3cret", goodUser: "alice", goodPass: "pw"}) + tgt := target(addr, "s3cret") + tgt.LocalAddr = laddr + if r := (Reachability{}).Run(context.Background(), tgt); r.Status != StatusPass { + t.Fatalf("bound reachability: got %s (%s), want pass", r.Status, r.Summary) + } + + // On timeout, the detected source IP is the bound address. + silent := startFakeServer(t, &fakeServer{secret: "s3cret", silent: true}) + stgt := target(silent, "s3cret") + stgt.Timeout = 300 * time.Millisecond + stgt.LocalAddr = laddr + r := (Reachability{}).Run(context.Background(), stgt) + if r.Status != StatusFail { + t.Fatalf("bound timeout: got %s, want fail", r.Status) + } + if r.Fields["source_ip"] != "127.0.0.1" { + t.Errorf("source_ip under --bind: got %q, want 127.0.0.1", r.Fields["source_ip"]) + } +} diff --git a/internal/check/ttls.go b/internal/check/ttls.go index 56f561a..ef59dc8 100644 --- a/internal/check/ttls.go +++ b/internal/check/ttls.go @@ -29,11 +29,12 @@ func (c EAPTTLS) Run(ctx context.Context, t Target) Result { } sess := &radius.EAPSession{ - Addr: t.Address, - Secret: t.Secret, - Timeout: t.Timeout, - Identity: c.User, - Attrs: commonAttrs(t), + Addr: t.Address, + Secret: t.Secret, + Timeout: t.Timeout, + Identity: c.User, + Attrs: commonAttrs(t), + LocalAddr: t.LocalAddr, } res, err := sess.AuthEAPTTLS(ctx, c.User, c.Pass, c.ServerName) diff --git a/internal/radius/client.go b/internal/radius/client.go index ed4a26c..18f76e2 100644 --- a/internal/radius/client.go +++ b/internal/radius/client.go @@ -12,13 +12,19 @@ import ( // and the round-trip time. A Message-Authenticator is always included, which // is both good hygiene and required by servers hardened against BlastRADIUS // (CVE-2024-3596). -func Exchange(addr string, secret string, p *Packet, timeout time.Duration) (reply *Packet, raw []byte, rtt time.Duration, err error) { +// +// localAddr, when non-nil, is the source address the UDP socket binds to +// (the --bind flag) — the way to pin the outgoing interface on a multi-homed +// host. The chosen source IP is what the RADIUS server sees, and it is exactly +// what TimeoutError.LocalIP reports back so the registration hint stays correct. +func Exchange(addr string, secret string, p *Packet, timeout time.Duration, localAddr net.Addr) (reply *Packet, raw []byte, rtt time.Duration, err error) { wire, err := p.encode(secret) if err != nil { return nil, nil, 0, err } - conn, err := net.Dial("udp", addr) + dialer := net.Dialer{LocalAddr: localAddr} + conn, err := dialer.Dial("udp", addr) if err != nil { return nil, nil, 0, err } diff --git a/internal/radius/client_test.go b/internal/radius/client_test.go index 56f0fc8..565f505 100644 --- a/internal/radius/client_test.go +++ b/internal/radius/client_test.go @@ -22,7 +22,7 @@ func TestExchangeTimeoutCarriesLocalIP(t *testing.T) { if err != nil { t.Fatal(err) } - _, _, _, err = Exchange(pc.LocalAddr().String(), "s3cret", p, 200*time.Millisecond) + _, _, _, err = Exchange(pc.LocalAddr().String(), "s3cret", p, 200*time.Millisecond, nil) if !errors.Is(err, ErrTimeout) { t.Fatalf("want ErrTimeout match, got %v", err) } diff --git a/internal/radius/eapsession.go b/internal/radius/eapsession.go index 8158452..c066113 100644 --- a/internal/radius/eapsession.go +++ b/internal/radius/eapsession.go @@ -22,11 +22,12 @@ import ( // certificate can be inspected. Inner authentication (PEAP-MSCHAPv2, EAP-TLS // client cert) builds on the same session and is a later milestone. type EAPSession struct { - Addr string - Secret string - Timeout time.Duration - Identity string - Attrs []Attribute // common NAS attributes added to every request + Addr string + Secret string + Timeout time.Duration + Identity string + Attrs []Attribute // common NAS attributes added to every request + LocalAddr net.Addr // source address to bind (--bind); nil = OS default radiusID byte state []byte // RADIUS State attribute to echo back @@ -58,7 +59,7 @@ func (s *EAPSession) send(eap []byte) (*EAPPacket, Code, error) { p.Add(AttrState, s.state) } - reply, _, _, err := Exchange(s.Addr, s.Secret, p, s.Timeout) + reply, _, _, err := Exchange(s.Addr, s.Secret, p, s.Timeout, s.LocalAddr) if err != nil { return nil, 0, err } diff --git a/internal/radius/mtu.go b/internal/radius/mtu.go index eb39772..413c1ca 100644 --- a/internal/radius/mtu.go +++ b/internal/radius/mtu.go @@ -2,6 +2,7 @@ package radius import ( "errors" + "net" "time" ) @@ -43,7 +44,7 @@ func (p *Packet) padToWithProxyState(target int) { // whether a reply came back — i.e. whether the network path carries RADIUS // packets of that size in both directions. Any reply (even Access-Reject) // counts; a timeout means the packet (or its reply) was dropped. -func MTUReachable(addr, secret string, target int, attrs []Attribute, timeout time.Duration) (ok bool, replied bool, err error) { +func MTUReachable(addr, secret string, target int, attrs []Attribute, timeout time.Duration, localAddr net.Addr) (ok bool, replied bool, err error) { p, err := NewAccessRequest(1) if err != nil { return false, false, err @@ -54,7 +55,7 @@ func MTUReachable(addr, secret string, target int, attrs []Attribute, timeout ti } p.padToWithProxyState(target) - _, _, _, err = Exchange(addr, secret, p, timeout) + _, _, _, err = Exchange(addr, secret, p, timeout, localAddr) if err == nil { return true, true, nil } diff --git a/internal/radius/packet.go b/internal/radius/packet.go index 61a055b..acab981 100644 --- a/internal/radius/packet.go +++ b/internal/radius/packet.go @@ -22,6 +22,7 @@ const ( AccessAccept Code = 2 AccessReject Code = 3 AccessChallenge Code = 11 + StatusServer Code = 12 // RFC 5997 liveness query; reply is an Access-Accept ) func (c Code) String() string { @@ -34,6 +35,8 @@ func (c Code) String() string { return "Access-Reject" case AccessChallenge: return "Access-Challenge" + case StatusServer: + return "Status-Server" default: return fmt.Sprintf("Code(%d)", byte(c)) } @@ -99,6 +102,19 @@ func NewAccessRequest(id byte) (*Packet, error) { return &Packet{Code: AccessRequest, Identifier: id, Authenticator: auth}, nil } +// NewStatusServer creates a Status-Server (RFC 5997) liveness query with a fresh +// authenticator. It carries no User-Name or password — it is a pure "are you +// alive?" ping that a server answers with an Access-Accept without ever +// consuming an authentication attempt. Exchange always appends the +// Message-Authenticator that RFC 5997 requires. +func NewStatusServer(id byte) (*Packet, error) { + auth, err := newAuthenticator() + if err != nil { + return nil, err + } + return &Packet{Code: StatusServer, Identifier: id, Authenticator: auth}, nil +} + func (p *Packet) Add(t AttrType, v []byte) { p.Attributes = append(p.Attributes, Attribute{Type: t, Value: v}) } diff --git a/internal/report/compare.go b/internal/report/compare.go new file mode 100644 index 0000000..b0cb3a9 --- /dev/null +++ b/internal/report/compare.go @@ -0,0 +1,20 @@ +package report + +import ( + "strings" + + "github.com/authhound/probe/internal/check" +) + +// ComparisonBlock renders the multi-server comparison for the terminal: the +// headline verdict (word-wrapped to match the rest of the output) followed by +// any per-check divergences between the servers that responded. +func ComparisonBlock(c check.Comparison) string { + var b strings.Builder + b.WriteString("Comparison across servers:\n") + b.WriteString(" " + wrap(c.Verdict, 70, " ") + "\n") + for _, d := range c.Divergent { + b.WriteString(" - " + d + "\n") + } + return b.String() +} diff --git a/internal/report/json.go b/internal/report/json.go index 013d6bf..08edc69 100644 --- a/internal/report/json.go +++ b/internal/report/json.go @@ -20,10 +20,12 @@ const SchemaVersion = "1" // in docs/json-schema.md and pinned by a golden-file test: a top-level // schema_version, the per-check results, and a summary tally. type JSONSink struct { - w io.Writer - results []check.Result - counts map[check.Status]int - repeat *repeatDoc // non-nil only in repeat mode (--count); see SetRepeat + w io.Writer + results []check.Result + counts map[check.Status]int + repeat *repeatDoc // non-nil only in repeat mode (--count); see SetRepeat + servers []serverDoc // non-nil only when comparing >1 --server; see SetServers + comparison *check.Comparison // the cross-server verdict, paired with servers } func NewJSONSink(w io.Writer) *JSONSink { @@ -46,16 +48,64 @@ type summaryCounts struct { Skip int `json:"skip"` } +// serverDoc is one server's block in a multi-server comparison. Top-level +// `results`/`summary` stay present (the combined tally across all servers, so +// `.summary.fail` still drives scripts); `servers[]` groups them per server. +type serverDoc struct { + Server string `json:"server"` + Results []check.Result `json:"results"` + Summary summaryCounts `json:"summary"` +} + +// SetServers attaches the per-server grouping and cross-server verdict for a +// multi-server run. It is additive: single-server documents omit both fields and +// are byte-for-byte unchanged. +func (s *JSONSink) SetServers(runs []check.ServerRun, cmp check.Comparison) { + for _, r := range runs { + s.servers = append(s.servers, serverDoc{ + Server: r.Server, + Results: r.Results, + Summary: tally(r.Results), + }) + } + s.comparison = &cmp +} + +// tally counts a result set into the fixed summary struct (all statuses present, +// including zeros). +func tally(results []check.Result) summaryCounts { + var c summaryCounts + for _, r := range results { + switch r.Status { + case check.StatusPass: + c.Pass++ + case check.StatusFail: + c.Fail++ + case check.StatusWarn: + c.Warn++ + case check.StatusInfo: + c.Info++ + case check.StatusSkip: + c.Skip++ + } + } + return c +} + func (s *JSONSink) Close() error { doc := struct { - SchemaVersion string `json:"schema_version"` - Results []check.Result `json:"results"` - Summary summaryCounts `json:"summary"` - Repeat *repeatDoc `json:"repeat,omitempty"` // additive: only with --count + SchemaVersion string `json:"schema_version"` + Results []check.Result `json:"results"` + Summary summaryCounts `json:"summary"` + Repeat *repeatDoc `json:"repeat,omitempty"` // additive: only with --count + Servers []serverDoc `json:"servers,omitempty"` // additive: only when comparing >1 --server + Comparison *check.Comparison `json:"comparison,omitempty"` // additive: paired with servers }{ SchemaVersion: SchemaVersion, Results: s.results, Repeat: s.repeat, + Servers: s.servers, + Comparison: s.comparison, Summary: summaryCounts{ Pass: s.counts[check.StatusPass], Fail: s.counts[check.StatusFail], diff --git a/test/freeradius-smoke.sh b/test/freeradius-smoke.sh index bed1554..ed3e2fb 100755 --- a/test/freeradius-smoke.sh +++ b/test/freeradius-smoke.sh @@ -13,7 +13,7 @@ cd "$(dirname "$0")/.." SECRET="testing123" work="$(mktemp -d)" -trap 'rm -rf "$work"; docker rm -f ah-freeradius ah-freeradius-nc ah-freeradius-flaky ah-netem ah-unhardened >/dev/null 2>&1 || true' EXIT +trap 'rm -rf "$work"; docker rm -f ah-freeradius ah-freeradius-nc ah-freeradius-flaky ah-netem ah-unhardened ah-secondary >/dev/null 2>&1 || true' EXIT # One test user for the PAP check, plus a machine identity for the NPS-style # machine-auth check. This file replaces the default authorize file; a single @@ -132,6 +132,21 @@ echo "== correct secret + PAP + PEAP-MSCHAPv2 + EAP-TTLS + EAP-TLS + MTU (expect --pap 'alice:pw' --peap 'alice:pw' --ttls 'alice:pw' \ --client-cert "$work/cert.pem" --client-key "$work/key.pem" --mtu --no-color || true +echo +echo "== Status-Server (RFC 5997): a server that supports it answers (expect PASS) ==" +# FreeRADIUS answers Status-Server by default: a liveness ping that consumes no +# auth attempt. The probe runs it first and reports PASS + supported=true. +set +e +ss_out="$("$work/authhound-probe" radius test --server 127.0.0.1 --secret "$SECRET" --no-color)" +ss_json="$("$work/authhound-probe" radius test --server 127.0.0.1 --secret "$SECRET" --json)" +set -e +echo "$ss_out" | grep -i "Status-Server" +echo "$ss_out" | grep -qi "answered Status-Server" || { echo "FAIL: Status-Server should PASS on a server that supports it"; exit 1; } +echo "$ss_json" | grep -q '"check": "status-server"' || { echo "FAIL: --json missing the status-server result"; exit 1; } +echo "$ss_json" | grep -q '"supported": "true"' || { echo "FAIL: --json should mark status-server supported=true"; exit 1; } +if echo "$ss_out$ss_json" | grep -q "$SECRET"; then echo "FAIL: secret leaked into Status-Server output"; exit 1; fi +echo "OK: Status-Server answered -> PASS, supported=true (no auth attempt consumed)" + echo echo "== BlastRADIUS posture: hardened FreeRADIUS signs replies (expect PASS) ==" # The probe always includes a Message-Authenticator in its request; a patched @@ -283,6 +298,55 @@ echo "$sjson" | grep -q '"warn": 1' || { echo "FAIL: --json summary.warn should echo "$sjson" | grep -q '"fail": 0' || { echo "FAIL: --json summary.fail should be present as 0"; exit 1; } echo "OK: schema_version present; per-status counts addressable (warn=1, fail=0)" +echo +echo "== multi-server comparison: two healthy servers, then one down ==" +# A second FreeRADIUS on a bridge port (the host-net primary already owns 1812), +# so the probe can compare two servers. Split-brain between RADIUS servers is the +# classic hidden cause of "intermittent" auth tickets. Permissive client entry +# (any source IP) because requests arrive via the Docker bridge gateway. +# Throwaway lab secret, never production. +cat > "$work/clients-secondary" <<'EOF' +client lab { + ipaddr = 0.0.0.0/0 + secret = testing123 +} +EOF +docker run -d --rm --name ah-secondary -p 127.0.0.1:11814:1812/udp \ + -v "$work/authorize:/etc/raddb/mods-config/files/authorize:ro" \ + -v "$work/clients-secondary:/etc/raddb/clients.conf:ro" \ + freeradius/freeradius-server:latest -fxx -l stdout >/dev/null +for i in $(seq 1 30); do + if docker logs ah-secondary 2>&1 | grep -q "Ready to process requests"; then break; fi + sleep 0.5 +done + +# Both healthy and matching -> "no split-brain", exit 0, per-server JSON blocks. +set +e +ms_ok="$("$work/authhound-probe" radius test --server 127.0.0.1,127.0.0.1:11814 --secret "$SECRET" --no-color)"; ms_ok_rc=$? +ms_json="$("$work/authhound-probe" radius test --server 127.0.0.1,127.0.0.1:11814 --secret "$SECRET" --json)" +set -e +echo "$ms_ok" | grep -A3 "Comparison across servers" +echo "$ms_ok" | grep -q "=== Server 1/2: 127.0.0.1:1812 ===" || { echo "FAIL: missing per-server banner"; exit 1; } +[ "$ms_ok_rc" -eq 0 ] || { echo "FAIL: two healthy servers should exit 0, got $ms_ok_rc"; exit 1; } +# Assert the verdict against the JSON (its verdict string is single-line; the +# text one is word-wrapped for the terminal and would split the phrase). +echo "$ms_json" | grep -q '"servers"' || { echo "FAIL: --json missing the servers[] block"; exit 1; } +echo "$ms_json" | grep -q '"comparison"' || { echo "FAIL: --json missing the comparison block"; exit 1; } +echo "$ms_json" | grep -q "no split-brain" || { echo "FAIL: two healthy servers should report no split-brain"; exit 1; } +if echo "$ms_ok$ms_json" | grep -q "$SECRET"; then echo "FAIL: secret leaked into multi-server output"; exit 1; fi +echo "OK: two healthy servers -> no split-brain; servers[]/comparison in JSON; exit 0" + +# Primary healthy, secondary (unused port) down -> the round-robin verdict, exit 1. +set +e +ms_down="$("$work/authhound-probe" radius test --server 127.0.0.1,127.0.0.1:11899 --secret "$SECRET" --timeout 2s --no-color)"; ms_down_rc=$? +set -e +echo "$ms_down" | grep -A3 "Comparison across servers" +echo "$ms_down" | grep -qi "round-robin" || { echo "FAIL: one-down comparison should name the round-robin risk"; exit 1; } +echo "$ms_down" | grep -q "50%" || { echo "FAIL: 1-of-2 down should read ~50%"; exit 1; } +[ "$ms_down_rc" -eq 1 ] || { echo "FAIL: a dead server should exit 1, got $ms_down_rc"; exit 1; } +echo "OK: primary healthy, secondary down -> ~50% round-robin verdict; exit 1" +docker rm -f ah-secondary >/dev/null 2>&1 || true + echo echo "== unwhitelisted probe (fresh server, no client entry -> expect registration hint) ==" # A fresh FreeRADIUS whose clients.conf contains only a dummy client, so requests @@ -313,8 +377,11 @@ if echo "$out" | grep -q "$SECRET"; then echo "FAIL: secret leaked into text out json="$(AUTHHOUND_SECRET="$SECRET" "$work/authhound-probe" radius test --server 127.0.0.1 --timeout 2s --json || true)" echo "$json" | grep -q '"hint"' || { echo "FAIL: --json missing hint field"; exit 1; } echo "$json" | grep -q '"source_ip": "127.0.0.1"' || { echo "FAIL: --json missing source_ip field"; exit 1; } +# Status-Server degrades gracefully: an unanswered probe is INFO (supported=false), +# never a FAIL — the reachability FAIL above carries the actionable guidance. +echo "$json" | grep -q '"supported": "false"' || { echo "FAIL: unanswered Status-Server should degrade to supported=false"; exit 1; } if echo "$json" | grep -q "$SECRET"; then echo "FAIL: secret leaked into JSON output"; exit 1; fi -echo "OK: registration hint present with detected IP; secret not leaked" +echo "OK: registration hint present with detected IP; Status-Server degraded to INFO; secret not leaked" echo echo "== flaky server: --count against induced packet loss (tc netem) ==" diff --git a/test/lab/docker-compose.yml b/test/lab/docker-compose.yml index ac7f418..36ae584 100644 --- a/test/lab/docker-compose.yml +++ b/test/lab/docker-compose.yml @@ -27,6 +27,25 @@ services: - ./config/authorize:/etc/raddb/mods-config/files/authorize:ro - ./config/radsec:/etc/raddb/sites-enabled/radsec:ro + # A second RADIUS server, for exercising multi-server comparison + # (`radius test --server 127.0.0.1,127.0.0.1:11814`). It's the fixture for the + # split-brain story: stop this one and the comparison verdict flips to + # "primary healthy, secondary NOT responding — ~50% of auths would fail". + # Bridge network with a published port so it doesn't clash with the host-net + # primary; requests arrive from the bridge gateway, so it accepts any source IP + # (throwaway lab secret, never production). + # + # docker compose -f test/lab/docker-compose.yml up # brings up both servers + # authhound-probe radius test --server 127.0.0.1,127.0.0.1:11814 --secret testing123 + freeradius-secondary: + image: freeradius/freeradius-server:latest + command: ["-fxx", "-l", "stdout"] + ports: + - "127.0.0.1:11814:1812/udp" + volumes: + - ./config/authorize:/etc/raddb/mods-config/files/authorize:ro + - ./config/clients-flaky:/etc/raddb/clients.conf:ro + # Flaky profile: the same FreeRADIUS behind induced packet loss and jitter, # for exercising `radius test --count` against genuinely intermittent # failures. Runs on a bridge network (not host) so netem can shape just this