From 55c47861985dfc4a48baf84d7d7cc46e1d3b31cd Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Wed, 15 Aug 2018 16:00:20 -0700 Subject: [PATCH] Add private key storage migration The current key storage can't handle separate keys for ssh and secrets. Nor can it handle key rollover. Upgrade the directory layout to support both, and support future upgrades with schema versions. Change-Id: Ifb292335b1b34d5e16be1c4d4e29aa843761411b --- tests/base.py | 8 +- tests/unit/test_keystorage.py | 68 +++++++++++++++++ tests/unit/test_v3.py | 4 +- zuul/cmd/client.py | 2 +- zuul/configloader.py | 47 ++++++------ zuul/lib/keystorage.py | 135 ++++++++++++++++++++++++++++++++++ zuul/manager/__init__.py | 2 +- zuul/scheduler.py | 13 ++-- 8 files changed, 245 insertions(+), 34 deletions(-) create mode 100644 tests/unit/test_keystorage.py create mode 100644 zuul/lib/keystorage.py diff --git a/tests/base.py b/tests/base.py index 89777529e5..45a0b6b131 100644 --- a/tests/base.py +++ b/tests/base.py @@ -2665,7 +2665,11 @@ class ZuulTestCase(BaseTestCase): key_root = os.path.join(self.state_root, 'keys') if not os.path.isdir(key_root): os.mkdir(key_root, 0o700) - private_key_file = os.path.join(key_root, source, project + '.pem') + fn = os.path.join(key_root, '.version') + with open(fn, 'w') as f: + f.write('1') + private_key_file = os.path.join( + key_root, 'secrets', 'project', source, project, '0.pem') private_key_dir = os.path.dirname(private_key_file) self.log.debug("Installing test keys for project %s at %s" % ( project, private_key_file)) @@ -2729,6 +2733,8 @@ class ZuulTestCase(BaseTestCase): key_root = os.path.join(self.state_root, 'keys') for root, dirname, files in os.walk(key_root): for fn in files: + if fn == '.version': + continue with open(os.path.join(root, fn)) as f: self.assertEqual(test_key, f.read()) diff --git a/tests/unit/test_keystorage.py b/tests/unit/test_keystorage.py new file mode 100644 index 0000000000..4df196c4cc --- /dev/null +++ b/tests/unit/test_keystorage.py @@ -0,0 +1,68 @@ +# Copyright 2018 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +import fixtures + +from zuul.lib import keystorage + +from tests.base import BaseTestCase + + +class TestKeyStorage(BaseTestCase): + + def _setup_keys(self, root, connection_name, project_name): + cn = os.path.join(root, connection_name) + if '/' in project_name: + pn = os.path.join(cn, os.path.dirname(project_name)) + os.makedirs(pn) + fn = os.path.join(cn, project_name + '.pem') + with open(fn, 'w'): + pass + + def assertFile(self, root, path, contents=None): + fn = os.path.join(root, path) + self.assertTrue(os.path.exists(fn)) + if contents: + with open(fn) as f: + self.assertEqual(contents, f.read()) + + def assertPaths(self, root, paths): + seen = set() + for dirpath, dirnames, filenames in os.walk(root): + for d in dirnames: + seen.add(os.path.join(dirpath[len(root) + 1:], d)) + for f in filenames: + seen.add(os.path.join(dirpath[len(root) + 1:], f)) + self.assertEqual(set(paths), seen) + + def test_key_storage(self): + root = self.useFixture(fixtures.TempDir()).path + self._setup_keys(root, 'gerrit', 'org/example') + keystorage.KeyStorage(root) + self.assertFile(root, '.version', '1') + self.assertPaths(root, [ + '.version', + 'secrets', + 'secrets/project', + 'secrets/project/gerrit', + 'secrets/project/gerrit/org', + 'secrets/project/gerrit/org/example', + 'secrets/project/gerrit/org/example/0.pem', + 'ssh', + 'ssh/project', + 'ssh/tenant', + ]) + # It shouldn't need to upgrade this time + keystorage.KeyStorage(root) diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 68a5157997..e37484df6b 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2686,7 +2686,9 @@ class TestProjectKeys(ZuulTestCase): def test_key_generation(self): key_root = os.path.join(self.state_root, 'keys') - private_key_file = os.path.join(key_root, 'gerrit/org/project.pem') + private_key_file = os.path.join( + key_root, + 'secrets/project/gerrit/org/project/0.pem') # Make sure that a proper key was created on startup with open(private_key_file, "rb") as f: private_key, public_key = \ diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py index c3bccccdf2..5a5c05607d 100755 --- a/zuul/cmd/client.py +++ b/zuul/cmd/client.py @@ -430,7 +430,7 @@ class Client(zuul.cmd.ZuulApp): self.configure_connections(source_only=True) sched.registerConnections(self.connections, load=False) loader = configloader.ConfigLoader( - sched.connections, sched, None) + sched.connections, sched, None, None) tenant_config, script = sched._checkTenantSourceConf(self.config) unparsed_abide = loader.readConfig(tenant_config, from_script=script) try: diff --git a/zuul/configloader.py b/zuul/configloader.py index 4bf64128e7..4a6813f12a 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -30,6 +30,7 @@ import zuul.manager.dependent import zuul.manager.independent import zuul.manager.supercedent from zuul.lib import encryption +from zuul.lib.keystorage import KeyStorage # Several forms accept either a single item or a list, this makes @@ -1213,11 +1214,12 @@ class ParseContext(object): class TenantParser(object): - def __init__(self, connections, scheduler, merger): + def __init__(self, connections, scheduler, merger, keystorage): self.log = logging.getLogger("zuul.TenantParser") self.connections = connections self.scheduler = scheduler self.merger = merger + self.keystorage = keystorage classes = vs.Any('pipeline', 'job', 'semaphore', 'project', 'project-template', 'nodeset', 'secret') @@ -1267,7 +1269,7 @@ class TenantParser(object): } return vs.Schema(tenant) - def fromYaml(self, abide, project_key_dir, conf): + def fromYaml(self, abide, conf): self.getSchema()(conf) tenant = model.Tenant(conf['name']) if conf.get('max-nodes-per-job') is not None: @@ -1281,8 +1283,7 @@ class TenantParser(object): tenant.unparsed_config = conf # tpcs is TenantProjectConfigs - config_tpcs, untrusted_tpcs = \ - self._loadTenantProjects(project_key_dir, conf) + config_tpcs, untrusted_tpcs = self._loadTenantProjects(conf) for tpc in config_tpcs: tenant.addConfigProject(tpc) for tpc in untrusted_tpcs: @@ -1343,10 +1344,9 @@ class TenantParser(object): branches = ['master'] + branches tpc.branches = branches - def _loadProjectKeys(self, project_key_dir, connection_name, project): - project.private_key_file = ( - os.path.join(project_key_dir, connection_name, - project.name + '.pem')) + def _loadProjectKeys(self, connection_name, project): + project.private_key_file = self.keystorage.getProjectSecretsKeyFile( + connection_name, project.name) self._generateKeys(project) self._loadKeys(project) @@ -1371,11 +1371,11 @@ class TenantParser(object): "Saving RSA keypair for project %s to %s" % ( project.name, project.private_key_file) ) - with open(project.private_key_file, 'wb') as f: - f.write(pem_private_key) # Ensure private key is read/write for zuul user only. - os.chmod(project.private_key_file, 0o600) + with open(os.open(project.private_key_file, + os.O_CREAT | os.O_WRONLY, 0o600), 'wb') as f: + f.write(pem_private_key) @staticmethod def _loadKeys(project): @@ -1451,7 +1451,7 @@ class TenantParser(object): raise Exception("Unable to parse project %s", conf) return projects - def _loadTenantProjects(self, project_key_dir, conf_tenant): + def _loadTenantProjects(self, conf_tenant): config_projects = [] untrusted_projects = [] @@ -1466,8 +1466,7 @@ class TenantParser(object): # tpcs = TenantProjectConfigs tpcs = self._getProjects(source, conf_repo, current_include) for tpc in tpcs: - self._loadProjectKeys( - project_key_dir, source_name, tpc.project) + self._loadProjectKeys(source_name, tpc.project) config_projects.append(tpc) current_include = frozenset(default_include - set(['pipeline'])) @@ -1475,8 +1474,7 @@ class TenantParser(object): tpcs = self._getProjects(source, conf_repo, current_include) for tpc in tpcs: - self._loadProjectKeys( - project_key_dir, source_name, tpc.project) + self._loadProjectKeys(source_name, tpc.project) untrusted_projects.append(tpc) return config_projects, untrusted_projects @@ -1845,11 +1843,16 @@ class TenantParser(object): class ConfigLoader(object): log = logging.getLogger("zuul.ConfigLoader") - def __init__(self, connections, scheduler, merger): + def __init__(self, connections, scheduler, merger, key_dir): self.connections = connections self.scheduler = scheduler self.merger = merger - self.tenant_parser = TenantParser(connections, scheduler, merger) + if key_dir: + self.keystorage = KeyStorage(key_dir) + else: + self.keystorage = None + self.tenant_parser = TenantParser(connections, scheduler, + merger, self.keystorage) def expandConfigPath(self, config_path): if config_path: @@ -1889,12 +1892,11 @@ class ConfigLoader(object): unparsed_abide.extend(data) return unparsed_abide - def loadConfig(self, unparsed_abide, project_key_dir): + def loadConfig(self, unparsed_abide): abide = model.Abide() for conf_tenant in unparsed_abide.tenants: # When performing a full reload, do not use cached data. - tenant = self.tenant_parser.fromYaml(abide, project_key_dir, - conf_tenant) + tenant = self.tenant_parser.fromYaml(abide, conf_tenant) abide.tenants[tenant.name] = tenant if len(tenant.layout.loading_errors): self.log.warning( @@ -1906,7 +1908,7 @@ class ConfigLoader(object): self.log.warning(err.error) return abide - def reloadTenant(self, project_key_dir, abide, tenant): + def reloadTenant(self, abide, tenant): new_abide = model.Abide() new_abide.tenants = abide.tenants.copy() new_abide.unparsed_project_branch_config = \ @@ -1915,7 +1917,6 @@ class ConfigLoader(object): # When reloading a tenant only, use cached data if available. new_tenant = self.tenant_parser.fromYaml( new_abide, - project_key_dir, tenant.unparsed_config) new_abide.tenants[tenant.name] = new_tenant if len(new_tenant.layout.loading_errors): diff --git a/zuul/lib/keystorage.py b/zuul/lib/keystorage.py new file mode 100644 index 0000000000..d4ccecec29 --- /dev/null +++ b/zuul/lib/keystorage.py @@ -0,0 +1,135 @@ +# Copyright 2018 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import tempfile +import logging +import os + + +class Migration(object): + log = logging.getLogger("zuul.KeyStorage") + version = 0 + parent = None + + def verify(self, root): + fn = os.path.join(root, '.version') + if not os.path.exists(fn): + return False + with open(fn) as f: + data = int(f.read().strip()) + if data == self.version: + return True + return False + + def writeVersion(self, root): + fn = os.path.join(root, '.version') + with open(fn, 'w') as f: + f.write(str(self.version)) + + def upgrade(self, root): + pass + + def verifyAndUpgrade(self, root): + if self.verify(root): + return + if self.parent: + self.parent.verifyAndUpgrade(root) + self.log.info("Upgrading key storage to version %s" % self.version) + self.upgrade(root) + self.writeVersion(root) + self.log.info("Finished upgrading key storage to version %s" % + self.version) + if not self.verify(root): + raise Exception("Inconsistent result after migration") + + +class MigrationV1(Migration): + version = 1 + parent = None + + """Upgrade from the unversioned schema to version 1. + + The original schema had secret keys in key_dir/connection/project.pem + + This updates us to: + key_dir/ + secrets/ + project/ + / + / + .pem + ssh/ + project/ + / + / + .pem + tenant/ + / + .pem + + Where keyids are integers to support future key rollover. In this + case, they will all be 0. + + """ + + def upgrade(self, root): + tmpdir = tempfile.mkdtemp(dir=root) + tmpdirname = os.path.basename(tmpdir) + connection_names = [] + for connection_name in os.listdir(root): + if connection_name == tmpdirname: + continue + # Move existing connections out of the way (in case one of + # them was called 'secrets' or 'ssh'. + os.rename(os.path.join(root, connection_name), + os.path.join(tmpdir, connection_name)) + connection_names.append(connection_name) + os.makedirs(os.path.join(root, 'secrets', 'project'), 0o700) + os.makedirs(os.path.join(root, 'ssh', 'project'), 0o700) + os.makedirs(os.path.join(root, 'ssh', 'tenant'), 0o700) + for connection_name in connection_names: + connection_root = os.path.join(tmpdir, connection_name) + for (dirpath, dirnames, filenames) in os.walk(connection_root): + subdir = os.path.relpath(dirpath, connection_root) + for fn in filenames: + key_name = os.path.join(subdir, fn) + project_name = key_name[:-len('.pem')] + key_dir = os.path.join(root, 'secrets', 'project', + connection_name, project_name) + os.makedirs(key_dir, 0o700) + old = os.path.join(tmpdir, connection_name, key_name) + new = os.path.join(key_dir, '0.pem') + self.log.debug("Moving key from %s to %s", old, new) + os.rename(old, new) + for (dirpath, dirnames, filenames) in os.walk( + connection_root, topdown=False): + os.rmdir(dirpath) + os.rmdir(tmpdir) + + +class KeyStorage(object): + current_version = MigrationV1 + + def __init__(self, root): + self.root = root + migration = self.current_version() + migration.verifyAndUpgrade(root) + + def getProjectSecretsKeyFile(self, connection, project, version=None): + """Return the path to the private key used for the project's secrets""" + # We don't actually support multiple versions yet + if version is None: + version = '0' + return os.path.join(self.root, 'secrets', 'project', + connection, project, version + '.pem') diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index fe109574d6..4d16042b0b 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -416,7 +416,7 @@ class PipelineManager(object): # Late import to break an import loop import zuul.configloader loader = zuul.configloader.ConfigLoader( - self.sched.connections, self.sched, None) + self.sched.connections, self.sched, None, None) self.log.debug("Loading dynamic layout") diff --git a/zuul/scheduler.py b/zuul/scheduler.py index e16e30a8e2..5ebb3a9e65 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -553,7 +553,7 @@ class Scheduler(threading.Thread): os.mkdir(d) return d - def _get_project_key_dir(self): + def _get_key_dir(self): state_dir = get_default(self.config, 'scheduler', 'state_dir', '/var/lib/zuul', expand_user=True) key_dir = os.path.join(state_dir, 'keys') @@ -645,13 +645,12 @@ class Scheduler(threading.Thread): connection.clearBranchCache() loader = configloader.ConfigLoader( - self.connections, self, self.merger) + self.connections, self, self.merger, + self._get_key_dir()) tenant_config, script = self._checkTenantSourceConf(self.config) self.unparsed_abide = loader.readConfig( tenant_config, from_script=script) - abide = loader.loadConfig( - self.unparsed_abide, - self._get_project_key_dir()) + abide = loader.loadConfig(self.unparsed_abide) for tenant in abide.tenants.values(): self._reconfigureTenant(tenant) self.abide = abide @@ -676,9 +675,9 @@ class Scheduler(threading.Thread): branch) old_tenant = self.abide.tenants[event.tenant_name] loader = configloader.ConfigLoader( - self.connections, self, self.merger) + self.connections, self, self.merger, + self._get_key_dir()) abide = loader.reloadTenant( - self._get_project_key_dir(), self.abide, old_tenant) tenant = abide.tenants[event.tenant_name] self._reconfigureTenant(tenant)