From 52d00e818b2c23438ad5bc0cf3e47651c3e866b2 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 26 Oct 2026 15:49:52 +0300 Subject: [PATCH] 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 | 3 ++- datasource/tests/test_datasource.py | 22 ++++++++++++++++++---- datasource/views.py | 26 ++++++++++++++------------ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/datasource/templates/user_data b/datasource/templates/user_data index f6a4a581..6e1409f5 100644 --- a/datasource/templates/user_data +++ b/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 %} diff --git a/datasource/tests/test_datasource.py b/datasource/tests/test_datasource.py index b71761f7..f72239ea 100644 --- a/datasource/tests/test_datasource.py +++ b/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]) diff --git a/datasource/views.py b/datasource/views.py index 4b0a5cac..45618515 100644 --- a/datasource/views.py +++ b/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)