saltstack / saltstack/salt

[BUG] vault.generate_token called internally with a bytes argument

Open Beginner friendly
#57,797 2 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug severity-medium ZD
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description

The vault.generate_token runner is called internally with a bytes value for the signature. Although the runner will work correctly if passed bytes or the decoded str, at least one event returner (SSE) expects this to be JSON serializable. I have not tried to reproduce with other returners, but at least the postgres returner would probably be impacted: https://github.com/saltstack/salt/blob/b95213ec903402f25c1e0aeb3990fe8452ab63ce/salt/returners/postgres.py#L276

Setup

Master conf:

peer_run:
  .*:
    - vault.generate_token

myvault:
  driver: vault

vault:
    url: http://vault:8200
    auth:
      method: approle
      role_id: some_uuid
      secret_id: some_uuid
    policies:
      - saltmaster

/srv/salt/_runners/call_vault.py:

import salt


def getpwd():
    pwd = salt.utils.sdb.sdb_get(
        "sdb://myvault/secret-v1/foo",
        __opts__,
        __utils__,
    )

/srv/salt/vault_orch.sls:

show_secrets:
  salt.runner:
    - name: call_vault.getpwd

Steps to Reproduce the behavior

salt-run state.orch vault_orch

This actually returns fine in 3001, but in the logs:

2020-06-25 13:01:36,650 [salt.utils.event :1280][ERROR   ][9856] Could not store events - returner 'sseapi.event_return' raised exception: Object of type 'bytes' is not JSON serializable
2020-06-25 13:01:36,650 [salt.utils.event :1286][DEBUG   ][9856] Event data that caused an exception: [{'data': {'fun': 'runner.vault.generate_token', 'jid': '20200625130136627218', 'user': 'UNKNOWN', 'fun_args': [{'minion_id': 'master1_master', 'signature': b'embOJBDbaNo1KumVRjq+pShjSPDTRRA5x+yhoHOIO+LdpjKtuVDGc/mRw04eiSy5dMqE53Ic7na434rIUTzANBKU00skx2A8I6rP1cMQTLKVC4mMGPGjXQQ4ZVJwdgxv6CVDfiNlagTYNXG7mtlJ6T7Sk2ya30ju5ICTKr5c0+nuLmReB7ZWV5KYnY2Dwvnh3dwCXn7J7gg1yi+DuXMIH2PEx49534GSISTt8CUrfU+vwJL0HaEd7j6RatrbXp7ZS5Bz3kX60eTi/Be0BdygsZLfMoTi1f98gystVKVK9dhZH3z1iDqhDoAcBxBXINbIbiuqqYu5Vgf/IWxuI2IpzQ==', 'impersonated_by_master': True, 'ttl': None, 'uses': None}], '_stamp': '2020-06-25T13:01:36.647658', '_master_path': ['master1_master']}, 'tag': 'salt/run/20200625130136627218/new'}]

Note that signature in fun_args has a bytes value.

Expected behavior

Orchestration runs OK.

Versions Report

salt --versions-report
Salt Version:
           Salt: 3001
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: Not Installed
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.0.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4
 
System Versions:
           dist: centos 7 Core
         locale: UTF-8
        machine: x86_64
        release: 5.3.0-59-generic
         system: Linux
        version: CentOS Linux 7 Core

Additional context

Potential fix:

diff --git a/salt/utils/vault.py b/salt/utils/vault.py
index aff38bdc09..c81e6fe9b1 100644
--- a/salt/utils/vault.py
+++ b/salt/utils/vault.py
@@ -16,8 +16,10 @@ import os
 import time
 
 import requests
+
 import salt.crypt
 import salt.exceptions
+import salt.utils.stringutils
 import salt.utils.versions
 
 log = logging.getLogger(__name__)
@@ -60,7 +62,9 @@ def _get_token_and_url_from_master():
     if __opts__.get("__role", "minion") == "minion":
         private_key = "{0}/minion.pem".format(pki_dir)
         log.debug("Running on minion, signing token request with key %s", private_key)
-        signature = base64.b64encode(salt.crypt.sign_message(private_key, minion_id))
+        signature = salt.utils.stringutils.to_str(
+            base64.b64encode(salt.crypt.sign_message(private_key, minion_id))
+        )
         result = __salt__["publish.runner"](
             "vault.generate_token", arg=[minion_id, signature, False, ttl, uses]
         )
@@ -71,7 +75,9 @@ def _get_token_and_url_from_master():
             minion_id,
             private_key,
         )
-        signature = base64.b64encode(salt.crypt.sign_message(private_key, minion_id))
+        signature = salt.utils.stringutils.to_str(
+            base64.b64encode(salt.crypt.sign_message(private_key, minion_id))
+        )
         result = __salt__["saltutil.runner"](
             "vault.generate_token",
             minion_id=minion_id,

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in salt/utils/vault.py at _get_token_and_url_from_master(), then reproduce the issue with salt-run state.orch vault_orch using the supplied configuration. Ensure the internally passed signature is JSON serializable and verify orchestration completes without the event-returner bytes error.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devops
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.