Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs #34295

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs #34295

merged 3 commits into from
Jan 19, 2023

Conversation

MakoWish
Copy link
Contributor

@MakoWish MakoWish commented Jan 17, 2023 •
edited
Loading

What does this PR do?

Some security events contain a source IP address of "LOCAL" or "Unknown" which are not valid IP addresses. This PR will correct the processing of events containing one of those values.

Why is it important?

This bug causes mapping exceptions and prevents these events from being ingested.

Checklist

  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

-fixes #34263

@MakoWish MakoWish requested a review from a team as a code owner January 17, 2023 21:28
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 17, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 17, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @MakoWish? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@MakoWish MakoWish mentioned this pull request Jan 17, 2023
Closed
1 task
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 17, 2023 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-19T00:54:51.954+0000

  • Duration: 39 min 51 sec

Test stats 🧪

Test Results
Failed 0
Passed 336
Skipped 0
Total 336

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 17, 2023
@efd6
Copy link
Contributor

efd6 commented Jan 17, 2023

/test

efd6
efd6 reviewed Jan 17, 2023
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Are you able to provide events to exercise this in tests?

@MakoWish
Copy link
Contributor Author

I unfortunately cannot provide EVTX files for this one, as the only record of this happening so far is from servers, and I do not have access to log into those servers. Since I have already made the changes to our Ingest Pipeline, I could provide the JSON of some of these documents in Elasticsearch for these events.

@MakoWish
Copy link
Contributor Author

Scratch that. I was able to find a client with some of these. As with the other PR, these events do contain sensitive information. Please let me know how/where I can send them to you, and I will do that.

@MakoWish
Copy link
Contributor Author

Attaching five scrubbed events of each where the source IP is either "LOCAL" or "Unknown".
source_ip_invalid.zip

@MakoWish
Copy link
Contributor Author

Can we back-port this?

@efd6 efd6 added bug backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify enhancement labels Jan 18, 2023
@efd6
add tests cases for unknown and local address
876f944 
Tests cases mechanically derived from user-provided XML scrubbed event data.
@efd6
Copy link
Contributor

efd6 commented Jan 18, 2023

I've added the tests temporarily in the collection testdata. I want to flesh this out with a way to just drop XML files into inputs, but this is a bigger change, so I will do that separately.

@efd6
Copy link
Contributor

efd6 commented Jan 18, 2023

/test

@efd6
Copy link
Contributor

efd6 commented Jan 19, 2023

/test

efd6
efd6 approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@efd6 efd6 merged commit 4a1e56f into elastic:main Jan 19, 2023
@mergify mergify bot mentioned this pull request Jan 19, 2023
Closed
mergify bot pushed a commit that referenced this pull request Jan 19, 2023
@MakoWish @mergify
Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs (#34295
5d13e19 
)

Tests cases mechanically derived from user-provided XML scrubbed event data.

Co-authored-by: Dan Kortschak <[email protected]>
(cherry picked from commit 4a1e56f)

# Conflicts:
#	x-pack/winlogbeat/module/security/ingest/security.yml
#	x-pack/winlogbeat/module/security/test/testdata/4778.evtx.golden.json
#	x-pack/winlogbeat/module/security/test/testdata/4778.golden.json
#	x-pack/winlogbeat/module/security/test/testdata/4779.evtx.golden.json
#	x-pack/winlogbeat/module/security/test/testdata/4779.golden.json
mergify bot pushed a commit that referenced this pull request Jan 19, 2023
@MakoWish @mergify
Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs (#34295
240ff45 
)

Tests cases mechanically derived from user-provided XML scrubbed event data.

Co-authored-by: Dan Kortschak <[email protected]>
(cherry picked from commit 4a1e56f)
@mergify mergify bot mentioned this pull request Jan 19, 2023
Merged
efd6 pushed a commit that referenced this pull request Jan 19, 2023
@mergify @MakoWish
Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs (#34295
837d967 
) (#34309)

Tests cases mechanically derived from user-provided XML scrubbed event data.

Co-authored-by: Dan Kortschak <[email protected]>
(cherry picked from commit 4a1e56f)

Co-authored-by: Eric <[email protected]>
@efd6 efd6 mentioned this pull request Mar 20, 2023
Closed
@MakoWish MakoWish deleted the fix_source_ip_local branch March 25, 2023 18:18
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
@MakoWish @efd6 @chrisberkhout
Validate Source IP "LOCAL" or "Unknown" in Windows Security Logs (#34295
b37800f 
)

Tests cases mechanically derived from user-provided XML scrubbed event data.

Co-authored-by: Dan Kortschak <[email protected]>
@chemamartinez chemamartinez mentioned this pull request Aug 3, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 approved these changes

Assignees

@MakoWish MakoWish

Labels
8.7-candidate backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify bug Winlogbeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression to #19627 for Security Events with Source IP "LOCAL"
3 participants
@MakoWish @elasticmachine @efd6