From 97719b43a8d52ab22cd8d7ad42c7c01c55c4e571 Mon Sep 17 00:00:00 2001 From: Jayson Grace Date: Thu, 30 Jul 2026 11:52:10 -0600 Subject: [PATCH] refactor: source ares technique credit from loot token_coverage **Changed:** - Technique credit now derives from the loot JSON's own `token_coverage` map instead of re-deriving categories from raw vuln_id prefixes, eliminating the second Redis round trip - `transport_ares.go`. ares owns the vuln_id-to-category derivation as of ares-cli #366, so `FetchReport` is now a single SSM call. - Replaced the prefix-matching credit path with a `creditableCategories` join table mapping ares category names to answer-key technique IDs, plus a `uncreditableCategories` refusal table for `other`, `golden_ticket`, `printnightmare`, and `zerologon` - `transport_ares.go`. `printnightmare` and `zerologon` are explicit refusals because ares mints them on evidence that precedes success. - Reworked `detectTokenCoverageDrift` to surface only categories that are neither creditable nor deliberately refused, failing closed on unknown categories so new ares techniques warn rather than silently dead-credit - `transport_ares.go`. - `writeTokenCoverageEntries` emits one finding per creditable category with a proven exploit, walking categories in sorted order for deterministic output; evidence now names the category and proven count since ares no longer exposes individual vuln_ids - `transport_ares.go`. - Restructured the test suite around the category join: added coverage for the credit table, refusal classification, drift detection, deterministic output, and evidence formatting - `transport_ares_test.go`. - Updated `synthesizeJSONL` call sites to drop the exploited-set argument and drive golden-ticket-adjacent verification through `TokenCoverage` - `verify_test.go`. **Removed:** - Removed the Redis fetch path (`fetchExploited`, `splitExploitedSets`, `exploitedSetMarker`) that read the `:exploited` and `:superseded` sets, since technique credit no longer needs the proven subset - `transport_ares.go`. - Removed the `aresExploitedToTechniqueIDs` prefix table and its aliases (`driftCategoryAliases`, `driftExemptCategories`), which duplicated ares's own categorisation and drifted from it silently - `transport_ares.go`. - Removed the obsolete tests for the prefix mapping and combined-SMEMBERS parsing (`TestAresExploitedToTechniqueIDs`, `TestSplitExploitedSets`, `TestDriftCategoriesCoverAresTokenCategory`) - `transport_ares_test.go`. --- cli/internal/scoreboard/transport_ares.go | 329 ++++------- .../scoreboard/transport_ares_test.go | 557 +++++++++--------- cli/internal/scoreboard/verify_test.go | 7 +- 3 files changed, 387 insertions(+), 506 deletions(-) diff --git a/cli/internal/scoreboard/transport_ares.go b/cli/internal/scoreboard/transport_ares.go index ee3cea47..77215f03 100644 --- a/cli/internal/scoreboard/transport_ares.go +++ b/cli/internal/scoreboard/transport_ares.go @@ -62,8 +62,9 @@ type aresLoot struct { } // aresTokenBucket is one entry in the ares loot JSON's `token_coverage` map, -// keyed by ares's own scoreboard-category name. We deliberately do not credit -// objectives from it (see detectTokenCoverageDrift) and only read Exploited. +// keyed by ares's own scoreboard-category name. Exploited counts only proven +// techniques as of ares-cli #366, which made it the technique credit source +// (see writeTokenCoverageEntries). type aresTokenBucket struct { Discovered int `json:"discovered"` Exploited int `json:"exploited"` @@ -100,11 +101,12 @@ type aresDomainCompromise struct { KrbtgtHashTypes []string `json:"krbtgt_hash_types"` } -// FetchReport runs `ares ops loot --latest --json` on the remote instance and, -// if successful, also fetches the proven subset of the `ares:op::exploited` -// Redis set so technique objectives can be credited directly. Both payloads are -// gzip+base64-encoded to sidestep SSM's 24KB stdout cap. Returns ErrNoReport -// when the operation hasn't produced any state yet. +// FetchReport runs `ares ops loot --latest --json` on the remote instance and +// translates the snapshot into synthetic findings. Technique credit comes from +// the loot JSON's own `token_coverage`, so this is a single round trip: no +// Redis read is needed. The payload is gzip+base64-encoded to sidestep SSM's +// 24KB stdout cap. Returns ErrNoReport when the operation hasn't produced any +// state yet. func (t *AresTransport) FetchReport(ctx context.Context) (string, error) { const jqFilter = `{operation_id, started_at,` + ` credentials: [.credentials[] | {username, password, domain, is_admin}],` + @@ -136,93 +138,112 @@ func (t *AresTransport) FetchReport(ctx context.Context) (string, error) { return "", fmt.Errorf("parse ares loot json: %w", err) } - proven, allExploited := t.fetchExploited(ctx, loot.OperationID) - - drift := detectTokenCoverageDrift(&loot, allExploited) + drift := detectTokenCoverageDrift(&loot) t.mu.Lock() t.drift = drift t.mu.Unlock() - return synthesizeJSONL(&loot, proven), nil + return synthesizeJSONL(&loot), nil } // Drift returns the ares token_coverage categories that reported at least one -// exploit which this transport's prefix table credited to nothing, as of the -// last FetchReport. Empty means the two mappings agree. +// proven exploit but are neither creditable nor deliberately refused, as of the +// last FetchReport. Empty means we can classify everything ares scored. func (t *AresTransport) Drift() []string { t.mu.Lock() defer t.mu.Unlock() return append([]string(nil), t.drift...) } -// driftExemptCategories are ares categories that are expected to produce no -// direct technique credit, so their presence is not evidence of a mapping bug. +// creditableCategories joins ares token_coverage category names to answer-key +// technique IDs. Most are identity — ares normalises its own aliases already +// (gpo_* to gpo_abuse, mssql_* to mssql_exploit, ntlmv1 to ntlmv1_downgrade) +// — but the join has to be written out because the two vocabularies are +// maintained in different repos and only this table is checked against the +// generated answer key (TestAresCreditedTechniquesExistInAnswerKey). +// +// This is deliberately not the old prefix table. That one re-derived the +// category from a raw vuln_id, duplicating ares's token_category, and drifted +// from it silently for months (acl_*, gpo_* and adcs_esc8 credited nothing). +// ares now does that derivation and we consume the result. +var creditableCategories = map[string]string{ + "acl_abuse": "acl_abuse", + "gpo_abuse": "gpo_abuse", + "adcs_esc1": "adcs_esc1", + "adcs_esc2": "adcs_esc2", + "adcs_esc3": "adcs_esc3", + "adcs_esc4": "adcs_esc4", + "adcs_esc6": "adcs_esc6", + "adcs_esc7": "adcs_esc7", + "adcs_esc8": "adcs_esc8", + "adcs_esc9": "adcs_esc9", + "adcs_esc10_case1": "adcs_esc10_case1", + "adcs_esc10_case2": "adcs_esc10_case2", + "adcs_esc11": "adcs_esc11", + "adcs_esc13": "adcs_esc13", + "adcs_esc15": "adcs_esc15", + "mssql_linked_server": "mssql_linked_server", + "mssql_exploit": "mssql_exploit", + "constrained_delegation": "constrained_delegation", + "unconstrained_delegation": "unconstrained_delegation", + "shadow_credentials": "shadow_credentials", + "ntlm_relay": "ntlm_relay", + "child_to_parent": "child_to_parent", + "forest_trust": "cross_forest_trust", + "sid_history_abuse": "sid_history_abuse", + "asrep_roast": "asrep_roast", + "seimpersonate": "seimpersonate", + "kerberoast": "kerberoast", + "ntlmv1_downgrade": "ntlmv1_downgrade", + "llmnr_nbtns_poisoning": "llmnr_nbtns_poisoning", + "gmsa_password_read": "gmsa_password_read", + "laps_password_read": "laps_password_read", + "rbcd": "rbcd", + "nopac": "nopac", +} + +// uncreditableCategories are ares categories that must never produce technique +// credit. They are refusals, not gaps, so they are also exempt from drift. // -// - "other" is ares's catch-all, a mix of ids DreadGOAD scores and ids it -// has no objective for, so the bucket is not actionable either way. -// - "printnightmare" and "zerologon" are deliberately never credited: -// ares's evidence gate fires on markers that precede success -// (printnightmare accepts "Stub loaded" / "[+] Triggering"; zerologon only -// ever runs the nxc check module, never the reset), so a credit there -// would score an attempt as an exploit. ares categorized both as "other" -// until l50/ares#366 gave them their own categories; without an explicit -// exemption they would warn on every poll once that lands. +// - "other" is ares's catch-all: discovery-only ids (smb_signing, spooler, +// dc_secretsdump) land here with no technique name attached. // - "golden_ticket" is flat in ares but per-domain in the answer key -// (golden_ticket-). That credit comes from domain_compromise[] -// instead, so a flat category with no matching credit is expected. -var driftExemptCategories = map[string]bool{ +// (golden_ticket-). That credit comes from domain_compromise[], +// which carries the domain the flat category drops. +// - "printnightmare" and "zerologon" are uncreditable by design: ares mints +// both on evidence that precedes success (printnightmare accepts "Stub +// loaded"/"[+] Triggering"; zerologon only runs the nxc check module, +// never the reset). ares-cli #366 promoted them out of "other", so without +// an explicit refusal they would start crediting attempts as exploits. +var uncreditableCategories = map[string]bool{ "other": true, "golden_ticket": true, "printnightmare": true, "zerologon": true, } -// driftCategoryAliases translates ares category names that differ from the -// answer-key technique ID for the same thing. ares returns the matched prefix -// verbatim unless it has an explicit alias, so `forest_trust_escalation_*` -// becomes category "forest_trust" while the objective is "cross_forest_trust". -// Every other category ares emits already matches its technique ID. -var driftCategoryAliases = map[string]string{ - "forest_trust": "cross_forest_trust", +// aresCategoryToTechniqueID returns the answer-key technique ID for an ares +// token_coverage category, or "" when the category must not be credited. +func aresCategoryToTechniqueID(category string) string { + return creditableCategories[category] } -// detectTokenCoverageDrift cross-checks ares's own category mapping against -// aresExploitedToTechniqueIDs. ares computes `token_coverage` with -// `token_category` (ares-cli `ops/loot/format/display.rs`), whose comment -// claims it is kept in lock-step with the Go table below; nothing enforces -// that, and it has silently diverged before (acl_*, gpo_*, and adcs_esc8 were -// all uncreditable for months). A category ares scores as exploited that maps -// to no technique here is the signature of that drift. -// -// This is a detector, not a credit source. token_coverage counts from the raw -// `:exploited` set (ares-core `state/reader.rs` does a plain SMEMBERS with no -// `:superseded` subtraction), so its exploited counts include back-credits for -// techniques ares never actually walked. Scoring off it would undo the -// superseded filtering in fetchExploited. +// detectTokenCoverageDrift reports ares categories we can neither credit nor +// explain. Since ares owns the vuln_id-to-category derivation, the surviving +// failure mode is vocabulary drift: ares adds or renames a category and this +// repo keeps scoring without it. Such a category credits nothing and would +// otherwise be invisible, so it is surfaced rather than silently dropped. // -// allExploited must therefore be the raw set, not the proven subset: this asks -// "can the table below name every id ares has?", which is a mapping question. -// Passing the filtered set would report the superseded filter as drift. -func detectTokenCoverageDrift(l *aresLoot, allExploited []string) []string { - if len(l.TokenCoverage) == 0 { - return nil - } - credited := map[string]bool{} - for _, entry := range allExploited { - for _, id := range aresExploitedToTechniqueIDs(entry) { - credited[id] = true - } - } +// Categories are matched against creditableCategories and uncreditableCategories +// only. An unknown category never credits — failing closed here means a new ares +// technique shows up as a warning to classify, not as a silent dead credit. +func detectTokenCoverageDrift(l *aresLoot) []string { var drifted []string for category, bucket := range l.TokenCoverage { - if bucket.Exploited == 0 || driftExemptCategories[category] { + if bucket.Exploited == 0 || uncreditableCategories[category] { continue } - techID := category - if alias, ok := driftCategoryAliases[category]; ok { - techID = alias - } - if !credited[techID] { + if aresCategoryToTechniqueID(category) == "" { drifted = append(drifted, category) } } @@ -230,72 +251,6 @@ func detectTokenCoverageDrift(l *aresLoot, allExploited []string) []string { return drifted } -// exploitedSetMarker separates the two SMEMBERS payloads in the combined -// Redis fetch. No ares vuln_id can collide with it: ids are built from -// hostnames, IPs, usernames and rights, all sanitized to alphanumerics, dots -// and underscores at the construction sites. -const exploitedSetMarker = "---SUPERSEDED---" - -// fetchExploited reads both `ares:op::exploited` and -// `ares:op::superseded` in a single SSM round trip. It returns the proven -// subset (exploited minus superseded) for scoring, and the raw exploited set -// for drift detection. Failures are non-fatal (just means no technique -// findings get emitted this poll). -// -// ares credits a vuln as exploited when a *different* path already reached the -// same goal, so the technique itself was never proven: an mssql_impersonation -// win back-credits the host's mssql_access, and a dc_secretsdump_ -// back-credits child_to_parent for that domain (ares-cli -// `orchestrator/state/dedup.rs`). Those ids are mirrored into `:superseded`, -// which ares documents as "subset of KEY_EXPLOITED; the technique itself was -// never proven to work", so only the proven subset is scored. -// -// Both sets are returned because drift detection must not see the filtering: -// token_coverage counts superseded ids too, so comparing it against the proven -// subset would flag the filter doing its job as a mapping bug. -func (t *AresTransport) fetchExploited(ctx context.Context, opID string) (proven, all []string) { - if opID == "" { - return nil, nil - } - cmd := fmt.Sprintf("redis-cli SMEMBERS %s; echo %s; redis-cli SMEMBERS %s", - shellQuote(fmt.Sprintf("ares:op:%s:exploited", opID)), - shellQuote(exploitedSetMarker), - shellQuote(fmt.Sprintf("ares:op:%s:superseded", opID))) - out, status, _, err := runSSMShell(ctx, t.Client, t.InstanceID, cmd) - if err != nil || status != ssmtypes.CommandInvocationStatusSuccess { - return nil, nil - } - return splitExploitedSets(out) -} - -// splitExploitedSets parses the combined SMEMBERS output into the proven -// subset and the raw exploited set. -func splitExploitedSets(out string) (proven, all []string) { - superseded := map[string]bool{} - seenMarker := false - for _, line := range strings.Split(out, "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - if line == exploitedSetMarker { - seenMarker = true - continue - } - if seenMarker { - superseded[line] = true - continue - } - all = append(all, line) - } - for _, entry := range all { - if !superseded[entry] { - proven = append(proven, entry) - } - } - return proven, all -} - func decodeGzipBase64(s string) ([]byte, error) { gz, err := base64.StdEncoding.DecodeString(s) if err != nil { @@ -323,72 +278,7 @@ func (t *AresTransport) DeleteReport(_ context.Context) (bool, error) { return false, nil } -// aresExploitedToTechniqueIDs maps an entry from `ares:op::exploited` to -// the answer-key technique IDs it represents. Returns nil for entries that -// don't correspond to any answer-key technique. The exploited set uses prefix -// names like `mssql_linked_server__` or bare names like -// `constrained_delegation_`; we match on the prefix. -func aresExploitedToTechniqueIDs(entry string) []string { - prefixes := []struct { - prefix string - ids []string - }{ - {"mssql_linked_server_", []string{"mssql_linked_server"}}, - {"mssql_impersonation_", []string{"mssql_exploit"}}, - {"mssql_", []string{"mssql_exploit"}}, - {"constrained_delegation_", []string{"constrained_delegation"}}, - {"unconstrained_delegation_", []string{"unconstrained_delegation"}}, - {"forest_trust_", []string{"cross_forest_trust"}}, - {"child_to_parent_", []string{"child_to_parent"}}, - // ares emits the granted right in the id (acl_genericall_*, - // acl_writeproperty_*, ...); they all collapse to one objective. - {"acl_", []string{"acl_abuse"}}, - {"asrep_roast_", []string{"asrep_roast"}}, - {"kerberoast_", []string{"kerberoast"}}, - {"llmnr_", []string{"llmnr_nbtns_poisoning"}}, - {"ntlm_relay_", []string{"ntlm_relay"}}, - {"ntlmv1_", []string{"ntlmv1_downgrade"}}, - {"seimpersonate_", []string{"seimpersonate"}}, - {"nopac_", []string{"nopac"}}, - {"adcs_esc1_", []string{"adcs_esc1"}}, - {"adcs_esc2_", []string{"adcs_esc2"}}, - {"adcs_esc3_", []string{"adcs_esc3"}}, // collapses ESC3 + ESC3-CRA - {"adcs_esc4_", []string{"adcs_esc4"}}, - {"adcs_esc6_", []string{"adcs_esc6"}}, - {"adcs_esc7_", []string{"adcs_esc7"}}, - {"adcs_esc8_", []string{"adcs_esc8"}}, - {"adcs_esc9_", []string{"adcs_esc9"}}, - {"adcs_esc10_case1_", []string{"adcs_esc10_case1"}}, - {"adcs_esc10_case2_", []string{"adcs_esc10_case2"}}, - {"adcs_esc11_", []string{"adcs_esc11"}}, - {"adcs_esc13_", []string{"adcs_esc13"}}, - {"adcs_esc15_", []string{"adcs_esc15"}}, - // Same shape as acl_: ares emits gpo___. - {"gpo_", []string{"gpo_abuse"}}, - {"gmsa_", []string{"gmsa_password_read"}}, - {"laps_", []string{"laps_password_read"}}, - {"sid_history_", []string{"sid_history_abuse"}}, - {"rbcd_", []string{"rbcd"}}, - {"shadow_credentials_", []string{"shadow_credentials"}}, - } - // Per-domain golden ticket: `golden_ticket_` → `golden_ticket-`. - // One scoreboard objective per domain because forging requires that domain's - // krbtgt hash; a multi-domain forest can have a separate GT per domain. - if strings.HasPrefix(entry, "golden_ticket_") { - domain := strings.ToLower(strings.TrimPrefix(entry, "golden_ticket_")) - if domain != "" { - return []string{"golden_ticket-" + domain} - } - } - for _, p := range prefixes { - if strings.HasPrefix(entry, p.prefix) || entry == strings.TrimSuffix(p.prefix, "_") { - return p.ids - } - } - return nil -} - -func synthesizeJSONL(l *aresLoot, exploited []string) string { +func synthesizeJSONL(l *aresLoot) string { var b strings.Builder writeJSONLEntry(&b, map[string]string{ "agent_id": "ares:" + l.OperationID, @@ -401,7 +291,7 @@ func synthesizeJSONL(l *aresLoot, exploited []string) string { writeHashEntry(&b, h) } emitted := map[string]bool{} - writeExploitedEntries(&b, exploited, emitted) + writeTokenCoverageEntries(&b, l.TokenCoverage, emitted) writeDomainCompromiseEntries(&b, l.DomainCompromise, emitted) return b.String() } @@ -450,19 +340,36 @@ func writeHashEntry(b *strings.Builder, h aresHashEntry) { }) } -func writeExploitedEntries(b *strings.Builder, exploited []string, emitted map[string]bool) { - for _, ex := range exploited { - for _, techID := range aresExploitedToTechniqueIDs(ex) { - if emitted[techID] { - continue - } - emitted[techID] = true - writeJSONLEntry(b, map[string]string{ - "target": "tech:" + techID, - "evidence": "ares: " + ex, - "description": "exploited", - }) +// writeTokenCoverageEntries credits one finding per creditable ares category +// that reported at least one proven exploit. Categories are walked in sorted +// order so the synthesized JSONL is deterministic across polls. +// +// The evidence string names the category and its proven count rather than a +// vuln_id, because token_coverage is aggregated: ares no longer hands us the +// individual ids. That is the cost of sourcing credit from ares's own +// categorisation instead of re-deriving it here. +func writeTokenCoverageEntries(b *strings.Builder, coverage map[string]aresTokenBucket, emitted map[string]bool) { + categories := make([]string, 0, len(coverage)) + for category := range coverage { + categories = append(categories, category) + } + sort.Strings(categories) + + for _, category := range categories { + bucket := coverage[category] + if bucket.Exploited == 0 { + continue + } + techID := aresCategoryToTechniqueID(category) + if techID == "" || emitted[techID] { + continue } + emitted[techID] = true + writeJSONLEntry(b, map[string]string{ + "target": "tech:" + techID, + "evidence": fmt.Sprintf("ares token_coverage: %s (%d proven)", category, bucket.Exploited), + "description": "exploited", + }) } } diff --git a/cli/internal/scoreboard/transport_ares_test.go b/cli/internal/scoreboard/transport_ares_test.go index 6c9fa7bd..a379fcbf 100644 --- a/cli/internal/scoreboard/transport_ares_test.go +++ b/cli/internal/scoreboard/transport_ares_test.go @@ -2,102 +2,102 @@ package scoreboard import ( "reflect" - "sort" + "strings" "testing" ) -// TestAresExploitedToTechniqueIDs pins the mapping against the vuln_id shapes -// ares actually SADDs into `ares:op::exploited`. The literals below are -// taken from the ares construction sites, not invented: acl_* from -// `orchestrator/result_processing/acl_grants.rs`, gpo_* from -// `ares-tools/src/parsers/ntsd.rs`, and the rest from -// `orchestrator/result_processing/mod.rs` and `orchestrator/automation/*`. -// ares mirrors this table in `ops/loot/format/display.rs` (token_category); -// the two must agree or the loot view and the status board disagree. -func TestAresExploitedToTechniqueIDs(t *testing.T) { +// aresTokenCategories mirrors every category name ares's token_category can +// return (ares-cli `ops/loot/format/display.rs`). It is the input domain of the +// credit path, so both the credit table and the refusal table are checked +// against it below. +var aresTokenCategories = []string{ + "acl_abuse", + "gpo_abuse", + "adcs_esc1", + "adcs_esc2", + "adcs_esc3", + "adcs_esc4", + "adcs_esc6", + "adcs_esc7", + "adcs_esc8", + "adcs_esc9", + "adcs_esc10_case1", + "adcs_esc10_case2", + "adcs_esc11", + "adcs_esc13", + "adcs_esc15", + "mssql_linked_server", + "mssql_exploit", + "constrained_delegation", + "unconstrained_delegation", + "shadow_credentials", + "ntlm_relay", + "child_to_parent", + "forest_trust", + "sid_history_abuse", + "asrep_roast", + "seimpersonate", + "kerberoast", + "ntlmv1_downgrade", + "llmnr_nbtns_poisoning", + "gmsa_password_read", + "laps_password_read", + "rbcd", + "nopac", + "printnightmare", + "zerologon", + "golden_ticket", + "other", +} + +// TestAresCategoryToTechniqueID pins the category join, including the refusals. +// ares owns the vuln_id-to-category derivation now, so the only thing this +// repo decides is which categories become answer-key credit. +func TestAresCategoryToTechniqueID(t *testing.T) { tests := []struct { - name string - entry string - want []string + name string + category string + want string }{ - // ares emits the granted right in the id, never the literal - // "acl_abuse". Matching on "acl_abuse_" credited nothing. - {"acl generic all", "acl_genericall_tywin.lannister_kingsguard", []string{"acl_abuse"}}, - {"acl write property", "acl_writeproperty_stannis.baratheon_dragonstone", []string{"acl_abuse"}}, - {"acl write dacl", "acl_writedacl_alice_dc01", []string{"acl_abuse"}}, - {"acl all extended rights", "acl_allextendedrights_bob_carol", []string{"acl_abuse"}}, - - // Same shape: gpo___. - {"gpo write property", "gpo_writeproperty_alice__31b2f340_016d_11d2_945f_00c04fb984f9_", []string{"gpo_abuse"}}, - {"gpo generic all", "gpo_genericall_bob_default_domain_policy", []string{"gpo_abuse"}}, - - // ESC8 is an answer-key objective and is in ares's - // EXPLOITABLE_ESC_TYPES, but had no entry in the prefix table. - {"adcs esc8", "adcs_esc8_192.168.58.50_ca01", []string{"adcs_esc8"}}, - - // ESC1 must not swallow the longer ESC10/ESC11/ESC13/ESC15 forms. - {"adcs esc1", "adcs_esc1_192.168.58.50_ESC1", []string{"adcs_esc1"}}, - {"adcs esc10 case1", "adcs_esc10_case1_192.168.58.50", []string{"adcs_esc10_case1"}}, - {"adcs esc10 case2", "adcs_esc10_case2_192.168.58.50", []string{"adcs_esc10_case2"}}, - {"adcs esc11", "adcs_esc11_192.168.58.50", []string{"adcs_esc11"}}, - {"adcs esc13", "adcs_esc13_192.168.58.50_group", []string{"adcs_esc13"}}, - {"adcs esc15", "adcs_esc15_192.168.58.50", []string{"adcs_esc15"}}, + {"identity mapping", "acl_abuse", "acl_abuse"}, + {"ares normalises its own alias", "gpo_abuse", "gpo_abuse"}, + {"esc8 credits", "adcs_esc8", "adcs_esc8"}, + {"long esc form is distinct", "adcs_esc10_case1", "adcs_esc10_case1"}, + {"nopac credits after ares #366", "nopac", "nopac"}, - // mssql_ is a prefix of the two longer forms; order decides. - {"mssql linked server", "mssql_linked_server_192_168_58_22_sql01", []string{"mssql_linked_server"}}, - {"mssql impersonation", "mssql_impersonation_192_168_58_22", []string{"mssql_exploit"}}, - {"mssql bare", "mssql_192_168_58_22", []string{"mssql_exploit"}}, + // The one name that differs between the two vocabularies. + {"forest_trust aliases to cross_forest_trust", "forest_trust", "cross_forest_trust"}, - {"kerberoast", "kerberoast_svc_sql", []string{"kerberoast"}}, - {"asrep roast", "asrep_roast_contoso.local", []string{"asrep_roast"}}, - {"ntlm relay", "ntlm_relay_192_168_58_10", []string{"ntlm_relay"}}, - {"ntlmv1 downgrade", "ntlmv1_192_168_58_12", []string{"ntlmv1_downgrade"}}, - {"seimpersonate", "seimpersonate_sql01", []string{"seimpersonate"}}, - {"nopac", "nopac_dc01", []string{"nopac"}}, - {"sid history", "sid_history_alice", []string{"sid_history_abuse"}}, - {"constrained delegation", "constrained_delegation_svc_web", []string{"constrained_delegation"}}, - {"unconstrained delegation", "unconstrained_delegation_dc01", []string{"unconstrained_delegation"}}, - {"shadow credentials", "shadow_credentials_dc01", []string{"shadow_credentials"}}, - {"rbcd", "rbcd_dc01", []string{"rbcd"}}, - {"gmsa", "gmsa_svc_gmsa", []string{"gmsa_password_read"}}, - {"laps", "laps_sql01", []string{"laps_password_read"}}, - {"forest trust", "forest_trust_escalation_fabrikam.local", []string{"cross_forest_trust"}}, - {"child to parent", "child_to_parent_contoso.local", []string{"child_to_parent"}}, + // Refusals: ares mints these on evidence that precedes success, so + // crediting them would score an attempt as an exploit. + {"printnightmare refused", "printnightmare", ""}, + {"zerologon refused", "zerologon", ""}, - // Per-domain objective, lowercased. - {"golden ticket", "golden_ticket_CONTOSO.LOCAL", []string{"golden_ticket-contoso.local"}}, + // Flat in ares, per-domain in the answer key; credited from + // domain_compromise[] instead, which still carries the domain. + {"golden_ticket refused", "golden_ticket", ""}, - // Deliberately unmapped despite ares minting the prefix: the ares - // evidence gate credits these on markers that precede success. - // printnightmare accepts "Stub loaded"/"[+] Triggering" and zerologon - // only ever runs the nxc check module, never the reset. Mapping them - // would credit an attempt as an exploit. - {"printnightmare unproven", "printnightmare_192_168_58_10", nil}, - {"zerologon check only", "zerologon_dc01", nil}, + // ares's catch-all holds discovery-only ids with no technique name. + {"other refused", "other", ""}, - // Discovery-only ids ares tracks that are not scoreboard techniques. - {"smb signing", "smb_signing_192_168_58_10", nil}, - {"webdav", "webdav_enabled_192_168_58_22", nil}, - {"spooler", "spooler_192_168_58_10", nil}, - {"ldap signing", "ldap_signing_192_168_58_10", nil}, - {"dc secretsdump", "dc_secretsdump_contoso.local", nil}, - {"empty", "", nil}, + {"unknown category never credits", "brand_new_ares_category", ""}, + {"empty", "", ""}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := aresExploitedToTechniqueIDs(tc.entry) - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("aresExploitedToTechniqueIDs(%q) = %v, want %v", tc.entry, got, tc.want) + if got := aresCategoryToTechniqueID(tc.category); got != tc.want { + t.Errorf("aresCategoryToTechniqueID(%q) = %q, want %q", tc.category, got, tc.want) } }) } } -// TestAresExploitedTechniquesExistInAnswerKey guards against the drift that -// motivated this file: a prefix that maps to a technique ID the generated -// answer key never declares is a silently dead credit. -func TestAresExploitedTechniquesExistInAnswerKey(t *testing.T) { +// TestAresCreditedTechniquesExistInAnswerKey is the guard that keeps the credit +// table honest: a category mapping to a technique ID the generated answer key +// never declares is a credit that can never land. Every creditable category is +// checked, not a sample, so adding one without an objective fails here. +func TestAresCreditedTechniquesExistInAnswerKey(t *testing.T) { ak, err := GenerateAnswerKey("../../../ad/GOAD/data/config.json") if err != nil { t.Fatal(err) @@ -109,80 +109,50 @@ func TestAresExploitedTechniquesExistInAnswerKey(t *testing.T) { } } - entries := []string{ - "acl_genericall_alice_bob", - "gpo_writeproperty_alice_policy", - "adcs_esc8_192.168.58.50_ca01", - "kerberoast_svc_sql", - "mssql_linked_server_192_168_58_22_sql01", - "nopac_dc01", - } - for _, entry := range entries { - ids := aresExploitedToTechniqueIDs(entry) - if len(ids) == 0 { - t.Errorf("%q maps to no technique", entry) - continue - } - for _, id := range ids { - if !known[id] { - t.Errorf("%q maps to %q, which is not an answer-key objective", entry, id) + for category, techID := range creditableCategories { + t.Run(category, func(t *testing.T) { + if !known[techID] { + t.Errorf("category %q credits %q, which is not an answer-key technique objective", category, techID) } - } + }) } } -// TestSplitExploitedSets covers the combined SMEMBERS parse: the proven subset -// drops superseded ids, while the raw set keeps them for drift detection. -func TestSplitExploitedSets(t *testing.T) { - tests := []struct { - name string - out string - wantProven []string - wantAll []string - }{ - { - name: "superseded removed from proven only", - out: "kerberoast_svc_sql\nmssql_access_sql01\n" + exploitedSetMarker + "\nmssql_access_sql01\n", - wantProven: []string{"kerberoast_svc_sql"}, - wantAll: []string{"kerberoast_svc_sql", "mssql_access_sql01"}, - }, - { - // ares only writes :superseded when something was superseded, so - // the second SMEMBERS is routinely empty. - name: "absent superseded key", - out: "kerberoast_svc_sql\nrbcd_dc01\n" + exploitedSetMarker + "\n", - wantProven: []string{"kerberoast_svc_sql", "rbcd_dc01"}, - wantAll: []string{"kerberoast_svc_sql", "rbcd_dc01"}, - }, - { - name: "everything superseded", - out: "child_to_parent_contoso.local\n" + exploitedSetMarker + "\nchild_to_parent_contoso.local\n", - wantProven: nil, - wantAll: []string{"child_to_parent_contoso.local"}, - }, - { - name: "both empty", - out: exploitedSetMarker + "\n", - wantProven: nil, - wantAll: nil, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - proven, all := splitExploitedSets(tc.out) - if !reflect.DeepEqual(proven, tc.wantProven) { - t.Errorf("proven = %v, want %v", proven, tc.wantProven) - } - if !reflect.DeepEqual(all, tc.wantAll) { - t.Errorf("all = %v, want %v", all, tc.wantAll) +// TestEveryAresCategoryIsClassified asserts each category ares can emit is +// either creditable or an explicit refusal. An unclassified one credits +// nothing and warns forever, which trains operators to ignore the drift signal. +func TestEveryAresCategoryIsClassified(t *testing.T) { + for _, category := range aresTokenCategories { + t.Run(category, func(t *testing.T) { + _, creditable := creditableCategories[category] + if creditable == uncreditableCategories[category] { + t.Errorf("category %q must be exactly one of creditable or uncreditable (creditable=%v, uncreditable=%v)", + category, creditable, uncreditableCategories[category]) } }) } } -// TestDetectTokenCoverageDrift pins the cross-check against ares's own -// category mapping. The regression it exists to catch is the acl_/gpo_/esc8 -// class: ares scores a category as exploited, our prefix table credits nothing. +// TestUncreditableCategoriesAreDeliberate pins the exact refusal set. Widening +// it is how a real technique goes quiet: a category moved here stops crediting +// and stops warning, so every entry has to be justified in the doc comment +// rather than added to silence a failing run. +func TestUncreditableCategoriesAreDeliberate(t *testing.T) { + want := map[string]bool{ + "other": true, + "golden_ticket": true, + "printnightmare": true, + "zerologon": true, + } + if !reflect.DeepEqual(uncreditableCategories, want) { + t.Errorf("uncreditableCategories = %v, want %v; a new refusal needs its rationale in the doc comment", + uncreditableCategories, want) + } +} + +// TestDetectTokenCoverageDrift covers the surviving failure mode. ares owns the +// categorisation, so drift is no longer two tables disagreeing — it is ares +// emitting a category name this repo has never classified. func TestDetectTokenCoverageDrift(t *testing.T) { cov := func(pairs ...any) map[string]aresTokenBucket { m := map[string]aresTokenBucket{} @@ -193,91 +163,57 @@ func TestDetectTokenCoverageDrift(t *testing.T) { } tests := []struct { - name string - coverage map[string]aresTokenBucket - exploited []string - want []string + name string + coverage map[string]aresTokenBucket + want []string }{ { - name: "no drift when the table credits the category", - coverage: cov("acl_abuse", 3, "kerberoast", 1), - exploited: []string{"acl_genericall_alice_bob", "kerberoast_svc_sql"}, - want: nil, + name: "no drift when every category is classified", + coverage: cov("acl_abuse", 3, "kerberoast", 1), + want: nil, }, { - // The exact shape of the bug this detector exists for: before the - // prefix fix, acl_* and gpo_* ids credited nothing at all. - name: "drift when a category credits nothing", - coverage: cov("acl_abuse", 3, "gpo_abuse", 1, "kerberoast", 1), - exploited: []string{"kerberoast_svc_sql"}, - want: []string{"acl_abuse", "gpo_abuse"}, + // A category ares adds upstream that nobody has classified here. + name: "unknown category with proven exploits drifts", + coverage: cov("acl_abuse", 3, "esc12_of_the_future", 1), + want: []string{"esc12_of_the_future"}, }, { - name: "discovered-but-not-exploited is not drift", - coverage: cov("adcs_esc1", 0), - exploited: nil, - want: nil, + name: "drifted categories are sorted", + coverage: cov("zzz_new", 1, "aaa_new", 1), + want: []string{"aaa_new", "zzz_new"}, }, { - // A mix of ids we score and ids we have no objective for, so the - // bucket says nothing about our mapping either way. - name: "other bucket is exempt", - coverage: cov("other", 5), - exploited: nil, - want: nil, + name: "discovered-but-not-exploited is not drift", + coverage: cov("adcs_esc1", 0), + want: nil, }, { - // l50/ares#366 promotes these out of "other" into their own - // categories. We deliberately never credit them (the ares evidence - // gate fires on attempt markers), so without the exemption they - // would warn on every poll once that lands. - name: "deliberately uncredited cve categories are exempt", - coverage: cov("printnightmare", 2, "zerologon", 1), - exploited: nil, - want: nil, + // An unclassified category with no proven exploit says nothing. + name: "unknown category without exploits is not drift", + coverage: cov("esc12_of_the_future", 0), + want: nil, }, { - // Same #366 change, opposite outcome: nopac leaves "other" too, - // but we do map it to a real objective, so it must credit. - name: "nopac credits once it leaves the other bucket", - coverage: cov("nopac", 1), - exploited: []string{"nopac_dc01"}, - want: nil, + name: "deliberate refusals are exempt", + coverage: cov("other", 5, "golden_ticket", 2, "printnightmare", 1, "zerologon", 1), + want: nil, }, { - // ares collapses golden_ticket_ to one flat category; the - // per-domain credit comes from domain_compromise[] instead. - name: "flat golden_ticket is exempt", - coverage: cov("golden_ticket", 2), - exploited: nil, - want: nil, + name: "forest_trust alias is classified", + coverage: cov("forest_trust", 1), + want: nil, }, { - // ares category name differs from the answer-key technique ID. - name: "forest_trust alias resolves to cross_forest_trust", - coverage: cov("forest_trust", 1), - exploited: []string{"forest_trust_escalation_fabrikam.local"}, - want: nil, - }, - { - // The superseded filter must not read as drift: the raw set still - // contains the id, so the mapping question answers "yes". - name: "superseded id still counts as mapped", - coverage: cov("child_to_parent", 1), - exploited: []string{"child_to_parent_contoso.local"}, - want: nil, - }, - { - name: "missing token_coverage disables the check", - coverage: nil, - exploited: nil, - want: nil, + name: "missing token_coverage disables the check", + coverage: nil, + want: nil, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := detectTokenCoverageDrift(&aresLoot{TokenCoverage: tc.coverage}, tc.exploited) + got := detectTokenCoverageDrift(&aresLoot{TokenCoverage: tc.coverage}) if !reflect.DeepEqual(got, tc.want) { t.Errorf("detectTokenCoverageDrift() = %v, want %v", got, tc.want) } @@ -285,106 +221,141 @@ func TestDetectTokenCoverageDrift(t *testing.T) { } } -// TestDriftCategoriesCoverAresTokenCategory asserts that every category name -// ares's token_category can return is either creditable by our prefix table, -// explicitly exempt, or explicitly aliased. A new ares category that is none -// of those would warn forever, which trains operators to ignore the signal. -// -// This covers only the creditable half. Exempt categories are asserted by -// TestDriftExemptCategoriesAreDeliberate instead, because -// detectTokenCoverageDrift short-circuits on them before it reads the -// exploited set: pairing one with a vuln_id here would pass for any string -// and assert nothing. -func TestDriftCategoriesCoverAresTokenCategory(t *testing.T) { - // Mirrors the creditable return values of token_category in ares-cli - // `ops/loot/format/display.rs`, paired with a representative vuln_id. - aresCategories := map[string]string{ - "acl_abuse": "acl_genericall_alice_bob", - "gpo_abuse": "gpo_writeproperty_alice_policy", - "adcs_esc1": "adcs_esc1_192.168.58.50_t", - "adcs_esc2": "adcs_esc2_192.168.58.50_t", - "adcs_esc3": "adcs_esc3_192.168.58.50_t", - "adcs_esc4": "adcs_esc4_192.168.58.50_t", - "adcs_esc6": "adcs_esc6_192.168.58.50_t", - "adcs_esc7": "adcs_esc7_192.168.58.50_t", - "adcs_esc8": "adcs_esc8_192.168.58.50_t", - "adcs_esc9": "adcs_esc9_192.168.58.50_t", - "adcs_esc10_case1": "adcs_esc10_case1_192.168.58.50", - "adcs_esc10_case2": "adcs_esc10_case2_192.168.58.50", - "adcs_esc11": "adcs_esc11_192.168.58.50", - "adcs_esc13": "adcs_esc13_192.168.58.50", - "adcs_esc15": "adcs_esc15_192.168.58.50", - "mssql_linked_server": "mssql_linked_server_sql01", - "mssql_exploit": "mssql_impersonation_sql01", - "constrained_delegation": "constrained_delegation_svc", - "unconstrained_delegation": "unconstrained_delegation_dc01", - "shadow_credentials": "shadow_credentials_dc01", - "ntlm_relay": "ntlm_relay_192_168_58_10", - "child_to_parent": "child_to_parent_contoso.local", - "forest_trust": "forest_trust_escalation_fabrikam.local", - "sid_history_abuse": "sid_history_alice", - "asrep_roast": "asrep_roast_contoso.local", - "seimpersonate": "seimpersonate_sql01", - "kerberoast": "kerberoast_svc_sql", - "ntlmv1_downgrade": "ntlmv1_192_168_58_12", - "llmnr_nbtns_poisoning": "llmnr_192_168_58_11", - "gmsa_password_read": "gmsa_svc", - "laps_password_read": "laps_sql01", - "rbcd": "rbcd_dc01", - // Added by l50/ares#366, which promotes nopac out of "other". The - // same change promotes printnightmare and zerologon, which are - // exempt rather than creditable; see the test below. - "nopac": "nopac_dc01", - } - - for category, vulnID := range aresCategories { +// TestNoAresCategoryDrifts runs the real category list through the detector. +// Any category ares can emit must be silent when it reports a proven exploit. +func TestNoAresCategoryDrifts(t *testing.T) { + for _, category := range aresTokenCategories { t.Run(category, func(t *testing.T) { - if driftExemptCategories[category] { - t.Fatalf("category %q is drift-exempt, so its vuln_id %q is never read and this case asserts nothing; move it to TestDriftExemptCategoriesAreDeliberate", - category, vulnID) - } drift := detectTokenCoverageDrift( &aresLoot{TokenCoverage: map[string]aresTokenBucket{category: {Exploited: 1}}}, - []string{vulnID}, ) if len(drift) != 0 { - t.Errorf("category %q with vuln_id %q reports drift %v; it needs a prefix entry, an alias, or an exemption", - category, vulnID, drift) + t.Errorf("category %q reports drift %v; it needs a credit entry or an explicit refusal", category, drift) } }) } } -// TestDriftExemptCategoriesAreDeliberate pins the categories ares reports that -// DreadGOAD refuses to credit. detectTokenCoverageDrift returns before it -// consults the exploited set for these, so there is no vuln_id to pair them -// with and the exploited set below is deliberately nil: the assertion is that -// the exemption exists and suppresses the warning, nothing more. -// -// The exact-set check is the point. Silently widening driftExemptCategories is -// how a real mapping bug gets muted, so a new entry must be added here and -// justified in the driftExemptCategories doc comment. -func TestDriftExemptCategoriesAreDeliberate(t *testing.T) { - want := []string{"golden_ticket", "other", "printnightmare", "zerologon"} - - got := make([]string, 0, len(driftExemptCategories)) - for category := range driftExemptCategories { - got = append(got, category) - } - sort.Strings(got) - if !reflect.DeepEqual(got, want) { - t.Fatalf("driftExemptCategories = %v, want %v; every exemption needs a documented reason", got, want) +// TestWriteTokenCoverageEntries covers what reaches the scoreboard: only +// categories with a proven exploit, only creditable ones, deduped against +// findings already emitted, and in a stable order. +func TestWriteTokenCoverageEntries(t *testing.T) { + tests := []struct { + name string + coverage map[string]aresTokenBucket + emitted map[string]bool + want []string + absent []string + }{ + { + name: "credits proven categories only", + coverage: map[string]aresTokenBucket{ + "acl_abuse": {Discovered: 4, Exploited: 2, Status: "partial"}, + "adcs_esc1": {Discovered: 1, Exploited: 0, Status: "missing"}, + }, + want: []string{"tech:acl_abuse"}, + absent: []string{"tech:adcs_esc1"}, + }, + { + // ares subtracts superseded ids upstream, so a category credited + // only by another path arrives as exploited=0 and must not score. + name: "fully superseded category does not credit", + coverage: map[string]aresTokenBucket{ + "child_to_parent": {Discovered: 1, Exploited: 0, Status: "missing"}, + }, + absent: []string{"tech:child_to_parent"}, + }, + { + name: "refusals never credit", + coverage: map[string]aresTokenBucket{ + "printnightmare": {Exploited: 3}, + "zerologon": {Exploited: 1}, + "golden_ticket": {Exploited: 2}, + "other": {Exploited: 9}, + }, + absent: []string{"tech:printnightmare", "tech:zerologon", "tech:golden_ticket", "tech:other"}, + }, + { + name: "alias is credited under the answer-key id", + coverage: map[string]aresTokenBucket{"forest_trust": {Exploited: 1}}, + want: []string{"tech:cross_forest_trust"}, + absent: []string{"tech:forest_trust"}, + }, + { + name: "already-emitted technique is not duplicated", + coverage: map[string]aresTokenBucket{"kerberoast": {Exploited: 2}}, + emitted: map[string]bool{"kerberoast": true}, + absent: []string{"tech:kerberoast"}, + }, + { + name: "unknown category is dropped, not credited", + coverage: map[string]aresTokenBucket{"esc12_of_the_future": {Exploited: 4}}, + absent: []string{"tech:esc12_of_the_future"}, + }, } - for _, category := range want { - t.Run(category, func(t *testing.T) { - drift := detectTokenCoverageDrift( - &aresLoot{TokenCoverage: map[string]aresTokenBucket{category: {Exploited: 1}}}, - nil, - ) - if len(drift) != 0 { - t.Errorf("exempt category %q reports drift %v; the exemption is not taking effect", category, drift) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var b strings.Builder + emitted := tc.emitted + if emitted == nil { + emitted = map[string]bool{} + } + writeTokenCoverageEntries(&b, tc.coverage, emitted) + out := b.String() + + for _, want := range tc.want { + if !strings.Contains(out, want) { + t.Errorf("missing %q in output:\n%s", want, out) + } + } + for _, absent := range tc.absent { + if strings.Contains(out, absent) { + t.Errorf("unexpected %q in output:\n%s", absent, out) + } } }) } } + +// TestWriteTokenCoverageEntriesIsDeterministic guards the sort: Go map order is +// randomised per run, and an unstable synthesized report would churn the +// scoreboard between otherwise identical polls. +func TestWriteTokenCoverageEntriesIsDeterministic(t *testing.T) { + coverage := map[string]aresTokenBucket{ + "rbcd": {Exploited: 1}, + "acl_abuse": {Exploited: 1}, + "kerberoast": {Exploited: 1}, + "nopac": {Exploited: 1}, + } + + var first string + for i := 0; i < 20; i++ { + var b strings.Builder + writeTokenCoverageEntries(&b, coverage, map[string]bool{}) + got := b.String() + if i == 0 { + first = got + continue + } + if got != first { + t.Fatalf("output is not deterministic:\nfirst:\n%s\ngot:\n%s", first, got) + } + } +} + +// TestTokenCoverageEvidenceNamesTheCategory documents the auditability tradeoff +// this credit source makes: the evidence string carries the category and its +// proven count, because token_coverage is aggregated and ares no longer hands +// over the individual vuln_ids. +func TestTokenCoverageEvidenceNamesTheCategory(t *testing.T) { + var b strings.Builder + writeTokenCoverageEntries(&b, map[string]aresTokenBucket{ + "acl_abuse": {Discovered: 7, Exploited: 3, Status: "partial"}, + }, map[string]bool{}) + + out := b.String() + if !strings.Contains(out, "ares token_coverage: acl_abuse (3 proven)") { + t.Errorf("evidence should name the category and proven count, got:\n%s", out) + } +} diff --git a/cli/internal/scoreboard/verify_test.go b/cli/internal/scoreboard/verify_test.go index a137ea30..4d4f7c8b 100644 --- a/cli/internal/scoreboard/verify_test.go +++ b/cli/internal/scoreboard/verify_test.go @@ -209,7 +209,7 @@ func TestSynthesizeJSONLDomainCompromise(t *testing.T) { }, }, } - jsonl := synthesizeJSONL(loot, nil) + jsonl := synthesizeJSONL(loot) report := ParseReport(jsonl) owned := domainsFromKrbtgt(report.Findings) @@ -265,8 +265,11 @@ func TestVerifyDomainCompromiseWithoutGoldenTicket(t *testing.T) { AdminUsers: []string{"administrator"}, }, }, + TokenCoverage: map[string]aresTokenBucket{ + "adcs_esc1": {Discovered: 1, Exploited: 1, Status: "ok"}, + }, } - report := ParseReport(synthesizeJSONL(loot, []string{"adcs_esc1_10.1.2.254"})) + report := ParseReport(synthesizeJSONL(loot)) status := VerifyReport(report, ak) verified := verifiedObjectiveIDs(status)