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 <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@thoughtworks.com>

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],
+        keyscan = subprocess.run(['ssh-keyscan', hostname],
-                              stdout=subprocess.PIPE,
+                                 stdout=subprocess.PIPE,
-                              stderr=subprocess.DEVNULL, check=True)
+                                 stderr=subprocess.DEVNULL)
-
+        keys = keyscan.stdout.decode().splitlines()
-        with tempfile.NamedTemporaryFile(delete=False) as tmpfil:
-            tmpfil.write(res1.stdout)
-
         # Generate user-friendly fingerprints of public keys
-        res2 = subprocess.run(['ssh-keygen', '-l', '-f', tmpfil.name],
+        keygen = subprocess.run(['ssh-keygen', '-l', '-f', '-'],
-                              stdout=subprocess.PIPE)
+                                  input=keyscan.stdout,
-        os.remove(tmpfil.name)
+                                  stdout=subprocess.PIPE)
-        keys = res2.stdout.decode().splitlines()
+        fingerprints = keygen.stdout.decode().splitlines()
 
-        # Create a list of tuples of (algorithm, fingerprint)
+        return zip(keys, fingerprints)
-        return [(key.rsplit(' ', 1)[-1].strip('()'), key) for key in keys]

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(
+            known_hosts_file.write(ssh_public_key + '\n')
-                ['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')
 
     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']
+        ssh_public_key = form.cleaned_data['ssh_public_key']
-        self._add_ssh_hostkey(self._get_hostname(), key_type)
+        self._add_ssh_hostkey(ssh_public_key)
         messages.success(self.request, _('SSH host verified.'))
         return self._add_remote_repository()