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

Update _ioc.py #311

Closed
wants to merge 1 commit into from
Closed

Update _ioc.py #311

wants to merge 1 commit into from

Conversation

aneisch
Copy link
Contributor

@aneisch aneisch commented Aug 23, 2021

Remove explicit ids={} from endpoint. Fixes issue blocking FQL filter use.

Remove ids={} from IOC DELETE endpoint

  • Enhancement
  • Major Feature update
  • Bug fixes
  • Breaking Change
  • Updated unit tests
  • Documentation

Remove explicit ids={} from endpoint. Fixes issue blocking FQL filter use.
Copy link
Member

@jshcodes jshcodes left a comment

Choose a reason for hiding this comment

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

Do you have a code example where this change resolves an issue? I ask because this text is stripped from the endpoint before it is used in Service Classes.

target_url = f"{calling_object.base_url}{target_endpoint[2]}".replace("?ids={}", "")

(Trying to determine if there is an issue with this functionality.) If we remove this from the endpoint at this point in time, it will negatively impact the Uber class.

@aneisch
Copy link
Contributor Author

aneisch commented Aug 23, 2021

Let me do some additional testing. I'm actually using the Uber class.

@jshcodes
Copy link
Member

Let me do some additional testing. I'm actually using the Uber class.

Oh! Then we definitely still need that. 😁

Leaving this PR open while you test.

@aneisch
Copy link
Contributor Author

aneisch commented Aug 23, 2021

Here's my test code:

.. creds json, etc..
falcon = Uber(creds=crowdstrike_creds)

def delete_expired_iocs():
    # https://github.com/CrowdStrike/falconpy/wiki/IOCs#queryiocs
    expiration_date = date.today() - timedelta(days=14)
    log.debug(expiration_date)
    log.info(f"Deleting IOCs which expired on or before {expiration_date}")

    PARAMS = {
        'filter': f"""source:'"IOC Automation*"'+(expiration:<'{expiration_date}')""",
        'comment': "Deleted by crowdstrike-ioc-sync 14 days after expiration"
    }

    response = falcon.command('indicator_delete_v1', parameters=PARAMS)

    if response["status_code"] == 200:
        number_deleted = response['body']['meta']['pagination']['total']
        log.info(f"Deleted {number_deleted} expired IOCs")

    else:
        log.error(f"CrowdStrike API error: {response}")
        remain = 0
delete_expired_iocs()

I'm using my fork for my testing.

When 192 = "/iocs/entities/indicators/v1":

crowdstrike-ioc-sync    | 2021-08-23 12:09:07,545 : DEBUG : https://api.crowdstrike.com:443 "DELETE /iocs/entities/indicators/v1?filter=source%3A%27%22IOC+Automation%2A%22%27%2B%28expiration%3A%3C%272021-08-09%27%29&comment=Deleted+by+crowdstrike-ioc-sync+14+days+after+expiration HTTP/1.1" 200 191
crowdstrike-ioc-sync    | 2021-08-23 12:09:07,548 : INFO : Deleted 0 expired IOCs

When 192 = "/iocs/entities/indicators/v1?ids={}":

crowdstrike-ioc-sync    | 2021-08-23 12:10:31,354 : DEBUG : https://api.crowdstrike.com:443 "DELETE /iocs/entities/indicators/v1?ids=%7B%7D&filter=source%3A%27%22IOC+Automation%2A%22%27%2B%28expiration%3A%3C%272021-08-09%27%29&comment=Deleted+by+crowdstrike-ioc-sync+14+days+after+expiration HTTP/1.1" 400 228
crowdstrike-ioc-sync   | 2021-08-23 12:10:31,358 : ERROR : CrowdStrike API error: {'status_code': 400, 'headers': {'Content-Encoding': 'gzip', 'Content-Length': '228', 'Content-Type': 'application/json', 'Date': 'Mon, 23 Aug 2021 17:10:31 GMT', 'Strict-Transport-Security': 'max-age=15724800; includeSubDomains', 'X-Cs-Region': 'us-1', 'X-Cs-Traceid': 'xx', 'X-Ratelimit-Limit': '6000', 'X-Ratelimit-Remaining': '5998'}, 'body': {'meta': {'query_time': 0.000358292, 'powered_by': 'ioc-manager', 'trace_id': 'xx'}, 'errors': [{'code': 400, 'message': 'Either IDs or the FQL filter must be provided, not both'}], 'resources': []}}

@jshcodes
Copy link
Member

I'm using my fork for my testing.

When 192 = "/iocs/entities/indicators/v1":

crowdstrike-ioc-sync    | 2021-08-23 12:09:07,545 : DEBUG : https://api.crowdstrike.com:443 "DELETE /iocs/entities/indicators/v1?filter=source%3A%27%22IOC+Automation%2A%22%27%2B%28expiration%3A%3C%272021-08-09%27%29&comment=Deleted+by+crowdstrike-ioc-sync+14+days+after+expiration HTTP/1.1" 200 191
crowdstrike-ioc-sync    | 2021-08-23 12:09:07,548 : INFO : Deleted 0 expired IOCs

When 192 = "/iocs/entities/indicators/v1?ids={}":

crowdstrike-ioc-sync    | 2021-08-23 12:10:31,354 : DEBUG : https://api.crowdstrike.com:443 "DELETE /iocs/entities/indicators/v1?ids=%7B%7D&filter=source%3A%27%22IOC+Automation%2A%22%27%2B%28expiration%3A%3C%272021-08-09%27%29&comment=Deleted+by+crowdstrike-ioc-sync+14+days+after+expiration HTTP/1.1" 400 228
crowdstrike-ioc-sync   | 2021-08-23 12:10:31,358 : ERROR : CrowdStrike API error: {'status_code': 400, 'headers': {'Content-Encoding': 'gzip', 'Content-Length': '228', 'Content-Type': 'application/json', 'Date': 'Mon, 23 Aug 2021 17:10:31 GMT', 'Strict-Transport-Security': 'max-age=15724800; includeSubDomains', 'X-Cs-Region': 'us-1', 'X-Cs-Traceid': 'xx', 'X-Ratelimit-Limit': '6000', 'X-Ratelimit-Remaining': '5998'}, 'body': {'meta': {'query_time': 0.000358292, 'powered_by': 'ioc-manager', 'trace_id': 'xx'}, 'errors': [{'code': 400, 'message': 'Either IDs or the FQL filter must be provided, not both'}], 'resources': []}}

This is perfect, thank you! I'm looking into this now.

We just received a question regarding this as well, also using the Uber class. I'm going to convert that discussion post to an issue so we have tracking.

@jshcodes jshcodes self-assigned this Aug 23, 2021
@jshcodes jshcodes marked this pull request as draft August 23, 2021 17:22
@jshcodes
Copy link
Member

jshcodes commented Aug 23, 2021

Ultimately the fix you are proposing is what will happen, in every endpoint module that references an ids parameter, but there is some minor work that needs to occur in the Uber class before that can happen.

In the interim, I propose we perform a replacement immediately after the Uber class URL is calculated. (Here: https://github.com/CrowdStrike/falconpy/blob/main/src/falconpy/_util.py#L325) This will work similarly to the replacement occurring in process_service_request (that Service Classes use) but should only impact calls where the endpoint still contains an ids array that is empty (versus one that is populated because our user is using the ids parameter).

The ver_0.6.2 branch contains a working example of this solution. Could I get you to test it for me with your code?

@aneisch
Copy link
Contributor Author

aneisch commented Aug 23, 2021

Your patch appears to have resolved this issue.

Semi-related: I see references to indicator.delete.v1 vs my use of indicator_delete_v1. indicator.delete.v1 is deprecated right?

@jshcodes
Copy link
Member

Your patch appears to have resolved this issue.

Excellent! I'll put together a PR for this now.

Semi-related: I see references to indicator.delete.v1 vs my use of indicator_delete_v1. indicator.delete.v1 is deprecated right?

Correct, but still allowed / maintained for now. This is primarily related to the Service Classes previously using this imported value as the name of the method. Some of the operation IDs as defined in swagger are just completely unacceptable Python syntax (not just to the linters), and had to be changed.

@jshcodes
Copy link
Member

Superseded by #315.

@jshcodes jshcodes closed this Aug 23, 2021
@jshcodes jshcodes added bug 🐛 Something isn't working iocs IOCs (both) issues and questions uber Questions related to Uber Class usage labels Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working iocs IOCs (both) issues and questions uber Questions related to Uber Class usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants