Bug#949222: Bug#959684: salt: CVE-2020-11651 and CVE-2020-11652

2020-05-05 Thread Simon McVittie
On Tue, 05 May 2020 at 17:37:53 +0200, Salvatore Bonaccorso wrote:
> Do you have respective stretch and buster setups which you could
> expose those packages to?

Sorry, no: the owner of the machines I was looking at asked me to switch
over to upstream's packages.

smcv



Bug#949222: Bug#959684: salt: CVE-2020-11651 and CVE-2020-11652

2020-05-05 Thread Salvatore Bonaccorso
Hi Simon,

On Tue, May 05, 2020 at 03:01:45PM +0100, Simon McVittie wrote:
> On Mon, 04 May 2020 at 01:34:33 +0200, Guilhem Moulin wrote:
> >   CVE-2020-11651
> >   CVE-2020-11652
> 
> I found myself needing to mitigate this for a salt deployment, so I
> tried backporting the upstream patches to buster.
> 
> The attached are not at all thoroughly-tested and should be reviewed
> carefully by someone who knows the codebase, but they seem to work, and
> the proof-of-concept from
> https://github.com/rossengeorgiev/salt-security-backports no longer reports
> that the master is vulnerable. This was only a stopgap, because that
> deployment is now using the packages from saltstack.com instead, but it
> might be useful to the salt maintainers.
> 
> There are also unofficial backports in
> https://github.com/rossengeorgiev/salt-security-backports - I tried doing
> the cherry-picks myself and then compared what I got with those, in an
> attempt to guard against mistakes (by either myself or the author of those
> backports).
> 
> Note that patch 0003 contains unofficial workarounds for regressions in the
> release that fixed those CVEs, which you might prefer to exclude from an
> official update.

I did actually work on that already yesterday and uploaded the
attached debdiffs to security-master *but* I'm in need of someone
using salt to effectively ina good way to test them. 

Do you have respective stretch and buster setups which you could
expose those packages to?

