Copy git ssh-id-wrapper to /tmp only if necessary (Fixes #10582, Fixes #19532)

This adds a check that only copies the ssh wrapper to a temporary
location if git is to be run by a specific user and at the same time
the predefined wrapper file is not executable by "others" by verifying
every path part for the others executable bit.

By doing this the temp file is kept only as a last resort which should
work with salt-ssh as per bug #19532, while avoiding a needless copy
on /tmp which could be potentially mounted with noexec as per
bug #10582.
This commit is contained in:
Massimiliano Torromeo 2017-09-27 11:14:40 +02:00
parent bd879eb66e
commit a63e6ca963
No known key found for this signature in database
GPG Key ID: 94DD2393DA2EE423

View File

@ -9,6 +9,7 @@ import copy
import logging
import os
import re
import stat
# Import salt libs
import salt.utils
@ -115,6 +116,22 @@ def _expand_path(cwd, user):
return os.path.join(os.path.expanduser(to_expand), str(cwd))
def _path_is_executable_others(path):
'''
Check every part of path for executable permission
'''
prevpath = None
while path and path != prevpath:
try:
if not os.stat(path).st_mode & stat.S_IXOTH:
return False
except OSError:
return False
prevpath = path
path, _ = os.path.split(path)
return True
def _format_opts(opts):
'''
Common code to inspect opts and split them if necessary
@ -214,11 +231,12 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
}
# copy wrapper to area accessible by ``runas`` user
# currently no suppport in windows for wrapping git ssh
# currently no support in windows for wrapping git ssh
ssh_id_wrapper = os.path.join(
salt.utils.templates.TEMPLATE_DIRNAME,
'git/ssh-id-wrapper'
)
tmp_ssh_wrapper = None
if salt.utils.is_windows():
for suffix in ('', ' (x86)'):
ssh_exe = (
@ -235,12 +253,14 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
# Use the windows batch file instead of the bourne shell script
ssh_id_wrapper += '.bat'
env['GIT_SSH'] = ssh_id_wrapper
elif not user or _path_is_executable_others(ssh_id_wrapper):
env['GIT_SSH'] = ssh_id_wrapper
else:
tmp_file = salt.utils.files.mkstemp()
salt.utils.files.copyfile(ssh_id_wrapper, tmp_file)
os.chmod(tmp_file, 0o500)
os.chown(tmp_file, __salt__['file.user_to_uid'](user), -1)
env['GIT_SSH'] = tmp_file
tmp_ssh_wrapper = salt.utils.files.mkstemp()
salt.utils.files.copyfile(ssh_id_wrapper, tmp_ssh_wrapper)
os.chmod(tmp_ssh_wrapper, 0o500)
os.chown(tmp_ssh_wrapper, __salt__['file.user_to_uid'](user), -1)
env['GIT_SSH'] = tmp_ssh_wrapper
if 'salt-call' not in _salt_cli \
and __salt__['ssh.key_is_encrypted'](id_file):
@ -270,13 +290,25 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
redirect_stderr=redirect_stderr,
**kwargs)
finally:
if not salt.utils.is_windows() and 'GIT_SSH' in env:
os.remove(env['GIT_SSH'])
# Cleanup the temporary ssh wrapper file
try:
__salt__['file.remove'](tmp_ssh_wrapper)
log.debug('Removed ssh wrapper file %s', tmp_ssh_wrapper)
except AttributeError:
# No wrapper was used
pass
except (SaltInvocationError, CommandExecutionError) as exc:
log.warning('Failed to remove ssh wrapper file %s: %s', tmp_ssh_wrapper, exc)
# Cleanup the temporary identity file
if tmp_identity_file and os.path.exists(tmp_identity_file):
log.debug('Removing identity file {0}'.format(tmp_identity_file))
try:
__salt__['file.remove'](tmp_identity_file)
log.debug('Removed identity file %s', tmp_identity_file)
except AttributeError:
# No identify file was used
pass
except (SaltInvocationError, CommandExecutionError) as exc:
log.warning('Failed to remove identity file %s: %s', tmp_identity_file, exc)
# If the command was successful, no need to try additional IDs
if result['retcode'] == 0: