Refs #28761 - Always set an empty REMOTE_USER for pulpcore API

[pdudley]

fixes a security flaw identified with GH-229

[wbclark]

thanks for the contribution! @jlsherrill or @ekohl can you confirm this is the right approach to resolve the issue?

[ekohl]

Code itself looks correct. Some more comments about the commit message and the organization. https://chris.beams.io/posts/git-commit/ has a lot of good suggestions about in general. We also have an issue that #nnn typically refers to Redmine issues where GH-nnn refers to Github issues.

[pdudley]

Fixed issue # - and thanks for the resource on commit messages!

[wbclark]

Code itself looks correct. Some more comments about the commit message and the organization. https://chris.beams.io/posts/git-commit/ has a lot of good suggestions about in general. We also have an issue that #nnn typically refers to Redmine issues where GH-nnn refers to Github issues.

Very helpful, thanks! Will also get this attached to the Redmine

[ekohl]

Very helpful, thanks! Will also get this attached to the Redmine

Normally what we do with these is use Refs #28761. That way, it is automatically linked. After merge it's also properly linked and our cherry pick scripts use that. We're unlikely to cherry pick this, but it's good to use the automation.

[ekohl]

My suggestion for a commit message (without hard line breaks as you'd normally add):

Refs #28761 - Always set an empty REMOTE_USER for pulpcore API

8be796383668528c3841d7378a2f3ef0dd6e86f7 started to pass the REMOTE_USER header to the pulpcore API when SSL authentication is present. Otherwise the REMOTE_USER header stays untouched. This allows attackers to impersonate any user. By always setting it to an empty string before optionally overriding, this security concern is addressed.

This links it back to our Redmine issue and in git log it explains what it's changing. It also explains why the change is made.

[ehelms]

Looks great, Thanks!

[ekohl]

In this particular case I didn't look too closely but I had intended it to use Refs #28654, the original issue that introduced the regression. As long as it's unreleased, that should be reused when making another fix. https://projects.theforeman.org/issues/28761 shouldn't have been created.