Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mask rendered data (in logs) #48291

Merged
merged 9 commits into from Jun 25, 2018

Conversation

Projects
None yet
3 participants
@isbm
Copy link
Contributor

commented Jun 25, 2018

What does this PR do?

  • Fixes a security issue
  • Adds an utility to further fix such cases

What issues does this PR fix or reference?

When you call salt-ssh -l debug, you will get the entire roster with the passwords directly in the log. This PR fixes this by matching key: value strings in the output data and replaces YAML values with the ** hidden ** string. This can be applied for passwords, user IDs, actual IP addresses etc.

NOTE: this is so far is limited only to key: value where value is replaced. Feel free to extend it for IP addresses etc.

Tests written?

Yes

@rallytime rallytime requested a review from saltstack/team-core Jun 25, 2018

for line in data.split(os.linesep):
if fnmatch.fnmatch(line.strip(), mask) and ':' in line:
key, value = line.split(':', 1)
out.append('{}: {}'.format(key.strip(), '** hidden **'))

This comment has been minimized.

Copy link
@terminalmage

terminalmage Jun 25, 2018

Contributor

Just as a precaution, key.strip() should probably be salt.utils.stringutils.to_unicode(key.strip()), to prevent a UnicodeDecodeError if key happens to be a str type with non-ascii unicode in it.

Note, this is working only when data is a single string,
ready for print or dump to the log. Also, when the data is formatted
as "key: value" in YAML syntax.

This comment has been minimized.

Copy link
@terminalmage

terminalmage Jun 25, 2018

Contributor

Can we reword the above two paragraphs like this?

    This can be used for cases where keys in your roster file may contain
    sensitive data such as IP addresses, passwords, user names, etc.

    Note that this works only when ``data`` is a single string (i.e. when the
    data in the roster is formatted as ``key: value`` pairs in YAML syntax).
@isbm

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2018

@rallytime rallytime merged commit b44f0f1 into saltstack:develop Jun 25, 2018

7 of 12 checks passed

jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10970 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #20053 — ABORTED
Details
codeclimate 1 issue to fix
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #6000 — FAILURE
Details
WIP ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #26204 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #18254 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23928 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22889 — SUCCESS
Details
jenkins/pr/lint The lint job has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.