search

redcanaryco / surveyor

A cross-platform baselining, threat hunting, and attack surface analysis tool for security teams.
MIT License
166 stars 62 forks source link

Fix SentinelOne limit error parsing #129

Closed xC0uNt3r7hr34t closed 11 months ago

xC0uNt3r7hr34t commented 12 months ago

removed 2000 limit option as this causes issues with multiple endpoints and is irrelevant with pagination.

resolves #128

TreWilkinsRC commented 12 months ago

Just tested the upper range. This change seems to be far more reliable. Thanks!

xC0uNt3r7hr34t commented 12 months ago

Both PQ and DV have limitations for 1000. DV endpoint allows 20000 limit but the other endpoints hit by surveyor do not. It is recommended to use 1000 for everything. currently your fix still sets things to 20000 when using DV

TreWilkinsRC commented 12 months ago

Okay, that makes sense. I'm assuming the concern here is the time it takes to get results returned. But I feel like that unnecessarily limits the API. If a user has a desired limit, they can set it using the --limit argument during the search. I definitely don't want to cap the use of Surveyor if there's no need, but also don't want to make the program less user-friendly. Any thoughts from this perspective?

Gonna hold of on merging in the meantime.

xC0uNt3r7hr34t commented 12 months ago

the /accounts endpoint is what had a limit of 1000 that threw an error. if we want the ability to do a 20000 limit, it should only be used when hitting the /dv/events endpoint as this is where results are returned. if the limit isn't maximized the results will be returned via pagination. If we are truly looking for speed, we can just set the default for that endpoint to 20000. this pretty much makes the entire function for pagination irrelevant for DV.

Edit: I reread your response and I think I better understand your questioning. If the user doesn't want the full results they can specify a smaller limit. If they want a larger limit (max of 20000) they could specify this. We should either set 20000 or 1000 as the default and then leave it to the user to either specify higher or lower limit. Regardless of that the only endpoint that should be higher than 1000 is the one that fetches the telemetry events as all other will never have that high of a limit or don't support it.

rc-abodkins commented 11 months ago

@xC0uNt3r7hr34t with the merge of #125 - can you test again and see if the issue is fixed?

xC0uNt3r7hr34t commented 11 months ago

Issue is fixed via #125. Thanks