zuul
/
zuul
Code Issues Proposed changes

Decrypt secrets on the executors 

Rather than decrypting secrets on the scheduler and sending them
to the executors unencrypted, now that the private keys are in ZK
and the executors have access to them, we can defer decryption to
the executors.  This means that when we move the build requests
from gearman to ZK, we avoid storing decrypted secrets in ZK.

We accomplish this by serializing the entire secret (parts or all
of which may be encrypted or plaintext) to YAML in the scheduler
and deserializing the YAML into a Secret object on the executor.
We do this because we already have support for indicating an
encrypted value via custom YAML tags.

This means that the build request (which is currently transmitted
via gearman and soon to be via ZK) serializes the rest of the job
to JSON.  This means we're storing a serialized-to-YAML secret as
a scalar value in a serialized-to-JSON data structure.  There's
nothing technically wrong with this, and it is the minimal version
of this change, however it's slightly unusual and may result in
a little extra work.  We may want to consider serializing the
entire job request as YAML instead.

Change-Id: I6d94c1d8da8b68e5fb60c27e73039155a02fb485
This commit is contained in:
James E. Blair 2021-05-05 19:05:32 -07:00
parent 707570e46b
commit fbb17e1f35
7 changed files with 132 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
tests/unit/test_connection.py
View File

@ -21,6 +21,7 @@ import types
import sqlalchemy as sa
import zuul
from zuul.lib.yamlutil import yaml
from tests.base import ZuulTestCase, FIXTURE_DIR, \
PostgresqlSchemaFixture, MySQLSchemaFixture, ZuulDBTestCase, \
BaseTestCase, AnsibleZuulTestCase
@ -716,8 +717,11 @@ class TestElasticsearchConnection(AnsibleZuulTestCase):
def _getSecrets(self, job, pbtype):
secrets = []
build = self.getJobFromHistory(job)
for pb in build.parameters[pbtype]:
secrets.append(pb['secrets'])
for pb in getattr(build.jobdir, pbtype):
if pb.secrets_content:
secrets.append(yaml.safe_load(pb.secrets_content))
else:
secrets.append({})
return secrets
def test_elastic_reporter(self):

22
tests/unit/test_v3.py
View File

@ -22,6 +22,7 @@ import textwrap
import gc
from time import sleep
from unittest import skip, skipIf
from zuul.lib.yamlutil import yaml
import paramiko
@ -4754,8 +4755,11 @@ class TestSecrets(ZuulTestCase):
def _getSecrets(self, job, pbtype):
secrets = []
build = self.getJobFromHistory(job)
for pb in build.parameters[pbtype]:
secrets.append(pb['secrets'])
for pb in getattr(build.jobdir, pbtype):
if pb.secrets_content:
secrets.append(yaml.safe_load(pb.secrets_content))
else:
secrets.append({})
return secrets
def test_secret_branch(self):
@ -4929,8 +4933,11 @@ class TestSecretInheritance(ZuulTestCase):
def _getSecrets(self, job, pbtype):
secrets = []
build = self.getJobFromHistory(job)
for pb in build.parameters[pbtype]:
secrets.append(pb['secrets'])
for pb in getattr(build.jobdir, pbtype):
if pb.secrets_content:
secrets.append(yaml.safe_load(pb.secrets_content))
else:
secrets.append({})
return secrets
def _checkTrustedSecrets(self):
@ -5034,8 +5041,11 @@ class TestSecretPassToParent(ZuulTestCase):
def _getSecrets(self, job, pbtype):
secrets = []
build = self.getJobFromHistory(job)
for pb in build.parameters[pbtype]:
secrets.append(pb['secrets'])
for pb in getattr(build.jobdir, pbtype):
if pb.secrets_content:
secrets.append(yaml.safe_load(pb.secrets_content))
else:
secrets.append({})
return secrets
def test_secret_no_pass_to_parent(self):

8
tests/zuul_client/test_zuulclient.py
View File

@ -21,6 +21,7 @@ import textwrap
import zuul.web
import zuul.rpcclient
from zuul.lib.yamlutil import yaml
from tests.base import iterate_timeout
from tests.base import ZuulDBTestCase
@ -50,8 +51,11 @@ class TestZuulClientEncrypt(BaseTestWeb):
def _getSecrets(self, job, pbtype):
secrets = []
build = self.getJobFromHistory(job)
for pb in build.parameters[pbtype]:
secrets.append(pb['secrets'])
for pb in getattr(build.jobdir, pbtype):
if pb.secrets_content:
secrets.append(yaml.safe_load(pb.secrets_content))
else:
secrets.append({})
return secrets
def test_encrypt_large_secret(self):

8
zuul/configloader.py
View File

@ -437,10 +437,12 @@ class EncryptedPKCS1_OAEP(yaml.YAMLObject):
def to_yaml(cls, dumper, data):
ciphertext = data.ciphertext
if isinstance(ciphertext, list):
ciphertext = [yaml.ScalarNode(value=base64.b64encode(x))
ciphertext = [yaml.ScalarNode(tag='tag:yaml.org,2002:str',
value=base64.b64encode(x))
for x in ciphertext]
else:
ciphertext = base64.b64encode(ciphertext)
return yaml.SequenceNode(tag=cls.yaml_tag,
value=ciphertext)
ciphertext = base64.b64encode(ciphertext)
return yaml.ScalarNode(tag=cls.yaml_tag, value=ciphertext)
def decrypt(self, private_key):

44
zuul/executor/server.py
View File

@ -39,11 +39,12 @@ from urllib.parse import urlsplit
from zuul.lib.ansible import AnsibleManager
from zuul.lib.gearworker import ZuulGearWorker
from zuul.lib.result_data import get_warnings_from_result_data
from zuul.lib.yamlutil import yaml
from zuul.lib import yamlutil as yaml
from zuul.lib.config import get_default
from zuul.lib.logutil import get_annotated_logger
from zuul.lib.statsd import get_statsd
from zuul.lib import filecomments
from zuul.lib.keystorage import ZooKeeperKeyStorage
import gear
@ -1767,7 +1768,7 @@ class AnsibleJob(object):
for role in playbook['roles']:
self.prepareRole(jobdir_playbook, role, args)
secrets = playbook['secrets']
secrets = self.decryptSecrets(playbook['secrets'])
if secrets:
check_varnames(secrets)
jobdir_playbook.secrets_content = yaml.safe_dump(
@ -1775,6 +1776,34 @@ class AnsibleJob(object):
self.writeAnsibleConfig(jobdir_playbook)
def decryptSecrets(self, secrets):
"""Decrypt the secrets dictionary provided by the scheduler
The input dictionary has a frozen secret dictionary as its
value (with encrypted data and the project name of the key to
use to decrypt it).
The output dictionary simply has decrypted data as its value.
:param dict secrets: The encrypted secrets dictionary from the
scheduler
:returns: A decrypted secrets dictionary
"""
ret = {}
for secret_name, frozen_secret in secrets.items():
secret = zuul.model.Secret(secret_name, None)
secret.secret_data = yaml.safe_load(
frozen_secret['encrypted_data'])
private_secrets_key, public_secrets_key = \
self.executor_server.keystore.getProjectSecretsKeys(
frozen_secret['connection_name'],
frozen_secret['project_name'])
secret = secret.decrypt(private_secrets_key)
ret[secret_name] = secret.secret_data
return ret
def checkoutTrustedProject(self, project, branch, args):
root = self.jobdir.getTrustedProject(project.canonical_name,
branch)
@ -2635,6 +2664,11 @@ class ExecutorServer(BaseMergeServer):
self.keep_jobdir = keep_jobdir
self.jobdir_root = jobdir_root
self.keystore = ZooKeeperKeyStorage(
self.zk_client,
password=self._get_key_store_password())
# TODOv3(mordred): make the executor name more unique --
# perhaps hostname+pid.
self.hostname = get_default(self.config, 'executor', 'hostname',
@ -2795,6 +2829,12 @@ class ExecutorServer(BaseMergeServer):
# Used to offload expensive operations to different processes
self.process_worker = None
def _get_key_store_password(self):
try:
return self.config["keystore"]["password"]
except KeyError:
raise RuntimeError("No key store password configured!")
def _getFunctionSuffixes(self):
suffixes = []
if self.zone:

2
zuul/lib/yamlutil.py
View File

@ -12,7 +12,7 @@
import types
import yaml
from yaml import ( # noqa: F401
YAMLObject, YAMLError, ScalarNode, MappingNode
YAMLObject, YAMLError, ScalarNode, MappingNode, SequenceNode
)
try:

76
zuul/model.py
View File

@ -29,7 +29,7 @@ import urllib.parse
import textwrap
import types
import itertools
import yaml
from zuul.lib import yamlutil as yaml
import jsonpath_rw
@ -924,6 +924,9 @@ class Secret(ConfigObject):
r.secret_data = self._decrypt(private_key, self.secret_data)
return r
def serialize(self):
return yaml.safe_dump(self.secret_data, default_flow_style=False)
class SecretUse(ConfigObject):
"""A use of a secret in a Job"""
@ -935,6 +938,25 @@ class SecretUse(ConfigObject):
self.pass_to_parent = False
class FrozenSecret(ConfigObject):
"""A frozen secret for use by the executor"""
def __init__(self, connection_name, project_name, name, encrypted_data):
super(FrozenSecret, self).__init__()
self.connection_name = connection_name
self.project_name = project_name
self.name = name
self.encrypted_data = encrypted_data
def toDict(self):
# Name is omitted since this is used in a dictionary
return dict(
connection_name=self.connection_name,
project_name=self.project_name,
encrypted_data=self.encrypted_data,
)
class ProjectContext(ConfigObject):
def __init__(self, project):
@ -1023,8 +1045,12 @@ class PlaybookContext(ConfigObject):
self.source_context = source_context
self.path = path
self.roles = roles
# The original SecretUse objects describing how the secret
# should be used
self.secrets = secrets
self.decrypted_secrets = ()
# FrozenSecret objects which contain only the info the
# executor needs
self.frozen_secrets = ()
def __repr__(self):
return '<PlaybookContext %s %s>' % (self.source_context,
@ -1073,26 +1099,30 @@ class PlaybookContext(ConfigObject):
secrets = []
for secret_use in self.secrets:
secret = layout.secrets.get(secret_use.name)
decrypted_secret = secret.decrypt(
self.source_context.project.private_secrets_key)
decrypted_secret.name = secret_use.alias
secrets.append(decrypted_secret)
self.decrypted_secrets = tuple(secrets)
secret_name = secret_use.alias
encrypted_secret_data = secret.serialize()
# Use *our* project, not the secret's, because we want to decrypt
# with *our* key.
connection_name = self.source_context.project.connection_name
project_name = self.source_context.project.name
secrets.append(FrozenSecret(connection_name, project_name,
secret_name, encrypted_secret_data))
self.frozen_secrets = tuple(secrets)
def addSecrets(self, decrypted_secrets):
current_names = set([s.name for s in self.decrypted_secrets])
new_secrets = [s for s in decrypted_secrets
def addSecrets(self, frozen_secrets):
current_names = set([s.name for s in self.frozen_secrets])
new_secrets = [s for s in frozen_secrets
if s.name not in current_names]
self.decrypted_secrets = self.decrypted_secrets + tuple(new_secrets)
self.frozen_secrets = self.frozen_secrets + tuple(new_secrets)
def toDict(self, redact_secrets=True):
# Render to a dict to use in passing json to the executor
secrets = {}
for secret in self.decrypted_secrets:
for secret in self.frozen_secrets:
if redact_secrets:
secrets[secret.name] = 'REDACTED'
else:
secrets[secret.name] = secret.secret_data
secrets[secret.name] = secret.toDict()
return dict(
connection=self.source_context.project.connection_name,
project=self.source_context.project.name,
@ -1678,15 +1708,21 @@ class Job(ConfigObject):
# Pass secrets to parents
secrets_for_parents = [s for s in other.secrets if s.pass_to_parent]
if secrets_for_parents:
decrypted_secrets = []
frozen_secrets = []
for secret_use in secrets_for_parents:
secret = layout.secrets.get(secret_use.name)
if secret is None:
raise Exception("Secret %s not found" % (secret_use.name,))
decrypted_secret = secret.decrypt(
other.source_context.project.private_secrets_key)
decrypted_secret.name = secret_use.alias
decrypted_secrets.append(decrypted_secret)
secret_name = secret_use.alias
encrypted_secret_data = secret.serialize()
# Use the other project, not the secret's, because we
# want to decrypt with the other project's key key.
connection_name = other.source_context.project.connection_name
project_name = other.source_context.project.name
frozen_secrets.append(FrozenSecret(
connection_name, project_name,
secret_name, encrypted_secret_data))
# Add the secrets to any existing playbooks. If any of
# them are in an untrusted project, then we've just given
# a secret to a playbook which can run in dynamic config,
@ -1696,7 +1732,7 @@ class Job(ConfigObject):
# trusted context.
for pb in itertools.chain(
self.pre_run, self.run, self.post_run, self.cleanup_run):
pb.addSecrets(decrypted_secrets)
pb.addSecrets(frozen_secrets)
if not pb.source_context.trusted:
self.post_review = True
@ -4219,7 +4255,7 @@ class ConfigItemErrorException(Exception):
# ordered by default. If this is a foreign config file or
# something the dump might be really long; hence the
# truncation.
extract = yaml.dump(conf, sort_keys=False)
extract = yaml.safe_dump(conf, sort_keys=False)
lines = extract.split('\n')
if len(lines) > 5:
lines = lines[0:4]