From fa3e2ea86b57283855b6f361e32696d339bbfb19 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 24 Jun 2019 17:25:11 -0700 Subject: [PATCH] backups: Fix issue with verifying SSH host keys Ensure that the fingerprint accepted is the one verified by user. If they fingerprints and public keys are retrieved separately, there is chance that what was verified by the user is not what is added to the known hosts file. - Avoid creating a temporary file when fetching keys Signed-off-by: Sunil Mohan Adapa Reviewed-by: Joseph Nuthalapati --- plinth/modules/backups/forms.py | 22 +++++++++------------- plinth/modules/backups/views.py | 13 ++++--------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/plinth/modules/backups/forms.py b/plinth/modules/backups/forms.py index a070046fd..b331f0334 100644 --- a/plinth/modules/backups/forms.py +++ b/plinth/modules/backups/forms.py @@ -199,18 +199,14 @@ class VerifySshHostkeyForm(forms.Form): def _get_all_public_keys(hostname): """Use ssh-keyscan to get all the SSH public keys of a host.""" # Fetch public keys of ssh remote - res1 = subprocess.run(['ssh-keyscan', hostname], - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, check=True) - - with tempfile.NamedTemporaryFile(delete=False) as tmpfil: - tmpfil.write(res1.stdout) - + keyscan = subprocess.run(['ssh-keyscan', hostname], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL) + keys = keyscan.stdout.decode().splitlines() # Generate user-friendly fingerprints of public keys - res2 = subprocess.run(['ssh-keygen', '-l', '-f', tmpfil.name], - stdout=subprocess.PIPE) - os.remove(tmpfil.name) - keys = res2.stdout.decode().splitlines() + keygen = subprocess.run(['ssh-keygen', '-l', '-f', '-'], + input=keyscan.stdout, + stdout=subprocess.PIPE) + fingerprints = keygen.stdout.decode().splitlines() - # Create a list of tuples of (algorithm, fingerprint) - return [(key.rsplit(' ', 1)[-1].strip('()'), key) for key in keys] + return zip(keys, fingerprints) diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py index 4ae106096..f74d8d32f 100644 --- a/plinth/modules/backups/views.py +++ b/plinth/modules/backups/views.py @@ -337,19 +337,14 @@ class VerifySshHostkeyView(SuccessMessageMixin, FormView): return hostname.split('%')[0] # XXX: Likely incorrect to split @staticmethod - def _add_ssh_hostkey(hostname, key_type): + def _add_ssh_hostkey(ssh_public_key): """Add the given SSH key to known_hosts.""" known_hosts_path = get_known_hosts_path() known_hosts_path.parent.mkdir(parents=True, exist_ok=True) known_hosts_path.touch() with known_hosts_path.open('a') as known_hosts_file: - key_line = subprocess.run( - ['ssh-keyscan', '-t', key_type, hostname], - stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, - check=True).stdout.decode().strip() - known_hosts_file.write(key_line) - known_hosts_file.write('\n') + known_hosts_file.write(ssh_public_key + '\n') def get(self, *args, **kwargs): """Skip this view if host is already verified.""" @@ -361,8 +356,8 @@ class VerifySshHostkeyView(SuccessMessageMixin, FormView): def form_valid(self, form): """Create and store the repository.""" - key_type = form.cleaned_data['ssh_public_key'] - self._add_ssh_hostkey(self._get_hostname(), key_type) + ssh_public_key = form.cleaned_data['ssh_public_key'] + self._add_ssh_hostkey(ssh_public_key) messages.success(self.request, _('SSH host verified.')) return self._add_remote_repository()