Regards,
Salvatore
diff -Nru salt-2016.11.2+ds/debian/changelog salt-2016.11.2+ds/debian/changelog
--- salt-2016.11.2+ds/debian/changelog  2018-04-20 14:33:54.0 +0200
+++ salt-2016.11.2+ds/debian/changelog  2020-05-04 14:29:16.0 +0200
@@ -1,3 +1,14 @@
+salt (2016.11.2+ds-1+deb9u3) stretch-security; urgency=high
+
+  * Non-maintainer upload by the Security Team.
+  * Address CVE-2020-11651 and CVE-2020-11652 (Closes: #959684)
+Thanks to Daniel Wozniak 
+  * Add note about log messages to hardening salt docs
+  * salt-api NET API with the ssh client enabled is vulnerable to command
+injection (CVE-2019-17361) (Closes: #949222)
+
+ -- Salvatore Bonaccorso   Mon, 04 May 2020 14:29:16 +0200
+
 salt (2016.11.2+ds-1+deb9u2) stretch; urgency=medium
 
   * Fix CVE-2017-8109: salt-ssh minion copied over configuration from the
diff -Nru 
salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
 
salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
--- 
salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
   1970-01-01 01:00:00.0 +0100
+++ 
salt-2016.11.2+ds/debian/patches/Add-note-about-log-messages-to-hardening-salt-docs.patch
   2020-05-04 14:29:16.0 +0200
@@ -0,0 +1,41 @@
+From: "Daniel A. Wozniak" 
+Date: Mon, 13 Apr 2020 07:01:07 +
+Subject: Add note about log messages to hardening salt docs
+Origin: 
https://github.com/saltstack/salt/commit/4631781376ddc9ee9d279f407ac3d0b78644fae7
+
+---
+ doc/topics/hardening.rst | 4 
+ salt/master.py   | 2 +-
+ 2 files changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/doc/topics/hardening.rst b/doc/topics/hardening.rst
+index c645b84128e4..569ad654af69 100644
+--- a/doc/topics/hardening.rst
 b/doc/topics/hardening.rst
+@@ -57,6 +57,10 @@ Salt hardening tips
+   particularly sensitive minions. There is also :ref:`salt-ssh` or the
+   :mod:`modules.sudo ` if you need to further restrict
+   a minion.
++- Monitor specific security releated log messages. Salt ``salt-master`` logs
++  attempts to access methods which are not exposed to network clients. These 
log
++  messages are logged at the ``error`` log level and start with ``Requested
++  method not exposed``.
+ 
+ .. _salt-users: https://groups.google.com/forum/#!forum/salt-users
+ .. _salt-announce: https://groups.google.com/forum/#!forum/salt-announce
+diff --git a/salt/master.py b/salt/master.py
+index 7d1444cf1221..aae55f1828e1 100644
+--- a/salt/master.py
 b/salt/master.py
+@@ -1162,7 +1162,7 @@ class TransportMethods(object):
+ try:
+ return getattr(self, name)
+ except AttributeError:
+-log.error("Expose method not found: %s", name)
++log.error("Requested method not exposed: %s", name)
+ else:
+ log.error("Requested method not exposed: %s", name)
+ 
+-- 
+2.20.1
+
diff -Nru 
salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
 
salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
--- 
salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
  1970-01-01 01:00:00.0 +0100
+++ 
salt-2016.11.2+ds/debian/patches/Fix-CVE-2020-11651-and-Fix-CVE-2020-11652-2016.11.2.patch
  2020-05-04 14:29:16.0 +0200
@@ -0,0 +1,237 @@
+From 006219501bbb3a81a9fb64975035011016d5a7eb Mon Sep 17 00:00:0

Bug#949222: Bug#959684: salt: CVE-2020-11651 and CVE-2020-11652

2020-05-05 Thread Simon McVittie
On Mon, 04 May 2020 at 01:34:33 +0200, Guilhem Moulin wrote:
>   CVE-2020-11651
>   CVE-2020-11652

I found myself needing to mitigate this for a salt deployment, so I
tried backporting the upstream patches to buster.

The attached are not at all thoroughly-tested and should be reviewed
carefully by someone who knows the codebase, but they seem to work, and
the proof-of-concept from
https://github.com/rossengeorgiev/salt-security-backports no longer reports
that the master is vulnerable. This was only a stopgap, because that
deployment is now using the packages from saltstack.com instead, but it
might be useful to the salt maintainers.

There are also unofficial backports in
https://github.com/rossengeorgiev/salt-security-backports - I tried doing
the cherry-picks myself and then compared what I got with those, in an
attempt to guard against mistakes (by either myself or the author of those
backports).

Note that patch 0003 contains unofficial workarounds for regressions in the
release that fixed those CVEs, which you might prefer to exclude from an
official update.

smcv
>From f17495581f0d7cc7bb94fa2c1f97517d16e2dd7a Mon Sep 17 00:00:00 2001
From: Simon McVittie 
Date: Mon, 4 May 2020 12:39:29 +0100
Subject: [PATCH 1/4] Apply upstream patch for CVE-2019-17361

Signed-off-by: Simon McVittie 
Closes: #949222
---
 .../0003-Use-renamed-python3-tornado4.patch   | 185 ++--
 ...17361-various-netapi-fixes-and-tests.patch | 280 ++
 debian/patches/series |   1 +
 3 files changed, 376 insertions(+), 90 deletions(-)
 create mode 100644 debian/patches/CVE-2019-17361-various-netapi-fixes-and-tests.patch

diff --git a/debian/patches/0003-Use-renamed-python3-tornado4.patch b/debian/patches/0003-Use-renamed-python3-tornado4.patch
index 01ad3154bf..1f0878a7a0 100644
--- a/debian/patches/0003-Use-renamed-python3-tornado4.patch
+++ b/debian/patches/0003-Use-renamed-python3-tornado4.patch
@@ -1,54 +1,53 @@
-From d17823525d0842ea3e643b024714643b4949ec1b Mon Sep 17 00:00:00 2001
 From: Benjamin Drung 
 Date: Mon, 17 Dec 2018 20:35:19 +0100
 Subject: [PATCH 3/3] Use renamed python3-tornado4
 
 ---
- doc/conf.py   | 17 
- salt/client/__init__.py   |  5 +++-
- salt/client/mixins.py |  5 +++-
- salt/crypt.py | 11 +---
- salt/engines/ircbot.py|  8 --
- salt/engines/webhook.py   | 11 +---
- salt/fileclient.py|  5 +++-
- salt/master.py|  5 +++-
- salt/minion.py| 17 
- salt/netapi/rest_tornado/__init__.py  | 13 +++---
- salt/netapi/rest_tornado/saltnado.py  | 20 +-
- .../rest_tornado/saltnado_websockets.py   |  9 ---
- salt/pillar/__init__.py   |  5 +++-
- salt/transport/client.py  |  9 +--
- salt/transport/ipc.py | 23 +++-
- salt/transport/mixins/auth.py |  5 +++-
- salt/transport/tcp.py | 26 +--
- salt/transport/zeromq.py  | 20 +-
- salt/utils/asynchronous.py|  8 --
- salt/utils/event.py   |  8 --
- salt/utils/gitfs.py   |  5 +++-
- salt/utils/http.py| 11 +---
- salt/utils/process.py |  5 +++-
- salt/utils/thin.py|  5 +++-
- salt/utils/zeromq.py  |  8 --
- salt/version.py   | 10 +--
- tests/integration/__init__.py |  8 --
- .../files/engines/runtests_engine.py  |  8 +++---
- tests/integration/modules/test_gem.py |  5 +++-
- tests/integration/modules/test_ssh.py |  5 +++-
- tests/support/helpers.py  |  8 --
- tests/unit/fileserver/test_gitfs.py   |  5 +++-
- tests/unit/modules/test_random_org.py |  5 +++-
- tests/unit/netapi/test_rest_tornado.py| 20 +-
- tests/unit/test_minion.py | 18 +
- tests/unit/transport/test_ipc.py  | 11 +---
- tests/unit/transport/test_tcp.py  | 14 +++---
- tests/unit/transport/test_zeromq.py   | 13 --
- tests/unit/utils/test_asynchronous.py | 11 +---
- tests/unit/utils/test_context.py  | 11 +---
- tests/unit/utils/test_event.py|  5 +++-
- 41 files changed, 314 insertions(+), 107 deletions(-)
+ doc/conf.py| 17 ++
+ salt/client/__init__.py|  5 -
+ salt/client/mixins.py  |  5 -
+ salt/crypt.py