Skip to content

fix(aws): keep MFA requirement checks out of sign-in success - #2660

Draft
kryonsx wants to merge 2 commits into
utmstack:v11from
kryonsx:codex/data-engine-aws-20260923
Draft

kryonsx wants to merge 2 commits into
utmstack:v11from
kryonsx:codex/data-engine-aws-20260923

Conversation

@kryonsx

@kryonsx kryonsx commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

CloudTrail CheckMfa: Success says the sign-in process checked whether MFA is required. Promoting that intermediate result to actionResult: success lets a success-only consumer treat it as completed authentication. The filter now preserves log.responseElements.CheckMfa and leaves the normalized result absent for this intermediate success. Failed checks still produce failure; completed console sign-ins, sign-in token issuance and API request outcomes retain their existing mappings.

Basis: AWS console sign-in events — sign-in process checks for MFA and CloudTrail record contents, reviewed against UTMStack ab96ae9a685c54c974253ec517c688a7557019e9 and SDK v1.1.33. API request success continues to describe the request, not completion of a later asynchronous job.

Validation:

  • Updated fabricated raw fixture and six dedicated raw authentication cases cover intermediate MFA success, failed check, completed/failed/missing login outcome, and denial overriding a reported success. Vendor result preservation and the actual SDK success-and-IP predicate are asserted.
  • AWS raw/normalization, rule predicate and mock-history suite: 195 passing tests/subtests; one external-document fixture test skipped. Dedicated authentication test and its six cases also pass.
  • All 73 AWS rule predicates were checked; no rule edit is needed. A generic root-activity predicate can lose an intermediate CheckMfa match if presented with that fabricated combination; this is intended under the final-authentication requirement.
  • Raw parsing is an explicit offline extraction model. The EventProcessor pipeline and actual alert creation were not executed in this source’s validation. No relevant AWS live records were available in the bounded review window; this correction is documentation-backed.

Compatibility: saved queries that counted successful CheckMfa records by normalized result should use the preserved vendor response with action: CheckMfa. No customer data or private identifiers are included. Draft only; no deployment or merge.

GitHub CI: dependency and prompt-discovery checks passed. The three AI-review jobs could not start because the workflow requires the missing THREATWINDS_API_KEY secret; the aggregate check therefore failed. This is separate from the local regression results above.

Companion dependency: a tested TI outcome-consumer compatibility correction is available as a separate draft. Review and roll it out together with the filter corrections. Deployed consumer-version equivalence remains unverified.

kryonsx and others added 2 commits September 23, 2026 09:16
The parser plugins remove every character except letters, digits and dots
from the field names they write. The AWS filter added its 22 correlation
markers with underscores (for example
log.correlationCandidate.mass_resource_deletion), so they were stored
without them (log.correlationCandidate.massresourcedeletion) and the 22
history rules that count them never matched.

Rename the markers to camelCase (for example
log.correlationCandidate.massResourceDeletion) in the filter and in every
rule that reads them. Rule conditions, thresholds and windows are unchanged.

Tests: the AWS parse model now stores grok, rename and add targets the way
the parser does, the history test counts each marker by its new name, and a
new test checks that every marker the filter adds and every marker an AWS
rule reads are the same name, made only of letters, digits and dots.
Fixtures that forge a marker in the input now use the new name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@osmontero

Copy link
Copy Markdown
Member

@kryonsx heads-up before you merge: I have an unmerged branch (fix/aws-filter-rules) that rewrites filters/aws/aws.yml to v2.1.0 (canonical-schema: eventName→action, sourceIPAddress→origin.ip, userIdentity.arn→origin.user, errorCode-derived actionResult, nested requestParameters/responseElements kept traversable) + migrates all 72 AWS rules to the new field paths. My branch is already based on your #2636, so we're not fighting over the v1.1.0 baseline — but my v2.1.0 actionResult block derives the result solely from errorCode presence and does not special-case CheckMfa, so if I merge over yours as-is I'd regress this fix.

Proposed sequence to avoid double-merging the same file:

  1. You land fix(aws): keep MFA requirement checks out of sign-in success #2660 (small, well-scoped, already has a contract test — good on the v1.1.x line).
  2. I rebase fix/aws-filter-rules onto the new v11 head, port the CheckMfa semantics into my v2.1.0 actionResult block, keep your aws_action_result_test.go green, and open my branch as a PR.

Happy to do the opposite (I port #2660 into my branch now and you can drop it from the v1.1.x line) if that's less churn — your call.

@kryonsx

kryonsx commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Change pushed (88b530b4): history markers renamed to names the parser keeps

What changed

  • filters/aws/aws.yml: the 22 log.correlationCandidate.* markers are now camelCase. For example, mass_resource_deletion is now massResourceDeletion.
  • The 22 rules that read them use exactly the same names (the Secrets Manager rule reads its marker twice). These rules have no # Rule version line, so no version was bumped.
  • plugins/alerts:
    • New aws_marker_names_test.go. It reads the filter and every AWS rule, and checks that each marker the filter adds and each marker a rule reads have the identical name, made only of letters, digits and dots. It also checks that each history rule counts the value the filter adds.
    • The parse model in the tests now cleans grok, rename and add target names the way the parser does, and the history tests look up the new names.

Why

  • The EventProcessor parser keeps only letters, digits and dots in the field names it writes (utils.SanitizeField). The filter wrote log.correlationCandidate.mass_resource_deletion, which is stored as log.correlationCandidate.massresourcedeletion, while the rules counted the underscored name. So the history rules never counted anything.
  • The old tests missed this because their parse model kept the underscores. Against the previous names, the new test and the history threshold tests fail for every marker. On this commit they pass.
  • The same underscored markers are already in v11 from Fix AWS CloudTrail outcomes, field mappings and correlation consumers #2636, so v11 has this problem until this merges.
  • Until the EventProcessor handles these characters, filters and rules should use only letters, digits and dots in field names.

Validation

  • None of the production instances we can read has AWS data, so the local EventProcessor playground ran on the 190 cases in this pull request's plugins/alerts/testdata/aws_raw.json, comparing this draft before and after the change.
    • All 190 events are parsed on both sides, with no event errors.
    • Before, 20 markers were stored on 36 events under the cleaned lowercase names. After, the same 20 markers are on the same 36 events under the camelCase names the rules read. For example, massResourceDeletion is on 11 events and iamPrivilegeEscalationPaths on 3.
    • samlProviderChange and consoleLoginImpossibleTravel are produced by neither version, because no test case triggers them.
    • Alerts are the same for every rule on both sides. No condition, window or grouping changed apart from the marker names.
    • The one "Circuit Breaker: AWS Mass Resource Deletion" alert appears on both sides. It comes from the playground having no search index.
  • go test ./... in plugins/alerts passes. The AWS filter and rules have no marker or top-level field names with _ or other special characters. Nested AWS keys such as x-amz-* are left as they are, because nested JSON keys keep their characters.

Not tested: the history thresholds themselves, because counting past events needs a search index. After deployment, only new events count toward the history windows.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants