Omnibus executor secret decrypt revert

Revert "Decrypt project ssh keys in executors"
This reverts commit 77bde6f765.

Revert "Decrypt secrets on the executors"
This reverts commit fbb17e1f35.

Revert "Support serializing encrypted secret objects"
This reverts commit 707570e46b.

We observed an error parsing the encrypted/pkcs1 yaml on the executors
in production on OpenDev.  We have not found the cause yet; revert this
until we identify it.

Change-Id: Icc751c54a376e68fd5d9e29dbbb67ed7aa6d67c5

tests/unit/test_connection.py

@@ -21,7 +21,6 @@ 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
@@ -746,11 +745,8 @@ class TestElasticsearchConnection(AnsibleZuulTestCase):
     def _getSecrets(self, job, pbtype):
         secrets = []
         build = self.getJobFromHistory(job)
-        for pb in getattr(build.jobdir, pbtype):
+        for pb in build.parameters[pbtype]:
-            if pb.secrets_content:
+            secrets.append(pb['secrets'])
-                secrets.append(yaml.safe_load(pb.secrets_content))
-            else:
-                secrets.append({})
         return secrets
 
     def test_elastic_reporter(self):

tests/unit/test_v3.py

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

tests/unit/test_web.py

@@ -1125,7 +1125,7 @@ class TestWebSecrets(BaseTestWeb):
             "project1-secret").json()
         self.assertEqual(
             {'secret_name': 'REDACTED'}, resp['playbooks'][0]['secrets'])
-        self.assertEqual('REDACTED', resp['ssh_keys'][0])
+        self.assertEqual('REDACTED', resp['ssh_keys'][0]['key'])
 
 
 class TestInfo(ZuulDBTestCase, BaseTestWeb):

tests/zuul_client/test_zuulclient.py

@@ -21,7 +21,6 @@ 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
@@ -51,11 +50,8 @@ class TestZuulClientEncrypt(BaseTestWeb):
     def _getSecrets(self, job, pbtype):
         secrets = []
         build = self.getJobFromHistory(job)
-        for pb in getattr(build.jobdir, pbtype):
+        for pb in build.parameters[pbtype]:
-            if pb.secrets_content:
+            secrets.append(pb['secrets'])
-                secrets.append(yaml.safe_load(pb.secrets_content))
-            else:
-                secrets.append({})
         return secrets
 
     def test_encrypt_large_secret(self):

zuul/configloader.py

@@ -412,7 +412,6 @@ repo {repo} on branch {branch}.  The error was:
 class EncryptedPKCS1_OAEP(yaml.YAMLObject):
     yaml_tag = u'!encrypted/pkcs1-oaep'
     yaml_loader = yaml.SafeLoader
-    yaml_dumper = yaml.SafeDumper
 
     def __init__(self, ciphertext):
         if isinstance(ciphertext, list):
@@ -433,18 +432,6 @@ class EncryptedPKCS1_OAEP(yaml.YAMLObject):
     def from_yaml(cls, loader, node):
         return cls(node.value)
 
-    @classmethod
-    def to_yaml(cls, dumper, data):
-        ciphertext = data.ciphertext
-        if isinstance(ciphertext, list):
-            ciphertext = [yaml.ScalarNode(tag='tag:yaml.org,2002:str',
-                                          value=base64.b64encode(x))
-                          for x in 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):
         if isinstance(self.ciphertext, list):
             return ''.join([

zuul/executor/common.py

@@ -134,11 +134,12 @@ def construct_gearman_params(uuid, sched, nodeset, job, item, pipeline,
     params['ssh_keys'] = []
     if pipeline.post_review:
         if redact_secrets_and_keys:
-            params['ssh_keys'].append("REDACTED")
+            ssh_key = "REDACTED"
         else:
-            params['ssh_keys'].append(dict(
+            ssh_key = item.change.project.private_ssh_key
-                connection_name=item.change.project.connection_name,
+        params['ssh_keys'].append(dict(
-                project_name=item.change.project.name))
+            name='%s project key' % item.change.project.canonical_name,
+            key=ssh_key))
     params['vars'] = job.combined_variables
     params['extra_vars'] = job.extra_variables
     params['host_vars'] = job.host_variables

zuul/executor/server.py

@@ -39,12 +39,11 @@ 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 import yamlutil as yaml
+from zuul.lib.yamlutil import 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
 
@@ -951,12 +950,7 @@ class AnsibleJob(object):
             self.ssh_agent.start()
             self.ssh_agent.add(self.private_key_file)
             for key in self.arguments.get('ssh_keys', []):
-                private_ssh_key, public_ssh_key = \
+                self.ssh_agent.addData(key['name'], key['key'])
-                    self.executor_server.keystore.getProjectSSHKeys(
-                        key['connection_name'],
-                        key['project_name'])
-                name = '%s project key' % (key['project_name'])
-                self.ssh_agent.addData(name, private_ssh_key)
             self.jobdir = JobDir(self.executor_server.jobdir_root,
                                  self.executor_server.keep_jobdir,
                                  str(self.job.unique))
@@ -1788,7 +1782,7 @@ class AnsibleJob(object):
         for role in playbook['roles']:
             self.prepareRole(jobdir_playbook, role, args)
 
-        secrets = self.decryptSecrets(playbook['secrets'])
+        secrets = playbook['secrets']
         if secrets:
             check_varnames(secrets)
             jobdir_playbook.secrets_content = yaml.safe_dump(
@@ -1796,34 +1790,6 @@ 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)
@@ -2684,11 +2650,6 @@ 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',
@@ -2849,12 +2810,6 @@ 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:

zuul/lib/yamlutil.py

@@ -9,11 +9,8 @@
 # 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 types
 import yaml
-from yaml import (  # noqa: F401
+from yaml import YAMLObject, YAMLError  # noqa: F401
-    YAMLObject, YAMLError, ScalarNode, MappingNode, SequenceNode
-)
 
 try:
     # Explicit type ignore to deal with provisional import failure
@@ -29,14 +26,9 @@ except ImportError:
     Mark = yaml.Mark
 
 
-yaml.add_representer(types.MappingProxyType,
-                     yaml.representer.SafeRepresenter.represent_dict,
-                     Dumper=SafeDumper)
-
-
 def safe_load(stream, *args, **kwargs):
     return yaml.load(stream, *args, Loader=SafeLoader, **kwargs)
 
 
-def safe_dump(data, *args, **kwargs):
+def safe_dump(stream, *args, **kwargs):
-    return yaml.dump(data, *args, Dumper=SafeDumper, **kwargs)
+    return yaml.dump(stream, *args, Dumper=SafeDumper, **kwargs)

zuul/model.py

@@ -29,7 +29,7 @@ import urllib.parse
 import textwrap
 import types
 import itertools
-from zuul.lib import yamlutil as yaml
+import yaml
 
 import jsonpath_rw
 
@@ -944,9 +944,6 @@ 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"""
@@ -958,25 +955,6 @@ 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):
@@ -1065,12 +1043,8 @@ 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
-        # FrozenSecret objects which contain only the info the
+        self.decrypted_secrets = ()
-        # executor needs
-        self.frozen_secrets = ()
 
     def __repr__(self):
         return '<PlaybookContext %s %s>' % (self.source_context,
@@ -1119,30 +1093,26 @@ class PlaybookContext(ConfigObject):
         secrets = []
         for secret_use in self.secrets:
             secret = layout.secrets.get(secret_use.name)
-            secret_name = secret_use.alias
+            decrypted_secret = secret.decrypt(
-            encrypted_secret_data = secret.serialize()
+                self.source_context.project.private_secrets_key)
-            # Use *our* project, not the secret's, because we want to decrypt
+            decrypted_secret.name = secret_use.alias
-            # with *our* key.
+            secrets.append(decrypted_secret)
-            connection_name = self.source_context.project.connection_name
+        self.decrypted_secrets = tuple(secrets)
-            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, frozen_secrets):
+    def addSecrets(self, decrypted_secrets):
-        current_names = set([s.name for s in self.frozen_secrets])
+        current_names = set([s.name for s in self.decrypted_secrets])
-        new_secrets = [s for s in frozen_secrets
+        new_secrets = [s for s in decrypted_secrets
                        if s.name not in current_names]
-        self.frozen_secrets = self.frozen_secrets + tuple(new_secrets)
+        self.decrypted_secrets = self.decrypted_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.frozen_secrets:
+        for secret in self.decrypted_secrets:
             if redact_secrets:
                 secrets[secret.name] = 'REDACTED'
             else:
-                secrets[secret.name] = secret.toDict()
+                secrets[secret.name] = secret.secret_data
         return dict(
             connection=self.source_context.project.connection_name,
             project=self.source_context.project.name,
@@ -1728,21 +1698,15 @@ 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:
-            frozen_secrets = []
+            decrypted_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,))
-                secret_name = secret_use.alias
+                decrypted_secret = secret.decrypt(
-                encrypted_secret_data = secret.serialize()
+                    other.source_context.project.private_secrets_key)
-                # Use the other project, not the secret's, because we
+                decrypted_secret.name = secret_use.alias
-                # want to decrypt with the other project's key key.
+                decrypted_secrets.append(decrypted_secret)
-                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,
@@ -1752,7 +1716,7 @@ class Job(ConfigObject):
             # trusted context.
             for pb in itertools.chain(
                     self.pre_run, self.run, self.post_run, self.cleanup_run):
-                pb.addSecrets(frozen_secrets)
+                pb.addSecrets(decrypted_secrets)
                 if not pb.source_context.trusted:
                     self.post_review = True
 
@@ -4284,7 +4248,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.safe_dump(conf, sort_keys=False)
+        extract = yaml.dump(conf, sort_keys=False)
         lines = extract.split('\n')
         if len(lines) > 5:
             lines = lines[0:4]