-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add compatibility for Satellite data format #8
Conversation
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.
@avirkud Looks good, can you verify that this matches all the new changes to Satellitev2?
pipeline/beam_tables.py
Outdated
'false_positive': ('boolean', 'nullable'), | ||
'indicators': ('string', 'nullable'), |
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.
Can we use the same exclude
and exclude_reason
format we use in Satellitev2, just to maintain compatibility?
pipeline/beam_tables.py
Outdated
@@ -122,6 +131,9 @@ | |||
ROWS_PCOLLECION_NAME = 'rows' | |||
|
|||
SATELLITE_TAGS = {'ip', 'http', 'asnum', 'asname', 'cert'} | |||
CDN_REGEX = re.compile("AMAZON|Akamai|OPENDNS|CLOUDFLARENET|GOOGLE") | |||
VERIFY_THRESHOLD = 1 |
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.
This should be at least 2 by default, I've seen that setting the value to 2 or 3 is best for getting a good FP:TP ratio. (maybe we could add that as a comment here).
pipeline/beam_tables.py
Outdated
@@ -800,8 +836,48 @@ def _calculate_confidence(scan: Dict[str, Any]) -> Dict[str, Any]: | |||
confidence['matches'].append(ip_match) | |||
|
|||
confidence['average'] = sum(confidence['matches']) / len(confidence['matches']) | |||
scan['confidence'] = confidence | |||
# Sanity check for untagged responses: do not claim interference | |||
if confidence['untagged_response'] and confidence['average'] > 0: |
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.
This is consistent with the latest additions to Satellite, right? Any answers which are untagged, any answers from control resolvers that are untagged should both not be marked as anomalies.
Yes - the fields, verify threshold, and untagged logic are updated |
Adding CI using github actions
Add compatibility for Satellite data format
Resolves #5