From c9d8b4c132667c9eba92ac696047892e6f65e4cf Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Wed, 13 May 2020 17:56:36 +0100 Subject: gerrit: Shell-quote ssh command arguments The Secure Shell protocol does not allow passing an argument vector, only a command line that will be interpreted by a specific program (usually a shell, but in this case Gerrit) on the remote host. OpenSSH's ssh command simply concatenates the remote command arguments with spaces between them, so we need to shell-quote them first. Closes #8. --- lorrycontroller/gerrit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lorrycontroller') diff --git a/lorrycontroller/gerrit.py b/lorrycontroller/gerrit.py index 6ae24e3..4e77ba7 100644 --- a/lorrycontroller/gerrit.py +++ b/lorrycontroller/gerrit.py @@ -34,7 +34,8 @@ class Gerrit(object): '%s@%s' % (user, host)] def _ssh_command(self, command): - out = cliapp.runcmd(self._ssh_command_args + command) + quoted_args = [cliapp.shell_quote(x) for x in command] + out = cliapp.runcmd(self._ssh_command_args + quoted_args) if isinstance(out, bytes): out = out.decode('utf-8', errors='replace') return out -- cgit v1.2.1 From 38697c01cc12c4e8cc57b3bb372c5cf5f34ce167 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 14 May 2020 00:21:01 +0100 Subject: Introduce and use SshCommand class for running commands via ssh Most of the ssh parameters are given by a URL, which should be configurable. Additional options can be set by keyword arguments. runcmd already has a function like this, but it doesn't take a URL and runcmd.shell_quote doesn't handle empty strings correctly. Use this in the Gerrit host connector. It could be used in the Gitano host connector as well, but I want to avoid making further changes there that I can't test. --- lorrycontroller/gerrit.py | 30 +++++++++--------------------- lorrycontroller/hosts.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 21 deletions(-) (limited to 'lorrycontroller') diff --git a/lorrycontroller/gerrit.py b/lorrycontroller/gerrit.py index c18f2ed..dad964f 100644 --- a/lorrycontroller/gerrit.py +++ b/lorrycontroller/gerrit.py @@ -32,27 +32,15 @@ class GerritDownstream(hosts.DownstreamHost): ''' def __init__(self, app_settings): - # XXX These need to be configurable - host = 'localhost' - port = 29418 - user = 'lorry' - - self._ssh_command_args = [ - 'ssh', '-oStrictHostKeyChecking=no', '-oBatchMode=yes', '-p%i' % port, - '%s@%s' % (user, host)] - - def _ssh_command(self, command): - quoted_args = [cliapp.shell_quote(x) for x in command] - out = cliapp.runcmd(self._ssh_command_args + quoted_args) - if isinstance(out, bytes): - out = out.decode('utf-8', errors='replace') - return out + # XXX This needs to be configurable + url = 'ssh://lorry@localhost:29418' + self._ssh_command = hosts.SshCommand(url, StrictHostKeyChecking='no') def _has_project(self, name): # There's no 'does this project exist' command in Gerrit 2.9.4; 'list # all projects with this prefix' is as close we can get. - output = self._ssh_command([ + output = self._ssh_command.run([ 'gerrit', 'ls-projects', '--type=ALL', '--prefix=%s' % name]) projects = output.strip().split('\n') @@ -72,17 +60,17 @@ class GerritDownstream(hosts.DownstreamHost): logging.info('Project %s exists in local Gerrit already.', name) else: - self._ssh_command(['gerrit', 'create-project', name]) + self._ssh_command.run(['gerrit', 'create-project', name]) logging.info('Created %s project in local Gerrit.', name) # We can only set this metadata if we're the owner of the # repository. For now, ignore failures. try: if 'head' in metadata: - self._ssh_command(['gerrit', 'set-head', name, - '--new-head', metadata['head']]) + self._ssh_command.run(['gerrit', 'set-head', name, + '--new-head', metadata['head']]) if 'description' in metadata: - self._ssh_command(['gerrit', 'set-project', name, - '-d', metadata['description']]) + self._ssh_command.run(['gerrit', 'set-project', name, + '-d', metadata['description']]) except cliapp.AppException: pass diff --git a/lorrycontroller/hosts.py b/lorrycontroller/hosts.py index 39cae57..fb892dc 100644 --- a/lorrycontroller/hosts.py +++ b/lorrycontroller/hosts.py @@ -14,6 +14,10 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import abc +import shlex +import urllib.parse + +import cliapp class DownstreamHost(abc.ABC): @@ -119,3 +123,37 @@ class UpstreamHost(abc.ABC): DownstreamHost.prepare_repo. ''' pass + + +class SshCommand: + def __init__(self, urlstring, **options): + try: + url = urllib.parse.urlsplit(urlstring, allow_fragments=False) + url.port + except ValueError: + raise cliapp.AppException('Invalid URL: %s' % urlstring) + + if url.scheme != 'ssh': + raise cliapp.AppException('Not an SSH URL: %s' % urlstring) + if url.path not in ['', '/']: + raise cliapp.AppException('Unexpected path part in SSH URL') + if url.query != '': + raise cliapp.AppException('Unexpected query part in SSH URL') + if url.password is not None: + raise cliapp.AppException('Unexpected password in SSH URL') + + self._ssh_args = ['ssh', '-oBatchMode=yes'] + for key, value in options.items(): + self._ssh_args.append('-o%s=%s' % (key, value)) + if url.username is not None: + self._ssh_args.append('-oUser=%s' % url.username) + if url.port is not None: + self._ssh_args.append('-p%i' % url.port) + self._ssh_args.append(url.hostname) + + def run(self, args): + quoted_args = [shlex.quote(arg) for arg in args] + stdout = cliapp.runcmd(self._ssh_args + quoted_args) + if isinstance(stdout, bytes): + stdout = stdout.decode('utf-8', errors='replace') + return stdout -- cgit v1.2.1 From ce311a3c803515eb95c761a3a2838b093a4ea3cd Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 14 May 2020 00:45:02 +0100 Subject: Introduce and use downstream-{http,ssh}-url settings Replace the current hardcoded Downstream Host parameters with application settings. In general, Downstream Host connectors will use some combination of HTTP(S) and/or SSH, so these are added in lorry-controller-webapp. Currently only the gerrit connector will use downstream-ssh-url and only the gitlab connector will use downstream-http-url. Turn *on* StrictHostKeyChecking for the gerrit connector when not talking to localhost. Closes #4. --- lorrycontroller/gerrit.py | 11 ++++++++--- lorrycontroller/gitlab.py | 15 +++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) (limited to 'lorrycontroller') diff --git a/lorrycontroller/gerrit.py b/lorrycontroller/gerrit.py index dad964f..0970d63 100644 --- a/lorrycontroller/gerrit.py +++ b/lorrycontroller/gerrit.py @@ -32,9 +32,14 @@ class GerritDownstream(hosts.DownstreamHost): ''' def __init__(self, app_settings): - # XXX This needs to be configurable - url = 'ssh://lorry@localhost:29418' - self._ssh_command = hosts.SshCommand(url, StrictHostKeyChecking='no') + url = app_settings['downstream-ssh-url'] + if url is None: + url = 'ssh://lorry@localhost:29418' + key_check = 'no' + else: + key_check = 'yes' + self._ssh_command = hosts.SshCommand( + url, StrictHostKeyChecking=key_check) def _has_project(self, name): # There's no 'does this project exist' command in Gerrit 2.9.4; 'list diff --git a/lorrycontroller/gitlab.py b/lorrycontroller/gitlab.py index 4f70f0a..58bf67f 100644 --- a/lorrycontroller/gitlab.py +++ b/lorrycontroller/gitlab.py @@ -37,9 +37,8 @@ class MissingGitlabModuleError(Exception): pass -def _init_gitlab(host, token): +def _init_gitlab(url, token): if gitlab: - url = "http://" + host return gitlab.Gitlab(url, token) else: raise MissingGitlabModuleError('gitlab module missing\n' @@ -61,10 +60,10 @@ class GitlabDownstream(hosts.DownstreamHost): app_settings.require('gitlab-private-token') def __init__(self, app_settings): - # XXX This needs to be configurable - host = 'localhost' - - self.gl = _init_gitlab(host, app_settings['gitlab-private-token']) + url = app_settings['downstream-http-url'] + if url is None: + url = 'http://localhost/' + self.gl = _init_gitlab(url, app_settings['gitlab-private-token']) def prepare_repo(self, repo_path, metadata): @@ -131,8 +130,8 @@ class GitlabUpstream(hosts.UpstreamHost): def __init__(self, host_info): self._protocol = host_info['protocol'] - self.gl = _init_gitlab(host_info['host'], - host_info['type_params']['private-token']) + url = 'http://%(host)s/' % host_info + self.gl = _init_gitlab(url, host_info['type_params']['private-token']) def list_repos(self): '''List projects on a GitLab instance.''' -- cgit v1.2.1 From f5d6b9a5124dd133251aed8202e0ba8a4dda4f4a Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 14 May 2020 00:51:49 +0100 Subject: GitlabUpstream: Use HTTPS unless explicitly configured not to Currently we always use the 'http' scheme to talk to GitLab's REST API. Use the 'https' scheme instead, if the configured protocol is 'https' or 'ssh' (as the SSH server doesn't expose this API). --- lorrycontroller/gitlab.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lorrycontroller') diff --git a/lorrycontroller/gitlab.py b/lorrycontroller/gitlab.py index 58bf67f..e77e4bb 100644 --- a/lorrycontroller/gitlab.py +++ b/lorrycontroller/gitlab.py @@ -130,7 +130,10 @@ class GitlabUpstream(hosts.UpstreamHost): def __init__(self, host_info): self._protocol = host_info['protocol'] - url = 'http://%(host)s/' % host_info + if self._protocol == 'ssh': + url = 'https://%(host)s/' % host_info + else: + url = '%(protocol)s://%(host)s/' % host_info self.gl = _init_gitlab(url, host_info['type_params']['private-token']) def list_repos(self): -- cgit v1.2.1