Repository navigation
PingID credential-reset correlation: normalize usernames and handle multiple resets - #4298
Conversation
Normalize UPN and domain-prefixed usernames on both sides of the Windows password-event join while preserving the original PingID user in findings. Consider all matching password events and retain the nearest qualifying reset per complete stats group, preserving source-distinct pairings. Add the synthetic regression fixture from splunk/attack_data#1230 and increment the detection version. Local unit tests return two findings for the original fixture and four for the new fixture. Separate local assertions verify the exact results and nearest qualifying reset; the current CI unit runner only requires nonzero results. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add five synthetic PingID device-pairing events and six Windows 4723/4724 password events for PingID New MFA Method After Credential Reset. Cover UPN-to-bare username matching, multiple resets and nearest-reset selection, exclusion outside the one-hour window, and simultaneous pairings from distinct source IPs. Register both files in the manifest. Use lab identities and reserved documentation IPs. Companion to splunk/security_content#4298. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f517cc5 to
3dfc0a2
Compare
In an eval string literal "\\" reaches the regex engine as a single backslash, so "[^\\]*\\|@.*$" parses as an unterminated character class and the search fails: "Regex: missing terminating ] for character class". \x5c survives both YAML and SPL unescaping. Also restores the ^ anchor on the PingID side to match the Windows side. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks for the reformat. One follow-up commit (1f9689d): in an eval string literal Repro: |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Thanks a lot for the fix, my main goal was making the regex easier to review but I see some special chars can cause an issue at times(\x27 for single-quote etc.) I just added a comment there for future reviewers/users who might not be familiar with corresponding data sources. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77812d5c6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The reformat normalized user in place, so findings and drilldowns showed the bare account name instead of the PingID username used by the other PingID detections. Join on join_user again and drop it before output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@sbaker-gre thanks for the update! Looks like you (or claude) nuked the comments added by @onurmerdogan - Let's add those back for clarity. While codex does a final review. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Yep, my bad. I trusted claude with that one and darn it... |
There was a problem hiding this comment.
LGTM. Tested locally and results are showing up (see my screenshot above).
PS: due to how ingest time work via HEC, I had to cheat a little by using the event time rather than the detection _time (but in practice this should not be an issue when ingesting via forwarders or uploading).
As an extra note. Since CI is failing for external contributors, I might have missed something. We will run full testing as always internally and fix any issues that comes up.
PingID New MFA Method After Credential Reset can't fire in environments where PingID
usernames are UPNs or email addresses.
The search joins PingID device-pairing events to Windows 4723/4724 on
user. PingIDreports the IdP username (for example
victim_user@example.com), while the Windowspassword events carry the bare account name (
victim_user).upper()alone never makes those match.The shipped fixture uses
victim_useron both sides, so the unit test passes anyway.Changes
join_user, with anyDOMAIN\prefix and@domainsuffix strippedon both sides. The finding keeps the original PingID
user.join max=0, so every password event is considered, not only the first one the subsearchreturns. Previously, a reset logged after the pairing could hide an earlier one that
qualifies.
dedup src, user, action, object, lastTime sortby +timeDiffRaw: one finding perstatsgroup, using the nearest qualifying reset. Pairings from different source IPsstay separate.
user, so it can't overwrite the PingID value.reset after the pairing ahead of one that qualifies, a reset outside the window, and
simultaneous pairings from two IPs.
Verification
current search returns 0. Both pass the original fixture (2 results). Run with
contentctl-ng 1.1.1, Splunk 10.4.3, Splunk_TA_windows 11.0.2 and Splunk_TA_fix_windows.
reset. The current CI unit runner checks for nonzero results; it does not enforce the
exact count or selected reset.
🤖 Generated with Claude Code