Merge "Do not leak credentials on leader-set failure"
This commit is contained in:
commit
c64776b628
|
@ -1474,7 +1474,7 @@ def set_admin_passwd(passwd, user=None):
|
|||
if user is None:
|
||||
user = config('admin-user')
|
||||
|
||||
leader_set({'{}_passwd'.format(user): passwd})
|
||||
_leader_set_secret({'{}_passwd'.format(user): passwd})
|
||||
|
||||
|
||||
def get_api_version():
|
||||
|
@ -1632,7 +1632,7 @@ def _migrate_admin_password():
|
|||
if is_leader() and os.path.exists(STORED_PASSWD):
|
||||
log('Migrating on-disk stored passwords to leader storage')
|
||||
with open(STORED_PASSWD) as fd:
|
||||
leader_set({"admin_passwd": fd.readline().strip('\n')})
|
||||
_leader_set_secret({"admin_passwd": fd.readline().strip('\n')})
|
||||
|
||||
os.unlink(STORED_PASSWD)
|
||||
|
||||
|
@ -1642,8 +1642,8 @@ def _migrate_service_passwords():
|
|||
if is_leader() and os.path.exists(SERVICE_PASSWD_PATH):
|
||||
log('Migrating on-disk stored passwords to leader storage')
|
||||
creds = load_stored_passwords()
|
||||
for k, v in creds.items():
|
||||
leader_set({"{}_passwd".format(k): v})
|
||||
_leader_set_secret({"{}_passwd".format(k): v
|
||||
for k, v in creds.items()})
|
||||
os.unlink(SERVICE_PASSWD_PATH)
|
||||
|
||||
|
||||
|
@ -1657,7 +1657,7 @@ def get_service_password(service_username):
|
|||
|
||||
|
||||
def set_service_password(passwd, user):
|
||||
leader_set({"{}_passwd".format(user): passwd})
|
||||
_leader_set_secret({"{}_passwd".format(user): passwd})
|
||||
|
||||
|
||||
def is_password_changed(username, passwd):
|
||||
|
@ -2555,7 +2555,7 @@ def key_leader_set():
|
|||
`CREDENTIAL_KEY_REPOSITORY` directories. Note that this function will fail
|
||||
if it is called on the unit that is not the leader.
|
||||
|
||||
:raises: :class:`subprocess.CalledProcessError` if the leader_set fails.
|
||||
:raises: :class:`RuntimeError` if the leader_set fails.
|
||||
"""
|
||||
disk_keys = {}
|
||||
for key_repository in [FERNET_KEY_REPOSITORY, CREDENTIAL_KEY_REPOSITORY]:
|
||||
|
@ -2564,7 +2564,7 @@ def key_leader_set():
|
|||
with open(os.path.join(key_repository, key_number),
|
||||
'r') as f:
|
||||
disk_keys[key_repository][key_number] = f.read()
|
||||
leader_set({'key_repository': json.dumps(disk_keys)})
|
||||
_leader_set_secret({'key_repository': json.dumps(disk_keys)})
|
||||
|
||||
|
||||
def key_write():
|
||||
|
@ -2722,3 +2722,18 @@ def endpoints_dict(settings):
|
|||
}
|
||||
|
||||
return endpoints
|
||||
|
||||
|
||||
def _leader_set_secret(secret_dict):
|
||||
"""Wrapper around leader_set doing its best to prevent leaking secrets."""
|
||||
if not is_leader():
|
||||
raise RuntimeError("This unit isn't the leader and therefore can't "
|
||||
"call leader_set() with the given secrets")
|
||||
try:
|
||||
leader_set(secret_dict)
|
||||
except subprocess.CalledProcessError as e:
|
||||
# Do not log 'e' or 'e.cmd' as this would leak secrets. Raise a
|
||||
# different exception to make sure that no one above will leak the
|
||||
# secrets by accident.
|
||||
raise RuntimeError('leader-set failed with {}: {}'.format(e.returncode,
|
||||
e.output))
|
||||
|
|
|
@ -7,6 +7,7 @@
|
|||
# requirements. They are intertwined. Also, Zaza itself should specify
|
||||
# all of its own requirements and if it doesn't, fix it there.
|
||||
#
|
||||
setuptools<50.0.0 # https://github.com/pypa/setuptools/commit/04e3df22df840c6bb244e9b27bc56750c44b7c85
|
||||
pbr>=1.8.0,<1.9.0
|
||||
simplejson>=2.2.0
|
||||
netifaces>=0.10.4
|
||||
|
|
|
@ -7,12 +7,14 @@
|
|||
# requirements. They are intertwined. Also, Zaza itself should specify
|
||||
# all of its own requirements and if it doesn't, fix it there.
|
||||
#
|
||||
setuptools<50.0.0 # https://github.com/pypa/setuptools/commit/04e3df22df840c6bb244e9b27bc56750c44b7c85
|
||||
charm-tools>=2.4.4
|
||||
requests>=2.18.4
|
||||
mock>=1.2
|
||||
flake8>=2.2.4,<=2.4.1
|
||||
flake8>=2.2.4
|
||||
stestr>=2.2.0
|
||||
coverage>=4.5.2
|
||||
pyudev # for ceph-* charm unit tests (need to fix the ceph-* charm unit tests/mocking)
|
||||
juju!=2.8.3 # this version causes spurious JujuAPIError's
|
||||
git+https://github.com/openstack-charmers/zaza.git#egg=zaza;python_version>='3.0'
|
||||
git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack
|
||||
|
|
2
tox.ini
2
tox.ini
|
@ -116,5 +116,5 @@ commands =
|
|||
functest-run-suite --keep-model --bundle {posargs}
|
||||
|
||||
[flake8]
|
||||
ignore = E402,E226
|
||||
ignore = E402,E226,W503,W504
|
||||
exclude = */charmhelpers
|
||||
|
|
|
@ -367,6 +367,7 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
**relation_data)
|
||||
|
||||
@patch.object(utils, 'get_real_role_names')
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
@patch.object(utils, 'leader_get')
|
||||
@patch.object(utils, 'get_api_version')
|
||||
|
@ -378,10 +379,11 @@ 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_api_version, leader_get,
|
||||
leader_set, _get_real_role_names, test_api_version=2):
|
||||
leader_set, is_leader, _get_real_role_names, test_api_version=2):
|
||||
_get_real_role_names.return_value = ['Member', 'SpecialRole']
|
||||
get_api_version.return_value = test_api_version
|
||||
leader_get.return_value = None
|
||||
is_leader.return_value = True
|
||||
relation_id = 'identity-service:0'
|
||||
remote_unit = 'unit/0'
|
||||
self.get_service_password.return_value = 'password'
|
||||
|
@ -1102,19 +1104,25 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
self.assertEqual(utils.get_admin_passwd(), 'admin')
|
||||
leader_get.assert_called_with('test_passwd')
|
||||
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
def test_set_admin_password(self, leader_set):
|
||||
def test_set_admin_password(self, leader_set, is_leader):
|
||||
is_leader.return_value = True
|
||||
utils.set_admin_passwd('secret')
|
||||
leader_set.assert_called_once_with({'admin_passwd': 'secret'})
|
||||
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
def test_set_admin_password_config_username(self, leader_set):
|
||||
def test_set_admin_password_config_username(self, leader_set, is_leader):
|
||||
is_leader.return_value = True
|
||||
self.test_config.set('admin-user', 'username')
|
||||
utils.set_admin_passwd('secret')
|
||||
leader_set.assert_called_once_with({'username_passwd': 'secret'})
|
||||
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
def test_set_admin_password_username(self, leader_set):
|
||||
def test_set_admin_password_username(self, leader_set, is_leader):
|
||||
is_leader.return_value = True
|
||||
utils.set_admin_passwd('secret', user='username')
|
||||
leader_set.assert_called_once_with({'username_passwd': 'secret'})
|
||||
|
||||
|
@ -1691,10 +1699,12 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
utils.fernet_rotate()
|
||||
self.subprocess.check_output.called_with(cmd)
|
||||
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
@patch('os.listdir')
|
||||
def test_key_leader_set(self, listdir, leader_set):
|
||||
def test_key_leader_set(self, listdir, leader_set, is_leader):
|
||||
listdir.return_value = ['0', '1']
|
||||
is_leader.return_value = True
|
||||
self.time.time.return_value = "the-time"
|
||||
with patch.object(builtins, 'open', mock_open(
|
||||
read_data="some_data")):
|
||||
|
@ -1884,6 +1894,7 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
mock_leader_get.assert_called_with('_charm-keystone-admin_passwd')
|
||||
|
||||
@patch.object(utils, 'get_manager')
|
||||
@patch.object(utils, 'is_leader')
|
||||
@patch.object(utils, 'leader_set')
|
||||
@patch.object(utils, 'resolve_address')
|
||||
@patch.object(utils, 'endpoint_url')
|
||||
|
@ -1898,6 +1909,7 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
mock_endpoint_url,
|
||||
mock_resolve_address,
|
||||
mock_leader_set,
|
||||
mock_is_leader,
|
||||
mock_get_manager):
|
||||
configs = MagicMock()
|
||||
mock_get_api_suffix.return_value = 'suffix'
|
||||
|
@ -1905,6 +1917,7 @@ class TestKeystoneUtils(CharmTestCase):
|
|||
mock_endpoint_url.side_effect = (
|
||||
lambda x, y, z: 'http://{}:{}/{}'.format(x, y, z))
|
||||
mock_leader_get.return_value = 'fakepassword'
|
||||
mock_is_leader.return_value = True
|
||||
mock_get_manager().resolve_user_id.return_value = 'fakeid'
|
||||
self.os_release.return_value = 'queens'
|
||||
utils.bootstrap_keystone(configs=configs)
|
||||
|
|
Loading…
Reference in New Issue