Skip to content

Commit

Permalink
fix(ec2): Fix broken uuid match with other-endianness (#5236)
Browse files Browse the repository at this point in the history
EC2 documents that the system-uuid may be reported in different endianness[1].

A user has reported a case where cloud-init is broken due to inability to
detect the system platform. Fix it.

Behavior change:

Cloud-init was previously making the assumption that uuid and serial would match
on ec2. This assumption was:

1) not documented as a valid way to identify ec2[1]

2) proven invalid on ec2 by the DMI_PRODUCT_SERIAL and DMI_PRODUCT_UUID reported in #5105

3) used in the logic which warns about not running on the "real" ec2

Preserving this warning logic exactly as it was presents several challenges:

a) Risk of regression outside of our control: Since this logic relied upon undocumented
   behavior, AWS could change this at any point, which would break all cloud-init
   instances.

b) Risk of incorrect implementation: What format is the uuid and product serial actually
   in? We don't know. It's easy and safe to just swap the byteorder of the first segment
   of the uuid because this is documented, but matching the whole uuid is problematic
   because UUID formats may be presented as mixed encoding (partially little endian and
   partially big endian). To implement this behavior while fixing this bug we would have
   to make even more assumptions than before. I propose we stop assuming and if a cloud
   happens to implement the same as EC2 (minus the serial/product match), then we just
   don't emit that warning. It's simpler, it's safer, and I really don't think that it is
   a huge change. This is a "change in behavior", but the change is that the code more
   correctly identifies EC2 and would no longer emit a warning on valid ec2 instances, so
   I don't think that this would require omitting this change from SRU.

c) Implementing whatever assumptions we make in b) would require implementing a
   byteswapping algorithm in POSIX shell, which is possible but best to avoid this if
   possible.

[1] https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/identify_ec2_instances.html

Fixes GH-5105
  • Loading branch information
holmanb authored May 1, 2024
1 parent 66a874b commit c1a19d7
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 47 deletions.
59 changes: 26 additions & 33 deletions cloudinit/sources/DataSourceEc2.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import logging
import os
import time
import uuid
from contextlib import suppress
from typing import Dict, List

from cloudinit import dmi, net, sources
Expand Down Expand Up @@ -794,11 +796,17 @@ def identify_aliyun(data):

def identify_aws(data):
# data is a dictionary returned by _collect_platform_data.
if data["uuid"].startswith("ec2") and (
data["uuid_source"] == "hypervisor" or data["uuid"] == data["serial"]
):
uuid_str = data["uuid"]
if uuid_str.startswith("ec2"):
# example same-endian uuid:
# EC2E1916-9099-7CAF-FD21-012345ABCDEF
return CloudNames.AWS

with suppress(ValueError):
if uuid.UUID(uuid_str).bytes_le.hex().startswith("ec2"):
# check for other endianness
# example other-endian uuid:
# 45E12AEC-DCD1-B213-94ED-012345ABCDEF
return CloudNames.AWS
return None


