Before this change, Ironic did not filter file:// paths when used as an
image source except to ensure they were a file (and not, e.g. a
character device). This is problematic from a security perspective
because you could end up with config files from well-known paths being
written to disk on a node.
The allowlist default list is huge, but it includes all known usages of
file:// URLs across Bifrost, Ironic, Metal3, and OpenShift in both CI
and default configuration.
For the backportable version of this patch for stable branches, we have
omitted the unconditional block of system paths in order to permit
operators using those branches to fully disable the new security
functionality.
Generated-by: Jetbrains Junie
Closes-bug: 2107847
Change-Id: I2fa995439ee500f9dd82ec8ccfa1a25ee8e1179c
We duplicated these disk_utils options in the security fix; disabling
use of them during doc runs.
Change-Id: I1a1eaf6f089c4fa52b030ce2fa82493cdc53f07a
The fix for CVE-2024-47211 results in image checksum being required in
all cases. However there is no requirement for checksums in
file:// based images.
This change checks for this situation. When checksum is missing for
file:// based image_source it is now calculated on-the-fly.
Change-Id: Ib2fd5ddcbee9a9d1c7e32770ec3d9b6cb20a2e2a
(cherry picked from commit b827c7bf72)
When we were fixing the qemu-img related CVE, in our rush we didn't
realize that the logic for storage sizing, which only falls back to
actual size didn't match the prior interface exactly. Instead of
disk_size, we have actual_size on the format inspector.
This was not discovered because all of the code handling that side
of the unit tests were mocked.
Anyhow, easy fix.
Closes-Bug: 2083520
Change-Id: Ic4390d578f564f245d7fb4013f2ba5531aee9ea9
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 <juliaashleykreger@gmail.com>
The partition image test job is known to be easy to cause to fail
because it is reliant upon CI scripting to try and make a partition
image during the job run, and it sometimes doesn't work which
increases the failure risk. Given this branch is moving un-maintained
in a few months, we can go ahead and remove this job with relatively
low risk at this point in time.
Also remove centos8, postgres, metalsmith, and non-voting bifrost jobs
to minimize resource waste.
Change-Id: I4cee2c24fc20227e84f7c25b5d24a4c9557b9614
It was recently learned by the OpenStack community that running qemu-img
on un-trusted images without a format pre-specified can present a
security risk. Furthermore, some of these specific image formats have
inherently unsafe features. This is rooted in how qemu-img operates
where all image drivers are loaded and attempt to evaluate the input data.
This can result in several different vectors which this patch works to
close.
This change imports the qemu-img handling code from Ironic-Lib into
Ironic, and image format inspection code, which has been developed by
the wider community to validate general safety of images before converting
them for use in a deployment.
This patch contains functional changes related to the hardening of these
calls including how images are handled, and updates documentation to
provide context and guidance to operators.
Closes-Bug: 2071740
Change-Id: I7fac5c64f89aec39e9755f0930ee47ff8f7aed47
Signed-off-by: Julia Kreger <juliaashleykreger@gmail.com>
ironic-tempest-partition-uefi-redfish-vmedia was renamed to
ironic-tempest-uefi-redfish-vmedia a long time ago
Change-Id: Iaa63e9cf12d47667955973033586fa65dd18e6b7
(cherry picked from commit 3f34f04bf0)
This reverts commit 86358c89e8.
Reason for revert: DIB appears to be still trying to merge filename in,
and as such we likely need to re-work our approach here.
Furthermore, explicitly force the redfish job to utilize just the wholedisk
test, instead of both the wholedisk and partition image build process, since
the partition image build test is easy to cause to fail.
Change-Id: I8c110deceda3a65e952cfb9b590d68b5d95efc16
Apparently, this has been around for ages, btu the error was likely
not exactly right as a result of this. Anyway, quick fix.
Change-Id: Idee3c1edfdd65928eaa5f8d30b62474d85dec277
(cherry picked from commit eaa0521bee)
On older CI nodes, we cannot use centos dib images in the pipeline
without explicitly pulling in an older image of centos because newer
centos images use an XFS feature in them which prevents us from
extracting image contents.
Change-Id: Ic14e75071651551c663a97d98c2226f8a375ec0b
To serve as a mechanism to allow an interlocking device identification
this patch injects a publisher id value into ISO images *and* the kernel
command line for any software running from the ISO image to match
the ISO in use to the location of data housed locally from within the
image.
Some differences exist with this patch due to refactoring changes in
I8567a10b77cdc3785686b79defcdafd75af53df0 where the basic flow and
logic was simplified just enough to require the logic to change
a little bit. Furthermore, even going to 2023.1, some default
configuration options were also removed as they were centered
around GRUB v1.
Related-Bug: 2032377
Change-Id: I9b74ec977fabc0a7f8ed6f113595a3f1624f6ee6
(cherry picked from commit fb850e7f00)
(cherry picked from commit 78c1d9a98d)
(cherry picked from commit c71e124f9c)
A temporary path forward to increase CI stability, by pinning
to what appears to be a "good working version" of upstream dnsmasq
which does not crash fon us.
Change-Id: I3295c92fd7b7871ad351b94f4c6cf0f554279db0
(cherry picked from commit f893c740d7)
(cherry picked from commit b32378e1cf)
Proliantutils 2.16.0 roughly times with the 2023.2
release of ironic and a switch to lextudio-pysnmp,
however in this branch of ironic, however this breaks
depending on order and collides with pysnmp namespace.
Also pins python-scciclient to <0.14.0 as to also not
pull in the dependency difference.
Also, Also, disables standalone, grenade, and metalsmith,
and snmp jobs from voting while we work to stabilize CI across
multiple branches.
Change-Id: Ibe3274d7fabfd4f06af8aba1af0957fa36e8d217
Special cases boot/uefi record setup to focus on UEFI
nvram updates instead of attempting nvram updates *and*
setting the boot device to disk.
Closes-Bug: 2053064
Change-Id: Ic6584479a47146577052d17fa3f697eef64ac73c
(cherry picked from commit 4fb1b813f4)
(cherry picked from commit 45d17c4abc)
In the early days of the neutron network interface, we had a hard
launch failure added to prevent ironic.conf from having a neutron
network configuration which was not valid when the neutron network
interface was in use.
But as time has moved on, these settings became node-settable,
and ironic configuration largely became mutable as well, so they
can always be added after the process has been launched.
But we kept the error being returned. Which doesn't make sense
now that it can always be back-filled into a working state
or just entirely be "user supplied" via the API by an appropriate
user.
Closes-Bug: 2054728
Change-Id: I33e76929ca9bf7869b3b4ef4d6501e692cf0a922
(cherry picked from commit 50ced3a3fa)
Turns out the service role support doesn't quite work,
because you could not enumerate nodes regardless of node
owner or lessee in order to enable services like Nova to
enumerate nodes to be able to schedule upon them, or
networking-baremetal to enumerate ports in update mapping
in Neutron.
So this change enables permissions to be modified to allow
service project users with the service role to enumerate the
list of resources, and grants rights similar to "system scoped
members" to the service project's users with the "service" role
which aligns with update actions to provision/unprovision nodes.
Adds some additional rbac testing to ensure we appropriately
covered these access rights.
Closes-Bug: 2051592
Change-Id: I2b4bcc748b6e43e4215dc45137becce301349032
(cherry picked from commit 0313ce26b5)
The kickstart unit tests were written in such a way that if
the tests are run on a system with kickstart validator present,
then the test behavior is different (and fails) than if it runs
without. Specifically, when it is present, an error is generated:
TypeError: write() argument must be str, not MagicMock
This is because we pass in a mock value for unit testing.
Removes the alternative path of if the validator is present
for unit testing, and locks the test into the false which
simplifies the validation path for the kickstart interface.
Change-Id: Idfb6b4f3b49901aa1a222c6fedc4367ef3bfd2a2
(cherry picked from commit bbc82fa148)
(cherry picked from commit 4895c687b1)
Fixes Secureboot with Anaconda deploy with PXE and iPXE
Story:2010356
Task: 46529
Change-Id: Id6262654bb5e41e02c7d90b9a9aaf395e7b6a088
(cherry picked from commit c5e004a73e)
The host currently hard-coded is not functioning. This replaces
the hard-coded mirror by the local CI mirror detected. In case
mirror info is not available then upstream centos mirror is used.
Change-Id: I96a8cb45154c9dbb50efecc22d34c4ff75c6722a
(cherry picked from commit 7032a0d9ac)
(cherry picked from commit aec3c072cd)
While os.link is supposed to follow symlinks, it's actually broken [1]
on Linux. As a result, Ironic may end up creating a hard link to
a symlink. If the symlink is relative, chances are high accessing the
resulting file will cause a FileNotFoundError.
[1] https://github.com/python/cpython/issues/81793
Change-Id: Ic52f0ddb0c94410dd854ee525e3c57b2e78ea84d
(cherry picked from commit 0b3ed093ea)
In the backports to fix the policy of the original change, Dmitry
noted that it was actually wrong, because we should have instead
raised NotAuthorized. Dmitry was absolutely correct, because in hind
sight I made the change trying to keep exactly the same behavior,
but the reality is this is a case where we should be explicit,
and tell the user they have done something forbidden.
This revert of the revert fixes that change.
Original Change: https://review.opendev.org/c/openstack/ironic/+/905038
Dmitry's Review Feedback: https://review.opendev.org/c/openstack/ironic/+/905088
Change-Id: I5727df00b8c4ae9495ed14b5cea1c0734b5f688d
(cherry picked from commit 4398c11a5f)
Before this change, if a user requested a node to be cleaned
or "managed" with cleaning enabled when the user is in the
system scope, Ironic would attempt to user's token to
make the request to Neutron.
This, unfortunately, does not work, as the neutron client explicitly
requires a project ID to make the request to Neutron. As a result,
Ironic now falls back to it's internal credential configuration to make
the forward request, which matches the behavior if a node has been
unprovisioned and the cleaning has been started automatically.
Closes-Bug: 2048416
Change-Id: Id91ec6afcf89642fb3069918e768016b8b657a31
(cherry picked from commit c3074524da)
This commit removes 'VolumeType' which param has long been
deprecated in DMTF Redfish schema, also removes 'Encrypted'
param as per discussion, and places 'Drives' inside 'Links'
as per the new DMTF schema.
Closes-Bug: 2045645
Change-Id: Ie3ae095fbc0a65e4bd43a98e6935da7c1288e883
Fixes an issue with debug logging referencing node vs node_uuid.
Change-Id: Ic7de9826fbec32038947be89b14f6dfdc2248de4
(cherry picked from commit 578c02813d)
When the per-node external_http_url feature was introduced by
c197a2d8b2, it only applied to a config
floppy. This fix ensures that it is also used for the boot ISO, both
when it is generated locally (by _prepare_iso_image()) or just cached
locally (by prepare_remote_image()).
Change-Id: Ic241da6845b4d97fd29888e28cc1d9ee34e182c1
Closes-Bug: #2044314
(cherry picked from commit 0d59e25cf8)
It's possible to use virtual media based provisioning on
servers that only support DVD MediaTypes and do not support CD
MediaTypes. The problem in this scenario is that Ironic will keep
the media attached since it will only eject the ones matching the
CD device, now we check if there is any DVD device with media inserted
when looking for CD devices.
Closes-Bug: 2039042
Change-Id: I7a5e871133300fea8a77ad5bfd9a0b045c24c201
When parsing redfish driver info wrap IPv6 address in brackets
before appending default scheme/authority.
Updated common.utils.wrap_ipv6() to ignore ValueError, e.g
simply return the string if ip is not an ipv6 address string.
Related: RHBZ#2239356
Closes-Bug: #2036454
Change-Id: Icefd96d6873474b4cfb7fbf3d8337cd42fd63ca6
(cherry picked from commit 72037b596a)
We've discovered we can deadlock on allocations, and reviewing
the code of both the test and the underlying db, it is sort of a
"multiple things contribute scenario", but first up here is to
streamline the allocations update process so we re-query after
closing out the transaction.
Change-Id: I46e78813787703819a61f69d4243271ec07e0983
Partial-Bug: #2028866
(cherry picked from commit cc9af373e7)
The PXE Annaconda dhcp cleanup test triggers the dhcp_factory clean
up code by default. Which is good! Problem is, if you don't have
dnsmasq installed, things blow up.
Specifically becuase it was called in such a way where it was
trying to clean up dhcp records for nodes. Example:
ironic.common.exception.InstanceDeployFailure: An error occurred
after deployment, while preparing to reboot the node
1be26c0b-03f2-4d2e-ae87-c02d7f33c123: [Errno 2] No such file
or directory:
'/etc/dnsmasq.d/hostsdir.d/ironic-52:54:00:cf:2d:31.conf'
Instead of executing that far, we just now check that we did, indeed
call for dhcp cleanup.
This was discovered while trying to fix unit test race conditions
and random failures in CI.
Change-Id: Id7b1e2e9ca97aeff786e9df06f35eca67dd36b58
(cherry picked from commit c392814ca8)
