-
Notifications
You must be signed in to change notification settings - Fork 407
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
[Crowdstrike] Adding new dashboards for Crowdstrike and Falcon #8478
Conversation
…t. Add validation bypass for searches
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
Some of the screenshots of the graphs is a bit weird because all the sample data is spread out over 10 years, they will look better with actual data. |
🌐 Coverage report
|
@@ -166,7 +166,7 @@ | |||
"preserve_original_event" | |||
], | |||
"threat": { | |||
"framework": "MITRE ATT&CK", | |||
"framework": "MITRE ATT\u0026CK", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this reverting to HTML-escaped? (throughout)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats because of our pipeline test code @efd6 . Its been an issue for a while. Though I was sure it was fixed. Its the json unmarshal that escapes things by default, though I believe we added an override for that at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is the reason. AIUI the html escaping was turned off, so we saw a whole heap of \uXXXX
→<real character>
. So I'm wondering why that is being reversed. Has this been changed back? I hope not for two reasons, it's unclear and it makes diff churn. @jsoriano Do you know why this is happening?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should be fixed, I will take a look. Thanks for the ping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reopening elastic/elastic-package#320.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will merge it for now then, I am checking with @jsoriano if we can take a look sooner rather than later, to see if we can resolve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @P1llus,
Is it possible that you have regenerated these files with an old version of elastic-package? If I try with 0.92, this change is reverted.
- "framework": "MITRE ATT\u0026CK",
+ "framework": "MITRE ATT&CK",
Both files are accepted by tests, probably because the default decoder makes parsing of entities transparent. I can try to disable this behaviour so this is detected, but please confirm what version of elastic-package you used to generate these files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try to disable this behaviour
It doesn't look so easy, HTML entities in JSON strings are always decoded in Go 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that you have regenerated these files with an old version of elastic-package? If I try with 0.92, this change is reverted.
This was my concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should have been made with elastic-package version-hash b2251c5-dirty (build time: 2023-11-13T11:17:14+01:00), but I can try it again with the latest build
Package crowdstrike - 1.25.0 containing this change is available at https://epr.elastic.co/search?package=crowdstrike |
This PR adds new dashboards for Crowdstrike (overview), and for Crowdstrike Falcon. It only adds slight changes to the existing FDR dashboard to fit the similar layout.
Added validation skip for searches (unsure why that is not allowed?), ran elastic-package format to clean up anything that was missing.
Checklist
changelog.yml
file.Related issues
Screenshots