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

Do not add auth and cookie header when redirecting #465

Open
cdenneen opened this issue Nov 16, 2021 · 2 comments
Open

Do not add auth and cookie header when redirecting #465

cdenneen opened this issue Nov 16, 2021 · 2 comments

Comments

@cdenneen
Copy link

Since archive is implementing it's own HTTP client PuppetX::Bodeco::Util for http downloads then the fix for PUP-11188 (puppetlabs/puppet@9a8d3ef) 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
Copy link
Author

def generate_request(uri)
header = @cookie && { 'Cookie' => @cookie }
request = Net::HTTP::Get.new(uri.request_uri, header)
request.basic_auth(@username, @password) if @username && @password
request
end
def follow_redirect(uri, option = { limit: FOLLOW_LIMIT }, &block)
http_opts = if uri.scheme == 'https'
{ use_ssl: true,
verify_mode: (@insecure ? OpenSSL::SSL::VERIFY_NONE : OpenSSL::SSL::VERIFY_PEER) }
else
{ use_ssl: false }
end
Net::HTTP.start(uri.host, uri.port, @proxy_addr, @proxy_port, http_opts) do |http|
http.request(generate_request(uri)) do |response|
case response
when Net::HTTPSuccess
yield response
when Net::HTTPRedirection
limit = option[:limit] - 1
raise Puppet::Error, "Redirect limit exceeded, last url: #{uri}" if limit < 0
location = safe_escape(response['location'])
new_uri = URI(location)
new_uri = URI(uri.to_s + location) if new_uri.relative?
follow_redirect(new_uri, limit: limit, &block)
else
raise Puppet::Error, "HTTP Error Code #{response.code}\nURL: #{uri}\nContent:\n#{response.body}"
end
end
end
end

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant