fix(datasource): cloud-init endpoints work without login, keys need a unique name

The OpenStack-style cloud-init endpoints (index, meta_data.json,
user_data) are fetched by booting VMs, which do not log in. Since the
login-required middleware they redirected to the login page, so the
datasource has not worked. They are login_not_required now; they expose
a hostname and the public SSH keys of a VM's owners.

A VM is found by the reverse DNS name of the client address, and VM
names are not unique (only compute and UUID are). user_data merged the
owners of every VM with that name, so a user who named their VM like
another user's VM got their key into that VM at its next cloud-init
run. Keys are now given only when exactly one VM has the name.

The client address is the last X-Forwarded-For entry, which nginx in
the default setup appends; gunicorn listens on 127.0.0.1 only. The bare
except around the reverse lookup is narrowed to OSError.

datasource/test_datasource.py

@@ -1,9 +1,13 @@
 import json
+from unittest.mock import patch
 
+from django.contrib.auth import get_user_model
 from django.test import RequestFactory, TestCase
 from django.urls import reverse
 
+from accounts.models import UserInstance, UserSSHKey
 from computes.models import Compute
+from instances.models import Instance
 from datasource.views import get_client_ip, get_hostname_by_ip
 
 
@@ -54,3 +58,42 @@ class DataSourceTestCase(TestCase):
     def test_vdi_url_not_found_compute(self):
         response = self.client.get(reverse("vdi_url", args=[9999, "vm1"]))
         self.assertEqual(response.status_code, 404)
+
+
+class CloudInitTestCase(TestCase):
+    """VMs fetch the cloud-init endpoints without logging in; keys go only to
+    a VM whose name is unique."""
+
+    KEY = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOwner owner@example"
+
+    def setUp(self):
+        self.compute = Compute.objects.create(name="ci-a", hostname="a.example", login="", password="", type=4)
+        self.owner = get_user_model().objects.create_user("ci_owner", password="pw")
+        UserSSHKey.objects.create(user=self.owner, keyname="k", keypublic=self.KEY)
+        self.vm = Instance.objects.create(compute=self.compute, name="web1", uuid="11111111-2222-3333-4444-999999999991")
+        UserInstance.objects.create(instance=self.vm, user=self.owner)
+        patcher = patch("datasource.views.get_hostname_by_ip", return_value="web1.lan")
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def userdata(self):
+        response = self.client.get(reverse("ds_openstack_userdata", args=["latest"]))
+        self.assertEqual(response.status_code, 200)
+        return response.content.decode()
+
+    def test_endpoints_need_no_login(self):
+        for name, args in (("ds_openstack_index", []), ("ds_openstack_metadata", ["latest"])):
+            self.assertEqual(self.client.get(reverse(name, args=args)).status_code, 200, name)
+        self.assertEqual(json.loads(self.client.get(reverse("ds_openstack_metadata", args=["latest"])).content)["hostname"], "web1.lan")
+
+    def test_owner_keys_for_a_unique_name(self):
+        self.assertIn(self.KEY, self.userdata())
+
+    def test_no_keys_when_the_name_is_taken_twice(self):
+        other = Compute.objects.create(name="ci-b", hostname="b.example", login="", password="", type=4)
+        Instance.objects.create(compute=other, name="web1", uuid="11111111-2222-3333-4444-999999999992")
+        self.assertNotIn("ssh", self.userdata())
+
+    def test_no_keys_for_an_unknown_name(self):
+        self.vm.delete()
+        self.assertNotIn("ssh", self.userdata())

datasource/views.py

@@ -2,6 +2,7 @@ import socket
 
 from accounts.models import UserInstance, UserSSHKey
 from computes.models import Compute
+from django.contrib.auth.decorators import login_not_required
 from django.core.exceptions import PermissionDenied
 from django.http import Http404, HttpResponse, JsonResponse
 from django.shortcuts import get_object_or_404, render
@@ -14,6 +15,11 @@ OS_VERSIONS = ["latest", ""]
 OS_UUID = "iid-dswebvirtcloud"
 
 
+# The cloud-init endpoints are fetched by booting VMs, which do not log in.
+# They expose only a hostname and the public SSH keys of a VM's owners.
+
+
+@login_not_required
 def os_index(request):
     """
     :param request:
@@ -23,6 +29,7 @@ def os_index(request):
     return HttpResponse(response)
 
 
+@login_not_required
 def os_metadata_json(request, version):
     """
     :param request:
@@ -40,6 +47,7 @@ def os_metadata_json(request, version):
         raise Http404(err)
 
 
+@login_not_required
 def os_userdata(request, version):
     """
     :param request:
@@ -52,12 +60,12 @@ def os_userdata(request, version):
         vname = hostname.split(".")[0]
 
         instance_keys = []
-        userinstances = UserInstance.objects.filter(instance__name=vname)
-
-        for ui in userinstances:
-            keys = UserSSHKey.objects.filter(user=ui.user)
-            for k in keys:
-                instance_keys.append(k.keypublic)
+        # VM names are not unique: with two VMs of this name, a user could put
+        # their key into the other VM by naming theirs the same. No keys then.
+        instances = list(Instance.objects.filter(name=vname)[:2])
+        if len(instances) == 1:
+            for ui in UserInstance.objects.filter(instance=instances[0]):
+                instance_keys += UserSSHKey.objects.filter(user=ui.user).values_list("keypublic", flat=True)
 
         return render(request, "user_data", locals())
     else:
@@ -85,7 +93,7 @@ def get_hostname_by_ip(ip):
     """
     try:
         addrs = socket.gethostbyaddr(ip)
-    except:
+    except OSError:  # no reverse DNS entry or an invalid address
         addrs = [ip]
     return addrs[0]
 

webvirtcloud/test_authorization_matrix.py

@@ -41,11 +41,9 @@ ALLOWED_FOR_ANY_USER = {
     "rest_framework:login": "DRF browsable API login",
     "rest_framework:logout": "DRF browsable API logout",
     "console": "renders an error without a valid token; access is checked per VM",
-    "ds_openstack_index": "static cloud-init datasource index",
-    # Meant for VMs: answers for the VM whose name matches the reverse DNS of
-    # the client IP. The design is broken and needs a redesign.
-    "ds_openstack_metadata": "cloud-init datasource keyed by client IP",
-    "ds_openstack_userdata": "cloud-init datasource keyed by client IP",
+    "ds_openstack_index": "cloud-init datasource, fetched by VMs",
+    "ds_openstack_metadata": "cloud-init datasource, fetched by VMs",
+    "ds_openstack_userdata": "cloud-init datasource, fetched by VMs",
     "instance-list": "API: filtered to the user's own instances",
     "compute-instance-list": "API: filtered to the user's own instances",
     "instance-flavor-list": "API: flavor catalogue",
@@ -62,6 +60,9 @@ ALLOWED_FOR_ANONYMOUS = {
     "accounts:login": "login page",
     "accounts:email_otp": "part of the login flow (its own issues are tracked separately)",
     "rest_framework:login": "DRF browsable API login page",
+    "ds_openstack_index": "cloud-init datasource, fetched by VMs that do not log in",
+    "ds_openstack_metadata": "cloud-init datasource, fetched by VMs that do not log in",
+    "ds_openstack_userdata": "cloud-init datasource, fetched by VMs that do not log in",
     "schema": "OpenAPI schema (SERVE_PERMISSIONS not restricted yet)",
     "schema-json": "OpenAPI schema (SERVE_PERMISSIONS not restricted yet)",
     "schema-redoc": "API docs (SERVE_PERMISSIONS not restricted yet)",