[foreman] Obfuscate http_proxy passwords. PR-3878 improvement

[pafernanr]

---

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

• Is the commit message split over multiple lines and hard-wrapped at 72 characters?
• Is the subject and message clear and concise?
• Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
• Does the commit contain a Signed-off-by: First Lastname [email protected]?
• Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
• Are all passwords or private data gathered by this PR obfuscated?

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo yum install -y dnf-plugins-core on RHEL 8
• sudo dnf install -y dnf-plugins-core on Fedora
• dnf copr enable packit/sosreport-sos-3881
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[jcastill]

@pafernanr can you squash these two commits into one please?

[jcastill]

Is is really necessary to remove this function? This obfuscates *_PROXY vars, and the change below doesn't do that, right?

[pafernanr]

do_paths_http_sub function replaces only one occurrence of r"(http(s)?://)\S+:\S+(@.*)", r"\1******:******\3").

In the foreman case there could be one or two occurrences on the same line. That's the reason to remove that function and improve the regex on foreman.py to capture one or multiple occurrences.

*_PROXY vars are being obfuscated at https://github.com/sosreport/sos/blob/main/sos/report/plugins/foreman.py#L299

[pafernanr]

hmmm. I think the description for do_paths_http_sub is not accurate:

• Obfuscate credentials in *_PROXY variables...

[pmoravec]

Yes, the description is a historic artifact from some first PR draft and should be updated.

[jcastill]

@pmoravec can you help with this?

[pmoravec]

Indeed, the do_paths_http_sub can match just one instance on a given line - that is why my #3878 already obfuscates the logfile(s) twice. Which is ridiculous and this PR aims to improve it.

[pafernanr]

If you agree, we might modify it to something like "Obfuscate BASIC_AUTH credentials in URLs for all files in the...".
But again that function suffer the same issue. Only one match is being replaced /o\

[pmoravec]

I created #3882 for that. Usually, there is just one password on a line, so usually the method is sufficient, I think.

Or are there instances where we need to obfuscate a secret on the same line multiple times? This is generic question, not specific to the do_paths_http_sub method. As the method just calls something internal that is used everywhere, incl. by do_path_regex_sub method.

(i.e. should we utilize repeatedly option of do_path_regex_sub?)

[pmoravec]

Works well for me, tested against all instances of the original PR reproducer.

[pmoravec]

Hold on merging this, @pafernanr should have an elegant improvement of do_paths_http_sub.

[pmoravec]

Cool! This makes my recent PR obsolete, I will drop it.

[arif-ali]

not for this PR, would we benefit on a test for multiple items on one line. I know I have the apt example in stagetwo already, which is a http_sub test for one item?

[pmoravec]

not for this PR, would we benefit on a test for multiple items on one line.

Yeah we should add some test for either proxy credentials or for multiple passwords on a line.

I know I have the apt example in stagetwo already, which is a http_sub test for one item?

I dont understand you here.

[arif-ali]

I know I have the apt example in stagetwo already, which is a http_sub test for one item?

I dont understand you here.

i.e. there is a stagetwo test for apt, that checks for http obfuscation, but that just has single http credentials on one line :)