-
Notifications
You must be signed in to change notification settings - Fork 908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sources/azure: validate IMDS network configuration metadata #1257
sources/azure: validate IMDS network configuration metadata #1257
Conversation
378e256
to
c34f236
Compare
@@ -178,6 +178,26 @@ def find_dev_from_busdev(camcontrol_out, busdev): | |||
return None | |||
|
|||
|
|||
def normalize_mac_address(mac: str): | |||
"""Normalize mac address with colons and lower-case.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What "non-colon" formats is this expected to normalize? Just curious since I don't see any new tests in this PR that appear to make use of this. This would obviously fail the format 'HHHH.HHHH.HHHH' that some network gear vendors use, but I assume that's out of scope for the inputs that this is supposed to normalize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't added tests yet, this was just a draft for early review :D
I expect get_interfaces() will always be in this lower-case, colon format - but I figured I'd pass it through regardless.
IMDS metadata uses upper-case w/o colons, so it needs the normalization for comparison.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use this method here too:
cloud-init/cloudinit/sources/DataSourceAzure.py
Line 2274 in c34f236
mac = ":".join(re.findall(r"..", intf["macAddress"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, thanks!
bd6a3d8
to
0ebae8c
Compare
0ebae8c
to
984d5b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
443cb30
to
eee1ee9
Compare
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>
eee1ee9
to
f893801
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Due to race conditions and caching, IMDS may return stale or incomplete
metadata. Add some validation to detect these scenarios and report
appropriate telemetry.
Signed-off-by: Chris Patterson cpatterson@microsoft.com