From b71794094ac93942020b5f68f8f62900e1d09c97 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Mon, 17 Apr 2023 16:04:52 -0400 Subject: azure: introduce identity module (#2116) - Add query_system_uuid() for getting system uuid from dmi in normalized (lower-cased) form. - Add byte_swap_system_uuid() to convert a system uuid for gen1 instances to the compute.vmId as presented by IMDS. - Add convert_system_uuid_to_vm() to convert system uuid to vm id depending on whether it is gen1 or gen2. - Add is_vm_gen1() to determine if VM is Azure's gen1 by checking for available of EFI (used in gen2). - Add query_vm_id() helper to get VM id without system uuid. - Move ChassisAssetTag from Azure helpers into identity. - Update DataSourceAzure._iid() to use this module. Signed-off-by: Chris Patterson --- cloudinit/sources/DataSourceAzure.py | 31 ++--- cloudinit/sources/azure/identity.py | 106 ++++++++++++++++ cloudinit/sources/helpers/azure.py | 57 +-------- tests/unittests/sources/azure/test_identity.py | 165 +++++++++++++++++++++++++ tests/unittests/sources/test_azure.py | 40 ++++-- tests/unittests/sources/test_azure_helper.py | 58 --------- 6 files changed, 310 insertions(+), 147 deletions(-) create mode 100644 cloudinit/sources/azure/identity.py create mode 100644 tests/unittests/sources/azure/test_identity.py diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 927e8cf0..aeec6a92 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -16,7 +16,6 @@ from pathlib import Path from time import sleep, time from typing import Any, Dict, List, Optional -from cloudinit import dmi from cloudinit import log as logging from cloudinit import net, sources, ssh_util, subp, util from cloudinit.event import EventScope, EventType @@ -27,12 +26,11 @@ from cloudinit.net.dhcp import ( ) from cloudinit.net.ephemeral import EphemeralDHCPv4 from cloudinit.reporting import events -from cloudinit.sources.azure import imds +from cloudinit.sources.azure import identity, imds from cloudinit.sources.helpers import netlink from cloudinit.sources.helpers.azure import ( DEFAULT_WIRESERVER_ENDPOINT, BrokenAzureDataSource, - ChassisAssetTag, NonAzureDataSource, OvfEnvXml, azure_ds_reporter, @@ -43,7 +41,6 @@ from cloudinit.sources.helpers.azure import ( get_ip_from_lease_value, get_metadata_from_fabric, get_system_info, - is_byte_swapped, push_log_to_kvp, report_diagnostic_event, report_failure_to_fabric, @@ -695,7 +692,7 @@ class DataSourceAzure(sources.DataSource): """Check platform environment to report if this datasource may run. """ - chassis_tag = ChassisAssetTag.query_system() + chassis_tag = identity.ChassisAssetTag.query_system() if chassis_tag is not None: return True @@ -857,24 +854,18 @@ class DataSourceAzure(sources.DataSource): prev_iid_path = os.path.join( self.paths.get_cpath("data"), "instance-id" ) - # Older kernels than 4.15 will have UPPERCASE product_uuid. - # We don't want Azure to react to an UPPER/lower difference as a new - # instance id as it rewrites SSH host keys. - # LP: #1835584 - system_uuid = dmi.read_dmi_data("system-uuid") - if system_uuid is None: - raise RuntimeError("failed to read system-uuid") - - iid = system_uuid.lower() + system_uuid = identity.query_system_uuid() if os.path.exists(prev_iid_path): previous = util.load_file(prev_iid_path).strip() - if previous.lower() == iid: - # If uppercase/lowercase equivalent, return the previous value - # to avoid new instance id. - return previous - if is_byte_swapped(previous.lower(), iid): + swapped_id = identity.byte_swap_system_uuid(system_uuid) + + # Older kernels than 4.15 will have UPPERCASE product_uuid. + # We don't want Azure to react to an UPPER/lower difference as + # a new instance id as it rewrites SSH host keys. + # LP: #1835584 + if previous.lower() in [system_uuid, swapped_id]: return previous - return iid + return system_uuid @azure_ds_telemetry_reporter def _wait_for_nic_detach(self, nl_sock): diff --git a/cloudinit/sources/azure/identity.py b/cloudinit/sources/azure/identity.py new file mode 100644 index 00000000..c371f10c --- /dev/null +++ b/cloudinit/sources/azure/identity.py @@ -0,0 +1,106 @@ +# Copyright (C) 2023 Microsoft Corporation. +# +# This file is part of cloud-init. See LICENSE file for license information. + +import enum +import logging +import os +import uuid +from typing import Optional + +from cloudinit import dmi +from cloudinit.sources.helpers.azure import report_diagnostic_event + +LOG = logging.getLogger(__name__) + + +def byte_swap_system_uuid(system_uuid: str) -> str: + """Byte swap system uuid. + + Azure always uses little-endian for the first three fields in the uuid. + This behavior was made strict in SMBIOS 2.6+, but Linux and dmidecode + follow RFC 4122 and assume big-endian for earlier SMBIOS versions. + + Azure's gen1 VMs use SMBIOS 2.3 which requires byte swapping to match + compute.vmId presented by IMDS. + + Azure's gen2 VMs use SMBIOS 3.1 which does not require byte swapping. + + :raises ValueError: if UUID is invalid. + """ + try: + original_uuid = uuid.UUID(system_uuid) + except ValueError: + msg = f"Failed to parse system uuid: {system_uuid!r}" + report_diagnostic_event(msg, logger_func=LOG.error) + raise + + return str(uuid.UUID(bytes=original_uuid.bytes_le)) + + +def convert_system_uuid_to_vm_id(system_uuid: str) -> str: + """Determine VM ID from system uuid.""" + if is_vm_gen1(): + return byte_swap_system_uuid(system_uuid) + + return system_uuid + + +def is_vm_gen1() -> bool: + """Determine if VM is gen1 or gen2. + + Gen2 guests use UEFI while gen1 is legacy BIOS. + """ + # Linux + if os.path.exists("/sys/firmware/efi"): + return False + + # BSD + if os.path.exists("/dev/efi"): + return False + + return True + + +def query_system_uuid() -> str: + """Query system uuid in lower-case.""" + system_uuid = dmi.read_dmi_data("system-uuid") + if system_uuid is None: + raise RuntimeError("failed to read system-uuid") + + # Kernels older than 4.15 will have upper-case system uuid. + system_uuid = system_uuid.lower() + LOG.debug("Read product uuid: %s", system_uuid) + return system_uuid + + +def query_vm_id() -> str: + """Query VM ID from system.""" + system_uuid = query_system_uuid() + return convert_system_uuid_to_vm_id(system_uuid) + + +class ChassisAssetTag(enum.Enum): + AZURE_CLOUD = "7783-7084-3265-9085-8269-3286-77" + + @classmethod + def query_system(cls) -> Optional["ChassisAssetTag"]: + """Check platform environment to report if this datasource may run. + + :returns: ChassisAssetTag if matching tag found, else None. + """ + asset_tag = dmi.read_dmi_data("chassis-asset-tag") + try: + tag = cls(asset_tag) + except ValueError: + report_diagnostic_event( + "Non-Azure chassis asset tag: %r" % asset_tag, + logger_func=LOG.debug, + ) + return None + + report_diagnostic_event( + "Azure chassis asset tag: %r (%s)" % (asset_tag, tag.name), + logger_func=LOG.debug, + ) + return tag diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index d4fc04e2..c0ffd760 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -1,6 +1,5 @@ # This file is part of cloud-init. See LICENSE file for license information. import base64 -import enum import json import logging import os @@ -17,7 +16,7 @@ from typing import Callable, List, Optional, TypeVar, Union from xml.etree import ElementTree from xml.sax.saxutils import escape -from cloudinit import distros, dmi, subp, temp_utils, url_helper, util, version +from cloudinit import distros, subp, temp_utils, url_helper, util, version from cloudinit.reporting import events from cloudinit.settings import CFG_BUILTIN @@ -65,34 +64,6 @@ def azure_ds_telemetry_reporter(func: Callable[..., T]) -> Callable[..., T]: return impl -def is_byte_swapped(previous_id, current_id): - """ - Azure stores the instance ID with an incorrect byte ordering for the - first parts. This corrects the byte order such that it is consistent with - that returned by the metadata service. - """ - if previous_id == current_id: - return False - - def swap_bytestring(s, width=2): - dd = [byte for byte in textwrap.wrap(s, 2)] - dd.reverse() - return "".join(dd) - - parts = current_id.split("-") - swapped_id = "-".join( - [ - swap_bytestring(parts[0]), - swap_bytestring(parts[1]), - swap_bytestring(parts[2]), - parts[3], - parts[4], - ] - ) - - return previous_id == swapped_id - - @azure_ds_telemetry_reporter def get_boot_telemetry(): """Report timestamps related to kernel initialization and systemd @@ -1079,32 +1050,6 @@ class NonAzureDataSource(Exception): pass -class ChassisAssetTag(enum.Enum): - AZURE_CLOUD = "7783-7084-3265-9085-8269-3286-77" - - @classmethod - def query_system(cls) -> Optional["ChassisAssetTag"]: - """Check platform environment to report if this datasource may run. - - :returns: ChassisAssetTag if matching tag found, else None. - """ - asset_tag = dmi.read_dmi_data("chassis-asset-tag") - try: - tag = cls(asset_tag) - except ValueError: - report_diagnostic_event( - "Non-Azure chassis asset tag: %r" % asset_tag, - logger_func=LOG.debug, - ) - return None - - report_diagnostic_event( - "Azure chassis asset tag: %r (%s)" % (asset_tag, tag.name), - logger_func=LOG.debug, - ) - return tag - - class OvfEnvXml: NAMESPACES = { "ovf": "http://schemas.dmtf.org/ovf/environment/1", diff --git a/tests/unittests/sources/azure/test_identity.py b/tests/unittests/sources/azure/test_identity.py new file mode 100644 index 00000000..53b1e18f --- /dev/null +++ b/tests/unittests/sources/azure/test_identity.py @@ -0,0 +1,165 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from unittest import mock + +import pytest + +from cloudinit.sources.azure import identity + + +@pytest.fixture(autouse=True) +def mock_read_dmi_data(): + with mock.patch.object( + identity.dmi, "read_dmi_data", return_value=None + ) as m: + yield m + + +@pytest.fixture(autouse=True) +def mock_os_path_exists(): + with mock.patch.object(identity.os.path, "exists") as m: + yield m + + +class TestByteSwapSystemUuid: + @pytest.mark.parametrize( + "system_uuid,swapped_uuid", + [ + ( + "527c2691-029f-fe4c-b1f4-a4da7ebac2cf", + "91267c52-9f02-4cfe-b1f4-a4da7ebac2cf", + ), + ( + "527C2691-029F-FE4C-B1F4-A4DA7EBAC2CD", + "91267c52-9f02-4cfe-b1f4-a4da7ebac2cd", + ), + ], + ) + def test_values(self, system_uuid, swapped_uuid): + assert identity.byte_swap_system_uuid(system_uuid) == swapped_uuid + + @pytest.mark.parametrize( + "system_uuid", + [ + "", + "g", + "91267c52-9f02-4cfe-b1f4-a4da7ebac2c", + "91267c52-9f02-4cfe-b1f4-a4da7ebac2ccc", + "-----", + ], + ) + def test_errors(self, system_uuid): + with pytest.raises(ValueError) as exc_info: + identity.byte_swap_system_uuid(system_uuid) + + assert exc_info.value.args[0] == "badly formed hexadecimal UUID string" + + +class TestConvertSystemUuidToVmId: + def test_gen1(self, monkeypatch): + system_uuid = "527c2691-029f-fe4c-b1f4-a4da7ebac2cf" + monkeypatch.setattr(identity, "is_vm_gen1", lambda: True) + + swapped_uuid = "91267c52-9f02-4cfe-b1f4-a4da7ebac2cf" + assert ( + identity.convert_system_uuid_to_vm_id(system_uuid) == swapped_uuid + ) + + def test_gen2(self, monkeypatch): + system_uuid = "527c2691-029f-fe4c-b1f4-a4da7ebac2cf" + monkeypatch.setattr(identity, "is_vm_gen1", lambda: False) + + assert ( + identity.convert_system_uuid_to_vm_id(system_uuid) == system_uuid + ) + + +class TestIsVmGen1: + def test_gen1(self, mock_os_path_exists) -> None: + mock_os_path_exists.side_effect = lambda _: False + + assert identity.is_vm_gen1() is True + + def test_gen2_freebsd(self, mock_os_path_exists) -> None: + mock_os_path_exists.side_effect = lambda x: x == "/dev/efi" + + assert identity.is_vm_gen1() is False + + def test_gen2_linux(self, mock_os_path_exists) -> None: + mock_os_path_exists.side_effect = lambda x: x == "/sys/firmware/efi" + + assert identity.is_vm_gen1() is False + + +class TestQuerySystemUuid: + @pytest.mark.parametrize( + "system_uuid", + [ + "527c2691-029f-fe4c-b1f4-a4da7ebac2cf", + "527C2691-029F-FE4C-B1F4-A4DA7EBAC2CF", + ], + ) + def test_values(self, mock_read_dmi_data, system_uuid): + mock_read_dmi_data.return_value = system_uuid + + assert identity.query_system_uuid() == system_uuid.lower() + assert mock_read_dmi_data.mock_calls == [mock.call("system-uuid")] + + def test_errors(self, mock_read_dmi_data): + mock_read_dmi_data.return_value = None + + with pytest.raises(RuntimeError) as exc_info: + identity.query_system_uuid() + + assert exc_info.value.args[0] == "failed to read system-uuid" + + +class TestQueryVmId: + def test_gen1(self, monkeypatch): + system_uuid = "527c2691-029f-fe4c-b1f4-a4da7ebac2cf" + swapped_uuid = "91267c52-9f02-4cfe-b1f4-a4da7ebac2cf" + monkeypatch.setattr(identity, "query_system_uuid", lambda: system_uuid) + monkeypatch.setattr(identity, "is_vm_gen1", lambda: True) + + assert identity.query_vm_id() == swapped_uuid + + def test_gen2(self, monkeypatch): + system_uuid = "527c2691-029f-fe4c-b1f4-a4da7ebac2cf" + monkeypatch.setattr(identity, "query_system_uuid", lambda: system_uuid) + monkeypatch.setattr(identity, "is_vm_gen1", lambda: False) + + assert identity.query_vm_id() == system_uuid + + +class TestChassisAssetTag: + def test_true_azure_cloud(self, caplog, mock_read_dmi_data): + mock_read_dmi_data.return_value = ( + identity.ChassisAssetTag.AZURE_CLOUD.value + ) + + asset_tag = identity.ChassisAssetTag.query_system() + + assert asset_tag == identity.ChassisAssetTag.AZURE_CLOUD + assert caplog.record_tuples == [ + ( + "cloudinit.sources.azure.identity", + 10, + "Azure chassis asset tag: " + "'7783-7084-3265-9085-8269-3286-77' (AZURE_CLOUD)", + ) + ] + + @pytest.mark.parametrize("tag", [None, "", "notazure"]) + def test_false_on_nonazure_chassis(self, caplog, mock_read_dmi_data, tag): + mock_read_dmi_data.return_value = tag + + asset_tag = identity.ChassisAssetTag.query_system() + + assert asset_tag is None + assert caplog.record_tuples == [ + ( + "cloudinit.sources.azure.identity", + 10, + "Non-Azure chassis asset tag: %r" % tag, + ) + ] diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py index 0648f08c..3c36c9c6 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -11,11 +11,12 @@ from pathlib import Path import pytest import requests -from cloudinit import distros, helpers, subp, url_helper +from cloudinit import distros, dmi, helpers, subp, url_helper from cloudinit.net import dhcp from cloudinit.sources import UNSET from cloudinit.sources import DataSourceAzure as dsaz from cloudinit.sources import InvalidMetaDataException +from cloudinit.sources.azure import identity from cloudinit.sources.helpers import netlink from cloudinit.util import ( MountFailedError, @@ -81,9 +82,9 @@ def mock_azure_report_failure_to_fabric(): @pytest.fixture def mock_chassis_asset_tag(): with mock.patch.object( - dsaz.ChassisAssetTag, + identity.ChassisAssetTag, "query_system", - return_value=dsaz.ChassisAssetTag.AZURE_CLOUD.value, + return_value=identity.ChassisAssetTag.AZURE_CLOUD.value, ) as m: yield m @@ -110,13 +111,14 @@ def mock_time(): def mock_dmi_read_dmi_data(): def fake_read(key: str) -> str: if key == "system-uuid": - return "fake-system-uuid" + return "50109936-ef07-47fe-ac82-890c853f60d5" elif key == "chassis-asset-tag": return "7783-7084-3265-9085-8269-3286-77" raise RuntimeError() - with mock.patch( - MOCKPATH + "dmi.read_dmi_data", + with mock.patch.object( + dmi, + "read_dmi_data", side_effect=fake_read, autospec=True, ) as m: @@ -1029,7 +1031,7 @@ scbus-1 on xpt0 bus 0 ), (dsaz.subp, "which", lambda x: True), ( - dsaz.dmi, + dmi, "read_dmi_data", self.m_read_dmi_data, ), @@ -3147,7 +3149,7 @@ class TestIsPlatformViable: @pytest.mark.parametrize( "tag", [ - dsaz.ChassisAssetTag.AZURE_CLOUD.value, + identity.ChassisAssetTag.AZURE_CLOUD.value, ], ) def test_true_on_azure_chassis( @@ -3422,7 +3424,7 @@ class TestInstanceId: def test_fallback(self, azure_ds, mock_dmi_read_dmi_data): id = azure_ds.get_instance_id() - assert id == "fake-system-uuid" + assert id == "50109936-ef07-47fe-ac82-890c853f60d5" class TestProvisioning: @@ -3543,7 +3545,10 @@ class TestProvisioning: mock.call("chassis-asset-tag"), mock.call("system-uuid"), ] - assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + assert ( + self.azure_ds.metadata["instance-id"] + == "50109936-ef07-47fe-ac82-890c853f60d5" + ) # Verify IMDS metadata. assert self.azure_ds.metadata["imds"] == self.imds_md @@ -3633,7 +3638,10 @@ class TestProvisioning: mock.call("chassis-asset-tag"), mock.call("system-uuid"), ] - assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + assert ( + self.azure_ds.metadata["instance-id"] + == "50109936-ef07-47fe-ac82-890c853f60d5" + ) # Verify IMDS metadata. assert self.azure_ds.metadata["imds"] == self.imds_md @@ -3749,7 +3757,10 @@ class TestProvisioning: mock.call("chassis-asset-tag"), mock.call("system-uuid"), ] - assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + assert ( + self.azure_ds.metadata["instance-id"] + == "50109936-ef07-47fe-ac82-890c853f60d5" + ) # Verify IMDS metadata. assert self.azure_ds.metadata["imds"] == self.imds_md @@ -3903,7 +3914,10 @@ class TestProvisioning: mock.call("chassis-asset-tag"), mock.call("system-uuid"), ] - assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + assert ( + self.azure_ds.metadata["instance-id"] + == "50109936-ef07-47fe-ac82-890c853f60d5" + ) # Verify IMDS metadata. assert self.azure_ds.metadata["imds"] == self.imds_md diff --git a/tests/unittests/sources/test_azure_helper.py b/tests/unittests/sources/test_azure_helper.py index 38a57b99..ac2126e8 100644 --- a/tests/unittests/sources/test_azure_helper.py +++ b/tests/unittests/sources/test_azure_helper.py @@ -196,28 +196,6 @@ class TestGoalStateParsing(CiTestCase): goal_state = self._get_goal_state(instance_id=instance_id) self.assertEqual(instance_id, goal_state.instance_id) - def test_instance_id_byte_swap(self): - """Return true when previous_iid is byteswapped current_iid""" - previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8" - current_iid = "544CDFD0-CB4E-4B4A-9954-5BDF3ED5C3B8" - self.assertTrue( - azure_helper.is_byte_swapped(previous_iid, current_iid) - ) - - def test_instance_id_no_byte_swap_same_instance_id(self): - previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8" - current_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8" - self.assertFalse( - azure_helper.is_byte_swapped(previous_iid, current_iid) - ) - - def test_instance_id_no_byte_swap_diff_instance_id(self): - previous_iid = "D0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8" - current_iid = "G0DF4C54-4ECB-4A4B-9954-5BDF3ED5C3B8" - self.assertFalse( - azure_helper.is_byte_swapped(previous_iid, current_iid) - ) - def test_certificates_xml_parsed_and_fetched_correctly(self): m_azure_endpoint_client = mock.MagicMock() certificates_url = "TestCertificatesUrl" @@ -1417,42 +1395,6 @@ class TestGetMetadataGoalStateXMLAndReportFailureToFabric(CiTestCase): ) -class TestChassisAssetTag: - def test_true_azure_cloud(self, caplog, mock_dmi_read_dmi_data): - mock_dmi_read_dmi_data.return_value = ( - azure_helper.ChassisAssetTag.AZURE_CLOUD.value - ) - - asset_tag = azure_helper.ChassisAssetTag.query_system() - - assert asset_tag == azure_helper.ChassisAssetTag.AZURE_CLOUD - assert caplog.record_tuples == [ - ( - "cloudinit.sources.helpers.azure", - 10, - "Azure chassis asset tag: " - "'7783-7084-3265-9085-8269-3286-77' (AZURE_CLOUD)", - ) - ] - - @pytest.mark.parametrize("tag", [None, "", "notazure"]) - def test_false_on_nonazure_chassis( - self, caplog, mock_dmi_read_dmi_data, tag - ): - mock_dmi_read_dmi_data.return_value = tag - - asset_tag = azure_helper.ChassisAssetTag.query_system() - - assert asset_tag is None - assert caplog.record_tuples == [ - ( - "cloudinit.sources.helpers.azure", - 10, - "Non-Azure chassis asset tag: %r" % tag, - ) - ] - - class TestOvfEnvXml: @pytest.mark.parametrize( "ovf,expected", -- cgit v1.2.1