From 5de1770931e886732870da1909f08279a0b804b4 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 2 Nov 2016 13:21:07 +0100 Subject: [PATCH] Create service credentials in SERVICE_DOMAIN Cleanup code that references users, projects or domains without necessary scoping or filtering throughout the charm. Add logging of domain name in contexts where this is relevant. Tighten rule:service_role to require role:service and token scoped to project config('service-tenant') created in SERVICE_DOMAIN. This ensures that if you have a deployment with end-user access to assign roles within their own domains they will not gain privileged access simply by assigning the service role to one of their own users. Allow users authorized by rule:service_role to perform identity:list_projects. This is required to allow Ceilometer to operate without Admin privileges. Services are given a user in project config('service-tenant') in SERVICE_DOMAIN for v3 authentication / authorization. As of Mitaka Keystone v3 policy the 'service' role is sufficient for services to validate tokens. Services are also given a user in project config('service-tenant') in DEFAULT_DOMAIN to support services still configured with v2.0 authentication / authorization. This will allow us to transition from v2.0 based authentication / authorization and existing services and charms will continue to operate as before. This will also allow the end-user to roll their deployment up to api_version 3 and back to api_version 2 as needed. Services and charms that has made the transition to fully use the v3 API for authentication and authorization will gain full access to domains and projects across the deployment. The first charm to make use of this is charm-ceilometer. Closes-Bug: 1636098 Change-Id: If1518029c43476a5e14bf94596197eabe663499c --- hooks/keystone_utils.py | 167 ++++++++++++++++++++---------- hooks/manager.py | 21 ++-- unit_tests/test_keystone_utils.py | 76 +++++++++----- 3 files changed, 175 insertions(+), 89 deletions(-) diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index c10d99fd..433a797d 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -92,6 +92,7 @@ from charmhelpers.core.hookenv import ( charm_dir, config, is_relation_made, + leader_set, log, local_unit, relation_get, @@ -204,6 +205,7 @@ SSL_SYNC_SEMAPHORE = threading.Semaphore() SSL_DIRS = [SSL_DIR, APACHE_SSL_DIR, CA_CERT_PATH] ADMIN_DOMAIN = 'admin_domain' DEFAULT_DOMAIN = 'default' +SERVICE_DOMAIN = 'service_domain' POLICY_JSON = '/etc/keystone/policy.json' TOKEN_FLUSH_CRON_FILE = '/etc/cron.d/keystone-token-flush' WSGI_KEYSTONE_CONF = '/etc/apache2/sites-enabled/wsgi-keystone.conf' @@ -737,14 +739,16 @@ def create_endpoint_template_v3(manager, region, service, publicurl, adminurl, ) -def create_tenant(name): +def create_tenant(name, domain): """Creates a tenant if it does not already exist""" manager = get_manager() - tenant = manager.resolve_tenant_id(name) + tenant = manager.resolve_tenant_id(name, domain=domain) if not tenant: manager.create_tenant(tenant_name=name, + domain=domain, description='Created by Juju') - log("Created new tenant: %s" % name, level=DEBUG) + log("Created new tenant '%s' in domain '%s'" % (name, domain), + level=DEBUG) return log("Tenant '%s' already exists." % name, level=DEBUG) @@ -789,14 +793,16 @@ def create_user(name, password, tenant=None, domain=None): """Creates a user if it doesn't already exist, as a member of tenant""" manager = get_manager() if user_exists(name, domain=domain): - log("A user named '%s' already exists" % name, level=DEBUG) + log("A user named '%s' already exists in domain '%s'" % (name, domain), + level=DEBUG) return tenant_id = None if tenant: - tenant_id = manager.resolve_tenant_id(tenant) + tenant_id = manager.resolve_tenant_id(tenant, domain=domain) if not tenant_id: - error_out('Could not resolve tenant_id for tenant %s' % tenant) + error_out("Could not resolve tenant_id for tenant '%s' in domain " + "'%s'" % (tenant, domain)) domain_id = None if domain: @@ -810,8 +816,8 @@ def create_user(name, password, tenant=None, domain=None): email='juju@localhost', tenant_id=tenant_id, domain_id=domain_id) - log("Created new user '%s' tenant: %s" % (name, tenant_id), - level=DEBUG) + log("Created new user '%s' tenant: '%s' domain: '%s'" % (name, tenant_id, + domain_id), level=DEBUG) def get_manager(api_version=None): @@ -834,33 +840,41 @@ def create_role(name, user=None, tenant=None, domain=None): return # NOTE(adam_g): Keystone client requires id's for add_user_role, not names - user_id = manager.resolve_user_id(user) + user_id = manager.resolve_user_id(user, user_domain=domain) role_id = manager.resolve_role_id(name) if None in [user_id, role_id]: - error_out("Could not resolve [%s, %s]" % - (user_id, role_id)) + error_out("Could not resolve [%s, %s] user_domain='%s'" % + (user_id, role_id, domain)) - grant_role(user, name, tenant, domain) + # default to grant role to project + grant_role(user, name, tenant=tenant, user_domain=domain, + project_domain=domain) -def grant_role(user, role, tenant=None, domain=None, user_domain=None): +def grant_role(user, role, tenant=None, domain=None, user_domain=None, + project_domain=None): """Grant user and tenant a specific role""" manager = get_manager() - log("Granting user '%s' role '%s' on tenant '%s'" % - (user, role, tenant)) + if domain: + log("Granting user '%s' role '%s' in domain '%s'" % + (user, role, domain)) + else: + log("Granting user '%s' role '%s' on tenant '%s' in domain '%s'" % + (user, role, tenant, project_domain)) user_id = manager.resolve_user_id(user, user_domain=user_domain) role_id = manager.resolve_role_id(role) if None in [user_id, role_id]: - error_out("Could not resolve [%s, %s]" % - (user_id, role_id)) + error_out("Could not resolve [%s, %s] user_domain='%s'" % + (user_id, role_id, user_domain)) tenant_id = None if tenant: - tenant_id = manager.resolve_tenant_id(tenant) + tenant_id = manager.resolve_tenant_id(tenant, domain=project_domain) if not tenant_id: - error_out('Could not resolve tenant_id for tenant %s' % tenant) + error_out("Could not resolve tenant_id for tenant '%s' in domain " + "'%s'" % (tenant, domain)) domain_id = None if domain: @@ -875,11 +889,19 @@ def grant_role(user, role, tenant=None, domain=None, user_domain=None): role=role_id, tenant=tenant_id, domain=domain_id) - log("Granted user '%s' role '%s' on tenant '%s'" % - (user, role, tenant), level=DEBUG) + if domain_id is None: + log("Granted user '%s' role '%s' on tenant '%s' in domain '%s'" % + (user, role, tenant, project_domain), level=DEBUG) + else: + log("Granted user '%s' role '%s' in domain '%s'" % + (user, role, domain), level=DEBUG) else: - log("User '%s' already has role '%s' on tenant '%s'" % - (user, role, tenant), level=DEBUG) + if domain_id is None: + log("User '%s' already has role '%s' on tenant '%s' in domain '%s'" + % (user, role, tenant, project_domain), level=DEBUG) + else: + log("User '%s' already has role '%s' in domain '%s'" + % (user, role, domain), level=DEBUG) def store_data(backing_file, data): @@ -962,12 +984,20 @@ def ensure_initial_admin(config): changes? """ if get_api_version() > 2: + manager = get_manager() default_domain_id = create_or_show_domain(DEFAULT_DOMAIN) store_default_domain_id(default_domain_id) admin_domain_id = create_or_show_domain(ADMIN_DOMAIN) store_admin_domain_id(admin_domain_id) - create_tenant("admin") - create_tenant(config("service-tenant")) + create_or_show_domain(SERVICE_DOMAIN) + create_tenant("admin", ADMIN_DOMAIN) + create_tenant(config("service-tenant"), SERVICE_DOMAIN) + leader_set({'service_tenant_id': manager.resolve_tenant_id( + config("service-tenant"), + domain=SERVICE_DOMAIN)}) + create_role('service') + create_tenant("admin", DEFAULT_DOMAIN) + create_tenant(config("service-tenant"), DEFAULT_DOMAIN) # User is managed by ldap backend when using ldap identity if not (config('identity-backend') == 'ldap' and config('ldap-readonly')): @@ -976,10 +1006,20 @@ def ensure_initial_admin(config): if get_api_version() > 2: create_user_credentials(config('admin-user'), passwd, domain=ADMIN_DOMAIN) - create_role(config('admin-role'), config('admin-user'), - domain=ADMIN_DOMAIN) + create_role('Member') + # Grant 'Member' role to user ADMIN_DOMAIN/admin-user in + # project ADMIN_DOMAIN/'admin' + # ADMIN_DOMAIN + grant_role(config('admin-user'), 'Member', tenant='admin', + user_domain=ADMIN_DOMAIN, + project_domain=ADMIN_DOMAIN) + create_role(config('admin-role')) + # Grant admin-role to user ADMIN_DOMAIN/admin-user in + # project ADMIN_DOMAIN/admin grant_role(config('admin-user'), config('admin-role'), - tenant='admin', user_domain=ADMIN_DOMAIN) + tenant='admin', user_domain=ADMIN_DOMAIN, + project_domain=ADMIN_DOMAIN) + # Grant domain level admin-role to ADMIN_DOMAIN/admin-user grant_role(config('admin-user'), config('admin-role'), domain=ADMIN_DOMAIN, user_domain=ADMIN_DOMAIN) else: @@ -1623,11 +1663,13 @@ def create_user_credentials(user, passwd, tenant=None, new_roles=None, level=DEBUG) update_user_password(user, passwd) else: - create_user(user, passwd, tenant, domain) + create_user(user, passwd, tenant=tenant, domain=domain) if grants: for role in grants: - grant_role(user, role, tenant, domain) + # grant role on project + grant_role(user, role, tenant=tenant, user_domain=domain, + project_domain=domain) else: log("No role grants requested for user '%s'" % (user), level=DEBUG) @@ -1636,7 +1678,7 @@ def create_user_credentials(user, passwd, tenant=None, new_roles=None, # Currently used by Swift and Ceilometer. for role in new_roles: log("Creating requested role '%s'" % role, level=DEBUG) - create_role(role, user, tenant, domain) + create_role(role, user=user, tenant=tenant, domain=domain) return passwd @@ -1644,21 +1686,36 @@ def create_user_credentials(user, passwd, tenant=None, new_roles=None, def create_service_credentials(user, new_roles=None): """Create credentials for service with given username. - Services are given a user under config('service-tenant') and are given the - config('admin-role') role. Tenant is assumed to already exist, + For Keystone v2.0 API compability services are given a user under + config('service-tenant') in DEFAULT_DOMAIN and are given the + config('admin-role') role. Tenant is assumed to already exist. + + For Keysteone v3 API compability services are given a user in project + config('service-tenant') in SERVICE_DOMAIN and are given the 'service' + role. + + As of Mitaka Keystone v3 policy the 'service' role is sufficient for + services to validate tokens. Project is assumed to already exist. """ tenant = config('service-tenant') if not tenant: raise Exception("No service tenant provided in config") - if get_api_version() == 2: - domain = None - else: + domain = None + if get_api_version() > 2: domain = DEFAULT_DOMAIN - return create_user_credentials(user, get_service_password(user), - tenant=tenant, new_roles=new_roles, - grants=[config('admin-role')], - domain=domain) + passwd = create_user_credentials(user, get_service_password(user), + tenant=tenant, new_roles=new_roles, + grants=[config('admin-role')], + domain=domain) + if get_api_version() > 2: + # v3 policy allows services to validate tokens when granted the + # 'service' role. + domain = SERVICE_DOMAIN + passwd = create_user_credentials(user, passwd, + tenant=tenant, new_roles=new_roles, + grants=['service'], domain=domain) + return passwd def add_service_to_keystone(relation_id=None, remote_unit=None): @@ -1783,16 +1840,12 @@ def add_service_to_keystone(relation_id=None, remote_unit=None): roles = get_requested_roles(settings) service_password = create_service_credentials(service_username, new_roles=roles) - - # As of https://review.openstack.org/#change,4675, all nodes hosting - # an endpoint(s) needs a service username and password assigned to - # the service tenant and granted admin role. - # note: config('service-tenant') is created in utils.ensure_initial_admin() - # we return a token, information about our API endpoints, and the generated - # service credentials + service_domain = None + if get_api_version() > 2: + service_domain = SERVICE_DOMAIN service_tenant = config('service-tenant') - domain_name = 'Default' if manager.api_version == 3 else None - grant_role(service_username, 'Admin', service_tenant, domain_name) + service_tenant_id = manager.resolve_tenant_id(service_tenant, + domain=service_domain) # NOTE(dosaboy): we use __null__ to represent settings that are to be # routed to relations via the cluster relation and set to None. @@ -1804,8 +1857,9 @@ def add_service_to_keystone(relation_id=None, remote_unit=None): "auth_port": config("admin-port"), "service_username": service_username, "service_password": service_password, + "service_domain": service_domain, "service_tenant": service_tenant, - "service_tenant_id": manager.resolve_tenant_id(service_tenant), + "service_tenant_id": service_tenant_id, "https_keystone": '__null__', "ssl_cert": '__null__', "ssl_key": '__null__', @@ -1860,16 +1914,17 @@ def add_credentials_to_keystone(relation_id=None, remote_unit=None): if get_api_version() == 2: domain = None + grants = [config('admin-role')] else: - domain = settings.get('domain') or DEFAULT_DOMAIN + domain = settings.get('domain') or SERVICE_DOMAIN + grants = ['service'] # Use passed project or the service project credentials_project = settings.get('project') or config('service-tenant') - create_tenant(credentials_project) + create_tenant(credentials_project, domain) - # Use passed grants or default to granting the Admin role - credentials_grants = (get_requested_grants(settings) or - [config('admin-role')]) + # Use passed grants or default grants + credentials_grants = (get_requested_grants(settings) or grants) # Create the user credentials_password = create_user_credentials( @@ -1891,7 +1946,7 @@ def add_credentials_to_keystone(relation_id=None, remote_unit=None): "credentials_password": credentials_password, "credentials_project": credentials_project, "credentials_project_id": - manager.resolve_tenant_id(credentials_project), + manager.resolve_tenant_id(credentials_project, domain=domain), "auth_protocol": protocol, "credentials_protocol": protocol, "api_version": get_api_version(), diff --git a/hooks/manager.py b/hooks/manager.py index 73382bb6..7b3a7559 100644 --- a/hooks/manager.py +++ b/hooks/manager.py @@ -91,13 +91,6 @@ def get_keystone_manager(endpoint, token, api_version=None): class KeystoneManager(object): - def resolve_tenant_id(self, name): - """Find the tenant_id of a given tenant""" - tenants = [t._info for t in self.api.tenants.list()] - for t in tenants: - if name.lower() == t['name'].lower(): - return t['id'] - def resolve_domain_id(self, name): pass @@ -150,6 +143,13 @@ class KeystoneManager2(KeystoneManager): def tenants_list(self): return self.api.tenants.list() + def resolve_tenant_id(self, name, domain=None): + """Find the tenant_id of a given tenant""" + tenants = [t._info for t in self.api.tenants.list()] + for t in tenants: + if name.lower() == t['name'].lower(): + return t['id'] + def create_tenant(self, tenant_name, description, domain='default'): self.api.tenants.create(tenant_name=tenant_name, description=description) @@ -182,11 +182,14 @@ class KeystoneManager3(KeystoneManager): keystone_session_v3 = session.Session(auth=keystone_auth_v3) self.api = keystoneclient_v3.Client(session=keystone_session_v3) - def resolve_tenant_id(self, name): + def resolve_tenant_id(self, name, domain=None): """Find the tenant_id of a given tenant""" + if domain: + domain_id = self.resolve_domain_id(domain) tenants = [t._info for t in self.api.projects.list()] for t in tenants: - if name.lower() == t['name'].lower(): + if name.lower() == t['name'].lower() and \ + (domain is None or t['domain_id'] == domain_id): return t['id'] def resolve_domain_id(self, name): diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index 8fba754d..29b2cdb7 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -261,15 +261,15 @@ class TestKeystoneUtils(CharmTestCase): def test_add_service_to_keystone_no_clustered_no_https_complete_values( self, KeystoneManager, add_endpoint, ensure_valid_service, _resolve_address, create_user, get_admin_domain_id, - get_api_version): + get_api_version, test_api_version=2): get_admin_domain_id.return_value = None - get_api_version.return_value = 2 + get_api_version.return_value = test_api_version relation_id = 'identity-service:0' remote_unit = 'unit/0' self.get_admin_token.return_value = 'token' self.get_service_password.return_value = 'password' self.test_config.set('service-tenant', 'tenant') - self.test_config.set('admin-role', 'admin') + self.test_config.set('admin-role', 'Admin') self.get_requested_roles.return_value = ['role1', ] _resolve_address.return_value = '10.0.0.3' self.test_config.set('admin-port', 80) @@ -279,6 +279,12 @@ class TestKeystoneUtils(CharmTestCase): self.get_local_endpoint.return_value = 'http://localhost:80/v2.0/' self.relation_ids.return_value = ['cluster/0'] + service_domain = None + service_role = 'Admin' + if test_api_version > 2: + service_domain = 'service_domain' + service_role = 'service' + mock_keystone = MagicMock() mock_keystone.resolve_tenant_id.return_value = 'tenant_id' KeystoneManager.return_value = mock_keystone @@ -299,23 +305,31 @@ class TestKeystoneUtils(CharmTestCase): internalurl='192.168.1.2') self.assertTrue(self.get_admin_token.called) self.get_service_password.assert_called_with('keystone') - create_user.assert_called_with('keystone', 'password', 'tenant', None) - self.grant_role.assert_called_with('keystone', 'Admin', 'tenant', - None) - self.create_role.assert_called_with('role1', 'keystone', 'tenant', - None) + create_user.assert_called_with('keystone', 'password', + domain=service_domain, + tenant='tenant') + self.grant_role.assert_called_with('keystone', service_role, + project_domain=service_domain, + tenant='tenant', + user_domain=service_domain) + self.create_role.assert_called_with('role1', user='keystone', + tenant='tenant', + domain=service_domain) - relation_data = {'admin_domain_id': None, 'auth_host': '10.0.0.3', + relation_data = {'admin_domain_id': None, + 'auth_host': '10.0.0.3', 'service_host': '10.0.0.3', 'admin_token': 'token', 'service_port': 81, 'auth_port': 80, 'service_username': 'keystone', 'service_password': 'password', + 'service_domain': service_domain, 'service_tenant': 'tenant', 'https_keystone': '__null__', 'ssl_cert': '__null__', 'ssl_key': '__null__', 'ca_cert': '__null__', 'auth_protocol': 'http', 'service_protocol': 'http', - 'service_tenant_id': 'tenant_id', 'api_version': 2} + 'service_tenant_id': 'tenant_id', + 'api_version': test_api_version} filtered = {} for k, v in relation_data.iteritems(): @@ -330,6 +344,12 @@ class TestKeystoneUtils(CharmTestCase): self.relation_set.assert_called_with(relation_id=relation_id, **filtered) + def test_add_service_to_keystone_no_clustered_no_https_complete_values_v3( + self): + return self.\ + test_add_service_to_keystone_no_clustered_no_https_complete_values( + test_api_version=3) + @patch('charmhelpers.contrib.openstack.ip.config') @patch.object(utils, 'ensure_valid_service') @patch.object(utils, 'add_endpoint') @@ -367,8 +387,9 @@ class TestKeystoneUtils(CharmTestCase): mock_user_exists): mock_user_exists.return_value = False utils.create_user_credentials('userA', 'passA', tenant='tenantA') - mock_create_user.assert_has_calls([call('userA', 'passA', 'tenantA', - None)]) + mock_create_user.assert_has_calls([call('userA', 'passA', + domain=None, + tenant='tenantA')]) mock_create_role.assert_has_calls([]) mock_grant_role.assert_has_calls([]) @@ -381,12 +402,16 @@ class TestKeystoneUtils(CharmTestCase): mock_user_exists.return_value = False utils.create_user_credentials('userA', 'passA', tenant='tenantA', grants=['roleA'], new_roles=['roleB']) - mock_create_user.assert_has_calls([call('userA', 'passA', 'tenantA', - None)]) - mock_create_role.assert_has_calls([call('roleB', 'userA', 'tenantA', - None)]) - mock_grant_role.assert_has_calls([call('userA', 'roleA', 'tenantA', - None)]) + mock_create_user.assert_has_calls([call('userA', 'passA', + tenant='tenantA', + domain=None)]) + mock_create_role.assert_has_calls([call('roleB', user='userA', + tenant='tenantA', + domain=None)]) + mock_grant_role.assert_has_calls([call('userA', 'roleA', + tenant='tenantA', + user_domain=None, + project_domain=None)]) @patch.object(utils, 'update_user_password') @patch.object(utils, 'user_exists') @@ -402,10 +427,13 @@ class TestKeystoneUtils(CharmTestCase): utils.create_user_credentials('userA', 'passA', tenant='tenantA', grants=['roleA'], new_roles=['roleB']) mock_create_user.assert_has_calls([]) - mock_create_role.assert_has_calls([call('roleB', 'userA', 'tenantA', - None)]) - mock_grant_role.assert_has_calls([call('userA', 'roleA', 'tenantA', - None)]) + mock_create_role.assert_has_calls([call('roleB', user='userA', + tenant='tenantA', + domain=None)]) + mock_grant_role.assert_has_calls([call('userA', 'roleA', + tenant='tenantA', + user_domain=None, + project_domain=None)]) mock_update_user_password.assert_has_calls([call('userA', 'passA')]) @patch.object(utils, 'get_manager') @@ -1079,7 +1107,7 @@ class TestKeystoneUtils(CharmTestCase): create_user_credentials.assert_called_with('requester', 'password', domain='Non-Default', new_roles=[], - grants=['Admin'], + grants=['service'], tenant='services') self.peer_store_and_set.assert_called_with(relation_id=relation_id, **relation_data) @@ -1130,7 +1158,7 @@ class TestKeystoneUtils(CharmTestCase): utils.add_credentials_to_keystone( relation_id=relation_id, remote_unit=remote_unit) - create_tenant.assert_called_with('myproject') + create_tenant.assert_called_with('myproject', None) create_user_credentials.assert_called_with('requester', 'password', domain=None, new_roles=['New', 'Member'],