Fix/sg known ip sources #97
Conversation
Updated with Insecure SG known ip sources
Updated with SG known IP sources.
Updated with InsecureSG known ips.
Updated with Insecure SG known CIDR range changes.
Updated with InsecureSG known source ips.
|
|
||
|
|
||
| class RestrictionStatus(Enum): | ||
| Restricted = "restricted" | ||
| OpenCompletely = "open_completely" | ||
| OpenPartly = "open_partly" | ||
| SafeIP = "safe_ips" |
There was a problem hiding this comment.
Maybe just safe to be consistent in naming?
There was a problem hiding this comment.
Updated and tested.
| if known_ip_cidr == source_cidr: | ||
| return True | ||
| elif source_ip.endswith("/32"): | ||
| for ip in known_ip_cidr: |
There was a problem hiding this comment.
It seems ipaddress module allows to check if ip belongs to network. We can do the check if we know source_cidr is /32:
if source_cidr[-1] in known_ip_cidr: return True
to avoid the loop over all addresses in subnet.
There was a problem hiding this comment.
Updated and tested.
| """elif source_cidr.subnet_of(known_ip_cidr): | ||
| return True""" | ||
| return False | ||
|
|
||
| def restriction_status(self, cidr): |
There was a problem hiding this comment.
I'm looking on this code https://github.com/dowjones/hammer/blob/dev/hammer/identification/lambdas/sg-issues-identification/describe_sec_grps_unrestricted_access.py#L57, it checks if the group is restricted and doesn't push it to db if it is. Should we do the same for safe groups? Any reason to save them to DB?
There was a problem hiding this comment.
As per testing it is not storing Safe groups in DDB. For Safe groups it is returning with Restricted status.
Updated with SG source ip review comments.
Updated with Review comments.
Upated with review comments.
Updated with known source ips related code changes.