kgess-mirror/openstack-ironic
1
0
Fork 0
mirror of https://opendev.org/openstack/ironic.git synced 2026-01-16 23:01:47 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge "OSSA-2025-001: Disallow unsafe image file:// paths" into unmaintained/2023.1

Browse source
This commit is contained in:
Zuul 2025-05-13 15:58:08 +00:00 • committed by Gerrit Code Review
commit c31ff499e6
8 changed files with 270 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
doc/source/install/standalone/enrollment.rst
View file

@ -32,13 +32,20 @@ There are however some limitations for different hardware interfaces:
$ sha256sum image.qcow2
9f6c942ad81690a9926ff530629fb69a82db8b8ab267e2cbd59df417c1a28060 image.qcow2
* :ref:`direct-deploy` started supporting ``file://`` images in the Victoria
release cycle, before that only HTTP(s) had been supported.
* If you're using :ref:`direct-deploy` with ``file://`` URLs, you have to
ensure the images meet all requirements:
* File images must be accessible to every conductor
* File images must be located in a path listed in
:oslo.config:option:`conductor.file_url_allowed_paths`
* File images must not be located in ``/dev``, ``/sys``, ``/proc``,
``/etc``, ``/boot``, ``/run`` or other system paths starting with the
Ironic 2025.2 release.
.. warning::
File images must be accessible to every conductor! Use a shared file
system if you have more than one conductor. The ironic CLI tool will not
transfer the file from a local machine to the conductor(s).
The Ironic CLI tool will not transfer the file from a local machine to the
conductor(s). Operators should use shared file systems or configuration
management to ensure consistent availability of images.
.. note::
The Bare Metal service tracks content changes for non-Glance images by

22
ironic/common/image_service.py
View file

@ -273,14 +273,32 @@ class FileImageService(BaseImageService):
:param image_href: Image reference.
:raises: exception.ImageRefValidationFailed if source image file
doesn't exist.
:returns: Path to image file if it exists.
doesn't exist, is in a blocked path, or is not in an allowed path.
:returns: Path to image file if it exists and is allowed.
"""
image_path = urlparse.urlparse(image_href).path
# Check if the path is in the blocklist
rpath = os.path.abspath(image_path)
# Check if the path is in the allowlist
for allowed in CONF.conductor.file_url_allowed_paths:
if rpath == allowed or rpath.startswith(allowed + os.sep):
break
else:
raise exception.ImageRefValidationFailed(
image_href=image_href,
reason=_(
"Security: Path %s is not allowed for image source "
"file URLs" % image_path)
)
# Check if the file exists
if not os.path.isfile(image_path):
raise exception.ImageRefValidationFailed(
image_href=image_href,
reason=_("Specified image file not found."))
return image_path
def download(self, image_href, image_file):

16
ironic/conf/conductor.py
View file

@ -19,6 +19,8 @@ from oslo_config import cfg
from oslo_config import types
from ironic.common.i18n import _
from ironic.conf import types as ir_types
opts = [
cfg.IntOpt('workers_pool_size',
@ -414,6 +416,20 @@ opts = [
'functionality by setting this option to True will '
'create a more secure environment, however it may '
'break users in an unexpected fashion.')),
cfg.ListOpt('file_url_allowed_paths',
default=['/var/lib/ironic', '/shared/html', '/templates',
'/opt/cache/files', '/vagrant'],
item_type=ir_types.ExplicitAbsolutePath(),
help=_(
'List of paths that are allowed to be used as file:// '
'URLs. Files in /boot, /dev, /etc, /proc, /sys and other'
'system paths are always disallowed for security reasons. '
'Any files in this path readable by ironic may be used as '
'an image source when deploying. Setting this value to '
'"" (empty) disables file:// URL support. Paths listed '
'here are validated as absolute paths and will be rejected'
'if they contain path traversal mechanisms, such as "..".'
)),
]

55
ironic/conf/types.py Normal file
View file

@ -0,0 +1,55 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy
# of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import os
from oslo_config import types
class ExplicitAbsolutePath(types.ConfigType):
"""Explicit Absolute path type.
Absolute path values do not get transformed and are returned as
strings. They are validated to ensure they are absolute paths and are equal
to os.path.abspath(value) -- protecting from path traversal issues.
Python path libraries generally define "absolute path" as anything
starting with a /, so tools like path.PurePath(str).is_absolute() is not
useful, because it will happily return that /tmp/../etc/resolv.conf is
absolute. This type is to be used in cases where we require the path to
be explicitly absolute.
:param type_name: Type name to be used in the sample config file.
"""
def __init__(self, type_name='explicit absolute path'):
super().__init__(type_name=type_name)
def __call__(self, value):
value = str(value)
# NOTE(JayF): This removes trailing / if provided, since
# os.path.abspath will not return a trailing slash.
if len(value) > 1:
value = value.rstrip('/')
absvalue = os.path.abspath(value)
if value != absvalue:
raise ValueError('Value must be an absolute path '
'containing no path traversal mechanisms. Config'
f'item was: {value}, but resolved to {absvalue}')
return value
def __repr__(self):
return 'explicit absolute path'
def _formatter(self, value):
return self.quote_trailing_and_leading_space(value)

44
ironic/tests/unit/common/test_image_service.py
View file

@ -482,20 +482,58 @@ class FileImageServiceTestCase(base.TestCase):
def setUp(self):
super(FileImageServiceTestCase, self).setUp()
self.service = image_service.FileImageService()
self.href = 'file:///home/user/image.qcow2'
self.href_path = '/home/user/image.qcow2'
self.href = 'file:///var/lib/ironic/images/image.qcow2'
self.href_path = '/var/lib/ironic/images/image.qcow2'
@mock.patch.object(os.path, 'isfile', return_value=True, autospec=True)
def test_validate_href(self, path_exists_mock):
self.service.validate_href(self.href)
path_exists_mock.assert_called_once_with(self.href_path)
@mock.patch.object(os.path, 'isfile', return_value=False, autospec=True)
@mock.patch.object(os.path, 'isfile', return_value=False,
autospec=True)
def test_validate_href_path_not_found_or_not_file(self, path_exists_mock):
self.assertRaises(exception.ImageRefValidationFailed,
self.service.validate_href, self.href)
path_exists_mock.assert_called_once_with(self.href_path)
@mock.patch.object(os.path, 'abspath', autospec=True)
def test_validate_href_empty_allowlist(self, abspath_mock):
abspath_mock.return_value = self.href_path
cfg.CONF.set_override('file_url_allowed_paths', [], 'conductor')
self.assertRaisesRegex(exception.ImageRefValidationFailed,
"is not allowed for image source file URLs",
self.service.validate_href, self.href)
@mock.patch.object(os.path, 'abspath', autospec=True)
def test_validate_href_not_in_allowlist(self, abspath_mock):
href = "file:///var/is/allowed/not/this/path/image.qcow2"
href_path = "/var/is/allowed/not/this/path/image.qcow2"
abspath_mock.side_effect = ['/var/lib/ironic', href_path]
cfg.CONF.set_override('file_url_allowed_paths', ['/var/lib/ironic'],
'conductor')
self.assertRaisesRegex(exception.ImageRefValidationFailed,
"is not allowed for image source file URLs",
self.service.validate_href, href)
@mock.patch.object(os.path, 'abspath', autospec=True)
@mock.patch.object(os.path, 'isfile',
return_value=True, autospec=True)
def test_validate_href_in_allowlist(self,
path_exists_mock,
abspath_mock):
href_dir = '/var/lib' # self.href_path is in /var/lib/ironic/images/
# First call is ironic.conf.types.ExplicitAbsolutePath
# Second call is in validate_href()
abspath_mock.side_effect = [href_dir, self.href_path]
cfg.CONF.set_override('file_url_allowed_paths', [href_dir],
'conductor')
result = self.service.validate_href(self.href)
self.assertEqual(self.href_path, result)
path_exists_mock.assert_called_once_with(self.href_path)
abspath_mock.assert_has_calls(
[mock.call(href_dir), mock.call(self.href_path)])
@mock.patch.object(os.path, 'getmtime', return_value=1431087909.1641912,
autospec=True)
@mock.patch.object(os.path, 'getsize', return_value=42, autospec=True)

34
ironic/tests/unit/conf/test_conductor.py Normal file
View file

@ -0,0 +1,34 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from ironic.conf import CONF
from ironic.tests.base import TestCase
class ValidateConductorAllowedPaths(TestCase):
def test_abspath_validation_bad_path_raises(self):
"""Verifies setting a relative path raises an error via oslo.config."""
self.assertRaises(
ValueError,
CONF.set_override,
'file_url_allowed_paths',
['bad/path'],
'conductor'
)
def test_abspath_validation_good_paths(self):
"""Verifies setting an absolute path works via oslo.config."""
CONF.set_override('file_url_allowed_paths', ['/var'], 'conductor')
def test_abspath_validation_good_paths_trailing_slash(self):
"""Verifies setting an absolute path works via oslo.config."""
CONF.set_override('file_url_allowed_paths', ['/var/'], 'conductor')

63
ironic/tests/unit/conf/test_types.py Normal file
View file

@ -0,0 +1,63 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from ironic.conf import types
from ironic.tests.base import TestCase
class ExplicitAbsolutePath(TestCase):
def test_explicit_absolute_path(self):
"""Verifies the Opt subclass used to validate absolute paths."""
good_paths = [
'/etc/passwd', # Valid
'/usr/bin/python', # Valid
'/home/user/file.txt', # Valid - dot in filename allowed
'/var/lib/ironic/.secretdir', # Valid - hidden directory allowed
'/var/lib/ironic/oslo.config', # Valid - dots in filename allowed
'/tmp/', # Valid
'/', # Valid (root directory)
'/.hidden_root_file', # Valid
'/path/including/a/numb3r', # Valid
'/a/path/with/a/trailing/slash/' # Valid
]
bad_paths = [
'relative/path', # Invalid - no leading slash
'./file.txt', # Invalid - relative path
'../file.txt', # Invalid - relative path
'file.txt', # Invalid - no leading slash
'', # Invalid - empty string
'/var/lib/ironic/../../../etc/passwd', # Invalid - path traversal
'/etc/../etc/passwd', # Invalid - path traversal
'/home/user/./config', # Invalid - contains current dir reference
'/home/user/../user/config', # Invalid - path traversal
'/../etc/passwd', # Invalid - path traversal at beginning
'/.', # Invalid - just current directory
'/..' # Invalid - just parent directory
]
eap = types.ExplicitAbsolutePath()
def _trypath(tpath):
try:
eap(tpath)
except ValueError:
return False
else:
return True
for path in good_paths:
self.assertTrue(_trypath(path),
msg=f"Improperly disallowed path: {path}")
for path in bad_paths:
self.assertFalse(_trypath(path),
msg=f"Improperly allowed path: {path}")

29
releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml Normal file
View file

@ -0,0 +1,29 @@
---
security:
- |
Fixes OSSA-2025-001, where Ironic did not properly filter file:// paths
when used as image sources. This would permit any file accessible by the
conductor to be used as an image to attempt deployment.
Adds ``CONF.conductor.file_url_allowed_paths``, an allowlist configuration
defaulting to ``/var/lib/ironic``, ``/shared/html``,
``/opt/cache/files``, ``/vagrant``, and ``/templates``,
permits operators to further restrict where the conductor will fetch
images for when provided a file:// URL. This default value was chosen
based on known usage by projects downstream of Ironic, including Metal3,
Bifrost, and OpenShift. These defaults may change to be more restrictive
at a later date. Operators using file:// URLs are encouraged to explicitly
set this value even if the current default is sufficient. Operators wishing
to fully disable the ability to deploy with a file:// URL should set this
configuration to "" (empty).
Operators wishing to restore the original insecure behavior should set
``CONF.conductor.file_url_allowed_paths`` to ``/``. Take note that in the
2025.2 release and later, ``/dev``, ``/sys``, ``/proc``, ``/run``, and
``/etc`` will be unconditionally blocked as a security measure.
This issue only poses a significant security risk when Ironic's
automated cleaning process is disabled and the service is configured in
such a way that permits direct deployment by an untrusted API user, such as
standalone Ironic installations or environments granting ownership of nodes
to projects.