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

Add GCPIAMCorpRuleEvent plugin. #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
98 changes: 72 additions & 26 deletions cloudmarker/clouds/gcpcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def _get_projects(self):
None, self._key_file_path))

yield ('firewall', project_index, project)
yield('iam-policy', project_index, project)

zones = _get_resource_iterator(compute_resource.zones(),
'items', self._key_file_path,
Expand Down Expand Up @@ -186,6 +187,7 @@ def _get_resources(self, record_type, project_index, project, zone=None):
return

project_id = project.get('projectId')
iterator = None
try:
if record_type == 'firewall':
resource = self._build_resource('compute', 'v1')
Expand All @@ -199,6 +201,13 @@ def _get_resources(self, record_type, project_index, project, zone=None):
'items', self._key_file_path,
project=project_id,
zone=zone)

elif record_type == 'iam-policy':
resource = self._build_resource('cloudresourcemanager', 'v1')
itr = resource.projects().getIamPolicy(resource=project_id,
body={}).execute()
iterator = itr['bindings']

else:
_log.warning('Unrecognized record_type: %s; %s', record_type,
util.outline_gcp_project(project_index,
Expand Down Expand Up @@ -233,35 +242,50 @@ def _make_record(self, iterator, gcp_record_type, project_index, project,
record_type_map = {
'compute': 'compute',
}
for i, raw_record in enumerate(iterator):
record = {
'raw': raw_record,
'ext': {
'cloud_type': 'gcp',
'record_type': gcp_record_type,
'project_id': project.get('projectId'),
'project_name': project.get('name'),
'zone': zone,
'key_file_path': self._key_file_path,
'client_email': self._client_email
},
'com': {
'cloud_type': 'gcp',
'record_type': record_type_map.get(gcp_record_type)

try:
for i, raw_record in enumerate(iterator):
record = {
'raw': raw_record,
'ext': {
'cloud_type': 'gcp',
'record_type': gcp_record_type,
'project_id': project.get('projectId'),
'project_name': project.get('name'),
'zone': zone,
'key_file_path': self._key_file_path,
'client_email': self._client_email
},
'com': {
'cloud_type': 'gcp',
'record_type': record_type_map.get(gcp_record_type)
}
}
}

_log.info('Found %s #%d: %s; %s', gcp_record_type, i,
raw_record.get('name'),
util.outline_gcp_project(project_index, project, zone,
self._key_file_path))
yield record
_log.info('Found %s #%d: %s; %s', gcp_record_type, i,
raw_record,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is no need to log the whole raw_record. Just raw_record.get('name') should be enough, as it was before.

Suggested change
raw_record,
raw_record.get('name'),

util.outline_gcp_project(project_index,
project,
zone,
self._key_file_path))
yield record

if gcp_record_type == 'firewall':
yield from _get_normalized_firewall_rules(record,
project_index,
project,
self._key_file_path)
if gcp_record_type == 'firewall':
yield from \
_get_normalized_firewall_rules(record,
project_index,
project,
self._key_file_path)

elif gcp_record_type == 'iam-policy':
yield from _get_iam_policy_rule(record)

except Exception as e:
_log.error('Failed to make record for %s; %s; error: %s: %s',
gcp_record_type,
util.outline_gcp_project(project_index, project, None,
self._key_file_path),
type(e).__name__, e)

def done(self):
"""Log a message that this plugin is done."""
Expand Down Expand Up @@ -299,6 +323,28 @@ def _get_resource_iterator(resource, key, key_file_path, **list_kwargs):
key_file_path, type(e).__name__, e)


def _get_iam_policy_rule(record):
"""Get the iam record to look for Gmail accounts.

Gmail accounts are personally created and controllable accounts.
Organizations seldom have any control over them.

Hence for each Google Cloud Platform project, an account configured in a
project shouldn't be a Gmail account

Arguments:
record (dict): IAM-Policy record generated by this plugin.

Yield:
dict: A normalized IAM corporate login policy rule record with ``com``
bucket populated with iam-corp-policy rule properties in common
notation.

"""
record['com']['record_type'] = 'iam_corp_login_policy_rule'
yield record


def _get_normalized_firewall_rules(firewall_record, project_index,
project, key_file_path):
"""Split a firewall record into multiple firewall rules.
Expand Down
82 changes: 82 additions & 0 deletions cloudmarker/events/gcpiamcorpruleevent.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
"""IAM-Policy rule event.

This module defines the :class:`GCPIAMCorpRuleEvent` class that identifies
weak IAM-Policy related rules. This plugin works on the GCP IAM properties
found in the ``com`` bucket of IAM-Policy rule records.
"""


import logging

from cloudmarker import util

_log = logging.getLogger(__name__)


class GCPIAMCorpRuleEvent:
"""IAM-Policy rule event plugin."""

def __init__(self):
"""Create an instance of :class:`GCPIAMCorpRuleEvent` plugin."""

def eval(self, record):
"""Evaluate IAM rules to check corporate login credentials.

Arguments:
record (dict): A IAM-Corp-Login-Policy rule record.

Yields:
dict: An event record representing a personal Gmail accounts.

"""
# If 'com' bucket is missing, we have a malformed record. Log a
# warning and ignore it.
com = record.get('com')
if com is None:
_log.warning('IAM-Policy rule record is missing com key: %r',
record)
return

# This plugin understands IAM-Policy rule records only, so ignore
# any other record types.
common_record_type = com.get('record_type')
if common_record_type != 'iam_corp_login_policy_rule':
return

members = record['raw']['members']
personal_account = None
for member in members:
if 'user' in member:
user = member.split('user:')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also share how a member record looks like in the raw bucket?

if user[1].endswith('gmail.com'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The user could be using any other mailing account as well, apart from gmail. Should we add checks for that too and make this generalised? I may be wrong here so please correct me.

personal_account = user[1]
reference = com.get('reference')
description = (
'Personal gmail account {} has been used.'
.format(personal_account)
)
recommendation = (
'Ensure that corporate login credentials are used instead of Gmail accounts.'
)

event_record = {
'ext': util.merge_dicts(record.get('ext', {}), {
'record_type': 'iam-policy-corp-login-rule-event'
}),

'com': {
'cloud_type': com.get('cloud_type'),
'record_type': 'iam-policy-corp-login-rule-event',
'reference': reference,
'description': description,
'recommendation': recommendation,
}
}
_log.info('Generating iam_policy_rule; %r', event_record)
yield event_record

def done(self):
"""Perform cleanup work.

Currently, this method does nothing. This may change in future.
"""
1 change: 1 addition & 0 deletions pkg-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ slackclient==1.3.1
uritemplate==3.0.0
urllib3==1.24.3
websocket-client==0.54.0
oauth2client==4.1.3
7 changes: 7 additions & 0 deletions pylama.ini
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ ignore = R0201
[pylama:cloudmarker/clouds/azpostgres.py]
ignore = R0913,R0201

# R0201 Method could be a function [pylint]
# C0301 Line too long (101/100) [pylint]
# E501 line too long (101 > 79 characters) [pycodestyle]

[pylama:cloudmarker/events/gcpiamcorpruleevent.py]
ignore = R0201,C0301,E501

# R0913 Too many arguments (8/5) [pylint]
# R0201 Method could be a function [pylint]

Expand Down
1 change: 1 addition & 0 deletions usr-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ pymongo
PyYAML
schedule
slackclient==1.3.1
oauth2client