Add service user password rotation feature

This patch adds the service user rotation feature, which provides two
actions:

 - list-service-usernames
 - rotate-service-user-password

The first lists the possible usernames that can be rotated.  The
second action rotates the service, and is tested via the func-test-pr.

Change-Id: Ia94ab3d54cd8a59e9ba5005b88d3ec1ff87019b1
func-test-pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/1029
This commit is contained in:
Alex Kavanagh 2023-03-23 15:45:47 +00:00
parent 09c507dd21
commit 42714adfde
10 changed files with 506 additions and 3 deletions

View File

@ -79,3 +79,20 @@ run-deferred-hooks:
show-deferred-events:
descrpition: |
Show the outstanding restarts
rotate-service-user-password:
description: |
Rotate the specified rabbitmq-server user's password. The current password
is replaced with a randomly generated password. The password is changed on
the relation to the user's units. This may result in a control plane outage
for the duration of the password changing process.
params:
service-user:
type: string
description: |
The username of the rabbitmq-server service as specified in
list-service-usernames.
list-service-usernames:
description: |
List the usernames of the passwords that have been provided on the
amqp relations. The service username passed to
'rotate-service-user-password' needs to be on this list.

View File

@ -19,6 +19,7 @@ import re
from collections import OrderedDict
from subprocess import check_output, CalledProcessError, PIPE
import sys
import traceback
_path = os.path.dirname(os.path.realpath(__file__))
@ -71,6 +72,9 @@ from hooks.rabbit_utils import (
list_vhosts,
vhost_queue_info,
rabbitmq_version_newer_or_equal,
get_usernames_for_passwords,
NotLeaderError,
InvalidServiceUserError,
)
@ -299,6 +303,39 @@ def show_deferred_events(args):
os_utils.show_deferred_events_action_helper()
def list_service_usernames(args):
"""List the service usernames known in this model that can be rotated."""
usernames = get_usernames_for_passwords()
action_set({'usernames': usernames or []})
def rotate_service_user_password(args):
"""Rotate the service user's password.
The parameter must be passed in the service-user parameter.
"""
service_user = action_get("service-user")
if service_user is None:
action_fail(
"The 'service-user' parameter was not passed and is required.")
return
try:
rabbitmq_server_relations.rotate_service_user_password(service_user)
except NotLeaderError:
action_fail(
"This unit either isn't the leader or is not ready to do "
"leader actions. The rotate-service-user-password action. "
"can't be run at the moment. Please verify that the unit is the "
"leader and that the cluster is ready.")
except InvalidServiceUserError:
action_fail(
"Service username {} is not valid for password rotation. Please "
"check the action 'list-service-users' for the correct username."
.format(service_user))
except Exception:
raise
# A dictionary of all the defined actions to callables (which take
# parsed arguments).
ACTIONS = {
@ -313,6 +350,8 @@ ACTIONS = {
"restart-services": restart,
"run-deferred-hooks": run_deferred_hooks,
"show-deferred-events": show_deferred_events,
"list-service-usernames": list_service_usernames,
"rotate-service-user-password": rotate_service_user_password,
}
@ -332,7 +371,10 @@ def main(args):
try:
action(args)
except Exception as e:
log("Action {} failed: {}\nTrackback:\n{}"
.format(action_name, str(e), traceback.format_exc()), ERROR)
action_fail("Action {} failed: {}".format(action_name, str(e)))
_run_atexit()

View File

@ -0,0 +1 @@
actions.py

View File

@ -0,0 +1 @@
actions.py

View File

@ -94,7 +94,7 @@ from charmhelpers.core.host import (
from charmhelpers.contrib.peerstorage import (
peer_store,
peer_retrieve
peer_retrieve,
)
from charmhelpers.fetch import (
@ -136,6 +136,7 @@ COORD_KEYS = [COORD_KEY_RESTART, COORD_KEY_PKG_UPGRADE, COORD_KEY_CLUSTER]
_named_passwd = '/var/lib/charm/{}/{}.passwd'
_local_named_passwd = '/var/lib/charm/{}/{}.local_passwd'
_service_password_glob = '/var/lib/charm/{}/*.passwd'
# hook_contexts are used as a convenient mechanism to render templates
@ -179,6 +180,16 @@ def CONFIG_FILES():
return _cfiles
class NotLeaderError(Exception):
"""Exception raised if not the leader."""
pass
class InvalidServiceUserError(Exception):
"""Exception raised if an invalid Service User is detected."""
pass
class ConfigRenderer(object):
"""
This class is a generic configuration renderer for
@ -590,6 +601,24 @@ def create_user(user, password, tags=[]):
apply_tags(user, tags)
def change_user_password(user, new_password):
"""Change the password of the rabbitmq user.
:param user: the user to change; must exist in the rabbitmq instance.
:type user: str
:param new_password: the password to change to.
:type new_password: str
:raises KeyError: if the user doesn't exist.
"""
exists = user_exists(user)
if not exists:
msg = "change_user_password: user '{}' doesn't exist.".format(user)
log(msg, ERROR)
raise KeyError(msg)
rabbitmqctl('change_password', user, new_password)
log("Changed password on rabbitmq for user: {}".format(user), INFO)
def grant_permissions(user, vhost):
"""Grant all permissions on a vhost to a user.
@ -1178,7 +1207,7 @@ def get_rabbit_password_on_disk(username, password=None, local=False):
def migrate_passwords_to_peer_relation():
'''Migrate any passwords storage on disk to cluster peer relation'''
for f in glob.glob('/var/lib/charm/{}/*.passwd'.format(service_name())):
for f in glob.glob(_service_password_glob.format(service_name())):
_key = os.path.basename(f)
with open(f, 'r') as passwd:
_value = passwd.read().strip()
@ -1190,6 +1219,48 @@ def migrate_passwords_to_peer_relation():
pass
def get_usernames_for_passwords_on_disk():
"""Return a list of usernames that have passwords on the disk.
Note this is only for non local passwords (i.e. that end in .passwd)
:returns: the list of usernames with passwords on the disk.
:rtype: List[str]
"""
return [
os.path.splitext(os.path.basename(f))[0]
for f in glob.glob(_service_password_glob.format(service_name()))]
def get_usernames_for_passwords():
"""Return a list of usernames that have passwords.
This checks BOTH the peer relationship (leader-storage, or the fallback to
the 'cluster' relation) and on disk. If the peer storage has usernames,
ignore the ones on disk (as they have already been migrated), otherwise
return the ones on disk.
The keys that have passwords in peer storage end with .passwd.
:returns: the list of usernames that have had passwords set.
:rtype: List[str]
"""
# first get from leader settings/peer relation, if available
peer_keys = None
try:
peer_keys = peer_retrieve(None)
except ValueError:
pass
if peer_keys is None:
peer_keys = {}
usernames = set(u[:-7] for u in peer_keys.keys() if u.endswith(".passwd"))
# if usernames were found in peer storage, return them.
if usernames:
return sorted(usernames)
# otherwise, return the ones on disk, if any
return sorted(get_usernames_for_passwords_on_disk())
def get_rabbit_password(username, password=None, local=False):
''' Retrieve, generate or store a rabbit password for
the provided username using peer relation cluster'''

View File

@ -100,6 +100,7 @@ from charmhelpers.core.hookenv import (
)
from charmhelpers.core.host import (
cmp_pkgrevno,
pwgen,
service_stop,
service_restart,
)
@ -315,6 +316,77 @@ def configure_amqp(username, vhost, relation_id, admin=False,
return password
def rotate_service_user_password(service_username):
"""Rotate the service username and update the relation.
This only works on the leader unit due to how peer storage is overlayed on
leader storage.
:param service_username: the username to rotate the password for.
:type service_username: str
:raises NotLeaderError: if the unit is not the leader.
:raises InvalidServiceUserError: if the service_username doesn't exist.
"""
if not rabbit.leader_node_is_ready():
raise rabbit.NotLeaderError(
"This unit can't perform leadership actions and so the password "
"cannot be rotated.")
if service_username not in rabbit.get_usernames_for_passwords():
raise rabbit.InvalidServiceUserError(
"Username {} is not valid for password rotation."
.format(service_username))
# pick a new password.
new_passwd = pwgen(length=64)
# Update the password in rabbitmq
rabbit.change_user_password(service_username, new_passwd)
# Update the setting either locally on disk, leader settings or peer
# storage.
try:
peer_store("{}.passwd".format(service_username), new_passwd)
except ValueError:
# if there was no cluster, just push to leader settings.
leader_set({"{}.passwd".format(service_username): new_passwd})
# Note that the password is not stored in the local cache, so the kv()
# won't need updating for this. The related unit with the username does
# need finding, though. Note that the username may have a '_' in it if
# multiple prefixes are used, but that was specified as service_username
pattern = re.compile(r"(\S+)_username")
for rid in relation_ids('amqp'):
key = None
for unit in related_units(rid):
current = relation_get(rid=rid, unit=unit) or {}
# the username is either as 'username' or '{previx}_username'
if 'username' in current:
key = 'password'
break
for key in current.keys():
match = pattern.match(key)
if match:
key = '_'.join((match[1], 'password'))
break
else:
continue
break
if key is not None:
log("Updating password on key {} on relation_id: {}"
.format(key, rid),
INFO)
relation_set(relation_id=rid,
relation_settings={key: new_passwd})
# set the password for the peer as well for update_client to work
# on the non-leader units
peer_key = "{}_{}".format(rid, key)
try:
peer_store(peer_key, new_passwd)
except ValueError:
# if there was no cluster, just push to leader settings.
leader_set({peer_key: new_passwd})
def update_clients(check_deferred_restarts=True):
"""Update amqp client relation hooks
@ -326,7 +398,7 @@ def update_clients(check_deferred_restarts=True):
:type check_deferred_events: bool
"""
if check_deferred_restarts and get_deferred_restarts():
log("Not sendinfg client update as a restart is pending.", INFO)
log("Not sending client update as a restart is pending.", INFO)
return
_leader_node_is_ready = rabbit.leader_node_is_ready()
_client_node_is_ready = rabbit.client_node_is_ready()

View File

@ -12,6 +12,7 @@ dev_bundles:
tests:
- zaza.openstack.charm_tests.rabbitmq_server.tests.RabbitMQDeferredRestartTest
- zaza.openstack.charm_tests.rabbitmq_server.tests.RmqTests
- zaza.openstack.charm_tests.rabbitmq_server.tests.RmqRotateServiceUserPasswordTests
tests_options:
force_deploy:

View File

@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import re
from unittest import mock
from functools import wraps
@ -307,3 +308,104 @@ class MainTestCase(RabbitActionTestCase):
with mock.patch.dict(actions.ACTIONS, {"foo": dummy_action}):
actions.main(["foo"])
self.assertEqual(dummy_calls, ["Action foo failed: uh oh"])
class ListServiceUsersTestCase(CharmTestCase):
def setUp(self):
super().setUp(actions, ['action_set', 'get_usernames_for_passwords'])
def test_list_service_usernames(self):
self.get_usernames_for_passwords.return_value = ['a', 'b']
actions.list_service_usernames([])
self.get_usernames_for_passwords.assert_called_once_with()
self.action_set.assert_called_once_with(
{'usernames': ['a', 'b']})
class RotateServiceUserPasswordTestCase(CharmTestCase):
def setUp(self):
super().setUp(actions, ['action_get', 'action_fail'])
def _assert_regex_in(self, regex, mock_item):
pattern = re.compile(regex)
calls = mock_item.call_args_list
for call in calls:
args = call[0]
msg = args[0]
print("message: {}".format(msg))
print("regex: {}".format(regex))
if pattern.match(msg):
print("pattern matched.")
return
self.fail("regex {} not found in any log.".format(regex))
@mock.patch.object(actions.rabbitmq_server_relations,
'rotate_service_user_password')
def test_rotate_service_user_password(
self, mock_rotate_service_user_password):
self.action_get.return_value = 'keystone'
actions.rotate_service_user_password([])
self.action_get.assert_called_once_with('service-user')
mock_rotate_service_user_password.assert_called_once_with('keystone')
@mock.patch.object(actions.rabbitmq_server_relations,
'rotate_service_user_password')
def test_rotate_service_user_password__service_user_none(
self, mock_rotate_service_user_password):
self.action_get.return_value = None
actions.rotate_service_user_password([])
self.action_fail.assert_called_once_with(
"The 'service-user' parameter was not passed and is required.")
mock_rotate_service_user_password.assert_not_called()
@mock.patch.object(actions.rabbitmq_server_relations,
'rotate_service_user_password')
def test_rotate_service_user_password__not_leader(
self, mock_rotate_service_user_password):
self.action_get.return_value = 'keystone'
def _error(*args, **kwargs):
raise actions.NotLeaderError('bang')
mock_rotate_service_user_password.side_effect = _error
actions.rotate_service_user_password([])
self._assert_regex_in(r"^This unit .* isn't the leader",
self.action_fail)
@mock.patch.object(actions.rabbitmq_server_relations,
'rotate_service_user_password')
def test_rotate_service_user_password__invalid_service_user(
self, mock_rotate_service_user_password):
self.action_get.return_value = 'keystone'
def _error(*args, **kwargs):
raise actions.InvalidServiceUserError('bang')
mock_rotate_service_user_password.side_effect = _error
actions.rotate_service_user_password([])
self._assert_regex_in(r"^Service username .* not valid",
self.action_fail)
@mock.patch.object(actions.rabbitmq_server_relations,
'rotate_service_user_password')
def test_rotate_service_user_password__other_exception(
self, mock_rotate_service_user_password):
self.action_get.return_value = 'keystone'
def _error(*args, **kwargs):
raise actions.Exception('bang')
mock_rotate_service_user_password.side_effect = _error
with self.assertRaises(Exception):
actions.rotate_service_user_password([])

View File

@ -1630,6 +1630,89 @@ class UtilsTests(CharmTestCase):
mock_rabbitmqctl.assert_called_once_with(
'set_user_tags', 'user1', 'monitor')
@mock.patch.object(rabbit_utils, 'rabbitmqctl')
@mock.patch.object(rabbit_utils, 'user_exists')
@mock.patch("rabbit_utils.log")
def test_change_user_password__user_doesnt_exist(
self,
mock_log,
mock_user_exists,
mock_rabbitmqctl
):
mock_user_exists.return_value = False
with self.assertRaises(KeyError):
rabbit_utils.change_user_password('a-user', 'new-password')
mock_rabbitmqctl.assert_not_called()
@mock.patch.object(rabbit_utils, 'rabbitmqctl')
@mock.patch.object(rabbit_utils, 'user_exists')
@mock.patch("rabbit_utils.log")
def test_change_user_password(
self,
mock_log,
mock_user_exists,
mock_rabbitmqctl
):
mock_user_exists.return_value = True
rabbit_utils.change_user_password('a-user', 'new-password')
mock_rabbitmqctl.assert_called_once_with(
'change_password', 'a-user', 'new-password')
@mock.patch.object(rabbit_utils, 'service_name')
@mock.patch('glob.glob')
def test_get_usernames_for_passwords_on_disk(
self, mock_glob, mock_service_name):
mock_glob.return_value = ['a.passwd', 'b.passwd']
mock_service_name.return_value = 'the-service'
self.assertEqual(rabbit_utils.get_usernames_for_passwords_on_disk(),
['a', 'b'])
mock_glob.assert_called_once_with(
rabbit_utils._service_password_glob.format('the-service'))
@mock.patch.object(rabbit_utils, 'peer_retrieve')
@mock.patch.object(rabbit_utils, 'get_usernames_for_passwords_on_disk')
def test_get_usernames_for_passwords__peer_retrieve_is_none(
self,
mock_get_usernames_for_passwords_on_disk,
mock_peer_retrieve,
):
mock_peer_retrieve.return_value = None
mock_get_usernames_for_passwords_on_disk.return_value = ['a']
self.assertEqual(rabbit_utils.get_usernames_for_passwords(), ['a'])
mock_peer_retrieve.assert_called_once_with(None)
@mock.patch.object(rabbit_utils, 'peer_retrieve')
@mock.patch.object(rabbit_utils, 'get_usernames_for_passwords_on_disk')
def test_get_usernames_for_passwords__peer_retrieve_raises_valueerror(
self,
mock_get_usernames_for_passwords_on_disk,
mock_peer_retrieve,
):
def _error(*args, **kwargs):
raise ValueError('bang')
mock_peer_retrieve.return_value = ['b.passwd']
mock_peer_retrieve.side_effect = _error
mock_get_usernames_for_passwords_on_disk.return_value = ['a']
self.assertEqual(rabbit_utils.get_usernames_for_passwords(), ['a'])
mock_peer_retrieve.assert_called_once_with(None)
@mock.patch.object(rabbit_utils, 'peer_retrieve')
@mock.patch.object(rabbit_utils, 'get_usernames_for_passwords_on_disk')
def test_get_usernames_for_passwords__peer_retrieve_ok(
self,
mock_get_usernames_for_passwords_on_disk,
mock_peer_retrieve,
):
mock_peer_retrieve.return_value = {
'c.passwd': 'c-pass',
'b.passwd': 'b-pass',
'z.thing': 'thing'
}
mock_get_usernames_for_passwords_on_disk.return_value = ['a']
self.assertEqual(rabbit_utils.get_usernames_for_passwords(),
['b', 'c'])
@mock.patch.object(rabbit_utils, 'rabbitmqctl')
@mock.patch.object(rabbit_utils, 'list_user_vhost_permissions')
def test_grant_permissions(self, mock_list_user_vhost_permissions,

View File

@ -328,6 +328,119 @@ class RelationUtil(CharmTestCase):
if os.path.exists(tmpdir):
shutil.rmtree(tmpdir)
@patch.object(rabbitmq_server_relations.rabbit, 'leader_node_is_ready')
def test_rotate_service_user_password__not_leader(
self,
mock_leader_node_is_ready,
):
mock_leader_node_is_ready.return_value = False
with self.assertRaises(
rabbitmq_server_relations.rabbit.NotLeaderError):
rabbitmq_server_relations.rotate_service_user_password('glance')
@patch.object(rabbitmq_server_relations.rabbit,
'get_usernames_for_passwords')
@patch.object(rabbitmq_server_relations.rabbit, 'leader_node_is_ready')
def test_rotate_service_user_password__invalid_username(
self,
mock_leader_node_is_ready,
mock_get_usernames_for_passwords,
):
mock_leader_node_is_ready.return_value = True
mock_get_usernames_for_passwords.return_value = ['cinder', 'glance']
with self.assertRaises(
rabbitmq_server_relations.rabbit.InvalidServiceUserError):
rabbitmq_server_relations.rotate_service_user_password('other')
@patch('rabbitmq_server_relations.relation_get')
@patch('rabbitmq_server_relations.relation_set')
@patch.object(rabbitmq_server_relations, 'leader_set')
@patch.object(rabbitmq_server_relations, 'peer_store')
@patch.object(rabbitmq_server_relations.rabbit, 'change_user_password')
@patch.object(rabbitmq_server_relations, 'pwgen')
@patch.object(rabbitmq_server_relations.rabbit,
'get_usernames_for_passwords')
@patch.object(rabbitmq_server_relations.rabbit, 'leader_node_is_ready')
def test_rotate_service_user_password__peer_store_valueerror(
self,
mock_leader_node_is_ready,
mock_get_usernames_for_passwords,
mock_pwgen,
mock_change_user_password,
mock_peer_store,
mock_leader_set,
mock_relation_set,
mock_relation_get
):
mock_leader_node_is_ready.return_value = True
mock_get_usernames_for_passwords.return_value = ['cinder', 'glance']
self.relation_ids.return_value = ['amqp:0']
self.related_units.return_value = ['glance/0']
mock_relation_get.return_value = {
'username': 'glance',
}
mock_pwgen.return_value = "new-password"
def _error(*args, **kwargs):
raise ValueError('bang')
mock_peer_store.side_effect = _error
rabbitmq_server_relations.rotate_service_user_password('glance')
mock_change_user_password.assert_called_once_with(
'glance', 'new-password')
mock_peer_store.assert_has_calls([
call('glance.passwd', 'new-password'),
call('amqp:0_password', 'new-password')])
mock_leader_set.assert_has_calls([
call({'glance.passwd': 'new-password'}),
call({'amqp:0_password': 'new-password'})])
mock_relation_set.assert_called_once_with(
relation_id='amqp:0',
relation_settings={'password': 'new-password'})
@patch('rabbitmq_server_relations.relation_get')
@patch('rabbitmq_server_relations.relation_set')
@patch.object(rabbitmq_server_relations, 'leader_set')
@patch.object(rabbitmq_server_relations, 'peer_store')
@patch.object(rabbitmq_server_relations.rabbit, 'change_user_password')
@patch.object(rabbitmq_server_relations, 'pwgen')
@patch.object(rabbitmq_server_relations.rabbit,
'get_usernames_for_passwords')
@patch.object(rabbitmq_server_relations.rabbit, 'leader_node_is_ready')
def test_rotate_service_user_password__peer_store_ok(
self,
mock_leader_node_is_ready,
mock_get_usernames_for_passwords,
mock_pwgen,
mock_change_user_password,
mock_peer_store,
mock_leader_set,
mock_relation_set,
mock_relation_get
):
mock_leader_node_is_ready.return_value = True
mock_get_usernames_for_passwords.return_value = ['cinder', 'glance']
self.relation_ids.return_value = ['amqp:0']
self.related_units.return_value = ['glance/0']
mock_relation_get.return_value = {
'some_username': 'glance',
}
mock_pwgen.return_value = "new-password"
rabbitmq_server_relations.rotate_service_user_password('glance')
mock_change_user_password.assert_called_once_with(
'glance', 'new-password')
mock_peer_store.assert_has_calls([
call('glance.passwd', 'new-password'),
call('amqp:0_some_password', 'new-password')])
mock_leader_set.assert_not_called()
mock_relation_set.assert_called_once_with(
relation_id='amqp:0',
relation_settings={'some_password': 'new-password'})
@patch('rabbit_utils.lsb_release')
@patch.object(rabbitmq_server_relations.rabbit, 'grant_permissions')
@patch('rabbit_utils.create_user')