Skip to content

Commit

Permalink
Disallow vague escalated permissions on deployerServiceAccount (#521)
Browse files Browse the repository at this point in the history
* Disallow vague escalated permissions on deployerServiceAccount

* Fix custom rule parsing

* Address PR comments
  • Loading branch information
eshiroma authored Jul 23, 2020
1 parent 6edf571 commit bad130d
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 1 deletion.
Binary file removed marketplace/deployer_util/.coverage
Binary file not shown.
28 changes: 28 additions & 0 deletions marketplace/deployer_util/config_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ def validate(self):
'x-google-marketplace.images')
if self._x_google_marketplace._deployer_service_account:
self._x_google_marketplace._deployer_service_account.validate()
# Move to validate() once enforced on SERVICE_ACCOUNT properties as well.
if (self._x_google_marketplace._deployer_service_account
.has_discouraged_cluster_scoped_permissions()):
raise InvalidSchema(
'Disallowed deployerServiceAccount role(s): '
'For `ClusterRole` roles, only the "view" predefined role is '
'allowed. Instead, use a "CUSTOM" role with specific '
'"apiGroups" and/or "resources".')

for _, p in self._properties.items():
if p.xtype == XTYPE_SERVICE_ACCOUNT:
Expand Down Expand Up @@ -941,6 +949,26 @@ def validate(self):
'https://github.com/GoogleCloudPlatform/marketplace-k8s-app-tools/blob/master/docs/schema.md#type-service_account'
)

def has_discouraged_cluster_scoped_permissions(self):
"""Returns true if the service account has discouraged permissions."""
# Consider all predefined roles except `view`.
if len(
list(
filter(lambda roleName: not roleName == 'view',
self.predefined_cluster_roles()))) > 0:
return True
# Consider apiGroups=['*'] + resources=['*'] + verbs=[<write>],
# which is essentially `cluster-admin`.
for rules in self.custom_cluster_role_rules():
for rule in rules:
write_verbs = set(
['*', 'create', 'update', 'patch', 'delete',
'deletecollection']).intersection(set(rule.get('verbs')))
if '*' in rule.get('apiGroups') and '*' in rule.get(
'resources') and write_verbs:
return True
return False


class SchemaXStorageClass:
"""Accesses STORAGE_CLASS property."""
Expand Down
93 changes: 92 additions & 1 deletion marketplace/deployer_util/config_helper_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ def test_deployer_service_account_missing_description_enforced_validate(self):
images: {}
deployerServiceAccount:
# Optional description would go here
# required description goes here
roles:
- type: Role
rulesType: PREDEFINED
Expand All @@ -689,6 +689,97 @@ def test_deployer_service_account_missing_description_enforced_validate(self):
'must have a `description`'):
schema.validate()

def test_deployer_service_account_cluster_scoped_write_predefined_role_enforced_validate(
self):
schema = config_helper.Schema.load_yaml("""
x-google-marketplace:
schemaVersion: v2
applicationApiVersion: v1beta1
publishedVersion: 6.5.130-metadata
publishedVersionMetadata:
releaseNote: Bug fixes
recommended: true
images: {}
deployerServiceAccount:
description: >
Asks for vague cluster-scoped permissions which is disallowed
roles:
- type: ClusterRole
rulesType: PREDEFINED
rulesFromRoleName: edit
properties:
simple:
type: string
""")
with self.assertRaisesRegex(config_helper.InvalidSchema,
'Disallowed deployerServiceAccount role'):
schema.validate()

def test_deployer_service_account_cluster_scoped_mock_cluster_admin_role_enforced_validate(
self):
schema = config_helper.Schema.load_yaml("""
x-google-marketplace:
schemaVersion: v2
applicationApiVersion: v1beta1
publishedVersion: 6.5.130-metadata
publishedVersionMetadata:
releaseNote: Bug fixes
recommended: true
images: {}
deployerServiceAccount:
description: >
Asks for vague cluster-scoped permissions which is disallowed
roles:
- type: ClusterRole
rulesType: CUSTOM
rules:
- apiGroups: ['*']
resources: ['*']
verbs: ['*']
properties:
simple:
type: string
""")
with self.assertRaisesRegex(config_helper.InvalidSchema,
'Disallowed deployerServiceAccount role'):
schema.validate()

def test_deployer_service_account_no_escalated_permissions_allowed_validate(
self):
schema = config_helper.Schema.load_yaml("""
x-google-marketplace:
schemaVersion: v2
applicationApiVersion: v1beta1
publishedVersion: 6.5.130-metadata
publishedVersionMetadata:
releaseNote: Bug fixes
recommended: true
images: {}
deployerServiceAccount:
description: >
Asks for vague namespaced permissions which is allowed
roles:
- type: Role
rulesType: PREDEFINED
rulesFromRoleName: edit
properties:
simple:
type: string
""")
schema.validate()

def test_service_account_missing_rulesType(self):
with self.assertRaisesRegex(
config_helper.InvalidSchema,
Expand Down

0 comments on commit bad130d

Please sign in to comment.