[OSSA-2024-004] Ironic sometimes fails to verify checksums of supplied image_source URLs (CVE-2024-47211)

[Julia Kreger]

Ironic contains a number of distinct features which also evolved over time to meet the needs of the community and infrastructure operators.

Originally, Image content only came from a trusted image store (Glance) where the checksum was known to have been pre-validated, and often just recorded by Glance, so ironic-python-agent could validate the checksum while streaming the image contents to disk in order to detect either a MITM or change in the source image which was not known, which failed the deploy to provide feedback to the operator.

As time evolved, capabilities were added to allow operators to optimize traffic and download images directly from the source to the agent. Luckily the agent's downloads do enforce checksums which is how this was observed

The issue arises when we've asked the conductor, via use of the "image_download_source" option, when set to local, with an image_source of a remote URL, the contents of the file are downloaded to the conductor, which is expected design. AT this point, If the forced conversion of the download to a raw image is not enabled, then the agent or deployment interface driver would then be responsible for validating the checksum of the file. But when you force the image to raw to facilitate streaming raw images to the agent which is a more peformant path, the checksum is *not* consulted properly before rewriting the image file contents, and the checksum on file is then replaced with the new image to be streamed out.

The net result, in a direct usage of ironic's API, an authenticated user with privileges to deploy a node, user may request a remote image from a URL which does not have it's checksum verified after transport, leaving that action open to a potential MITM attack.

The chain of this issue appears to start at:

https://github.com/openstack/ironic/blob/master/ironic/common/images.py#L389-L393

And then the image_to_raw method *should* checksum the file, but it does not:

https://github.com/openstack/ironic/blob/master/ironic/common/images.py#L389-L393

Once the newly converted raw image is on disk, then ironic continues with it's process and generates a new checksum, which is made available for IPA to verify the download of the content.

https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/deploy_utils.py#L1157-L1197

In essence, ironic needs to perform the same basic check that IPA performs to validate the checksum is also what the user supplied and that there is not a mismatch in the value. This should realistically be done prior to converting the image, but could potentially be done after fetching the file. The major issue is part of ironic's contract is that the checksum is performed for disk images to ensure that nothing has happened in the mean time since the URL was retrieve and validated or occurred while in transport.

Assigning to myself as I'm likely the one who will code and test the fix for this.

[Julia Kreger (juliaashleykreger)]

Attached is a first pass fix. If I can get a sanity check on it from another ironic-core, that would be much appreciated.

[Julia Kreger (juliaashleykreger)]

I discussed this with JayF who is an ironic-core and on the OpenStack VMT team. Overall, pretty good and the flow makes sense given structure/pattern, but some notes below.

common/images.py:

* We likely need to check for errors coming from oslo.utils.fileutils, specifically if we get an unsupported hashlib algorithm supplied.
* We *likely* need to adopt length based fingerprint checking to mirror IPA in standalone cases.
* Current handling of md5 does not respect the default md5 option knob, that needs to be fixed.
* Upper case input testing is missing.
* Normalize checksum algo to lower, and test as well, just to be on the extra safe side.

conf/conductor.py

* Restructure the help text to say "In the default case, files have their checksum verified prior to conversion"

drivers/modules/deploy_utils.py:

* _get_checksum_and_algo -> Might be able to do the algorithm guess here, but we should consult what IPA does presently.

* Some excess whitespace changes in deploy_utils in the patch, needs to be fixed.

* One other thing to consider, we likely need to validate against available hashlib algorithms Data example below. IPA explicitly, at least *newer* IPA versions have specific guess matching.

>>> hashlib.algorithms_available
{'blake2b', 'shake_256', 'sha512_224', 'blake2s', 'sha384', 'sha3_384', 'sha512_256', 'sha3_512', 'md5-sha1', 'shake_128', 'sha3_256', 'sha224', 'sm3', 'sha3_224', 'md5', 'sha1', 'sha512', 'sha256'}

As an action item, we also wanted to check what glance defaults to, and it is sha512, but that is on storage of an artifact based upon the default configuration.

[Julia Kreger (juliaashleykreger)]

After discussing with JayF more yesterday, after taking a holistic look at the usage of the image_checksum field and the support which was added into the ironic-python-agent for URL based checksum payload files, a consensus was reached that we should discuss with Dmitry Tantsur, and I took the action item to do so first thing this morning.

The TL;DR:

Even with the scoping context, Dmitry thinks this is still a CVE, albeit minor given the number of aspect s which need to line up outside of the "normal" settings, and the biggest challenge with this is the disjointed context regarding purpose of the field, i.e. is it for data validation or is it for security.

A risk of regression which Dmitry was also able to highlight is a potential case of a fix breaking IPA users if the are opting into the conductor local conversion and stream out to the ramdisk, where today "it silently works". The net result seems to be that we will need to bolster the checksum support in ironic to reach parity with IPA.

A noted major outcome also needs to be to improve the documentation to appropriately set user context. i.e. is the checksum a security feature, or a data integrity feature. The necessary logic fix itself is relatively simple even though it really requires working all the way through the cache interaction.

[Julia Kreger (juliaashleykreger)]

Revised patch for review.

[Dmitry Tantsur (divius)]

In exceptions:

+class ImageChecksumError(ImageUnacceptable):

ImageUnacceptable seems to have HTTP code 500, which is wrong here. Btw the same affects the existing ImageInvalid.

In documentation:

> Use of the node ``instance_info\image_checksum`` field **is** discouraged

I disagree with this statement. It makes sense from the Glance perspective, but for a standalone user, image_checksum is easy to use and to remember (how do you remember the "_os_" part?), especially since the hash algorithm is mostly redundant in real world.

> [conductor]disable_conductor_support_for_checksum_files

nit: no need to repeat "conductor" in the option name

In def validate_checksum:

> use_checksum_algo is not None and use_checksum_algo == ''

The first part is completely redundant: if it's '' it's not None. Or did you mean to check use_checksum instead?

+ if not use_checksum_algo:
+ # This is backwards compatible support for a bare checksum.
+ if not CONF.agent.allow_md5_checksum:

I'd prefer the md5 logic moved to _get_checksum_and_algo where the detection of other algorithms is happening.

+ LOG.error("Failed to read file %(path)s to compute checksum.",
+ {"path": path})
+ raise exception.ImageChecksumError()

This is not a checksum error though, you're unable to read the *image file*, right? I'd handle it differently.

+ if calculated.lower() != use_checksum.lower():

This will crash if use_checksum is None, which seems to be an accepted case.

In image_cache:

+def _fetch(context, image_href, path, force_raw=False, expected_format=None,
+ expected_checksum=None, expected_checksum_algo=None):

New parameters completely unused?

General comment: could you group the new checksum code in a new module, like ironic.common.checksum_utils? Essentially all completely new functions could go there. I'm especially concerned about growing deploy_utils further: it's already a hard to manage mess.

[Jay Faulkner (jason-oldos)]

Draft OSSA attached.

[Jay Faulkner (jason-oldos)]

CVE Request 1731951 is in to MITRE to get a CVE assigned.

[Julia Kreger (juliaashleykreger)]

Jay, regarding the draft. It looks good except for "As usual, we will provide updated releaes off..."

One thought, we could frame it with "image_download_source" as "local" as well. When set to HTTP, raw image streaming and conversion doesn't take place, and the alternate path is swift/glance. Technically, this is an evolution involving "image_download_source" option "local" which happened in Ussuri if I'm recalling correctly.

That being said, one could view the same issue if they were to emulate glance v1 and MITM the interaction which would definitely also mean prior versions are impacted as well. I guess the thought processes and question is more how do we present the slice of the issue, or not.

[Julia Kreger (juliaashleykreger)]

Hey Dmitry, Thanks for the feedback.

I'm unsure if the HTTP return code matters in this case, since we're past task.process_event launching a new worker to handle the next steps of the deployment process. From a point of order standpoint, I'll fix it just in case it ever gets changed/evolved in the future to cover the base concern since I do agree in principal.

Regarding docs, I'll attempt to better split the hair there. Bottom line boils down to we can't block on precise wording for this, but I see your point.

Overall, I suspect some of your comments are driven by some context needing to be added which we already discussed, but you've definitely got some points I likely need to address and I'll move it all to a separate file to lessen back port pain. I may have also misread one of your comments when we discussed, but I should have a new revision by your tomorrow afternoon. Thanks!

[Julia Kreger (juliaashleykreger)]

Dmitry, regarding your concern of new parameters completely unused, I added them as positional on the callers. I'll fix. Thanks!

[Julia Kreger (juliaashleykreger)]

Greetings Dmitry, I'm attaching a new version to take a look at. Please let me know what you think. I do need to do a little more work on the unit tests since shifting some of the code over.

Thanks!

[Dmitry Tantsur (divius)]

Just a thought on disable_support_for_checksum_files: we probably need a knob in IPA to achieve the same effect.

Are you sure you've added checksum_utils to the patch? I cannot see it.

[Dmitry Tantsur (divius)]

> Dmitry, regarding your concern of new parameters completely unused, I added them as positional on the callers. I'll fix. Thanks!

Seems not fixed still?

[Julia Kreger (juliaashleykreger)]

Sorry about that, now with checksum_utils.

[Julia Kreger (juliaashleykreger)]

Regarding IPA, I think that is a hard-no, because we've *always* had the checksum requirement, and so adding a parity feature to it as part of a CVE fix seems like a bad path to take. We set that expectation, and the knob is only to disable the operation on the conductor to enable short term route-around issues.

Regarding the new parameters, they are passed and that explicit change to keyword arg parameters forced unit test changes along the code involved, but sometimes they can be optional based upon the overall code path's invocation. The *key* is the images we deploy though and that is the contract set up which matches IPA's behavior on image downloads and write-outs. The contract being, if someone says "this image with this checksum", we need to verify it, not just ignore the checksum and overwrite the checksum when converted to raw locally.

[Julia Kreger (juliaashleykreger)]

Chatted with Dmitry because it seemed we were not on the same page. I misread his prior comment about IPA. That would be a feature to have support for that lockout to be extended, the only reason to have it in ironic is because it is technically a feature in parity to help address the overall problem.

He was also able to point out the line I wasn't seeing in the code otherwise, so we're good now. I just need to do some minor cleanup and test changes and we should be good to begin testing in devstack soon().

[Julia Kreger (juliaashleykreger)]

Dmitry, revised patch is attached with moved testing for the most part. I didn't completely duplicate testing where it made sense as well. Trying to walk a reasonable path there. Please let me know! Thanks!

I'll likely spin up a devstack tomorrow and run this patch through it's paces .

[Dmitry Tantsur (divius)]

Overall note: I'd appreciate a bit more detailed docstrings for new functions, especially around return values and exceptions (anything for get_checksum_from_url).

---

In def get_checksum_from_url(checksum, image_source):

nit: a few cases of re.findall which can be replaced by search since we only care about the first instance (yes, it was probably me who wrote this code initially, I know :)

+ reason='Checksum file empty.')

missing _()

+ reason=("Checksum file does not contain name %s" % expected_fname))

should be

+ reason=_("Checksum file does not contain name %s") % expected_fname)

---

In def get_checksum_and_algo(instance_info):

+ # NOTE(TheJulia): This is all based on SHA-2 lengths.
+ # SHA-3 would require a hint, thus ValueError because
+ # it may not be a fixed length. That said, SHA-2 is not
+ # as of this not being added, being withdrawn standards wise.

I'm a bit confused. First, I don't see what ValueError refers to. Second, double "not" in the last sentence. Third, did you mean SHA-3 being withdrawn?

---

In unit tests

+class IronicChecksumUtilsTestCase(base.TestCase):

I'd prefer several classes for different functions (bonus: you can move @mock decorators to the class level).

===

None of the comments are critical, so revision 4 is good to go as far as I'm concerned. Thank you for working on this issue!

[Jay Faulkner (jason-oldos)]

The diff looks fine to me, I have no additional comments other than this general question:
Why are we even giving people a knob to disable checking the checksum? If we can't articulate a use case for it, I'd be hard pressed to think that hatch should exist -- at least in the master version of the patch. If we want to add it for stable to avoid breaking folks, I can understand that a little.

If there *is* a known case where this can break existing installs; that's also a case that's potentially being exploited today. Why is it OK to let them disable checksum checking?

[Jay Faulkner (jason-oldos)]

A second note: I wouldn't hate having disable_support_for_checksum_files being passed through to IPA, but that's an enhancement for IPA later, not a backportable security fix.

Note: I still have not been issued a CVE number by mitre. Technically the 5 business days -- the maximum they suggest it will take -- is in a couple of course. I will follow up Monday if I haven't received one yet.

[Julia Kreger (juliaashleykreger)]

Greetings Jay, The general knob to disable checksum is the escape latch switch for operators to be able to pull when they have applied the fix, where they discover they have users which were sending an incorrect checksum the entire time.

I'm basically -2 to the approach of only having a knob in stable backports because of the increased risk for issues with backports. Given how limited my time is, this change needs to be as clean a fix as possible with as minimal risk as possible.

If at all possible, I need to get this fix shipped next week as well. I'll be testing this week.

If y'all decide we would rather not have a knob, I'm fine with that as well. The bottom line is the knob is an attempt to be nice to operators for when this drops because rebuilds may suddenly break if the original request had a bad checksum. This is also why we loaded in the support for checksums from URLs, because that is a feature which was added to IPA and Ironic needs to be aware of it to navigate how to handle the request if the operator has configured for the image to be converted by the conductor.

[Dmitry Tantsur (divius)]

Maybe we should insta-deprecate the knob? Otherwise, I agree with Julia's line of thoughts.

[Julia Kreger (juliaashleykreger)]

I'm totally cool with doing so.

[Jay Faulkner (jason-oldos)]

CVE-2024-47211 has been assigned to this security issue.

[Jay Faulkner (jason-oldos)]

As far as I'm concerned, marking it as deprecated in backports and removing it on master ASAP is essentially a perfect implementation of my suggestion. I'm onboard to keep the knob but insta-deprecate it.

Thanks!

[Jay Faulkner (jason-oldos)]

Updated OSSA with CVE reference attached.

[Julia Kreger (juliaashleykreger)]

Swapped the knob to insta-deprecate.

While doing basic testing, I realized a slight problem with the original approach which was to only checksum *if* we're doing a conversion. I changed that to go ahead and just always run the checksum if the image fetch is triggered and we have a checksum, from the simple standpoint that it will also provide faster feedback for users. The issue with my approach is the image conversion to raw is split into two parts, the fetch and then later the force to raw, and due to the modeling the checksum was only getting passed in on the first call. Stepping back a little made a bit more sense.

Changed documentation and removed the newly added knob and adjusted the release note.

Overall, a bit more simple. Seems better. I'm post a new patch here in a little bit and y'all can take a look.

Dmitry, also made some of the changes noted, but do also want to sort of get this locked down and begin backporting this week.

[Julia Kreger (juliaashleykreger)]

Jay, the summary text in the OSSA does need to be revised since we've cut a release last week.

[Dmitry Tantsur (divius)]

In def get_checksum_from_url(checksum, image_source):

+ raise exception.ImageDownloadfailed(
+ image_href=checksum,
+ reason=("Checksum file does not contain name %s" % expected_fname))

Misspelled exception name ImageDownloadfailed vs ImageDownloadFailed. Makes me suspect that this code path is not covered by tests.

Also missing i18n around the error message.

+ # NOTE(TheJulia): This testing file *lacks* the url testing explicitly,
+ # however that is presently covered by invocation through code testing
+ # in test_deploy_utils.py, which seems like a happy middle ground.

We probably should get some tests as the issue above shows. I don't think the parsing part is actually covered indirectly.

[Julia Kreger (juliaashleykreger)]

Attached is a quick diff for Dmitry.

[Julia Kreger (juliaashleykreger)]

Attached is the latest version of the fix, with the changes Dmitry requested *and* one spelling fix codespell found.

[Jay Faulkner (jason-oldos)]

