Skip to content
This repository has been archived by the owner on Aug 1, 2019. It is now read-only.

Commit

Permalink
Add private key storage migration
Browse files Browse the repository at this point in the history
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
  • Loading branch information
James E. Blair committed Sep 4, 2018
1 parent 85f6598 commit 55c4786
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 34 deletions.
8 changes: 7 additions & 1 deletion tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2665,7 +2665,11 @@ def setupProjectKeys(self, source, project):
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))
Expand Down Expand Up @@ -2729,6 +2733,8 @@ def assertNoGeneratedKeys(self):
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())

Expand Down
68 changes: 68 additions & 0 deletions tests/unit/test_keystorage.py
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 3 additions & 1 deletion tests/unit/test_v3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = \
Expand Down
2 changes: 1 addition & 1 deletion zuul/cmd/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def validate(self):
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:
Expand Down
47 changes: 24 additions & 23 deletions zuul/configloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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
Expand Down Expand Up @@ -1213,11 +1214,12 @@ def getImpliedBranches(self, source_context):


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')
Expand Down Expand Up @@ -1267,7 +1269,7 @@ def getSchema(self):
}
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:
Expand All @@ -1281,8 +1283,7 @@ def fromYaml(self, abide, project_key_dir, conf):

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:
Expand Down Expand Up @@ -1343,10 +1344,9 @@ def _getProjectBranches(self, tenant, tpc):
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)
Expand All @@ -1371,11 +1371,11 @@ def _generateKeys(self, project):
"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):
Expand Down Expand Up @@ -1451,7 +1451,7 @@ def _getProjects(self, source, conf, current_include):
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 = []

Expand All @@ -1466,17 +1466,15 @@ def _loadTenantProjects(self, project_key_dir, conf_tenant):
# 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']))
for conf_repo in conf_source.get('untrusted-projects', []):
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
Expand Down Expand Up @@ -1845,11 +1843,16 @@ def _parseLayout(self, tenant, data, loading_errors):
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:
Expand Down Expand Up @@ -1889,12 +1892,11 @@ def readConfig(self, config_path, from_script=False):
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(
Expand All @@ -1906,7 +1908,7 @@ def loadConfig(self, unparsed_abide, project_key_dir):
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 = \
Expand All @@ -1915,7 +1917,6 @@ def reloadTenant(self, project_key_dir, abide, tenant):
# 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):
Expand Down
135 changes: 135 additions & 0 deletions zuul/lib/keystorage.py
Original file line number Diff line number Diff line change
@@ -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/
<connection>/
<project>/
<keyid>.pem
ssh/
project/
<connection>/
<project>/
<keyid>.pem
tenant/
<tenant>/
<keyid>.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')
Loading

0 comments on commit 55c4786

Please sign in to comment.