diff options
author | Chris Patterson <cpatterson@microsoft.com> | 2022-02-14 14:53:36 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-14 13:53:36 -0600 |
commit | 32fcbb580d6eacb06c901bc291e0fa118bb9b646 (patch) | |
tree | dae2663e521288317bcd00137f85435f9696d68d | |
parent | 1d3ddaf93378d1de6be34c7e6430e9b64d42b3f7 (diff) | |
download | vyos-cloud-init-32fcbb580d6eacb06c901bc291e0fa118bb9b646.tar.gz vyos-cloud-init-32fcbb580d6eacb06c901bc291e0fa118bb9b646.zip |
sources/azure: validate IMDS network configuration metadata (#1257)
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 <cpatterson@microsoft.com>
-rwxr-xr-x | cloudinit/sources/DataSourceAzure.py | 82 | ||||
-rw-r--r-- | 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 2566bd8e..f8e1dd02 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 @@ class DataSourceAzure(sources.DataSource): # 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 @@ class DataSourceAzure(sources.DataSource): 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 bd525b13..ecedc54d 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -37,6 +37,8 @@ from tests.unittests.helpers import ( 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 @@ IMDS_NETWORK_METADATA = { ] } -MOCKPATH = "cloudinit.sources.DataSourceAzure." EXAMPLE_UUID = "d0df4c54-4ecb-4a4b-9954-5bdf3ed5c3b8" @@ -764,6 +781,13 @@ scbus-1 on xpt0 bus 0 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 @@ scbus-1 on xpt0 bus 0 ), (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 @@ scbus-1 on xpt0 bus 0 ) 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 @@ class TestRandomSeed(CiTestCase): 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 |