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.

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

Fix and enhance handling of crowdstrike.DomainName #12509

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

peterydzynski
Copy link
Contributor

Proposed commit message

This PR fixes a few bugs and also enhances the handling of domain names.

List of bug fixes:

  • Fixed the logic surrounding the setting of url.scheme which would set all logs that have no destination port to url.scheme: http even if no network activity was involved. This logic is still entirely based on port which is obviously not ideal but I left that alone.
  • Fixed the handling of DNS fields to apply to all DNS log types. The old logic missed a few other types of events like SuspiciousDnsRequest.
  • Fixed the setting of dns.question.name which was handled by the registered domain processor which meant that all internal domains and PTR queries were not being set properly.

Enhancement:

  • Added handling for crowdstrike.DomainName when the event type is not DNS related. I have only observed SMB communications with crowdstrike.DomainName so there may still be some other event types that need handling but I do not have examples.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@peterydzynski peterydzynski requested a review from a team as a code owner January 28, 2025 22:15
@efd6
Copy link
Contributor

efd6 commented Jan 28, 2025

/test

@andrewkroh andrewkroh added enhancement New feature or request Integration:crowdstrike CrowdStrike Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations] labels Jan 28, 2025
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@peterydzynski
Copy link
Contributor Author

Thanks for the fixes @efd6

@efd6
Copy link
Contributor

efd6 commented Jan 29, 2025

/test

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@efd6
Copy link
Contributor

efd6 commented Jan 29, 2025

The tests are failing with

test case failed: one or more problems with fields found in documents: [0] field "crowdstrike.ContextBaseFileName" is undefined
[1] field "crowdstrike.EventOrigin" is undefined
[2] field "crowdstrike.QueryStatus" is undefined
[3] field "crowdstrike.SmbShareName" is undefined

Please add these fields to the field definitions.

@peterydzynski
Copy link
Contributor Author

Added mappings and actually tested on the right branch, apologies.

@efd6
Copy link
Contributor

efd6 commented Jan 30, 2025

/test

@elasticmachine
Copy link

elasticmachine commented Jan 30, 2025

💔 Build Failed

Failed CI Steps

History

@efd6
Copy link
Contributor

efd6 commented Jan 30, 2025

Please run elastic-package build.

@qcorporation qcorporation requested review from a team as code owners February 4, 2025 03:56
@andrewkroh andrewkroh added Integration:1password 1Password Integration:abnormal_security Abnormal Security New Integration Issue or pull request for creating a new integration package. labels Feb 4, 2025
@peterydzynski
Copy link
Contributor Author

Hey @efd6, I committed the updated readme after building. Any insight into what happened in the upstream that caused all these merge conflicts and what we need to do to get this merged in?

@efd6
Copy link
Contributor

efd6 commented Feb 6, 2025

There has been a history rewrite. Please rebase onto the new main and sync your main to that ref.

@peterydzynski peterydzynski force-pushed the crowdstrike-domain-fix branch from 3d1145c to 09ae88c Compare February 7, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:abnormal_security Abnormal Security Integration:1password 1Password New Integration Issue or pull request for creating a new integration package. Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants