Set allowed-projects on untrusted jobs with secrets
It is possible to circumvent the use of `allowed-projects` in untrusted projects by creating a change which `Depends-On` a change which alters a project definition. This behavior may be unexpected, so documentation has been updated with warnings to avoid relying on it in sensitive cases. It may have been possible to expose a secret, or use resources protected by a secret, if a job using a secret was defined in an untrusted project on a system with an independent pre-merge post-review pipeline -- that is, a pipeline with `post-review` set to true, `manager` set to `independent`, and which operated on changes before they merged. To prevent disclosure or use in this situation, `allowed-projects` is now automatically set to the current project when a secret is used in a job defined in an untrusted project, and it can not be overridden. The test_trusted_secret_inheritance_gate test is removed because it only tested that jobs with secrets in an untrusted repo were able to run in a trusted repo. That is no longer possible. Change-Id: I77f6a011bca08a2433137dc29597b7cc2757adb1 Story: 2004837 Task: 29037
This commit is contained in:
parent
6fccffe49b
commit
ed7f9da75e
|
@ -562,6 +562,13 @@ Here is an example of two job definitions:
|
|||
specified in a project's pipeline, set this attribute to
|
||||
``true``.
|
||||
|
||||
.. warning::
|
||||
|
||||
It is possible to circumvent the use of `final` in an
|
||||
:term:`untrusted-project` by creating a change which
|
||||
`Depends-On` a change which alters `final`. This limitation
|
||||
does not apply to jobs in a :term:`config-project`.
|
||||
|
||||
.. attr:: protected
|
||||
:default: false
|
||||
|
||||
|
@ -569,12 +576,28 @@ Here is an example of two job definitions:
|
|||
from this job. Once this is set to ``true`` it cannot be reset to
|
||||
``false``.
|
||||
|
||||
.. warning::
|
||||
|
||||
It is possible to circumvent the use of `protected` in an
|
||||
:term:`untrusted-project` by creating a change which
|
||||
`Depends-On` a change which alters `protected`. This
|
||||
limitation does not apply to jobs in a
|
||||
:term:`config-project`.
|
||||
|
||||
.. attr:: abstract
|
||||
:default: false
|
||||
|
||||
To indicate a job is not intended to be run directly, but
|
||||
instead must be inherited from, set this attribute to ``true``.
|
||||
|
||||
.. warning::
|
||||
|
||||
It is possible to circumvent the use of `abstract` in an
|
||||
:term:`untrusted-project` by creating a change which
|
||||
`Depends-On` a change which alters `abstract`. This
|
||||
limitation does not apply to jobs in a
|
||||
:term:`config-project`.
|
||||
|
||||
.. attr:: success-message
|
||||
:default: SUCCESS
|
||||
|
||||
|
@ -1009,6 +1032,19 @@ Here is an example of two job definitions:
|
|||
it should be able to run this job, then it must be explicitly
|
||||
listed. By default, all projects may use the job.
|
||||
|
||||
If a :attr:`job.secrets` is used in a job definition in an
|
||||
:term:`untrusted-project`, `allowed-projects` is automatically
|
||||
set to the current project only, and can not be overridden.
|
||||
|
||||
.. warning::
|
||||
|
||||
It is possible to circumvent the use of `allowed-projects` in
|
||||
an :term:`untrusted-project` by creating a change which
|
||||
`Depends-On` a change which alters `allowed-projects`. This
|
||||
limitation does not apply to jobs in a
|
||||
:term:`config-project`, or jobs in an `untrusted-project`
|
||||
which use a secret.
|
||||
|
||||
.. attr:: post-review
|
||||
:default: false
|
||||
|
||||
|
@ -1022,6 +1058,15 @@ Here is an example of two job definitions:
|
|||
it will remain set for all child jobs and variants (it can not be
|
||||
set to ``false``).
|
||||
|
||||
.. warning::
|
||||
|
||||
It is possible to circumvent the use of `post-review` in an
|
||||
:term:`untrusted-project` by creating a change which
|
||||
`Depends-On` a change which alters `post-review`. This
|
||||
limitation does not apply to jobs in a
|
||||
:term:`config-project`, or jobs in an `untrusted-project`
|
||||
which use a secret.
|
||||
|
||||
.. attr:: branches
|
||||
|
||||
A regular expression (or list of regular expressions) which
|
||||
|
@ -1372,7 +1417,9 @@ indicate the job should only run in post-review pipelines.
|
|||
|
||||
If a job with secrets is unsafe to be used by other projects, the
|
||||
:attr:`job.allowed-projects` attribute can be used to restrict the
|
||||
projects which can invoke that job.
|
||||
projects which can invoke that job. If a job with secrets is defined
|
||||
in an `untrusted-project`, `allowed-projects` is automatically set to
|
||||
that project only, and can not be overridden.
|
||||
|
||||
Secrets, like most configuration items, are unique within a tenant,
|
||||
though a secret may be defined on multiple branches of the same
|
||||
|
|
|
@ -0,0 +1,23 @@
|
|||
---
|
||||
security:
|
||||
- |
|
||||
Jobs with secrets in untrusted projects now automatically set
|
||||
`allowed-projects`.
|
||||
|
||||
It is possible to circumvent the use of `allowed-projects` in
|
||||
untrusted projects by creating a change which `Depends-On` a
|
||||
change which alters a project definition. This behavior may be
|
||||
unexpected, so documentation has been updated with warnings to
|
||||
avoid relying on it in sensitive cases.
|
||||
|
||||
It may have been possible to expose a secret, or use resources
|
||||
protected by a secret, if a job using a secret was defined in an
|
||||
untrusted project on a system with an independent pre-merge
|
||||
post-review pipeline -- that is, a pipeline with `post-review` set
|
||||
to true, `manager` set to `independent`, and which operated on
|
||||
changes before they merged.
|
||||
|
||||
To prevent disclosure or use in this situation, `allowed-projects`
|
||||
is now automatically set to the current project when a secret is
|
||||
used in a job defined in an untrusted project, and it can not be
|
||||
overridden.
|
|
@ -3,6 +3,12 @@
|
|||
parent: restricted-job
|
||||
allowed-projects:
|
||||
- org/project2
|
||||
|
||||
- job:
|
||||
name: test-project2b
|
||||
parent: restricted-job
|
||||
allowed-projects:
|
||||
- org/project2
|
||||
|
||||
- project:
|
||||
name: org/project2
|
||||
|
|
|
@ -62,11 +62,6 @@
|
|||
- trusted-secrets
|
||||
- trusted-secrets-trusted-child
|
||||
- trusted-secrets-untrusted-child
|
||||
gate:
|
||||
jobs:
|
||||
- untrusted-secrets
|
||||
- untrusted-secrets-trusted-child
|
||||
- untrusted-secrets-untrusted-child
|
||||
|
||||
- secret:
|
||||
name: trusted-secret
|
||||
|
|
|
@ -728,6 +728,77 @@ class TestAllowedProjects(ZuulTestCase):
|
|||
dict(name='restricted-job', result='SUCCESS', changes='1,1'),
|
||||
], ordered=False)
|
||||
|
||||
def test_allowed_projects_dynamic_config(self):
|
||||
# It is possible to circumvent allowed-projects with a
|
||||
# depends-on.
|
||||
in_repo_conf2 = textwrap.dedent(
|
||||
"""
|
||||
- job:
|
||||
name: test-project2b
|
||||
parent: restricted-job
|
||||
allowed-projects:
|
||||
- org/project1
|
||||
""")
|
||||
in_repo_conf1 = textwrap.dedent(
|
||||
"""
|
||||
- project:
|
||||
check:
|
||||
jobs:
|
||||
- test-project2b
|
||||
""")
|
||||
|
||||
file_dict = {'zuul.yaml': in_repo_conf2}
|
||||
A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A',
|
||||
files=file_dict)
|
||||
file_dict = {'zuul.yaml': in_repo_conf1}
|
||||
B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B',
|
||||
files=file_dict)
|
||||
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
B.subject, A.data['id'])
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
self.assertHistory([
|
||||
dict(name='test-project2b', result='SUCCESS', changes='1,1 2,1'),
|
||||
], ordered=False)
|
||||
|
||||
def test_allowed_projects_dynamic_config_secret(self):
|
||||
# It is not possible to circumvent allowed-projects with a
|
||||
# depends-on if there is a secret involved.
|
||||
in_repo_conf2 = textwrap.dedent(
|
||||
"""
|
||||
- secret:
|
||||
name: project2_secret
|
||||
data: {}
|
||||
- job:
|
||||
name: test-project2b
|
||||
parent: restricted-job
|
||||
secrets: project2_secret
|
||||
allowed-projects:
|
||||
- org/project1
|
||||
""")
|
||||
in_repo_conf1 = textwrap.dedent(
|
||||
"""
|
||||
- project:
|
||||
check:
|
||||
jobs:
|
||||
- test-project2b
|
||||
""")
|
||||
|
||||
file_dict = {'zuul.yaml': in_repo_conf2}
|
||||
A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A',
|
||||
files=file_dict)
|
||||
file_dict = {'zuul.yaml': in_repo_conf1}
|
||||
B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B',
|
||||
files=file_dict)
|
||||
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
B.subject, A.data['id'])
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
self.assertHistory([])
|
||||
self.assertEqual(B.reported, 1)
|
||||
self.assertIn('Project org/project1 is not allowed '
|
||||
'to run job test-project2b', B.messages[0])
|
||||
|
||||
|
||||
class TestCentralJobs(ZuulTestCase):
|
||||
tenant_config_file = 'config/central-jobs/main.yaml'
|
||||
|
@ -3764,21 +3835,6 @@ class TestSecretInheritance(ZuulTestCase):
|
|||
|
||||
self._checkTrustedSecrets()
|
||||
|
||||
def test_untrusted_secret_inheritance_gate(self):
|
||||
A = self.fake_gerrit.addFakeChange('common-config', 'master', 'A')
|
||||
A.addApproval('Code-Review', 2)
|
||||
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
|
||||
self.waitUntilSettled()
|
||||
self.assertHistory([
|
||||
dict(name='untrusted-secrets', result='SUCCESS', changes='1,1'),
|
||||
dict(name='untrusted-secrets-trusted-child',
|
||||
result='SUCCESS', changes='1,1'),
|
||||
dict(name='untrusted-secrets-untrusted-child',
|
||||
result='SUCCESS', changes='1,1'),
|
||||
], ordered=False)
|
||||
|
||||
self._checkUntrustedSecrets()
|
||||
|
||||
def test_untrusted_secret_inheritance_check(self):
|
||||
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
|
||||
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
|
||||
|
|
|
@ -662,9 +662,14 @@ class JobParser(object):
|
|||
# A job in an untrusted repo that uses secrets requires
|
||||
# special care. We must note this, and carry this flag
|
||||
# through inheritance to ensure that we don't run this job in
|
||||
# an unsafe check pipeline.
|
||||
# an unsafe check pipeline. We must also set allowed-projects
|
||||
# to only the current project, as otherwise, other projects
|
||||
# might be able to cause something to happen with the secret
|
||||
# by using a depends-on header.
|
||||
if secrets and not conf['_source_context'].trusted:
|
||||
job.post_review = True
|
||||
job.allowed_projects = frozenset((
|
||||
conf['_source_context'].project.name,))
|
||||
|
||||
if (conf.get('timeout') and
|
||||
self.pcontext.tenant.max_job_timeout != -1 and
|
||||
|
@ -798,7 +803,8 @@ class JobParser(object):
|
|||
job.group_variables = group_variables
|
||||
|
||||
allowed_projects = conf.get('allowed-projects', None)
|
||||
if allowed_projects:
|
||||
# See note above at "post-review".
|
||||
if allowed_projects and not job.allowed_projects:
|
||||
allowed = []
|
||||
for p in as_list(allowed_projects):
|
||||
(trusted, project) = self.pcontext.tenant.getProject(p)
|
||||
|
|
Loading…
Reference in New Issue