[Bug 1219985] New: Symlinked states not found after fix-cve-2024-22231-and-cve-2024-22232-bsc-1219430-bs.patch
https://bugzilla.suse.com/show_bug.cgi?id=1219985 Bug ID: 1219985 Summary: Symlinked states not found after fix-cve-2024-22231-and-cve-2024-22232-bsc-1219430-bs.p atch Classification: openSUSE Product: openSUSE Distribution Version: Leap 15.5 Hardware: Other OS: openSUSE Leap 15.5 Status: NEW Severity: Normal Priority: P5 - None Component: Salt Assignee: salt-maintainers@suse.de Reporter: georg.pfuetzenreuter@suse.com QA Contact: qa-bugs@suse.de Target Milestone: --- Found By: --- Blocker: --- Hi, originating from https://gitlab.infra.opensuse.org/infra/salt/-/issues/20 (internal link) - in the openSUSE infrastructure we notice our complete Salt test suite to fail. Upon investigation I found the culprit to be a combination of our use of symlinks to install certain formula states, and this update in the Salt package: ``` Thu Feb 1 12:07:20 UTC 2024 - Pablo Suárez Hernández <pablo.suarezhernandez@suse.com> - Prevent directory traversal when creating syndic cache directory on the master (CVE-2024-22231, bsc#1219430) - Prevent directory traversal attacks in the master's serve_file method (CVE-2024-22232, bsc#1219431) - Added: * fix-cve-2024-22231-and-cve-2024-22232-bsc-1219430-bs.patch ``` More specifically, the problem lies with the changes in salt/fileserver/roots.py. To reproduce, set up the following structure: ``` # more /srv/salt/test.sls first_state: test.succeed_without_changes # more /srv/formula/myformula/init.sls myformula: test.succeed_without_changes # ls -ld /srv/salt/myformula lrwxrwxrwx 1 root root 22 Feb 15 19:53 /srv/salt/myformula -> /srv/formula/myformula ``` Before the patch, both the regular state file, as well as the symlink work: ``` # salt-call --local state.test test local: ---------- ID: first_state Function: test.succeed_without_changes Result: True Comment: Success! Started: 19:58:21.228111 Duration: 1.578 ms Changes: Summary for local ------------ Succeeded: 1 Failed: 0 ------------ Total states run: 1 Total run time: 1.578 ms # salt-call --local state.test myformula local: ---------- ID: myformula Function: test.succeed_without_changes Result: True Comment: Success! Started: 19:58:25.466603 Duration: 1.506 ms Changes: Summary for local ------------ Succeeded: 1 Failed: 0 ------------ Total states run: 1 Total run time: 1.506 ms ``` After the patch, only the regular state works, and the symlinked one fails with a very misleading error message: ``` # salt-call --local state.test test local: ---------- ID: first_state Function: test.succeed_without_changes Result: True Comment: Success! Started: 20:03:29.581793 Duration: 1.299 ms Changes: Summary for local ------------ Succeeded: 1 Failed: 0 ------------ Total states run: 1 Total run time: 1.299 ms # salt-call --local state.test myformula local: Data failed to compile: ---------- No matching sls found for 'myformula' in env 'base' ``` The debug output merely states: ``` [DEBUG ] Could not find file 'salt://myformula.sls' in saltenv 'base' [DEBUG ] Could not find file 'salt://myformula/init.sls' in saltenv 'base' ``` The following is a patch to revert only the problematic parts of fix-cve-2024-22231-and-cve-2024-22232-bsc-1219430-bs.patch: ``` 52237e8b96c0:/usr/lib/python3.6/site-packages/salt # diff -u fileserver/roots.py.orig fileserver/roots.py --- fileserver/roots.py.orig 2024-02-15 19:53:39.824084515 +0000 +++ fileserver/roots.py 2024-02-15 19:54:03.144242042 +0000 @@ -101,8 +101,8 @@ full = os.path.join(root, path) # Refuse to serve file that is not under the root. - if not salt.utils.verify.clean_path(root, full, subdir=True): - continue + #if not salt.utils.verify.clean_path(root, full, subdir=True): + # continue if os.path.isfile(full) and not salt.fileserver.is_file_ignored(__opts__, full): fnd["path"] = full @@ -135,24 +135,24 @@ gzip = load.get("gzip", None) fpath = os.path.normpath(fnd["path"]) - actual_saltenv = saltenv = load["saltenv"] - if saltenv not in __opts__["file_roots"]: - if "__env__" in __opts__["file_roots"]: - log.debug( - "salt environment '%s' maps to __env__ file_roots directory", saltenv - ) - saltenv = "__env__" - else: - return fnd - file_in_root = False - for root in __opts__["file_roots"][saltenv]: - if saltenv == "__env__": - root = root.replace("__env__", actual_saltenv) - # Refuse to serve file that is not under the root. - if salt.utils.verify.clean_path(root, fpath, subdir=True): - file_in_root = True - if not file_in_root: - return ret + #actual_saltenv = saltenv = load["saltenv"] + #if saltenv not in __opts__["file_roots"]: + # if "__env__" in __opts__["file_roots"]: + # log.debug( + # "salt environment '%s' maps to __env__ file_roots directory", saltenv + # ) + # saltenv = "__env__" + # else: + # return fnd + #file_in_root = False + #for root in __opts__["file_roots"][saltenv]: + # if saltenv == "__env__": + # root = root.replace("__env__", actual_saltenv) + # # Refuse to serve file that is not under the root. + # if salt.utils.verify.clean_path(root, fpath, subdir=True): + # file_in_root = True + #if not file_in_root: + # return ret with salt.utils.files.fopen(fpath, "rb") as fp_: fp_.seek(load["loc"]) ``` Would appreciate assistance, as this breaks our setup. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1219985 Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pablo.suarezhernandez@suse. | |com -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1219985 https://bugzilla.suse.com/show_bug.cgi?id=1219985#c1 --- Comment #1 from Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com> --- Upstream discussion: https://github.com/saltstack/salt/issues/65977. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1219985 https://bugzilla.suse.com/show_bug.cgi?id=1219985#c8 --- Comment #8 from Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com> --- I now tried your suggestion. This works: ``` c0c6bbd75dbf:/srv/formula-src # ls -ld chrony lrwxrwxrwx 1 root root 21 Feb 16 13:24 chrony -> chrony-formula/chrony c0c6bbd75dbf:/srv/formula-src # more /etc/salt/minion.d/roots.conf file_roots: base: - /srv/salt - /usr/share/salt-formulas/states - /srv/formula-src c0c6bbd75dbf:/srv/formula-src # salt-call --local state.show_sls chrony local: ---------- chrony-package-install-pkg-installed: .... ``` However this, in my opinion slightly cleaner variant, does not: ``` c0c6bbd75dbf:/srv/formula-src # ls -ld states/chrony lrwxrwxrwx 1 root root 24 Feb 16 13:23 states/chrony -> ../chrony-formula/chrony c0c6bbd75dbf:/srv/formula-src # more /etc/salt/minion.d/roots.conf file_roots: base: - /srv/salt - /usr/share/salt-formulas/states - /srv/formula-src/states c0c6bbd75dbf:/srv/formula-src # salt-call --local state.show_sls chrony local: - No matching sls found for 'chrony' in env 'base' ``` Then again, this variant does: ``` c0c6bbd75dbf:/srv/formula-src # ls -ld chrony lrwxrwxrwx 1 root root 34 Feb 16 13:32 chrony -> repositories/chrony-formula/chrony c0c6bbd75dbf:/srv/formula-src # more /etc/salt/minion.d/roots.conf file_roots: base: - /srv/salt - /usr/share/salt-formulas/states - /srv/formula-src c0c6bbd75dbf:/srv/formula-src # salt-call --local state.show_sls chrony local: ---------- chrony-package-install-pkg-installed: ---------- ... ``` I think I will opt for this last variant as to not clutter our repository top level with both, submodules and symlinks.
would still affect symlinks that are targetting files which are part of the same file root.
Thanks, makes sense. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1219985 Christian Boltz <suse-beta@cboltz.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |suse-beta@cboltz.de -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1219985 https://bugzilla.suse.com/show_bug.cgi?id=1219985#c9 --- Comment #9 from Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com> --- Some big refactoring in our Salt setup later, we now have an operational setup again: https://code.opensuse.org/heroes/salt/c/37c6d020e8e6e154717503d8bddacafb3be6... https://code.opensuse.org/heroes/salt-formulas-git/c/7e7a1d137c46d24ea8362fa... Now I can think of at least two other Salt infrastructures I maintain where the problematic symlink setup is used .. assuming this new behavior is now expected, I guess I will have to do the refactoring work there as well. Thank you for the help. -- You are receiving this mail because: You are on the CC list for the bug.
participants (1)
-
bugzilla_noreply@suse.com