salt.transport.ipc: msgpack.Unpacker exceeding max_str_len(1048576)

[arizvisa]

Description of Issue

2019-10-04 02:34:30,824 [salt.transport.ipc:198 ][ERROR   ][26] Exception occurred while handling stream: 3239303 exceeds max_str_len(1048576)

This is actually a dupe of issue #53230 which is closed but discussion is still occurring. I'm not the original reporter, so I'm unable to re-open. According to discussion on the original issue, some msgpack-python versions set some unreasonable values in order to prevent some lame DoS attacks. It seems that msgpack-pure doesn't have these constraints.

The settings can be specified using kwargs to msgpack.Unpacker. I'm hitting max_str_len specifically, however the original reporter has hit max_bin_len. There's a few others that are set pretty conservatively in these distro versions of msgpack. The easiest fix would be to warn the user if they're using a dumb version of the module.

I looked around to see if there was any documentation for supported Python module versions but was unable to locate this. This is problematic because salt-bootstrap appears to grab msgpack out of the distro's package manager which can result in snagging one of these busted versions.

A better fix would probably be to expose these parameters via salt's configuration files, but either documenting the issue of older msgpack versions, trading performance of msgpack-python for msgpack-pure or updating salt-bootstrap to force install a more recent version of msgpack-python using pip will all work around the problem.

Setup

I hit this by simply using pkg.list_repo_pkgs on a minion. Really you need some way to exercise transferring a large amount of data with msgpack.Unpacker. Since this is spotted in a couple of places in salt.transport.ipc or salt.transport.tcp either of those will work.

Steps to Reproduce Issue

Since all calls to msgpack.Unpacker are using the defaults, anything that results in transferring d
ata larger than 1mb will result in the error. I simply ran the following against a default ubuntu vm which results in the call failing and the above message logged.

$ salt $id pkg.list_repo_pkgs

Versions Report

Note that msgpack-python is 0.5.6.. This is the most recent version as installed by salt-bootstrap.

           Salt: 2019.2.1

Dependency Versions:
           cffi: 1.11.5
       cherrypy: Not Installed
       dateutil: 2.8.0
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.14
          ioflo: 1.7.6
         Jinja2: 2.10.1
        libgit2: 0.27.8
        libnacl: 1.6.1
       M2Crypto: Not Installed
           Mako: 1.1.0
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: 3.9.0
         pygit2: 0.27.4
         Python: 2.7.16 (default, Apr 30 2019, 15:54:43)
   python-gnupg: Not Installed
         PyYAML: 5.1
          PyZMQ: 17.0.0
           RAET: 0.6.8
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 5.0.2
            ZMQ: 4.3.2

System Versions:
           dist: fedora 30 Thirty
         locale: UTF-8
        machine: x86_64
        release: 4.14.96-coreos
         system: Linux
        version: Fedora 30 Thirty

[arizvisa]

Actually downgrading to msgpack_pure as I suggested is not possible on 2019.2.1 without backporting commit 8e6546a, as 2019.2.1's salt.transport imports msgpack directly instead of using salt.utils.msgpack.

[johnj]

I had a similar issue with the TCP transport. I ended up pinning msgpack to 0.5.6 in our pex single-binary build for now.

https://github.com/johnj/salt-pex

[flassman]

I've been hit by this issue after upgrading to salt 3000 (with Python 3.6.8), after having happily used msgpack 0.5.6 and 2019.2.2 (with Python 2.7.5) before. As a workaround I have downloaded msgpack-0.5.6 from https://pypi.org/project/msgpack/0.5.6/#files and installed it like this (I'm on CentOS 7.7):

# rpm -e --nodeps python36-msgpack-0.6.2-2.el7.x86_64
# pip3 install msgpack-0.5.6-cp36-cp36m-manylinux1_x86_64.whl

[Athos85]

I've been hit by this issue after upgrading to salt 3000 (with Python 3.6.8), after having happily used msgpack 0.5.6 and 2019.2.2 (with Python 2.7.5) before. As a workaround I have downloaded msgpack-0.5.6 from https://pypi.org/project/msgpack/0.5.6/#files and installed it like this (I'm on CentOS 7.7):

