fix(gstfsd): random password salt, timeouts, loopback-only daemon

- Hash the root password with a random SHA-512 salt instead of the fixed
  "$6$kgPoiREy".
- Talk to gstfsd through one helper with a 120 s socket timeout; an
  unreachable daemon or a bad reply is an error message, not a 500.
- gstfsd refuses to listen on a non-loopback address unless
  GSTFSD_ALLOW_REMOTE=1: it is an unauthenticated root service.
- gstfsd: encode replies (sendall(str) raised TypeError on Python 3, so
  the success path never worked) and use octal chmod modes.
- README: what gstfsd does and how to allow it on a remote compute.

README.md

@@ -315,7 +315,7 @@ sudo -u www-data ssh-copy-id root@<compute-node-ip>
 
 ### 3. Install or Update `gstfsd` Daemon
 
-The `gstfsd` daemon provides guest filesystem inspection and stats on hypervisors:
+The `gstfsd` daemon sets the root password and SSH key of a shut-off VM. It listens on 127.0.0.1 only; for a remote compute set `GSTFSD_BIND_HOST` and `GSTFSD_ALLOW_REMOTE=1` (it is unauthenticated, so trusted networks only):
 
 ```bash
 curl -fsSL https://raw.githubusercontent.com/retspen/webvirtcloud/master/conf/daemon/gstfsd | sudo tee /usr/local/bin/gstfsd > /dev/null

conf/daemon/gstfsd

@@ -3,16 +3,37 @@
 # gstfsd - WebVirtCloud daemon for managing VM's filesystem
 #
 
+import ipaddress
 import os
 import socketserver
 import json
-import guestfs
 import re
+import sys
 
 PORT = int(os.environ.get("GSTFSD_PORT", 16510))
 ADDRESS = os.environ.get("GSTFSD_BIND_HOST", "127.0.0.1")
 
 
+def is_loopback(host):
+    try:
+        return ipaddress.ip_address(host).is_loopback
+    except ValueError:
+        return host == "localhost"
+
+
+# gstfsd is an unauthenticated root service that rewrites any VM's
+# /etc/shadow and authorized_keys, so it only listens on loopback unless
+# explicitly allowed (ROADMAP S-08).
+if not is_loopback(ADDRESS) and os.environ.get("GSTFSD_ALLOW_REMOTE") != "1":
+    sys.stderr.write(
+        "gstfsd: refusing to listen on %s (unauthenticated root service). "
+        "Set GSTFSD_ALLOW_REMOTE=1 to allow it on a trusted network.\n" % ADDRESS
+    )
+    sys.exit(1)
+
+import guestfs  # noqa: E402
+
+
 class MyTCPServer(socketserver.ThreadingTCPServer):
     allow_reuse_address = True
 
@@ -38,15 +59,15 @@ class MyTCPServerHandler(socketserver.BaseRequestHandler):
                             new_root_hash = "root:" + data['passwd'] + ":"
                             file_shadow_new = re.sub('^root:.*?:', new_root_hash, file_shadow)
                             gfs.write('/etc/shadow', file_shadow_new)
-                            gfs.chmod(640, '/etc/shadow')
-                            self.request.sendall(json.dumps({'return': 'success'}))
+                            gfs.chmod(0o640, '/etc/shadow')
+                            self.request.sendall(json.dumps({'return': 'success'}).encode())
                         if data['action'] == 'publickey':
                             if not gfs.is_dir('/root/.ssh'):
                                 gfs.mkdir('/root/.ssh')
-                                gfs.chmod(700, "/root/.ssh")
+                                gfs.chmod(0o700, "/root/.ssh")
                             gfs.write('/root/.ssh/authorized_keys', data['key'])
-                            gfs.chmod(600, '/root/.ssh/authorized_keys')
-                            self.request.sendall(json.dumps({'return': 'success'}))
+                            gfs.chmod(0o600, '/root/.ssh/authorized_keys')
+                            self.request.sendall(json.dumps({'return': 'success'}).encode())
                     gfs.umount(part)
                 except RuntimeError:
                     pass

instances/test_gstfsd.py

@@ -0,0 +1,31 @@
+import os
+import signal
+import subprocess
+import sys
+import unittest
+from pathlib import Path
+
+GSTFSD = Path(__file__).resolve().parent.parent / "conf" / "daemon" / "gstfsd"
+
+
+class GstfsdBindTestCase(unittest.TestCase):
+    """ROADMAP S-08: gstfsd is an unauthenticated root service; no remote bind by default."""
+
+    def _run(self, **env):
+        # novncd (imported by other tests) sets SIGCHLD to SIG_IGN, which hides exit codes.
+        previous = signal.signal(signal.SIGCHLD, signal.SIG_DFL)
+        try:
+            return subprocess.run(
+                [sys.executable, str(GSTFSD)],
+                env=dict(os.environ, **env),
+                capture_output=True,
+                text=True,
+                timeout=20,
+            )
+        finally:
+            signal.signal(signal.SIGCHLD, previous)
+
+    def test_refuses_non_loopback_bind_without_opt_in(self):
+        result = self._run(GSTFSD_BIND_HOST="0.0.0.0")
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("GSTFSD_ALLOW_REMOTE", result.stderr)

instances/test_root_pass.py

@@ -1,4 +1,5 @@
 import json
+import socket
 from unittest.mock import MagicMock, patch
 
 from django.test import TestCase
@@ -43,8 +44,9 @@ class SetRootPassTestCase(TestCase):
         data = json.loads(sent_bytes.decode())
         self.assertEqual(data["action"], "password")
         self.assertEqual(data["vname"], "root-pass-vm")
-        # SHA-512 crypt starts with $6$kgPoiREy$
-        self.assertTrue(data["passwd"].startswith("$6$kgPoiREy$"))
+        # SHA-512 crypt with a random salt (ROADMAP S-08), not the old fixed one
+        self.assertTrue(data["passwd"].startswith("$6$"))
+        self.assertFalse(data["passwd"].startswith("$6$kgPoiREy$"))
 
     def test_set_root_pass_get_not_allowed(self):
         response = self.client.get(reverse("instances:rootpasswd", args=[self.instance.id]))
@@ -53,3 +55,54 @@ class SetRootPassTestCase(TestCase):
     def test_set_root_pass_not_found(self):
         response = self.client.post(reverse("instances:rootpasswd", args=[99999]), {"passwd": "secret"})
         self.assertEqual(response.status_code, 404)
+
+    @patch("socket.socket")
+    @patch.object(Instance, "proxy")
+    def test_same_password_gets_a_different_salt_each_time(self, mock_proxy, mock_socket_cls):
+        mock_proxy.get_status.return_value = 5
+        mock_sock = MagicMock()
+        mock_socket_cls.return_value = mock_sock
+        mock_sock.recv.return_value = json.dumps({"return": "success"}).encode()
+        url = reverse("instances:rootpasswd", args=[self.instance.id])
+
+        self.client.post(url, {"passwd": "samePassword1"})
+        self.client.post(url, {"passwd": "samePassword1"})
+
+        hashes = [json.loads(c[0][0].decode())["passwd"] for c in mock_sock.send.call_args_list]
+        self.assertEqual(len(hashes), 2)
+        self.assertNotEqual(hashes[0], hashes[1])
+
+    @patch("socket.socket")
+    @patch.object(Instance, "proxy")
+    def test_unreachable_gstfsd_is_an_error_message_not_a_crash(self, mock_proxy, mock_socket_cls):
+        mock_proxy.get_status.return_value = 5
+        mock_sock = MagicMock()
+        mock_socket_cls.return_value = mock_sock
+        mock_sock.connect.side_effect = socket.timeout("timed out")
+
+        response = self.client.post(
+            reverse("instances:rootpasswd", args=[self.instance.id]), {"passwd": "x"}
+        )
+
+        self.assertEqual(response.status_code, 302)
+        mock_sock.settimeout.assert_called()
+
+    @patch("socket.socket")
+    @patch.object(Instance, "proxy")
+    def test_add_public_key_unreachable_gstfsd_is_an_error_message(self, mock_proxy, mock_socket_cls):
+        from accounts.models import UserSSHKey
+        from django.contrib.auth import get_user_model
+
+        admin = get_user_model().objects.get(username="admin")
+        key = UserSSHKey.objects.create(user=admin, keyname="k", keypublic="ssh-ed25519 AAAA k")
+        mock_proxy.get_status.return_value = 5
+        mock_sock = MagicMock()
+        mock_socket_cls.return_value = mock_sock
+        mock_sock.connect.side_effect = ConnectionRefusedError()
+
+        response = self.client.post(
+            reverse("instances:add_public_key", args=[self.instance.id]), {"sshkeyid": key.id}
+        )
+
+        self.assertEqual(response.status_code, 302)
+        mock_sock.settimeout.assert_called()

instances/views.py

@@ -610,6 +610,24 @@ def migrate(request, pk):
     )
 
 
+GSTFSD_PORT = 16510
+GSTFSD_TIMEOUT = 120  # gstfsd starts a guestfs appliance, which takes a while
+
+
+def gstfsd_request(hostname, data):
+    """Send one request to gstfsd and return its JSON reply, or an error reply."""
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    s.settimeout(GSTFSD_TIMEOUT)
+    try:
+        s.connect((hostname, GSTFSD_PORT))
+        s.send(json.dumps(data).encode())
+        return json.loads(s.recv(1024).strip())
+    except (OSError, ValueError) as err:
+        return {"return": "error", "message": _("gstfsd error: %(err)s") % {"err": err}}
+    finally:
+        s.close()
+
+
 @require_POST
 @serialize_instance_mutation
 def set_root_pass(request, pk):
@@ -618,16 +636,11 @@ def set_root_pass(request, pk):
     if request.method == "POST":
         passwd = request.POST.get("passwd", None)
         if passwd:
-            passwd_hash = crypt.crypt(passwd, "$6$kgPoiREy")
+            passwd_hash = crypt.crypt(passwd, crypt.mksalt(crypt.METHOD_SHA512))
             data = {"action": "password", "passwd": passwd_hash, "vname": instance.name}
 
             if instance.proxy.get_status() == 5:
-                s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-                s.connect((instance.compute.hostname, 16510))
-                s.send(bytes(json.dumps(data).encode()))
-                d = s.recv(1024).strip()
-                result = json.loads(d)
-                s.close()
+                result = gstfsd_request(instance.compute.hostname, data)
                 if result["return"] == "success":
                     msg = _("Reset root password")
                     addlogmsg(
@@ -659,11 +672,7 @@ def add_public_key(request, pk):
         }
 
         if instance.proxy.get_status() == 5:
-            s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-            s.connect((instance.compute.hostname, 16510))
-            s.send(json.dumps(data).encode())
-            result = json.loads(s.recv(1024))
-            s.close()
+            result = gstfsd_request(instance.compute.hostname, data)
             if result["return"] == "error":
                 msg = result["message"]
             else: