[Pkg-salt-team] Bug#985085: source review findings - SUSE specific and CVE-2021-25315 does not apply to Debian

Sun Mar 28 05:55:50 BST 2021

In brief, like carnil had suggested above, my review and findings concur that
#985085 and CVE-2021-25315 are SUSE specific and do not apply to Debian.
Thanks to Miuku of #suse on freenode for his helpful feedback accessing the
SUSE src.rpm .  I'll leave the bug open a few days, allowing for additional
review and feedback.

Looking closer, as can be seen from the link carnil provided, the SUSE
bugzilla tracker for this issue notes:

    "Hi. Upstream was not affected with this issue.
    The issue was caused by overlapping of upstream patch and one of our patches."
    Victor Zhestkov 2021-03-13 13:54:38 UTC
    https://bugzilla.suse.com/show_bug.cgi?id=1182382#c16

Reviewing the mitre description for this CVE, it's corrected in the following
two SUSE packages and versions:

    - SUSE Linux Enterprise Server 15 SP 3 salt version 3002.2-3
    - openSUSE Tumbleweed salt versions after 3002.2-2.1

While I have not yet been able to access the source for the Enterprise Server
fixed salt version (salt-3002.2-8.33.1.src.rpm), the openSUSE fixed salt
version (salt-3002.2-4.1.src.rpm) can be downloaded from the following link
(click "Grab binary packages directly" and then the src.rpm).

    https://software.opensuse.org//download.html?project=openSUSE%3AFactory&package=salt

    https://download.opensuse.org/repositories/openSUSE:/Factory/standard/src/salt-3002.2-4.1.src.rpm

The SUSE salt.spec changelog for salt-3002.2-4.1.src.rpm documents fixing this
CVE (by adding the patch Elimar Riesebieter linked to earlier):
    * Mon Mar  1 2021 Alexander Graul <alexander.graul at suse.com>
    - Bring missing part of async batch implementation back (bsc#1182382)
      (CVE-2021-25315)
    https://bugzilla.suse.com/attachment.cgi?id=846239

This same fixed message can be seen in the SUSE Customer Center website, with
links to the fixed SUSE Linux Enterprise Server salt-3002.2-8.33.1.src.rpm
package, that requires a subscription to access (enter "CVE-2021-25315" at the
search prompt).
        https://scc.suse.com/patches/

After downloading the src.rpm, source review can mostly be performed on Debian
(see below for sample commands to begin; I did use an openSUSE VM to run
rpmbuild on the src.rpm to get their patched source tree).  
Reading the "fix patch", it only changes one file -- salt/client/__init__.py .

    https://bugzilla.suse.com/attachment.cgi?id=846239

Comparing the current Debian testing and unstable salt package version
(3002.5+dfsg1-1) with the latest fixed openSUSE salt version
(salt-3002.2-4.1.src.rpm), the initial upstream client/__init__.py file is
identical.  

The Debian package only applies a single patch to that file, which corrects a
comment typo elsewhere ( debian/patches/Fix-various-spelling-mistakes.patch ).

The SUSE salt RPM has ~160 patches applied to it (vs ~20 patches applied to
the Debian one).  Three of the SUSE salt patches modify the client/__init__.py
file.

While I admit to not being well versed in the Salt codebase, comparing the
various patches it appears that SUSE adopted one approach to implement "eauth"
(their Patch40 async-batch-implementation.patch), and upstream Salt (which
Debian matches) implemented another, and in the process reset the initial SUSE
authentication token parsing.  This becomes very clear comparing an upstream
version of client/__init__.py versus the SUSE patched version (diff or
gvimdiff).  A little curious SUSE continues with their approach and now
patches out the current upstream, but that may be related to their other
patches and the fact that their solution has been in place for several years
now.  

Regardless, I don't see #985085 and CVE-2021-25315 appling to Debian, and
recommend this bug be closed.

regards,
donfede

#####
# spec file excerpts and comments, followed by my summary notes at dash (-),
# of the 3x patches affecting client/__init__.py from salt.spec file in
# openSUSE salt-3002.2-4.1.src.rpm 

    Patch40: async-batch-implementation.patch
    Date: Fri, 16 Nov 2018 17:05:29 +0100 From: Mihai Dinca <mdinca at suse.de>
    # PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/50546
    # PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/51863
     - >> This patch introduces the "overlap" code, including a call to
       batch_get_eauth() nearby the "fix patch" code.

    Patch63: fix-memory-leak-produced-by-batch-async-find_jobs-me.patch
    Date: Mon, 16 Sep 2019 11:27:30 +0200 From: Mihai Dinca <mdinca at suse.de>
    # PATCH-FIX_OPENSUSE: https://github.com/openSUSE/salt/commit/6af07030a502c427781991fc9a2b994fa04ef32e
     - Minor addition elsewhere.

    Patch151: async-batch-implementation-fix-320.patch
    Date: Wed, 17 Feb 2021 16:47:11 +0300 From: Victor Zhestkov <35733135+vzhestkov at users.noreply.github.com>
    # PATCH-FIX_OPENSUSE: https://github.com/openSUSE/salt/pull/320
     - This is the new "fix patch" code from the SUSE CVE fix; this code is
       present in Debian, but is the only eauth parameter processing present.

#####
# sample commands to begin code review of suse src.rpm 

/tmp/hack_salt$ wget https://download.opensuse.org/repositories/openSUSE:/Factory/standard/src/salt-3002.2-4.1.src.rpm
...
Length: 26473319 (25M) [application/x-redhat-package-manager]
...
2021-03-27 23:20:33 (2.18 MB/s) - ‘salt-3002.2-4.1.src.rpm’ saved

/tmp/hack_salt$ mkdir x ; cd x
/tmp/hack_salt/x$ rpm2cpio ../salt-3002.2-4.1.src.rpm | cpio -id
55842 blocks
/tmp/hack_salt/x$ ls | wc -l
168
/tmp/hack_salt/x$ file * | grep -v ASCII\ text
add-supportconfig-module-for-remote-calls-and-saltss.patch: Python script, UTF-8 Unicode text executable
batch.py-avoid-exception-when-minion-does-not-respon.patch: unified diff output, UTF-8 Unicode text
fix-memory-leak-produced-by-batch-async-find_jobs-me.patch: unified diff output, UTF-8 Unicode text
html.tar.bz2:                                               bzip2 compressed data, block size = 900k
opensuse-3000-libvirt-engine-fixes-251.patch:               unified diff output, UTF-8 Unicode text
open-suse-3002.2-bigvm-310.patch:                           Python script, UTF-8 Unicode text executable
salt.spec:                                                  UTF-8 Unicode text
v3002.2.tar.gz:                                             gzip compressed data, from Unix, original size modulo 2^32 76451840
/tmp/hack_salt/x$ 
/tmp/hack_salt/x$ grep -l "client/__init__" *
async-batch-implementation-fix-320.patch
async-batch-implementation.patch
fix-memory-leak-produced-by-batch-async-find_jobs-me.patch
/tmp/hack_salt/x$ 

#####
# sample commands run on an openSUSE system to review patched salt src.rpm 

zypper source-install salt
zypper install rpm-build
cd /usr/src/packages/SPECS
rpmbuild -bp salt.spec

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-salt-team/attachments/20210328/e08ba640/attachment.sig>