fix(datasource): no SSH keys for an address without a reverse DNS entry

- A booting VM is named by its address's reverse DNS entry. Without one,
  the address itself was taken as the name, so its first number ("10",
  "192") named the VM: a user who made a VM with that name had their keys
  given to every such VM. Such an address now gets no keys; its metadata
  hostname is still the address.
- Keys are written as quoted YAML strings, so a comment with an apostrophe
  or ": " no longer breaks the key or the file.

datasource/templates/user_data

@@ -1,6 +1,7 @@
 #cloud-config
-{% if instance_keys %}
+{% autoescape off %}{% if instance_keys %}
 ssh_authorized_keys:
 {% for key in instance_keys %}  - {{ key }}
 {% endfor %}
 {% endif %}
+{% endautoescape %}

datasource/tests/test_datasource.py

@@ -1,4 +1,6 @@
 import json
+
+import yaml
 from unittest.mock import patch
 
 from django.contrib.auth import get_user_model
@@ -51,9 +53,8 @@ class DataSourceTestCase(TestCase):
         request.META["HTTP_X_FORWARDED_FOR"] = "10.0.0.1, 192.168.1.50"
         self.assertEqual(get_client_ip(request), "192.168.1.50")
 
-    def test_get_hostname_by_ip_fallback(self):
-        # Invalid IP will cause gethostbyaddr to fail and return the IP string itself
-        self.assertEqual(get_hostname_by_ip("256.256.256.256"), "256.256.256.256")
+    def test_get_hostname_by_ip_without_an_entry(self):
+        self.assertIsNone(get_hostname_by_ip("256.256.256.256"))
 
     def test_vdi_url_not_found_compute(self):
         response = self.client.get(reverse("vdi_url", args=[9999, "vm1"]))
@@ -73,7 +74,7 @@ class CloudInitTestCase(TestCase):
         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.reverse_dns = patcher.start()
         self.addCleanup(patcher.stop)
 
     def userdata(self):
@@ -97,3 +98,16 @@ class CloudInitTestCase(TestCase):
     def test_no_keys_for_an_unknown_name(self):
         self.vm.delete()
         self.assertNotIn("ssh", self.userdata())
+
+    def test_no_keys_for_an_address_without_a_reverse_dns_entry(self):
+        # the address's first number is no VM's name: a VM named "10" gets nothing
+        self.reverse_dns.return_value = None
+        Instance.objects.create(compute=self.compute, name="127", uuid="11111111-2222-3333-4444-999999999993")
+        self.assertNotIn("ssh", self.userdata())
+        metadata = json.loads(self.client.get(reverse("ds_openstack_metadata", args=["latest"])).content)
+        self.assertEqual(metadata["hostname"], "127.0.0.1")
+
+    def test_a_key_comment_stays_one_string(self):
+        key = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOwner it's: mine"
+        UserSSHKey.objects.filter(user=self.owner).update(keypublic=key)
+        self.assertEqual(yaml.safe_load(self.userdata())["ssh_authorized_keys"], [key])

datasource/views.py

@@ -1,3 +1,4 @@
+import json
 import socket
 
 from accounts.models import UserInstance, UserSSHKey
@@ -39,7 +40,7 @@ def os_metadata_json(request, version):
 
     if version == "latest":
         ip = get_client_ip(request)
-        hostname = get_hostname_by_ip(ip)
+        hostname = get_hostname_by_ip(ip) or ip
         response = {"uuid": OS_UUID, "hostname": hostname}
         return JsonResponse(response)
     else:
@@ -55,17 +56,19 @@ def os_userdata(request, version):
     :return:
     """
     if version == "latest":
-        ip = get_client_ip(request)
-        hostname = get_hostname_by_ip(ip)
-        vname = hostname.split(".")[0]
+        hostname = get_hostname_by_ip(get_client_ip(request))
 
         instance_keys = []
-        # 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])
+        # The VM is named by its address's reverse DNS entry; without one there
+        # is no name (the address's first number is no VM's name). 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=hostname.split(".")[0])[:2]) if hostname else []
         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)
+        # quoted, so a comment such as "it's: mine" stays one YAML string
+        instance_keys = [json.dumps(key) for key in instance_keys]
 
         return render(request, "user_data", locals())
     else:
@@ -89,13 +92,12 @@ def get_client_ip(request):
 def get_hostname_by_ip(ip):
     """
     :param ip:
-    :return:
+    :return: the address's reverse DNS name, or None
     """
     try:
-        addrs = socket.gethostbyaddr(ip)
+        return socket.gethostbyaddr(ip)[0]
     except OSError:  # no reverse DNS entry or an invalid address
-        addrs = [ip]
-    return addrs[0]
+        return None
 
 
 def get_vdi_url(request, compute_id, vname):
@@ -115,7 +117,7 @@ def get_vdi_url(request, compute_id, vname):
             compute.hostname, compute.login, compute.password, compute.type, vname, uuid=instance.uuid
         )
 
-        fqdn = get_hostname_by_ip(compute.hostname)
+        fqdn = get_hostname_by_ip(compute.hostname) or compute.hostname
         url = f"{conn.get_console_type()}://{fqdn}:{conn.get_console_port()}"
         response = url
         return HttpResponse(response)