A couple minor nitpicks in security.rst:
- When we reference subfields in JSON-based db fields, we usually use forward slash (instance_info/image_checksum) not reverse slash (instance_info\image_checksum)
- When you reference the config item, you can use the new oslo config reference syntax that we've made consistent throughout the docs.

Neither of these would prevent me from +2'ing the patch and it's backports.

[Julia Kreger (juliaashleykreger)]

Greetings Jay, Done. Diff below.

+++ b/doc/source/admin/security.rst
@@ -38,21 +38,21 @@ Ironic users leverage, outside of the "general" use case capabilities provided
 by the ``direct`` deployment interface.

 .. NOTE::
- Use of the node ``instance_info\image_checksum`` field is discouraged
+ Use of the node ``instance_info/image_checksum`` field is discouraged
    for integrated OpenStack Users as usage of the matching Glance Image
    Service field is also deprecated. That being said, Ironic retains this
    feature by popular demand while also enabling also retain simplified
    operator interaction.
    The newer field values supported by Glance are also specifically
- supported by Ironic as ``instance_info\image_os_hash_value`` for
- checksum values and ``instance_info\image_os_hash_algo`` field for
+ supported by Ironic as ``instance_info/image_os_hash_value`` for
+ checksum values and ``instance_info/image_os_hash_algo`` field for
    the checksum algorithm.

 .. WARNING::
    Setting a checksum value to a URL is supported, *however* doing this is
    making a "tradeoff" with security as the remote checksum *can* change.
    Conductor support this functionality can be disabled using the
- ``[conductor]disable_support_for_checksum_files``.
+ :oslo.config:option:`conductor.disable_support_for_checksum_files` setting.

 REST API: user roles and policy settings
 ========================================

[Dmitry Tantsur (divius)]

> Attached is a quick diff for Dmitry.

LGTM (also with the changes Jay requested).

[Jay Faulkner (jason-oldos)]

embargo-notice and linux-distros list informed per VMT process

https://security.openstack.org/vmt-process.html#downstream-stakeholders-notification-email-private-issues

[Jay Faulkner (jason-oldos)]

A `git am` friendly version of the OSSA patch, since I may be out on disclosure date. Please be sure to edit the yaml to update the gerrit links once they exist.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ironic/+/931293

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2024.2
Review: https://review.opendev.org/c/openstack/ironic/+/931294

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2024.1
Review: https://review.opendev.org/c/openstack/ironic/+/931295

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: bugfix/26.0
Review: https://review.opendev.org/c/openstack/ironic/+/931296

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: bugfix/25.0
Review: https://review.opendev.org/c/openstack/ironic/+/931297

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: bugfix/24.0
Review: https://review.opendev.org/c/openstack/ironic/+/931298

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2023.2
Review: https://review.opendev.org/c/openstack/ironic/+/931299

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/2023.1
Review: https://review.opendev.org/c/openstack/ironic/+/931300

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: unmaintained/zed
Review: https://review.opendev.org/c/openstack/ironic/+/931301

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: unmaintained/yoga
Review: https://review.opendev.org/c/openstack/ironic/+/931302

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: unmaintained/xena
Review: https://review.opendev.org/c/openstack/ironic/+/931303

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: unmaintained/wallaby
Review: https://review.opendev.org/c/openstack/ironic/+/931304

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: unmaintained/victoria
Review: https://review.opendev.org/c/openstack/ironic/+/931305

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931296
Committed: https://opendev.org/openstack/ironic/commit/6f653d7b52c49a4d82c1cbdf53be97c4fd79da4a
Submitter: "Zuul (22348)"
Branch: bugfix/26.0

commit 6f653d7b52c49a4d82c1cbdf53be97c4fd79da4a
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931297
Committed: https://opendev.org/openstack/ironic/commit/799900d1f180ffb785a2a5da29e49b089f608a3f
Submitter: "Zuul (22348)"
Branch: bugfix/25.0

commit 799900d1f180ffb785a2a5da29e49b089f608a3f
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931298
Committed: https://opendev.org/openstack/ironic/commit/c164641b4526fad904825f3dae8dfc5041e91295
Submitter: "Zuul (22348)"
Branch: bugfix/24.0