# rpm -e --nodeps python36-msgpack-0.6.2-2.el7.x86_64
# pip3 install msgpack-0.5.6-cp36-cp36m-manylinux1_x86_64.whl

Hi, Any news on this issue? we entered the same issue after upgrading to salt 3000 :/

our stream is about 6.6 MB big
Exception occurred while handling stream: 6607286 exceeds max_str_len(1048576)

@flassman how did you manage to "pin" salt to the older msgpack release after installing it? I followed your instruction but I dont find the place where I should take the config change on salt in order to use msgpack-0.5.6 with salt 3000. do you have any hint?

Thanks a lot.

[Athos85]

Update
Because of a major impact in our infrastructure we had to downgrade from salt 3000 back to salt 2019.2

[sancex]

rpm -e --nodeps python36-msgpack-0.6.2-2.el7.x86_64

pip3 install msgpack-0.5.6-cp36-cp36m-manylinux1_x86_64.whl

Worked like a charm... thanks for the solution.

[psyer]

I'm shocked this is not blowing up peoples clusters all over the place. I guess they are not sending/receiving messages over 1MB. This was a show stopper for us (and others). @sagetherage can we please bump this higher than low priority?

To be clear this is not a Salt issue, this is an issue with the default settings that were changed on a hard Salt dependency which is causing the issue. The Salt code can work around this issue until the dependency is fixed (in msgpack 1.0). RedHat/CentOS/Salt don't have packages for msgpack 1.0 yet, which is why we are stuck.

There are a bunch of issues opened and closed issues about this (consolidation?)

#53230
#55180
#57026

@lukasraska describes the issue in the best way I've seen in #57026.

We can not pin the version of msgpack to 0.5.6 to fix this in 3001, because of a hard dependency of salt 3001 on msgpack 0.6.
The msgpack people don't fix this issue until 1.0. We want to have the latest Salt installed, so we had to patch our Salt to fix this.

We created the following patch (to 3001.1 utils/msgpack.py) to fix this issue. I would do a PR, but it is unlikely I can write a integration test for this. If someone wants to do the test code, I would be glad to do the PR. I appreciate @lukasraska for the idea, but his promised PR or code never materialized.

92,98d91
< def _fix_msgpack_unpack_kwargs(kwargs):
<     assert isinstance(kwargs, dict)
<
<     If version >= (0, 6, 0) and version < (1, 0, 0):
<         kwargs["max_buffer_size"] = 100 * 1024 * 1024
<
<     return _sanitize_msgpack_unpack_kwargs(kwargs)
107c100
<     self, *args, **_fix_msgpack_unpack_kwargs(kwargs)
---
>     self, *args, **_sanitize_msgpack_unpack_kwargs(kwargs)
108a102
>

[sagetherage]

@waynew maybe this is a good one for the Test Clinic today at 2 PM Central?

[sagetherage]

@psyer go ahead and put in a PR -- that will help to move it along and I will try to get this on Wayne's Test Clinic agenda if not today then I think he has another Test Clinic on Thursday this week - are you able to join either of those? you can see those on the community calendar here

[psyer]

@psyer go ahead and put in a PR -- that will help to move it along and I will try to get this on Wayne's Test Clinic agenda if not today then I think he has another Test Clinic on Thursday this week - are you able to join either of those? you can see those on the community calendar here

@sagetherage I will put in a PR, but unfortunately I work in an environment during the day, where I would not be able to attend Wanye's Clinic. I'll look into possibly being able to write some type of test, as all I need to do is put a message on the bus larger than 1MB when msgpack >.6 and < 1.0 is installed.

[sagetherage]

@psyer you can also work asyn with Wayne on writing tests - he is on the Community Slack as Wayne Werner (Core Eng.) and he is on IRC as waynew. Let me know if I can help facilitate in another way, open to ideas, too. :)

[arizvisa]

awesome.