Skip to content

Commit

Permalink
loosen rbac for get cert - avoid checking every DNS in SAN (#444)
Browse files Browse the repository at this point in the history
Previous rbac for get certificate:
* CN, and all DNS in SAN needs to match a single regex, and the outcome
of 'service_name' needs to be matched to the kms auth identity.

Problem: this limits a lot of the freedom of how DNS in SAN can be
defined, especially when 'service_name' does not need to be existent in
SAN DNS, even just missing in one of the entries.

Change: loosen the rbac of get cert to just match against CN. For DNS
list in SAN, we will skip checking against kms auth identity.
  • Loading branch information
meng-han authored Nov 7, 2024
1 parent bb8cdf5 commit 1b4b1c4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 30 deletions.
22 changes: 9 additions & 13 deletions confidant/authnz/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,15 @@ def default_acl(*args, **kwargs):
if not ca_object.settings['name_regex']:
return False
cert_pattern = re.compile(ca_object.settings['name_regex'])
domains = [resource_id]
domains.extend(resource_kwargs.get('san', []))
# Ensure the CN and every value in the SAN is allowed for this
# user.
for domain in domains:
match = cert_pattern.match(domain)
if not match:
return False
service_name = match.group('service_name')
if not service_name:
return False
if not authnz.user_is_service(service_name):
return False
domain = resource_id
match = cert_pattern.match(domain)
if not match:
return False
service_name = match.group('service_name')
if not service_name:
return False
if not authnz.user_is_service(service_name):
return False
return True
return False
else:
Expand Down
20 changes: 3 additions & 17 deletions tests/unit/confidant/authnz/rbac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,30 +85,16 @@ def test_default_acl(mocker: MockerFixture):
kwargs={'ca': 'development'},
) is False
# Test for user type is service, with certificate resource and get
# action, with a valid CN, but an invalid SAN
assert rbac.default_acl(
resource_type='certificate',
action='get',
resource_id='test-service.example.com',
kwargs={
'ca': 'development',
'san': ['bad-service.example.com'],
},
) is False
# Test for user type is service, with certificate resource and get
# action, with a valid CN, but a mix of valid and invalid SAN values
# action, with a valid CN
assert rbac.default_acl(
resource_type='certificate',
action='get',
resource_id='test-service.example.com',
kwargs={
'ca': 'development',
'san': [
'bad-service.example.com',
'test-service.example.com',
],
'san': ['test-service.sub.example.com'],
},
) is False
) is True
# Test for user type is service, and an allowed resource, with
# disallowed fake action
assert rbac.default_acl(resource_type='service', action='fake') is False
Expand Down

0 comments on commit 1b4b1c4

Please sign in to comment.