saltstack / saltstack/salt

[BUG] file.manage comment does not redact credentials when using http as source and it has failed to cache the file

Open
#58,470 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Description
Given a file.manage state set to source from http, when the state is unable to connect, the message in the ret['comment'] is not redacted revealing used credentials in the URL.

minionwin1:

      ID: http_hashed_file
Function: file.managed
    Name: C:\test.txt
  Result: False
 Comment: Failed to cache http://<redacted>@172.31.26.239:5000/filer?file=test.txt: Error: HTTP 401: UNAUTHORIZED reading http://adriano:1234@172.31.26.239:5000/filer?file=test.txt

The problem seems to be in salt/modules/file.py, in the function def get_managed().
The return includes the exc message which contains the failed http url

except Exception as exc:  # pylint: disable=broad-except
  # A 404 or other error code may raise an exception, catch it
  # and return a comment that will fail the calling state.
  _source = salt.utils.url.redact_http_basic_auth(source)
  return "", {}, "Failed to cache {0}: {1}".format(_source, exc)

This specifc fix worked fine in this module, but I believe it's a change that should go up the tree maybe to prevent this error more globally

                _source = salt.utils.url.redact_http_basic_auth(source)
                _exc = salt.utils.url.redact_http_basic_auth(str(exc))
                return "", {}, "Failed to cache {0}: {1}".format(_source, _exc)

Setup

cat /srv/salt/userpwd_redacted/cp_secret.sls

http_hashed_file:
  file.managed:
{% if grains['os'] == 'Windows' %}
    - name: 'C:\test.txt'
{% else %}
    - name: '/tmp/test.txt'
{% endif %}
    - source: 'http://3adrian:1234@172.31.26.239:5000/filer?file=test.txt'. #username is wrong
    - source_hash: b05403212c66bdc8ccc597fedf6cd5fe

Steps to Reproduce the behavior
Run state using invalid credentials and observe non redacted credentials info

Expected behavior
Credentials must be redacted

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report
3001.1

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/modules/file.py at get_managed() and inspect how the exception text is included in the cache failure comment. Reproduce the file.managed state with invalid HTTP credentials, then verify that credentials are redacted in the complete returned comment, including the URL contained in the exception message.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.