From f8938017e597cf3c820390e4057c0670e3a478ed Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Mon, 14 Feb 2022 08:52:32 -0500 Subject: [PATCH] sources/azure: validate IMDS network configuration metadata Due to race conditions and caching, IMDS may return stale or incomplete metadata. Add some validation to detect these scenarios and report appropriate telemetry. Introduce normalize_mac_address() to allow for comparison of mac addresses, replacing that found inline in: _generate_network_config_from_imds_metadata() Add validation of final fetch of IMDS metadata. Signed-off-by: Chris Patterson --- cloudinit/sources/DataSourceAzure.py | 82 +++++++- tests/unittests/sources/test_azure.py | 260 +++++++++++++++++++++++++- 2 files changed, 339 insertions(+), 3 deletions(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 2566bd8e9e9..f8e1dd02114 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -172,6 +172,26 @@ def find_dev_from_busdev(camcontrol_out: str, busdev: str) -> Optional[str]: return None +def normalize_mac_address(mac: str) -> str: + """Normalize mac address with colons and lower-case.""" + if len(mac) == 12: + mac = ":".join( + [mac[0:2], mac[2:4], mac[4:6], mac[6:8], mac[8:10], mac[10:12]] + ) + + return mac.lower() + + +@azure_ds_telemetry_reporter +def get_hv_netvsc_macs_normalized() -> List[str]: + """Get Hyper-V NICs as normalized MAC addresses.""" + return [ + normalize_mac_address(n[1]) + for n in net.get_interfaces() + if n[2] == "hv_netvsc" + ] + + def execute_or_debug(cmd, fail_ret=None) -> str: try: return subp.subp(cmd)[0] # type: ignore @@ -513,6 +533,9 @@ def crawl_metadata(self): # fetch metadata again as it has changed after reprovisioning imds_md = self.get_imds_data_with_api_fallback(retries=10) + # Report errors if IMDS network configuration is missing data. + self.validate_imds_network_metadata(imds_md=imds_md) + self.seed = metadata_source crawled_data.update( { @@ -1532,6 +1555,54 @@ def network_config(self): def region(self): return self.metadata.get("imds", {}).get("compute", {}).get("location") + @azure_ds_telemetry_reporter + def validate_imds_network_metadata(self, imds_md: dict) -> bool: + """Validate IMDS network config and report telemetry for errors.""" + local_macs = get_hv_netvsc_macs_normalized() + + try: + network_config = imds_md["network"] + imds_macs = [ + normalize_mac_address(i["macAddress"]) + for i in network_config["interface"] + ] + except KeyError: + report_diagnostic_event( + "IMDS network metadata has incomplete configuration: %r" + % imds_md.get("network"), + logger_func=LOG.warning, + ) + return False + + missing_macs = [m for m in local_macs if m not in imds_macs] + if not missing_macs: + return True + + report_diagnostic_event( + "IMDS network metadata is missing configuration for NICs %r: %r" + % (missing_macs, network_config), + logger_func=LOG.warning, + ) + + if not self._ephemeral_dhcp_ctx or not self._ephemeral_dhcp_ctx.iface: + # No primary interface to check against. + return False + + primary_mac = net.get_interface_mac(self._ephemeral_dhcp_ctx.iface) + if not primary_mac or not isinstance(primary_mac, str): + # Unexpected data for primary interface. + return False + + primary_mac = normalize_mac_address(primary_mac) + if primary_mac in missing_macs: + report_diagnostic_event( + "IMDS network metadata is missing primary NIC %r: %r" + % (primary_mac, network_config), + logger_func=LOG.warning, + ) + + return False + def _username_from_imds(imds_data): try: @@ -2179,6 +2250,7 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict: # If there are no available IP addresses, then we don't # want to add this interface to the generated config. if not addresses: + LOG.debug("No %s addresses found for: %r", addr_type, intf) continue has_ip_address = True if addr_type == "ipv4": @@ -2203,7 +2275,7 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict: "{ip}/{prefix}".format(ip=privateIp, prefix=netPrefix) ) if dev_config and has_ip_address: - mac = ":".join(re.findall(r"..", intf["macAddress"])) + mac = normalize_mac_address(intf["macAddress"]) dev_config.update( {"match": {"macaddress": mac.lower()}, "set-name": nicname} ) @@ -2214,6 +2286,14 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict: if driver and driver == "hv_netvsc": dev_config["match"]["driver"] = driver netconfig["ethernets"][nicname] = dev_config + continue + + LOG.debug( + "No configuration for: %s (dev_config=%r) (has_ip_address=%r)", + nicname, + dev_config, + has_ip_address, + ) return netconfig diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py index bd525b13b67..ecedc54d6eb 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -37,6 +37,8 @@ wrap_and_call, ) +MOCKPATH = "cloudinit.sources.DataSourceAzure." + @pytest.fixture def azure_ds(request, paths): @@ -45,6 +47,22 @@ def azure_ds(request, paths): yield dsaz.DataSourceAzure(sys_cfg={}, distro=mock.Mock(), paths=paths) +@pytest.fixture +def mock_get_interfaces(): + """Mock for net.get_interfaces().""" + with mock.patch(MOCKPATH + "net.get_interfaces", return_value=[]) as m: + yield m + + +@pytest.fixture +def mock_get_interface_mac(): + """Mock for net.get_interface_mac().""" + with mock.patch( + MOCKPATH + "net.get_interface_mac", return_value="001122334455" + ) as m: + yield m + + def construct_valid_ovf_env( data=None, pubkeys=None, userdata=None, platform_settings=None ): @@ -196,7 +214,6 @@ def construct_valid_ovf_env( ] } -MOCKPATH = "cloudinit.sources.DataSourceAzure." EXAMPLE_UUID = "d0df4c54-4ecb-4a4b-9954-5bdf3ed5c3b8" @@ -764,6 +781,13 @@ def _load_possible_azure_ds(seed_dir, cache_dir): self.m_is_platform_viable = mock.MagicMock(autospec=True) self.m_get_metadata_from_fabric = mock.MagicMock(return_value=[]) self.m_report_failure_to_fabric = mock.MagicMock(autospec=True) + self.m_get_interfaces = mock.MagicMock( + return_value=[ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("eth0", "00:15:5d:69:63:ba", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + ) self.m_list_possible_azure_ds = mock.MagicMock( side_effect=_load_possible_azure_ds ) @@ -799,6 +823,16 @@ def _dmi_mocks(key): ), (dsaz, "get_boot_telemetry", mock.MagicMock()), (dsaz, "get_system_info", mock.MagicMock()), + ( + dsaz.net, + "get_interface_mac", + mock.MagicMock(return_value="00:15:5d:69:63:ba"), + ), + ( + dsaz.net, + "get_interfaces", + self.m_get_interfaces, + ), (dsaz.subp, "which", lambda x: True), ( dsaz.dmi, @@ -1940,7 +1974,7 @@ def test_blacklist_through_distro(self, m_net_get_interfaces): ) distro.networking.get_interfaces_by_mac() - m_net_get_interfaces.assert_called_with( + self.m_get_interfaces.assert_called_with( blacklist_drivers=dsaz.BLACKLIST_DRIVERS ) @@ -3534,4 +3568,226 @@ def test_non_ascii_seed_is_serializable(self): self.assertEqual(deserialized["seed"], result) +class TestValidateIMDSMetadata: + @pytest.mark.parametrize( + "mac,expected", + [ + ("001122aabbcc", "00:11:22:aa:bb:cc"), + ("001122AABBCC", "00:11:22:aa:bb:cc"), + ("00:11:22:aa:bb:cc", "00:11:22:aa:bb:cc"), + ("00:11:22:AA:BB:CC", "00:11:22:aa:bb:cc"), + ("pass-through-the-unexpected", "pass-through-the-unexpected"), + ("", ""), + ], + ) + def test_normalize_scenarios(self, mac, expected): + normalized = dsaz.normalize_mac_address(mac) + assert normalized == expected + + def test_empty( + self, azure_ds, caplog, mock_get_interfaces, mock_get_interface_mac + ): + imds_md = {} + + assert azure_ds.validate_imds_network_metadata(imds_md) is False + assert ( + "cloudinit.sources.DataSourceAzure", + 30, + "IMDS network metadata has incomplete configuration: None", + ) in caplog.record_tuples + + def test_validates_one_nic( + self, azure_ds, mock_get_interfaces, mock_get_interface_mac + ): + + mock_get_interfaces.return_value = [ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + azure_ds._ephemeral_dhcp_ctx = mock.Mock(iface="test0") + + imds_md = { + "network": { + "interface": [ + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "001122334455", + } + ] + } + } + + assert azure_ds.validate_imds_network_metadata(imds_md) is True + + def test_validates_multiple_nic( + self, azure_ds, mock_get_interfaces, mock_get_interface_mac + ): + + mock_get_interfaces.return_value = [ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"), + ("test1", "01:11:22:33:44:55", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + azure_ds._ephemeral_dhcp_ctx = mock.Mock(iface="test0") + + imds_md = { + "network": { + "interface": [ + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "001122334455", + }, + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "011122334455", + }, + ] + } + } + + assert azure_ds.validate_imds_network_metadata(imds_md) is True + + def test_missing_all( + self, azure_ds, caplog, mock_get_interfaces, mock_get_interface_mac + ): + + mock_get_interfaces.return_value = [ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"), + ("test1", "01:11:22:33:44:55", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + azure_ds._ephemeral_dhcp_ctx = mock.Mock(iface="test0") + + imds_md = {"network": {"interface": []}} + + assert azure_ds.validate_imds_network_metadata(imds_md) is False + assert ( + "cloudinit.sources.DataSourceAzure", + 30, + "IMDS network metadata is missing configuration for NICs " + "['00:11:22:33:44:55', '01:11:22:33:44:55']: " + f"{imds_md['network']!r}", + ) in caplog.record_tuples + + def test_missing_primary( + self, azure_ds, caplog, mock_get_interfaces, mock_get_interface_mac + ): + + mock_get_interfaces.return_value = [ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"), + ("test1", "01:11:22:33:44:55", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + azure_ds._ephemeral_dhcp_ctx = mock.Mock(iface="test0") + + imds_md = { + "network": { + "interface": [ + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "011122334455", + }, + ] + } + } + + assert azure_ds.validate_imds_network_metadata(imds_md) is False + assert ( + "cloudinit.sources.DataSourceAzure", + 30, + "IMDS network metadata is missing configuration for NICs " + f"['00:11:22:33:44:55']: {imds_md['network']!r}", + ) in caplog.record_tuples + assert ( + "cloudinit.sources.DataSourceAzure", + 30, + "IMDS network metadata is missing primary NIC " + f"'00:11:22:33:44:55': {imds_md['network']!r}", + ) in caplog.record_tuples + + def test_missing_secondary( + self, azure_ds, mock_get_interfaces, mock_get_interface_mac + ): + + mock_get_interfaces.return_value = [ + ("dummy0", "9e:65:d6:19:19:01", None, None), + ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"), + ("test1", "01:11:22:33:44:55", "hv_netvsc", "0x3"), + ("lo", "00:00:00:00:00:00", None, None), + ] + azure_ds._ephemeral_dhcp_ctx = mock.Mock(iface="test0") + + imds_md = { + "network": { + "interface": [ + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "001122334455", + }, + ] + } + } + + assert azure_ds.validate_imds_network_metadata(imds_md) is False + + # vi: ts=4 expandtab