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.

Sign up for GitHub

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

Jump to bottom

Fix/sg known ip sources #97

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Fix/sg known ip sources #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b27b41a
Updated with Insecure SG known ip sources
vigneswararaomacharla Aug 7, 2019
e970bb1
Updated with SG known IP sources.
vigneswararaomacharla Aug 7, 2019
1377437
Updated with InsecureSG known ips.
vigneswararaomacharla Aug 7, 2019
cfad136
Updated with Insecure SG known CIDR range changes.
vigneswararaomacharla Aug 8, 2019
585e52a
Updated with InsecureSG known source ips.
vigneswararaomacharla Aug 8, 2019
b828715
Updated with SG source ip review comments.
vigneswararaomacharla Aug 19, 2019
9fc5fbf
Updated with Review comments.
vigneswararaomacharla Aug 19, 2019
35dc6fa
Upated with review comments.
vigneswararaomacharla Aug 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion deployment/configs/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@
"reporting": false,
"remediation": false,
"remediation_accounts": ["210987654321", "654321210987"],
"remediation_retention_period": 21
"remediation_retention_period": 21,
"known_ip_sources": ["205.203.150.72", "205.203.130.22"]
},
"user_inactivekeys": {
"enabled": true,
Expand Down
37 changes: 34 additions & 3 deletions hammer/library/aws/security_groups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
from library.utility import jsonDumps
from library.aws.s3 import S3Operations
from library.aws.utility import convert_tags
from library.config import Config


class RestrictionStatus(Enum):
Restricted = "restricted"
OpenCompletely = "open_completely"
OpenPartly = "open_partly"
Safe = "safe"


class SecurityGroupOperations:
Expand Down Expand Up @@ -372,6 +374,28 @@ def __str__(self):
perms = ", ".join([str(perm) for perm in self.permissions])
return f"{self.__class__.__name__}(Name={self.name}, Id={self.id}, Permissions=[{perms}])"

def validate_known_ip_soource(self, source_ip):
"""

:param source_ip: ip address
:return: boolean
"""
config = Config()
known_ip_sources = config.sg.known_ip_sources
source_cidr = ipaddress.ip_network(source_ip)

for known_ip in known_ip_sources:
known_ip_cidr = ipaddress.ip_network(known_ip)
if known_ip_cidr == source_cidr:
return True
elif source_ip.endswith("/32"):
if source_cidr[-1] in known_ip_cidr:
return True
# ipaddress.subnet_of() function new to Python 3.7. Not available in 3.6
"""elif source_cidr.subnet_of(known_ip_cidr):
return True"""
return False

def restriction_status(self, cidr):
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per testing it is not storing Safe groups in DDB. For Safe groups it is returning with Restricted status.

"""
Check restriction status of cidr
Expand All @@ -380,11 +404,17 @@ def restriction_status(self, cidr):

:return: RestrictionStatus with check result
"""
is_known_ip = self.validate_known_ip_soource(cidr)

status = RestrictionStatus.Restricted
if cidr.endswith("/0"):
status = RestrictionStatus.OpenCompletely
elif ipaddress.ip_network(cidr).is_global:
status = RestrictionStatus.OpenPartly

if is_known_ip:
status = RestrictionStatus.Safe

logging.debug(f"Checked '{cidr}' - '{status.value}'")
return status

Expand All @@ -406,10 +436,11 @@ def check(self, restricted_ports):
logging.debug(f"Checking '{perm.protocol}' '{perm.from_port}-{perm.to_port}' ports for {ip_range}")
# first condition - CIDR is Global/Public
status = self.restriction_status(ip_range.cidr)
if status == RestrictionStatus.Restricted:
logging.debug(f"Skipping restricted '{ip_range}'")
if status in (RestrictionStatus.Restricted, RestrictionStatus.Safe):
logging.debug(f"Skipping restricted/safe IP address '{ip_range}'")
continue
# second - check if ports from `restricted_ports` list has intersection with ports from FromPort..ToPort range
# second - check if ports from `restricted_ports` list has intersection with ports from FromPort..
# ToPort range
if perm.from_port is None or perm.to_port is None:
logging.debug(f"Marking world-wide open all ports from '{ip_range}'")
ip_range.status = status
Expand Down