From fb3766ef9c39af6caa7ba928529d25ced9446db3 Mon Sep 17 00:00:00 2001 From: Pino de Candia Date: Mon, 26 Feb 2018 21:39:49 +0000 Subject: [PATCH] Fixed processing of role assignment deletions. Change-Id: Ib791702e2a09e7f907e664b1a262544cd9484735 Signed-off-by: Pino de Candia --- tatu/db/models.py | 25 +++++++++++++++++++--- tatu/notifications.py | 50 +++++++++++++++++++++++++++++-------------- tatu/pat.py | 2 +- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/tatu/db/models.py b/tatu/db/models.py index 515c40c..62a7218 100644 --- a/tatu/db/models.py +++ b/tatu/db/models.py @@ -18,11 +18,13 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.declarative import declarative_base import sshpubkeys +from oslo_log import log as logging from tatu.castellano import get_secret, store_secret from tatu.ks_utils import getProjectRoleNamesForUser from tatu.utils import canonical_uuid_string, generateCert, revokedKeysBase64, random_uuid Base = declarative_base() +LOG = logging.getLogger(__name__) class Authority(Base): @@ -167,7 +169,24 @@ def getRevokedKeysBase64(session, auth_id): def revokeUserCert(session, cert): cert.revoked = True session.add(cert) - session.add(db.RevokedKey(cert.auth_id, serial=cert.serial)) + session.add(RevokedKey(auth_id=cert.auth_id, serial=cert.serial)) + session.commit() + + +def revokeUserCertsForRoleChange(session, user_id, proj_id, new_roles): + new_roles = set(new_roles) + for cert in session.query(UserCert).filter(UserCert.user_id == user_id).filter(UserCert.auth_id == proj_id): + if cert.revoked: continue + # A certificate is too permissive if it allows roles that were removed in Keystone. + old_roles = cert.principals.split(",") + removed_roles = set(old_roles) - new_roles + if len(removed_roles) > 0: + LOG.info("Revoking certificate with serial {} for user {}" + " because roles/principals {} were removed." + .format(cert.serial, cert.user_name, removed_roles)) + cert.revoked = True + session.add(cert) + session.add(RevokedKey(auth_id=cert.auth_id, serial=cert.serial)) session.commit() @@ -176,7 +195,7 @@ def revokeUserCerts(session, user_id): for u in session.query(UserCert).filter(UserCert.user_id == user_id): u.revoked = True session.add(u) - session.add(RevokedKey(u.auth_id, serial=u.serial)) + session.add(RevokedKey(auth_id=u.auth_id, serial=u.serial)) session.commit() @@ -185,7 +204,7 @@ def revokeUserCertsInProject(session, user_id, project_id): for u in session.query(UserCert).filter(UserCert.user_id == user_id).filter(UserCert.auth_id == project_id): u.revoked = True session.add(u) - session.add(RevokedKey(u.auth_id, serial=u.serial)) + session.add(RevokedKey(auth_id=u.auth_id, serial=u.serial)) session.commit() diff --git a/tatu/notifications.py b/tatu/notifications.py index 41b6a75..3f38f0f 100644 --- a/tatu/notifications.py +++ b/tatu/notifications.py @@ -69,13 +69,16 @@ class NotificationEndpoint(object): # TODO: look for domain if project isn't available proj_id = payload['project'] for user_id in users: + roles = ks_utils.getProjectRoleNamesForUser(proj_id, user_id) try: - db.revokeUserCertsInProject(se, user_id, proj_id) + se = self.Session() + db.revokeUserCertsForRoleChange(se, user_id, proj_id, roles) except Exception as e: LOG.error( "Failed to revoke user {} certificates in project {} " - "after role assignment change, due to exception {}" - .format(user_id, proj_id, e)) + "after role {} was removed, due to exception {}" + .format(user_id, proj_id, payload['role'], e)) + import traceback; traceback.print_exc() se.rollback() self.Session.remove() elif event_type == 'identity.user.deleted': @@ -92,7 +95,7 @@ class NotificationEndpoint(object): se.rollback() self.Session.remove() elif event_type == 'compute.instance.delete.end': - instance_id = payload.get('instance_id') + instance_id = canonical_uuid_string(payload.get('instance_id')) host = db.getHost(se, instance_id) if host is not None: _deleteHost(self.Session, host) @@ -175,24 +178,39 @@ def sync(engine): LOG.info("Revoke user certificates if user was deleted or lost a role.") for cert in db.getUserCerts(session_factory()): - # Invalidate the cert if the user was removed from Keystone - if cert.user_id not in ks_user_ids: - db.revokeUserCert(cert) - continue + if cert.revoked: continue + se = session_factory() - # Invalidate the cert if it has any principals that aren't current - p = ks_utils.getProjectRoleNamesForUser(cert.auth_id, cert.user_id) - cert_p = cert.principals.split(",") - if len(set(cert_p) - set(p)) > 0: - se = session_factory() - db.revokeUserCert(cert) + try: + # Invalidate the cert if the user was removed from Keystone + if cert.user_id not in ks_user_ids: + db.revokeUserCert(se, cert) + continue + + # Invalidate the cert if it has any principals that aren't current + roles = ks_utils.getProjectRoleNamesForUser(cert.auth_id, + cert.user_id) + old_roles = cert.principals.split(",") + removed_roles = set(old_roles) - set(roles) + if len(removed_roles) > 0: + LOG.info("Revoking certificate with serial {} for user {}" + " because roles/principals {} were removed." + .format(cert.serial, cert.user_name, removed_roles)) + db.revokeUserCert(se, cert) + + except: + LOG.error( + "Failed to delete certificate with serial {} for user {}" + .format(cert.serial, cert.user_id)) + se.rollback() + session_factory.remove() # Iterate through all the instance IDs in Tatu. Clean up DNS and PAT for # any that no longer exist in Nova. LOG.info("Delete DNS and PAT resources of any server that was deleted.") instance_ids = set() for instance in nova.servers.list(search_opts={'all_tenants': True}): - instance_ids.add(instance.id) + instance_ids.add(canonical_uuid_string(instance.id)) for host in db.getHosts(session_factory()): if host.id not in instance_ids: _deleteHost(session_factory, host) @@ -200,7 +218,7 @@ def sync(engine): def main(): transport = oslo_messaging.get_notification_transport(CONF) - targets = [oslo_messaging.Target(topic='notifications')] + targets = [oslo_messaging.Target(topic='tatu_notifications')] storage_engine = create_engine(CONF.tatu.sqlalchemy_engine) endpoints = [NotificationEndpoint(storage_engine)] diff --git a/tatu/pat.py b/tatu/pat.py index 15d5cda..740caef 100644 --- a/tatu/pat.py +++ b/tatu/pat.py @@ -103,7 +103,7 @@ def deletePatEntries(ip_port_tuples): pat_entries = DRAGONFLOW.get_all(PATEntry) tuples = set(ip_port_tuples) for entry in pat_entries: - if (entry.pat.id, entry.pat_l4_port) in tuples: + if (entry.pat.id, str(entry.pat_l4_port)) in tuples: DRAGONFLOW.delete(entry)