diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b1f69ee479ef..420a58ac254f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,8 @@ repos: - --markdown-linebreak-ext=md exclude: > (?x)^( - pkg/macos/pkg-resources/.*\.rtf + pkg/macos/pkg-resources/.*\.rtf| + pkg/patches/.*\.patch )$ - id: mixed-line-ending # Replaces or checks mixed line ending. diff --git a/pkg/patches/pip-urllib3/_version.py.patch b/pkg/patches/pip-urllib3/_version.py.patch new file mode 100644 index 000000000000..6eca20d59475 --- /dev/null +++ b/pkg/patches/pip-urllib3/_version.py.patch @@ -0,0 +1,31 @@ +--- a/pip/_vendor/urllib3/_version.py ++++ b/pip/_vendor/urllib3/_version.py +@@ -1,2 +1,26 @@ +-# This file is protected via CODEOWNERS +-__version__ = "1.26.20" ++# This file is a Salt-maintained security patch of pip's vendored urllib3. ++# ++# The underlying code is urllib3 1.26.20 (the version vendored by pip 25.2) ++# with the following CVE fixes backported from upstream urllib3 2.6.3: ++# ++# CVE-2025-66418 (GHSA-gm62-xv2j-4w53): Unbounded Content-Encoding ++# decompression chain -- MultiDecoder now enforces a 5-link limit. ++# Upstream fix: urllib3 2.6.0 (commit 24d7b67). ++# ++# CVE-2026-21441 (GHSA-38jv-5279-wg99): drain_conn unnecessarily ++# decompressed the full body of HTTP redirect responses, creating a ++# decompression-bomb vector. Fixed by adding _has_decoded_content ++# tracking and only decoding in drain_conn when decoding was already ++# in progress. ++# Upstream fix: urllib3 2.6.3 (commit 8864ac4). ++# ++# CVE-2025-66471 (GHSA-2xpw-w6gg-jr37): Decompression bomb in the ++# streaming API via max_length parameter. NOT backported -- requires a ++# full 2.x streaming infrastructure refactor. Ubuntu did not backport ++# this to 1.26.x either. pip maintainers confirmed pip is not ++# affected because all pip network calls use decode_content=False. ++# ++# The version string "2.6.3" reflects the highest upstream release from ++# which fixes have been backported. The underlying API remains urllib3 ++# 1.26.x -- this is NOT a port to urllib3 2.x. ++__version__ = "2.6.3" diff --git a/pkg/patches/pip-urllib3/response.py.patch b/pkg/patches/pip-urllib3/response.py.patch new file mode 100644 index 000000000000..4bd47c69c053 --- /dev/null +++ b/pkg/patches/pip-urllib3/response.py.patch @@ -0,0 +1,64 @@ +--- a/pip/_vendor/urllib3/response.py ++++ b/pip/_vendor/urllib3/response.py +@@ -129,8 +129,18 @@ + they were applied. + """ + ++ # Maximum allowed number of chained HTTP encodings in the ++ # Content-Encoding header. CVE-2025-66418 (GHSA-gm62-xv2j-4w53). ++ max_decode_links = 5 ++ + def __init__(self, modes): +- self._decoders = [_get_decoder(m.strip()) for m in modes.split(",")] ++ encodings = [m.strip() for m in modes.split(",")] ++ if len(encodings) > self.max_decode_links: ++ raise DecodeError( ++ "Too many content encodings in the chain: " ++ "%d > %d" % (len(encodings), self.max_decode_links) ++ ) ++ self._decoders = [_get_decoder(e) for e in encodings] + + def flush(self): + return self._decoders[0].flush() +@@ -222,6 +232,9 @@ + self.reason = reason + self.strict = strict + self.decode_content = decode_content ++ # CVE-2026-21441: tracks whether content decoding has been ++ # initiated so drain_conn can skip decompression on redirects. ++ self._has_decoded_content = False + self.retries = retries + self.enforce_content_length = enforce_content_length + self.auto_close = auto_close +@@ -286,7 +299,11 @@ + Unread data in the HTTPResponse connection blocks the connection from being released back to the pool. + """ + try: +- self.read() ++ self.read( ++ # CVE-2026-21441: Do not spend resources decoding the ++ # content unless decoding has already been initiated. ++ decode_content=self._has_decoded_content, ++ ) + except (HTTPError, SocketError, BaseSSLError, HTTPException): + pass + +@@ -394,11 +411,18 @@ + Decode the data passed in and potentially flush the decoder. + """ + if not decode_content: ++ # CVE-2026-21441: guard against toggling after decoding started. ++ if self._has_decoded_content: ++ raise RuntimeError( ++ "Calling read(decode_content=False) is not supported after " ++ "read(decode_content=True) was called." ++ ) + return data + + try: + if self._decoder: + data = self._decoder.decompress(data) ++ self._has_decoded_content = True + except self.DECODER_ERROR_CLASSES as e: + content_encoding = self.headers.get("content-encoding", "").lower() + raise DecodeError( diff --git a/salt/utils/http.py b/salt/utils/http.py index 50f1f3428bbd..0636e5c5222e 100644 --- a/salt/utils/http.py +++ b/salt/utils/http.py @@ -636,12 +636,13 @@ def query( except salt.ext.tornado.httpclient.HTTPError as exc: ret["status"] = exc.code ret["error"] = str(exc) - ret["body"], _ = _decode_result( - exc.response.body, - exc.response.headers, - backend, - decode_body=decode_body, - ) + if exc.response is not None: + ret["body"], _ = _decode_result( + exc.response.body, + exc.response.headers, + backend, + decode_body=decode_body, + ) return ret except (socket.herror, OSError, TimeoutError, socket.gaierror) as exc: if status is True: diff --git a/tests/pytests/pkg/integration/test_pip_urllib3_patch.py b/tests/pytests/pkg/integration/test_pip_urllib3_patch.py new file mode 100644 index 000000000000..13563abe6740 --- /dev/null +++ b/tests/pytests/pkg/integration/test_pip_urllib3_patch.py @@ -0,0 +1,91 @@ +import pathlib +import re +import subprocess +import zipfile + +import pytest + +PATCHED_URLLIB3_VERSION = "2.6.3" + + +@pytest.fixture(autouse=True) +def skip_on_prev_version(install_salt): + """ + Skip urllib3 patch tests when running against the previous (downgraded) + Salt version, which does not contain the CVE backports. + """ + if install_salt.use_prev_version: + pytest.skip("urllib3 CVE patch is not present in the previous Salt version") + + +def _site_packages(install_salt) -> pathlib.Path: + """Return the site-packages directory for the installed Salt Python.""" + ret = subprocess.run( + install_salt.binary_paths["python"] + + [ + "-c", + "import pip, pathlib; print(pathlib.Path(pip.__file__).parent.parent)", + ], + capture_output=True, + text=True, + check=False, + ) + assert ret.returncode == 0, ret.stderr + return pathlib.Path(ret.stdout.strip()) + + +def test_pip_vendored_urllib3_version(install_salt): + """ + Verify that pip's vendored urllib3 in the installed Salt package + reports the security-patched version string. + """ + ret = subprocess.run( + install_salt.binary_paths["python"] + + [ + "-c", + "import pip._vendor.urllib3; print(pip._vendor.urllib3.__version__)", + ], + capture_output=True, + text=True, + check=False, + ) + assert ret.returncode == 0, ret.stderr + version = ret.stdout.strip() + assert ( + version == PATCHED_URLLIB3_VERSION + ), f"pip's vendored urllib3 is {version!r}; expected {PATCHED_URLLIB3_VERSION!r}" + + +def test_virtualenv_embedded_pip_wheel_urllib3_version(install_salt): + """ + Verify that the pip wheel bundled inside virtualenv's seed/wheels/embed + directory also contains the security-patched urllib3. New virtualenvs + seeded from this wheel will inherit the CVE fixes. + """ + site_packages = _site_packages(install_salt) + embed_dir = site_packages / "virtualenv" / "seed" / "wheels" / "embed" + + if not embed_dir.is_dir(): + pytest.skip(f"virtualenv embed directory not found: {embed_dir}") + + pip_wheels = sorted(embed_dir.glob("pip-*.whl")) + if not pip_wheels: + pytest.skip(f"No pip wheel found in {embed_dir}") + + pip_wheel = pip_wheels[-1] + with zipfile.ZipFile(pip_wheel) as zf: + try: + with zf.open("pip/_vendor/urllib3/_version.py") as f: + content = f.read().decode("utf-8") + except KeyError: + pytest.fail( + f"pip/_vendor/urllib3/_version.py not found inside {pip_wheel.name}" + ) + + match = re.search(r'^__version__\s*=\s*["\']([^"\']+)["\']', content, re.MULTILINE) + assert match, f"Could not parse __version__ from {pip_wheel.name}" + version = match.group(1) + assert version == PATCHED_URLLIB3_VERSION, ( + f"Embedded pip wheel {pip_wheel.name} contains urllib3 {version!r}; " + f"expected {PATCHED_URLLIB3_VERSION!r}" + ) diff --git a/tests/pytests/unit/utils/test_http.py b/tests/pytests/unit/utils/test_http.py index ba4e0b6a2e2c..62a169ff9a47 100644 --- a/tests/pytests/unit/utils/test_http.py +++ b/tests/pytests/unit/utils/test_http.py @@ -166,6 +166,29 @@ def test_query_error_handling(): assert isinstance(ret.get("error", None), str) +def test_query_tornado_httperror_no_response(): + """ + Tests that http.query handles a Tornado HTTPError where exc.response is None. + This happens on connection-level failures such as a connect timeout (HTTP 599) + where no HTTP response is ever received from the server. + """ + import salt.ext.tornado.httpclient + + http_error = salt.ext.tornado.httpclient.HTTPError(599, "Timeout while connecting") + assert http_error.response is None + + mock_client = MagicMock() + mock_client.fetch.side_effect = http_error + + with patch("salt.utils.http.HTTPClient", return_value=mock_client): + ret = http.query("https://example.com/test", backend="tornado") + + assert isinstance(ret, dict) + assert ret.get("status") == 599 + assert "Timeout while connecting" in ret.get("error", "") + assert "body" not in ret + + def test_parse_cookie_header(): header = "; ".join( [ diff --git a/tools/pkg/build.py b/tools/pkg/build.py index 886ea70a6667..b4b557460f24 100644 --- a/tools/pkg/build.py +++ b/tools/pkg/build.py @@ -5,13 +5,19 @@ # pylint: disable=resource-leakage,broad-except from __future__ import annotations +import base64 +import csv +import hashlib +import io import json import logging import os import pathlib import re import shutil +import sys import tarfile +import tempfile import zipfile from typing import TYPE_CHECKING @@ -21,6 +27,172 @@ log = logging.getLogger(__name__) +# Cached path to the patched pip wheel built by _build_patched_pip_wheel. +# None until first call; reused across all build steps in the same process. +_PATCHED_PIP_WHEEL: pathlib.Path | None = None + + +def _apply_unified_diff(original_text: str, patch_text: str) -> str: + """ + Apply a unified diff patch to *original_text* and return the result. + + This is a minimal pure-Python applier sufficient for the well-formed, + non-fuzzy patches stored in pkg/patches/pip-urllib3/. It handles the + standard unified diff hunk format produced by difflib.unified_diff and + GNU diff, including the '\\' (no newline at end of file) marker. + """ + orig_lines = original_text.splitlines(True) + result: list[str] = [] + orig_idx = 0 + + patch_lines = patch_text.splitlines(True) + i = 0 + + # Skip the file-header lines (--- / +++) before the first hunk. + while i < len(patch_lines) and not patch_lines[i].startswith("@@"): + i += 1 + + while i < len(patch_lines): + line = patch_lines[i] + if line.startswith("@@"): + m = re.match(r"^@@ -(\d+)(?:,\d+)? \+\d+(?:,\d+)? @@", line) + if not m: + i += 1 + continue + orig_start = int(m.group(1)) - 1 # convert 1-based → 0-based + + # Copy unchanged original lines that precede this hunk. + result.extend(orig_lines[orig_idx:orig_start]) + orig_idx = orig_start + i += 1 + + # Process hunk body lines. + while i < len(patch_lines): + hunk_line = patch_lines[i] + if hunk_line.startswith("@@"): + break # next hunk starts + if hunk_line.startswith("+"): + result.append(hunk_line[1:]) + elif hunk_line.startswith("-"): + orig_idx += 1 + elif hunk_line.startswith(" "): + result.append(orig_lines[orig_idx]) + orig_idx += 1 + # "\\" → "No newline at end of file" marker; skip. + i += 1 + else: + i += 1 + + # Copy any original lines that follow the last hunk. + result.extend(orig_lines[orig_idx:]) + return "".join(result) + + +def _patch_pip_wheel_urllib3(wheel_path: pathlib.Path) -> None: + """ + Rewrite *wheel_path* in-place so that the urllib3 vendored inside pip + contains the Salt security backports defined in pkg/patches/pip-urllib3/. + + Patches applied (unified diff format): + response.py.patch — CVE-2025-66418, CVE-2026-21441 + _version.py.patch — version bumped to "2.6.3" + + Each patch is applied to the file as extracted from the wheel, so the + original sources do not need to be stored in the repository. The wheel's + RECORD file is updated with correct sha256 hashes and sizes for the two + patched files so that the installed dist-info stays valid. + """ + patches_dir = tools.utils.REPO_ROOT / "pkg" / "patches" / "pip-urllib3" + patch_map = { + "pip/_vendor/urllib3/response.py": ( + patches_dir / "response.py.patch" + ).read_text(encoding="utf-8"), + "pip/_vendor/urllib3/_version.py": ( + patches_dir / "_version.py.patch" + ).read_text(encoding="utf-8"), + } + + def _record_hash(content: bytes) -> str: + digest = hashlib.sha256(content).digest() + return "sha256=" + base64.urlsafe_b64encode(digest).decode().rstrip("=") + + tmp_path = wheel_path.with_suffix(".tmp.whl") + try: + with zipfile.ZipFile(wheel_path, "r") as zin: + with zipfile.ZipFile( + tmp_path, "w", compression=zipfile.ZIP_DEFLATED + ) as zout: + record_name: str | None = None + record_rows: list[list[str]] = [] + patched: dict[str, bytes] = {} + + for item in zin.infolist(): + if item.filename.endswith(".dist-info/RECORD"): + record_name = item.filename + raw = zin.read(item.filename).decode("utf-8") + record_rows = list(csv.reader(raw.splitlines())) + continue # written last after we know the new hashes + if item.filename in patch_map: + original = zin.read(item.filename).decode("utf-8") + patched_text = _apply_unified_diff( + original, patch_map[item.filename] + ) + patched_bytes = patched_text.encode("utf-8") + patched[item.filename] = patched_bytes + zout.writestr(item, patched_bytes) + else: + zout.writestr(item, zin.read(item.filename)) + + # Update RECORD rows for patched files and write it back. + if record_name: + new_rows = [] + for row in record_rows: + if len(row) >= 1 and row[0] in patched: + content = patched[row[0]] + new_rows.append( + [row[0], _record_hash(content), str(len(content))] + ) + else: + new_rows.append(row) + buf = io.StringIO() + csv.writer(buf).writerows(new_rows) + zout.writestr(record_name, buf.getvalue()) + + tmp_path.replace(wheel_path) + except Exception: + tmp_path.unlink(missing_ok=True) + raise + + +def _build_patched_pip_wheel(ctx: Context) -> pathlib.Path: + """ + Download pip==25.2 into a temporary directory, patch its vendored urllib3, + and return the path to the patched wheel. The result is cached for the + lifetime of the current process so subsequent calls are free. + """ + global _PATCHED_PIP_WHEEL + if _PATCHED_PIP_WHEEL is not None: + return _PATCHED_PIP_WHEEL + + tmpdir = pathlib.Path(tempfile.mkdtemp(prefix="salt-pip-patch-")) + ctx.info("Downloading pip==25.2 for urllib3 security patching ...") + ctx.run( + sys.executable, + "-m", + "pip", + "download", + "pip==25.2", + "--no-deps", + "--dest", + str(tmpdir), + ) + wheel = next(tmpdir.glob("pip-*.whl")) + ctx.info(f"Patching urllib3 CVEs inside {wheel.name} ...") + _patch_pip_wheel_urllib3(wheel) + _PATCHED_PIP_WHEEL = wheel + return wheel + + # Define the command group build = command_group( name="build", @@ -276,6 +448,20 @@ def macos( ctx.info("Installing salt into the relenv python") ctx.run("./install_salt.sh") + # Patch pip's vendored urllib3 in the standalone macOS build. + # install_salt.sh uses the relenv pip but does not upgrade it, so we + # install the security-patched pip wheel and replace the copy that + # virtualenv embeds so that new environments also get the fixed pip. + build_env = checkout / "pkg" / "macos" / "build" / "opt" / "salt" + python_bin = build_env / "bin" / "python3" + patched_pip = _build_patched_pip_wheel(ctx) + ctx.run(str(python_bin), "-m", "pip", "install", str(patched_pip)) + for old_pip in (build_env / "lib").glob( + "python*/site-packages/virtualenv/seed/wheels/embed/pip-*.whl" + ): + old_pip.unlink() + shutil.copy(str(patched_pip), str(old_pip.parent / patched_pip.name)) + if sign: ctx.info("Signing binaries") with ctx.chdir(checkout / "pkg" / "macos"): @@ -614,10 +800,25 @@ def onedir_dependencies( "install", "-U", "setuptools", - "pip", "wheel", env=env, ) + # Install pip from the security-patched wheel instead of pulling from PyPI, + # so that pip's vendored urllib3 never contains the vulnerable version. + # --force-reinstall is required because relenv ships with pip pre-installed + # at the same version (25.2), so without it pip would skip the install as + # "already satisfied" and leave the unpatched copy in site-packages. + patched_pip = _build_patched_pip_wheel(ctx) + ctx.run( + str(python_bin), + "-m", + "pip", + "install", + "--force-reinstall", + "--no-deps", + str(patched_pip), + env=env, + ) ctx.run( str(python_bin), "-m", @@ -834,17 +1035,22 @@ def errfn(fn, path, err): env["PIP_CONSTRAINT"] = str( tools.utils.REPO_ROOT / "requirements" / "constraints.txt" ) + # Download setuptools and wheel normally; pip is handled separately below + # so that the security-patched wheel is used instead of the PyPI version. ctx.run( str(python_executable), "-m", "pip", "download", "setuptools", - "pip", "wheel", "--dest", str(embed_dir), ) + # Copy the security-patched pip wheel into the embed directory so that + # virtualenv seeds new environments with pip that has the urllib3 fixes. + patched_pip = _build_patched_pip_wheel(ctx) + shutil.copy(str(patched_pip), str(embed_dir / patched_pip.name)) # Update __init__.py with the new versions