commit c164641b4526fad904825f3dae8dfc5041e91295
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931293
Committed: https://opendev.org/openstack/ironic/commit/00c5e0faf8e435bf3ceb24399ef0d853f3058590
Submitter: "Zuul (22348)"
Branch: master

commit 00c5e0faf8e435bf3ceb24399ef0d853f3058590
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931299
Committed: https://opendev.org/openstack/ironic/commit/b3d85c494af6f692ebb80918a06d36de7756c9ad
Submitter: "Zuul (22348)"
Branch: stable/2023.2

commit b3d85c494af6f692ebb80918a06d36de7756c9ad
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931294
Committed: https://opendev.org/openstack/ironic/commit/afe4f480905fb80cb7cfe98c43f15b03ea011fea
Submitter: "Zuul (22348)"
Branch: stable/2024.2

commit afe4f480905fb80cb7cfe98c43f15b03ea011fea
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931295
Committed: https://opendev.org/openstack/ironic/commit/0d6b5c9a283b186caae5c83477ea58f5a3543b56
Submitter: "Zuul (22348)"
Branch: stable/2024.1

commit 0d6b5c9a283b186caae5c83477ea58f5a3543b56
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931301
Committed: https://opendev.org/openstack/ironic/commit/336d76a111987206a64e3bb89a63cc4a7d181c55
Submitter: "Zuul (22348)"
Branch: unmaintained/zed

commit 336d76a111987206a64e3bb89a63cc4a7d181c55
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 23.0.3 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 24.1.3 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 26.1.1 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931305
Committed: https://opendev.org/openstack/ironic/commit/0942a1b198f8035d30fb40d512d268e128336b8d
Submitter: "Zuul (22348)"
Branch: unmaintained/victoria

commit 0942a1b198f8035d30fb40d512d268e128336b8d
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931304
Committed: https://opendev.org/openstack/ironic/commit/2f514be6205c00ca3245cd5d7ccd80f8cc5d6f0d
Submitter: "Zuul (22348)"
Branch: unmaintained/wallaby

commit 2f514be6205c00ca3245cd5d7ccd80f8cc5d6f0d
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931303
Committed: https://opendev.org/openstack/ironic/commit/7afa6ba056969e42ac6fcf707a95b78345d009c7
Submitter: "Zuul (22348)"
Branch: unmaintained/xena

commit 7afa6ba056969e42ac6fcf707a95b78345d009c7
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931300
Committed: https://opendev.org/openstack/ironic/commit/b7cc66502e0628c22f6e9a600b188a0bf6c2c398
Submitter: "Zuul (22348)"
Branch: stable/2023.1

commit b7cc66502e0628c22f6e9a600b188a0bf6c2c398
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/ironic/+/931302
Committed: https://opendev.org/openstack/ironic/commit/2ea32ad666cfa2e3929e370701b20d78731e27af
Submitter: "Zuul (22348)"
Branch: unmaintained/yoga

commit 2ea32ad666cfa2e3929e370701b20d78731e27af
Author: Julia Kreger <email address hidden>
Date: Tue Sep 17 09:57:19 2024 -0700

    Checksum files before raw conversion

    While working another issue, we discovered that support added to
    the ironic-conductor process combined the image_download_source
    option of "local" with the "force_raw" option resulted in a case
    where Ironic had no concept to checksum the files *before* the
    conductor process triggered an image format conversion and
    then records new checksum values.

    In essence, this opened the user requested image file to be
    suspetible to a theoretical man-in-the-middle attack OR
    the remote server replacing the content with an unknown file,
    such as a new major version.

    The is at odds with Ironic's security model where we do want to
    ensure the end user of ironic is asserting a known checksum for
    the image artifact they are deploying, so they are aware of the
    present state. Due to the risk, we chose to raise this as a CVE,
    as infrastructure operators should likely apply this patch.

    As a note, if your *not* forcing all images to be raw format
    through the conductor, then this issue is likely not a major
    issue for you, but you should still apply the patch.

    This is being tracked as CVE-2024-47211.

    Closes-Bug: 2076289
    Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a
    Signed-off-by: Julia Kreger <email address hidden>

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 21.4.4 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 27.0.0 release.