*** mhen_ is now known as mhen | 01:33 | |
d34dh0r53 | no reviewathon today :) | 12:52 |
---|---|---|
breton | hello | 14:55 |
breton | i have a question about https://review.opendev.org/c/openstack/keystone/+/851845 | 14:56 |
breton | so... it got merged. It introduced a uniquness constraint in https://review.opendev.org/c/openstack/keystone/+/851845/6/keystone/common/sql/migrations/versions/bobcat/expand/b4f8b3f584e0_fix_incorrect_constraints.py | 14:57 |
breton | how? There was no uniqueness before, users created a bunch of trusts that are duplicate, and now there is a constraint added | 14:58 |
breton | for me the upgrade fails with oslo_db.exception.DBDuplicateEntry | 15:00 |
gtema | breton - unfortunately you would need to resolve those manually. Duplicated entries are a potential security vulnerability, so you definitely don't want them to be present | 15:02 |
breton | why are they a security vulnerability? Was it discussed somewhere? | 15:05 |
gtema | no sure. Potential vulnerability is that you have multiple trusts with the same params and when you delete it you may have an impression there is no trust anymore, but it is still there (as duplicate) | 15:06 |
gtema | remaining_uses is also not being properly counted, as well as redelegation_count | 15:06 |
breton | why? A trust has an id. And that id is required to use it. | 15:12 |
breton | it also means that the users who recorded that id will now lose it. | 15:13 |
gtema | but when you login using trust its ID is not passed. Keystone picks one (eventually a random one) | 15:13 |
breton | no, it is passed. | 15:13 |
gtema | oh, right | 15:14 |
breton | https://docs.openstack.org/api-ref/identity/v3-ext/#consuming-a-trust | 15:14 |
gtema | breton - anyway, the expected constraint was added 10 years ago in https://opendev.org/openstack/keystone/commit/59b09b50ff15df9975832dbfba42e0c984591e48 so it is expected that it is there. Perhaps (and most likely this was the point of the change you referred to) it was not properly applied due to the code bug | 15:21 |
breton | it was never added, and was never expected in OpenStack. https://opendev.org/openstack/glance/src/branch/master/glance/common/trust_auth.py#L47-L52 - Glance doesn't even consider that a trust creation can fail normally | 15:43 |
breton | mistral, same: https://github.com/openstack/mistral/blob/688c4e5743df5b9c2c81ac43fcdbdd817e25cf1d/mistral/services/security.py#L110 | 15:45 |
breton | magnum, same - no logic around existing trust: https://github.com/openstack/magnum/blob/ea302babcbe00f2c3d3503bda133fcc5a58dbe1b/magnum/common/keystone.py#L209 | 15:46 |
breton | zaqar, same: https://github.com/openstack/zaqar/blob/master/zaqar/notification/tasks/trust.py#L60 | 15:47 |
breton | aodh: https://github.com/openstack/aodh/blob/master/aodh/api/controllers/v2/alarms.py#L463 | 15:48 |
breton | i am investigating it further... it seems that there was some activity around this issue in yoga, commit afce7ca8eb | 15:57 |
breton | alright, i still cannot fully describe the issue, but it is not as bad as it seems | 20:21 |
breton | the unique constraint almost never works even today | 20:22 |
breton | the constraint gets triggered if one tries to create a trust with expiration date. Without expiration the trust can be created with the same params as many times as needed. | 20:23 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!