voxpupuli / puppet-archive

Compressed archive file download and extraction with native types/providers for Windows and Unix
https://forge.puppet.com/puppet/archive
Apache License 2.0
60 stars 176 forks source link

Do not add auth and cookie header when redirecting #465

Open cdenneen opened 3 years ago

cdenneen commented 3 years ago

Since archive is implementing it's own HTTP client PuppetX::Bodeco::Util for http downloads then the fix for PUP-11188 (https://github.com/puppetlabs/puppet/commit/9a8d3ef017cf63ce0f848ec64394f7bad287e825) needs to be implemented here as well or need to move away from this library in favor of the default Puppet::Network::HTTP.

The underlying problem in an example is JFrog Cloud will redirect authenticated header/cookie information from the session to the s3 bucket for download. The s3 bucket only needs the Signature that JFrog will provide based on the storage configuration it has not the session/auth from the client -> JFrog part.

Passing this info on from the client auth is potentially a security risk but causes the client to fail to download due to more than one auth being sent:

Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified

cdenneen commented 3 years ago

https://github.com/voxpupuli/puppet-archive/blob/2f4cb0fb9e557da6e0e63d631045d2d4c8fec730/lib/puppet_x/bodeco/util.rb#L86-L118

since generate_request is request.basic_auth(@username, @password) if @username && @password regardless if the initial request or a redirect the username/password is getting passed on to the redirection which leads to potential leak of credentials but larger issue is any redirect to something like an s3 bucket will have a signature in the redirect and can't have additional basic auth or causes the Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified error.

cdenneen commented 3 years ago

I know this code hasn't been touched since initially created by @nanliu so not sure if anyone wants to tackle this.