Salt: Slack Engine failing to load (develop branch)

[pmusolino-rms]

Description of Issue/Question

The documentation for /etc/salt/master states to do:

engines:
slack:

However it complains that it wants a list

2017-06-21 18:52:15,533 [salt.config                              ][WARNING ][22786] Key 'engines' with value {'slack': {'control': True, 'fire_all': False, 'token': 'xxxx-xxxxxxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxx', 'groups': {'default': {'commands': ['test.ping', 'net.arp', 'net.ping', 'list_jobs', 'list_commands'], 'users': ['soandso'], 'aliases': {'list_commands': {'cmd': 'pillar.get salt:engines:slack:valid_commands target=saltmaster'}, 'ping_google_from_xyz': {'cmd': 'net.ping 8.8.8.8 target=xyz'}, 'get_arp_tables': {'cmd': 'net.arp target=xyz'}, 'list_jobs': {'cmd': 'jobs.list_jobs'}}}}, 'groups_pillar_name': 'slack_engine:groups_pillar'}} has an invalid type of dict, a list is required for this value
2017-06-21 18:52:15,615 [salt.loaded.int.engines.slack            ][ERROR   ][22751] Slack bot token not found, bailing...
2017-06-21 18:52:15,615 [salt.engines                             ][CRITICAL][22751] Engine <salt.loader.LazyLoader object at 0x3d80f50> could not be started! Error: Slack Engine bot token not configured

If I do

engines:
  - slack

the invalid types errors disappear however the slackbot token not found error persists.

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

engines:
  - slack:
      token: 'xxxx-xxxxxxxxx-xxxxxxxxx-xxxxxxxxxx-xxxxxx'
      control: True
      fire_all: False
      groups_pillar_name: "slack_engine:groups_pillar"
      groups:
        default:
          users:
            - soandso
          commands:
            - test.ping
            - net.arp
            - net.ping
            - list_jobs
            - list_commands
          aliases:
            list_jobs:
              cmd: jobs.list_jobs
            list_commands:
              cmd: pillar.get salt:engines:slack:valid_commands target=saltmaster
            get_arp_tables:
              cmd: net.arp target=xyz
            ping_google_from_xyz:
              cmd: net.ping 8.8.8.8 target=xyz

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)
Try to start salt-master on host

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
           Salt: 2017.7.0-2512-ge9bb2b6

Dependency Versions:
           cffi: 1.10.0
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.6
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: 0.8.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: 1.2.5
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Nov  6 2016, 00:28:07)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.3.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.1
            ZMQ: 3.2.5

System Versions:
           dist: centos 7.3.1611 Core
        machine: x86_64
        release: 3.10.0-514.16.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.3.1611 Core

[Ch3LL]

@garethgreenaway I can replicate the warning when using a dict vs a list. Do you know what the correct data structure is? I am not sure if the doc is wrong or the warning is incorrect?

[garethgreenaway]

@Ch3LL Taking a look.

[garethgreenaway]

@Ch3LL Looks like the correct value is a list and the documentation is wrong.

[lahdekorpi]

When using:

engines:
  slack:
    token: 'xoxb-xxx-xxx'
    control: True
...

Everything works perfectly. But I do get a:

[WARNING ] Key 'engines' with value {'slack': {'control': True, 'valid_commands': ['test.ping', 'test.version', 'status.uptime', 'status.meminfo', 'status.version', 'status.cpustats', 'status.diskstats', 'status.diskusage', 'status.w', 'list_jobs', 'list_commands', 'jobs.list_jobs'], 'token': 'xoxb-xxx-xxx', 'valid_users': ['xxx', 'xxx'], 'aliases': {'list_commands': {'cmd': 'pillar.get salt:engines:slack:valid_commands target=saltmaster tgt_type=list'}, 'list_jobs': {'cmd': 'jobs.list_jobs'}}}} has an invalid type of dict, a list is required for this value

When using:

engines:
  - slack:
    token: 'xoxb-xxx-xxx'
    control: True
...

Slack isn't loaded at all.

Salt Version:
           Salt: 2017.7.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Nov 19 2016, 06:48:10)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-92-generic
         system: Linux
        version: Ubuntu 16.04 xenial

[Ch3LL]

@garethgreenaway thanks for the input seems we might need to change the code so it loads with a list. So i'll label this a bug.

[garethgreenaway]

@ch3ll will take a look.

[lahdekorpi]

Any workarounds for this?

[lahdekorpi]

Nothing?

[lahdekorpi]

Would this issue qualify as a security issue since it does output the actual Slack token to restricted users when they run Salt commands?

[lahdekorpi]

Just verified that this still exists in 2017.7.3.

[lahdekorpi]

Oops. It's an indentation issue.

engines:
  - slack:
    token: 'xoxb-xxx-xxx'
    control: True

is wrong.

While:

engines:
  - slack:
      token: 'xoxb-xxx-xxx'
      control: True

is correct.

[garethgreenaway]

@lahdekorpi Can you provide some addition information for where you're seeing a potential security issue and in what scenarios you're seeing the Slack token? Thanks!

[lahdekorpi]

@garethgreenaway When the engines in the master config had incorrect indentation, Slack integration would work perfectly but Salt would output a:

[WARNING ] Key 'engines' with value {'slack': {'control': True, 'valid_commands': ['test.ping', 'test.version', 'status.uptime', 'status.meminfo', 'status.version', 'status.cpustats', 'status.diskstats', 'status.diskusage', 'status.w', 'list_jobs', 'list_commands', 'jobs.list_jobs'], 'token': 'xoxb-xxx-xxx', 'valid_users': ['xxx', 'xxx'], 'aliases': {'list_commands': {'cmd': 'pillar.get salt:engines:slack:valid_commands target=saltmaster tgt_type=list'}, 'list_jobs': {'cmd': 'jobs.list_jobs'}}}} has an invalid type of dict, a list is required for this value

warning to the output of every user and script, even restricted, running Salt including a private API key for Slack that can be used to spy on Slack messages / do other things.

This does not, however, happen if the master is correctly configured.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.