Expand Down Expand Up @@ -853,7 +861,6 @@ def _collect_platform_data():
Keys in the dictionary are as follows:
uuid: system-uuid from dmi or /sys/hypervisor
uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi'
serial: dmi 'system-serial-number' (/sys/.../product_serial)
asset_tag: 'dmidecode -s chassis-asset-tag'
vendor: dmi 'system-manufacturer' (/sys/.../sys_vendor)
Expand All @@ -862,37 +869,23 @@ def _collect_platform_data():
On Ec2 instances experimentation is that product_serial is upper case,
and product_uuid is lower case. This returns lower case values for both.
"""
data = {}
try:
uuid = None
with suppress(OSError, UnicodeDecodeError):
uuid = util.load_text_file("/sys/hypervisor/uuid").strip()
data["uuid_source"] = "hypervisor"
except Exception:
uuid = dmi.read_dmi_data("system-uuid")
data["uuid_source"] = "dmi"

if uuid is None:
uuid = ""
data["uuid"] = uuid.lower()

serial = dmi.read_dmi_data("system-serial-number")
if serial is None:
serial = ""

data["serial"] = serial.lower()

asset_tag = dmi.read_dmi_data("chassis-asset-tag")
if asset_tag is None:
asset_tag = ""
uuid = uuid or dmi.read_dmi_data("system-uuid") or ""
serial = dmi.read_dmi_data("system-serial-number") or ""
asset_tag = dmi.read_dmi_data("chassis-asset-tag") or ""
vendor = dmi.read_dmi_data("system-manufacturer") or ""
product_name = dmi.read_dmi_data("system-product-name") or ""

data["asset_tag"] = asset_tag.lower()

vendor = dmi.read_dmi_data("system-manufacturer")
data["vendor"] = (vendor if vendor else "").lower()

product_name = dmi.read_dmi_data("system-product-name")
data["product_name"] = (product_name if product_name else "").lower()

return data
return {
"uuid": uuid.lower(),
"serial": serial.lower(),
"asset_tag": asset_tag.lower(),
"vendor": vendor.lower(),
"product_name": product_name.lower(),
}


def _build_nic_order(
Expand Down
24 changes: 19 additions & 5 deletions tests/unittests/sources/test_ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ class TestEc2:

valid_platform_data = {
"uuid": "ec212f79-87d1-2f1d-588f-d86dc0fd5412",
"uuid_source": "dmi",
"serial": "ec212f79-87d1-2f1d-588f-d86dc0fd5412",
}

Expand Down Expand Up @@ -855,7 +854,7 @@ def test_unknown_platform_with_strict_true(self, mocker, tmpdir):
"""Unknown platform data with strict_id true should return False."""
uuid = "ab439480-72bf-11d3-91fc-b8aded755F9a"
ds = self._setup_ds(
platform_data={"uuid": uuid, "uuid_source": "dmi", "serial": ""},
platform_data={"uuid": uuid, "serial": ""},
sys_cfg={"datasource": {"Ec2": {"strict_id": True}}},
md={"md": DEFAULT_METADATA},
mocker=mocker,
Expand All @@ -869,7 +868,7 @@ def test_unknown_platform_with_strict_false(self, mocker, tmpdir):
"""Unknown platform data with strict_id false should return True."""
uuid = "ab439480-72bf-11d3-91fc-b8aded755F9a"
ds = self._setup_ds(
platform_data={"uuid": uuid, "uuid_source": "dmi", "serial": ""},
platform_data={"uuid": uuid, "serial": ""},
sys_cfg={"datasource": {"Ec2": {"strict_id": False}}},
md={"md": DEFAULT_METADATA},
mocker=mocker,
Expand Down Expand Up @@ -1673,20 +1672,35 @@ def test_convert_ec2_metadata_gets_macs_from_get_interfaces_by_mac(self):
)


class TesIdentifyPlatform:
class TestIdentifyPlatform:
def collmock(self, **kwargs):
"""return non-special _collect_platform_data updated with changes."""
unspecial = {
"asset_tag": "3857-0037-2746-7462-1818-3997-77",
"serial": "H23-C4J3JV-R6",
"uuid": "81c7e555-6471-4833-9551-1ab366c4cfd2",
"uuid_source": "dmi",
"vendor": "tothecloud",
"product_name": "cloudproduct",
}
unspecial.update(**kwargs)
return unspecial

@mock.patch("cloudinit.sources.DataSourceEc2._collect_platform_data")
def test_identify_aws(self, m_collect):
"""aws should be identified if uuid starts with ec2"""
m_collect.return_value = self.collmock(
uuid="ec2E1916-9099-7CAF-FD21-012345ABCDEF"
)
assert ec2.CloudNames.AWS == ec2.identify_platform()

@mock.patch("cloudinit.sources.DataSourceEc2._collect_platform_data")
def test_identify_aws_endian(self, m_collect):
"""aws should be identified if uuid starts with ec2"""
m_collect.return_value = self.collmock(
uuid="45E12AEC-DCD1-B213-94ED-012345ABCDEF"
)
assert ec2.CloudNames.AWS == ec2.identify_platform()

@mock.patch("cloudinit.sources.DataSourceEc2._collect_platform_data")
def test_identify_aliyun(self, m_collect):
"""aliyun should be identified if product name equals to
Expand Down
14 changes: 14 additions & 0 deletions tests/unittests/test_ds_identify.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,13 @@ def test_aws_ec2_hvm_env(self):
"""
self._test_ds_found("Ec2-hvm-env")

def test_aws_ec2_hvm_endian(self):
"""EC2: hvm instances use system-uuid and may have swapped endianness
test using SYSTEMD_VIRTUALIZATION, not systemd-detect-virt
"""
self._test_ds_found("Ec2-hvm-swap-endianness")

def test_aws_ec2_xen(self):
"""EC2: sys/hypervisor/uuid starts with ec2."""
self._test_ds_found("Ec2-xen")
Expand Down Expand Up @@ -1592,6 +1599,13 @@ def _print_run_output(rc, out, err, cfg, files):
P_PRODUCT_UUID: "EC23AEF5-54BE-4843-8D24-8C819F88453E\n",
},
},
"Ec2-hvm-swap-endianness": {
"ds": "Ec2",
"mocks": [{"name": "detect_virt", "RET": "kvm", "ret": 0}],
"files": {
P_PRODUCT_UUID: "AB232AEC-54BE-4843-8D24-8C819F88453E\n",
},
},
"Ec2-hvm-env": {
"ds": "Ec2",
"mocks": [{"name": "detect_virt_env", "RET": "vm:kvm", "ret": 0}],
Expand Down
25 changes: 16 additions & 9 deletions tools/ds-identify
Original file line number Diff line number Diff line change
Expand Up @@ -1347,16 +1347,23 @@ ec2_identify_platform() {
return 0
fi

# product uuid and product serial start with case insensitive
local uuid="${DI_DMI_PRODUCT_UUID}"
case "$uuid:$serial" in
[Ee][Cc]2*:[Ee][Cc]2*)
# both start with ec2, now check for case insensitive equal
nocase_equal "$uuid" "$serial" &&
{ _RET="AWS"; return 0; };;
# keep only the first octet
local start_uuid="${DI_DMI_PRODUCT_UUID%%-*}"
case "$start_uuid" in
# example ec2 uuid:
# EC2E1916-9099-7CAF-FD21-012345ABCDEF
[Ee][Cc]2*)
_RET="AWS"
;;
# example ec2 uuid:
# 45E12AEC-DCD1-B213-94ED-012345ABCDEF
*2[0-9a-fA-F][Ee][Cc])
_RET="AWS"
;;
*)
_RET="$default"
;;
esac

_RET="$default"
return 0;
}

Expand Down

0 comments on commit c1a19d7

Please sign in to comment.