Use the security context of the playbook when checking out roles

When the executor checks out and runs content, the security context
(trusted or untrusted) comes in to play in two ways: whether
speculative merging should be used when checking out the content
and the level of access to ansible.

This is straightforward for playbooks: when running an untrusted
playbook, use the speculatively merged repo and the untrusted
ansible environment.  When running a trusted playbook, only use
the branch tip of the playbook's repo, and the trusted ansible
environment.

When we consider roles, we also need to consider whether to use
the speculatively merged role repo, or the branch tip.  The current
code uses the security context of the role repo to decide which to
do (untrusted role repo uses the speculatively merged repo,
trusted role repo uses branch tip).  However this presents a problem.

Consider a job defined in a trusted repo which uses a role defined
in an untrusted repo.  The playbook will be run in the trusted
execution context, but the role it depends on will come from the
speculatively-merged role repo.  This means a user could propose a
change which Depends-On a change to the role repo and cause mischief.

The author of the job in the trusted repo should be able to rely on
the fact that when that job runs, both the playbook and the roles
used by that playbook only contain code that is actually in the
respective repositories.  Likewise, should a job in an untrusted
repo inherit from that job, when its playbook runs, it should be
able to use a speculative change to the role repo.

In short, when we are running a trusted playbook, we should use the
branch tip of all role repos used by that playbook.  And when we run
an untrusted playbook, we should use any speculatively merged changes
to those roles.

Since we can run both kinds of playbooks in a single job, this change
prepares roles in both manners, if necessary.  If any playbook run by
the job is untrusted, we will prepare the speculatively-merged repo
as a role.  If any playbook is trusted (or the role does not appear in
the dependency chain for the change), we prepare the branch tip of the
role repo.  When we run the playbooks, we use the appropriate version
of each role based on the security context of the playbook.

Change-Id: I06dd3851a8f805dba9afe1b4a0eaa1b2fdd4efa2
This commit is contained in:
James E. Blair 2017-04-28 08:14:48 -07:00
parent 2a53567014
commit 6563e4bc49
3 changed files with 102 additions and 35 deletions

View File

@ -408,7 +408,7 @@ class JobParser(object):
return model.ZuulRole(role.get('name', name),
project.connection_name,
project.name, trusted)
project.name)
class ProjectTemplateParser(object):

View File

@ -108,7 +108,8 @@ class JobDir(object):
self.pre_playbooks = []
self.post_playbooks = []
self.roles = []
self.roles_path = []
self.trusted_roles_path = []
self.untrusted_roles_path = []
self.untrusted_config = os.path.join(
self.ansible_root, 'untrusted.cfg')
self.trusted_config = os.path.join(self.ansible_root, 'trusted.cfg')
@ -142,6 +143,10 @@ class JobDir(object):
count = len(self.roles)
root = os.path.join(self.ansible_root, 'role_%i' % (count,))
os.makedirs(root)
trusted = os.path.join(root, 'trusted')
os.makedirs(trusted)
untrusted = os.path.join(root, 'untrusted')
os.makedirs(untrusted)
self.roles.append(root)
return root
@ -601,9 +606,9 @@ class AnsibleJob(object):
repo.delete_remote(repo.remotes.origin)
# is the playbook in a repo that we have already prepared?
self.preparePlaybookRepos(args)
trusted, untrusted = self.preparePlaybookRepos(args)
self.prepareRoles(args)
self.prepareRoles(args, trusted, untrusted)
# TODOv3: Ansible the ansible thing here.
self.prepareAnsibleFiles(args)
@ -737,15 +742,24 @@ class AnsibleJob(object):
return None
def preparePlaybookRepos(self, args):
trusted = untrusted = False
for playbook in args['pre_playbooks']:
jobdir_playbook = self.jobdir.addPrePlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=True)
if playbook['trusted']:
trusted = True
else:
untrusted = True
for playbook in args['playbooks']:
jobdir_playbook = self.jobdir.addPlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=False)
if playbook['trusted']:
trusted = True
else:
untrusted = True
if jobdir_playbook.path is not None:
self.jobdir.playbook = jobdir_playbook
break
@ -756,6 +770,11 @@ class AnsibleJob(object):
jobdir_playbook = self.jobdir.addPostPlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=True)
if playbook['trusted']:
trusted = True
else:
untrusted = True
return (trusted, untrusted)
def preparePlaybookRepo(self, jobdir_playbook, playbook, args, required):
self.log.debug("Prepare playbook repo for %s" % (playbook,))
@ -799,11 +818,11 @@ class AnsibleJob(object):
required=required,
trusted=playbook['trusted'])
def prepareRoles(self, args):
def prepareRoles(self, args, trusted, untrusted):
for role in args['roles']:
if role['type'] == 'zuul':
root = self.jobdir.addRole()
self.prepareZuulRole(args, role, root)
self.prepareZuulRole(args, role, root, trusted, untrusted)
def findRole(self, path, trusted=False):
d = os.path.join(path, 'tasks')
@ -826,17 +845,22 @@ class AnsibleJob(object):
self._blockPluginDirs(os.path.join(path, entry))
return path
def prepareZuulRole(self, args, role, root):
def prepareZuulRole(self, args, role, root, trusted, untrusted):
self.log.debug("Prepare zuul role for %s" % (role,))
# Check out the role repo if needed
source = self.executor_server.connections.getSource(
role['connection'])
project = source.getProject(role['project'])
role_repo = None
if not role['trusted']:
# This is a project repo, so it is safe to use the already
# checked out version (from speculative merging) of the
# role
untrusted_role_repo = None
trusted_role_repo = None
trusted_root = os.path.join(root, 'trusted')
untrusted_root = os.path.join(root, 'untrusted')
name = role['target_name']
if untrusted:
# There is at least one untrusted playbook. For that
# case, use the already checked out version (from
# speculative merging) of the role.
for i in args['items']:
if (i['connection'] == role['connection'] and
@ -847,27 +871,70 @@ class AnsibleJob(object):
path = os.path.join(self.jobdir.src_root,
project.canonical_hostname,
project.name)
link = os.path.join(root, role['name'])
# The name of the symlink is the requested name of
# the role (which may be the repo name or may be
# something else; this can come into play if this
# is a bare role).
link = os.path.join(untrusted_root, name)
link = os.path.realpath(link)
if not link.startswith(os.path.realpath(untrusted_root)):
raise Exception("Invalid role name %s", name)
os.symlink(path, link)
role_repo = link
untrusted_role_repo = link
break
# The role repo is either a config repo, or it isn't in
# the stack of changes we are testing, so check out the branch
# tip into a dedicated space.
if not role_repo:
merger = self.executor_server._getMerger(root)
if trusted or not untrusted_role_repo:
# There is at least one trusted playbook which will need a
# trusted checkout of the role, or the role did not appear
# in the dependency chain for the change (in which case,
# there is no existing untrusted checkout of it). Check
# out the branch tip into a dedicated space.
merger = self.executor_server._getMerger(trusted_root)
merger.checkoutBranch(role['connection'], project.name,
'master')
role_repo = os.path.join(root, project.canonical_hostname,
project.name)
orig_repo_path = os.path.join(trusted_root,
project.canonical_hostname,
project.name)
if name != project.name:
# The requested name of the role is not the same as
# the project name, so rename the git repo as the
# requested name. It is the only item in this
# directory, so we don't need to worry about
# collisions.
target = os.path.join(trusted_root,
project.canonical_hostname,
name)
target = os.path.realpath(target)
if not target.startswith(os.path.realpath(trusted_root)):
raise Exception("Invalid role name %s", name)
os.rename(orig_repo_path, target)
trusted_role_repo = target
else:
trusted_role_repo = orig_repo_path
role_path = self.findRole(role_repo, trusted=role['trusted'])
if role_path is None:
# In the case of a bare role, add the containing directory
role_path = os.path.join(root, project.canonical_hostname)
self.jobdir.roles_path.append(role_path)
if not untrusted_role_repo:
# In the case that there was no untrusted checkout,
# use the trusted checkout.
untrusted_role_repo = trusted_role_repo
untrusted_root = trusted_root
if untrusted:
untrusted_role_path = self.findRole(untrusted_role_repo,
trusted=False)
if untrusted_role_path is None:
# In the case of a bare role, add the containing directory
untrusted_role_path = os.path.join(untrusted_root,
project.canonical_hostname)
self.jobdir.untrusted_roles_path.append(untrusted_role_path)
if trusted:
trusted_role_path = self.findRole(trusted_role_repo,
trusted=True)
if trusted_role_path is None:
# In the case of a bare role, add the containing directory
trusted_role_path = os.path.join(trusted_root,
project.canonical_hostname)
self.jobdir.trusted_roles_path.append(trusted_role_path)
def prepareAnsibleFiles(self, args):
keys = []
@ -909,9 +976,6 @@ class AnsibleJob(object):
config.write('gathering = explicit\n')
config.write('library = %s\n'
% self.executor_server.library_dir)
if self.jobdir.roles_path:
config.write('roles_path = %s\n' %
':'.join(self.jobdir.roles_path))
config.write('command_warnings = False\n')
config.write('callback_plugins = %s\n'
% self.executor_server.callback_dir)
@ -924,6 +988,12 @@ class AnsibleJob(object):
% self.executor_server.action_dir)
config.write('lookup_plugins = %s\n'
% self.executor_server.lookup_dir)
roles_path = self.jobdir.untrusted_roles_path
else:
roles_path = self.jobdir.trusted_roles_path
if roles_path:
config.write('roles_path = %s\n' % ':'.join(roles_path))
# On trusted jobs, we want to prevent the printing of args,
# since trusted jobs might have access to secrets that they may

View File

@ -673,11 +673,10 @@ class Role(object):
class ZuulRole(Role):
"""A reference to an ansible role in a Zuul project."""
def __init__(self, target_name, connection_name, project_name, trusted):
def __init__(self, target_name, connection_name, project_name):
super(ZuulRole, self).__init__(target_name)
self.connection_name = connection_name
self.project_name = project_name
self.trusted = trusted
def __repr__(self):
return '<ZuulRole %s %s>' % (self.project_name, self.target_name)
@ -687,8 +686,7 @@ class ZuulRole(Role):
return False
return (super(ZuulRole, self).__eq__(other) and
self.connection_name == other.connection_name,
self.project_name == other.project_name,
self.trusted == other.trusted)
self.project_name == other.project_name)
def toDict(self):
# Render to a dict to use in passing json to the executor
@ -696,7 +694,6 @@ class ZuulRole(Role):
d['type'] = 'zuul'
d['connection'] = self.connection_name
d['project'] = self.project_name
d['trusted'] = self.trusted
return d