[BUG] Regression caused by PR#62641

[vzhestkov]

Description PR https://github.com/saltstack/salt/pull/62641 caused the regression for some particular cases, and could be also harmful for the other cases.

In case if the password is used for the authentication to remote SSH system and the password is not really complex in case of matching any part of the output from the client side any occurance of the password is replaced with ******* not just for logging, but also inside the resulting response.

So there are 2 most problematic cases:

1. If the password used for the user on the remote system is password or Password this word is wiped of of the prompt to enter password in case if jump host is used for ssh and salt-ssh processing got stuck on waiting for extra output from the client as the promt for the password is not detected in this case. On my tests it happens with keyboard interactive authentication for ssh client. Can be reproduced with such roster file (but it could be default settings on ssh server side):

   alma8.tf.local:
   host: alma8.tf.local
   user: root
   passwd: Password
   ssh_options: ["PreferredAuthentications=keyboard-interactive"]

2. If the password is matching any part of the output, this part will be sanitized. As the result is the broken data or even the broken JSON if the matching part is a part of JSON output. For example in case of having linux as a pasword we could have such output:

   # salt-ssh --roster=flat --roster-file=/tmp/ssh/alma8.sls -i alma8.tf.local grains.item kernelparams
   alma8.tf.local:
   ----------
   kernelparams:
       |_
         - BOOT_IMAGE
         - (hd0,msdos1)/vmlinuz-4.18.0-477.10.1.el8_8.x86_64
       |_
         - root
         - /dev/mapper/alma******-root
       |_
         - ro
         - None
       |_
         - crashkernel
         - auto
       |_
         - resume
         - /dev/mapper/alma******-swap
       |_
         - rd.lvm.lv
         - alma******/root
       |_
         - rd.lvm.lv
         - alma******/swap
       |_
         - rhgb
         - None
       |_
         - quiet
         - None

Setup (Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

• [x] on-prem machine
• [x] VM (Virtualbox, KVM, etc. please specify)
• [x] VM running on a cloud service, please be explicit and add details
• [ ] container (Kubernetes, Docker, containerd, etc. please specify)
• [ ] or a combination, please be explicit
• [ ] jails if it is FreeBSD
• [x] classic packaging
• [x] onedir packaging
• [x] used bootstrap to install

Steps to Reproduce the behavior Just require remote ssh system with password set to something lilke 'Password', 'password' or even matching any part of the output which can be produced with the specified call.

One of the examples of the roster file used for testing:

alma8.tf.local:
  host: alma8.tf.local
  user: root
  passwd: Password
  ssh_options: ["PreferredAuthentications=keyboard-interactive"]

Expected behavior No any stucks on processing salt-ssh events and relevant output with no wiping some data which is not really related to the password itself.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3006.0 Python Version: Python: 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] Dependency Versions: cffi: 1.13.2 cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 2.10.1 libgit2: 1.3.0 looseversion: 1.0.2 M2Crypto: 0.38.0 Mako: Not Installed msgpack: 0.5.6 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 20.3 pycparser: 2.17 pycrypto: 3.9.0 pycryptodome: Not Installed pygit2: 1.7.0 python-gnupg: Not Installed PyYAML: 5.4.1 PyZMQ: 17.1.2 relenv: Not Installed smmap: Not Installed timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.2.3 System Versions: dist: opensuse-leap 15.5 locale: UTF-8 machine: x86_64 release: 5.14.21-150500.55.31-default system: Linux version: openSUSE Leap 15.5 ```

Additional context The newer versions seems also affected.

[welcome[bot]]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[vzhestkov]

@Ch3LL could you please take a look on this issue?

[twangboy]

This may be a documentation issue. If the password matches any output from the command it will be replaced with "*" to avoid leaking passwords in the logs. There is no way to guess what is a password in the output and what isn't. This would also be an incentive to use more complex passwords. Even something like P@ssw0rd would avoid this edge case.

[vzhestkov]

I think it's not just a documentation issue, looks like the data is wiping too early and as I specified it could potentially wipe any part of the response from json if it's matching the password just by accident, in this case we can't really say the customers not to use the passwords which could potentially match any part of the output. Check the example with grains I put in the original description. For me it sounds too weird to ask the users to take care of it themselves. And especially because it became a problem after the certain fix.

[agraul]

By replacing all strings that match the password, a known string that is replaced shows what the password is. In other words, the current situation does not prevent leaking the password. I agree it reduces the likelihood.

The bigger problem is that salt-ssh tries to find the password prompt in the filtered ssh output. When someone uses a part of the password prompt string as their password, salt-ssh is unable to "enter" the password.

[dwoz]

I agree this is not a documentation issue. We should not be doing a string replacement on the output and we should not be logging the output. If we absolutely need to log the output for some reason it should be to debug which we already have documented could log sensitive data.