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

crowdstrike: remove arbitrary filter and limit #1008

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

juju4
Copy link
Contributor

@juju4 juju4 commented Oct 8, 2022

Minor change to remove hardcoded settings that likely don't fit many environments

@juju4
Copy link
Contributor Author

juju4 commented Oct 15, 2022

Any comment on this one?

Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

Hi! Sorry for the lateness on reviews and thanks for your PR.

I don't know much about Crowdstrike, but do you think that this filter string should be passable as a param from the cartography CLI?

cc: @ryan-lane who knows more here.

cartography/intel/crowdstrike/endpoints.py Show resolved Hide resolved
@juju4
Copy link
Contributor Author

juju4 commented Nov 13, 2022

it's possible to add a cli param but usually you would want to get all assets. imho, more a edge case

Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

@juju4 - sorry for the extremely delayed review, I'm finally able to catch up a bit.

From my brief read, it seems like endpoints.py is written specifically to load in CrowdstrikeHosts assuming that they are not on AWS EC2. If we remove the filter, then will the rest of the code still work? Will it fail if there are CrowdstrikeHosts that are not on AWS EC2?

Again, I'm not familiar with Crowdstrike, just doing a read and seeing that the filter may have been put there for a reason.

@juju4
Copy link
Contributor Author

juju4 commented Jan 7, 2023

Example filter from falconpy docs
https://www.falconpy.io/Service-Collections/Hosts.html#available-filters

empty filter is valid meaning no filter. It is working, used in production.
existing code restricts to 400 devices and only AWS_EC2 provider

@juju4
Copy link
Contributor Author

juju4 commented Feb 25, 2023

is there something that I can help to move this forward?

achantavy
achantavy previously approved these changes Mar 30, 2023
Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

Approving, let's open up an issue to expose the parameters in config.py.

@achantavy achantavy enabled auto-merge (squash) March 30, 2023 16:16
@achantavy achantavy disabled auto-merge March 31, 2023 04:30
@chandanchowdhury chandanchowdhury force-pushed the devel-crowdstrike branch 2 times, most recently from 6512c92 to df79710 Compare June 26, 2024 22:15
@chandanchowdhury chandanchowdhury merged commit 07243c2 into cartography-cncf:master Jun 26, 2024
5 checks passed
SecPrez pushed a commit to SecPrez/cartography that referenced this pull request Nov 10, 2024
Minor change to remove hardcoded settings that likely don't fit many
environments

Co-authored-by: i_virus <[email protected]>
chandanchowdhury added a commit to chandanchowdhury/cartography that referenced this pull request Nov 27, 2024
Minor change to remove hardcoded settings that likely don't fit many
environments

Co-authored-by: i_virus <[email protected]>
Signed-off-by: chandanchowdhury <[email protected]>
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.

3 participants