Enhance security by removing Authorization header on HTTP redirects

[otegami]

When redirects occur, the Authorization header containing authorization information is transferred to the redirect destination host. This can expose sensitive credentials to unintended hosts, posing a security risk.

I will introduce a case where the Authorization header is unintentionally transferred during redirection.

Case

The issue occurs when downloading GitHub Actions Artifacts using the GitHub REST API.

• https://docs.github.com/ja/rest/actions/artifacts#download-an-artifact

The process is as follows.

1. Authorize using the Authorization header.
2. Redirect to a URL containing a Shared Access Signature (SAS) for downloading the artifact.
3. Download the artifact.

During the redirect in step 2, the Authorization header from step 1 is retained and transferred to the redirect destination.
This results in an authorization error due to the redirection destination misinterpreting the Authorization header content as the SAS.

Reproduce code

Here is a Ruby code example that demonstrates the problem, resulting in a 403 error because the Authorization header does not match the expected signature format.

require 'open-uri'

uri = ENV['ARTIFACT_URL']
access_token = ENV['GITHUB_ACCESS_TOKEN']

URI(uri).open("Authorization" => "token #{access_token}")

$ ARTIFACT_URL="https://api.github.com/repos/:owner/sandbox-github-actions-artifacts-v4/actions/artifacts/:artifact_id/zip" \
 GITHUB_ACCESS_TOKEN="your_access_token" \
 ruby download_artifact.rb

/home/kodama/.rbenv/versions/3.3.2/lib/ruby/3.3.0/open-uri.rb:376:in `open_http': 403 Server failed to authenticate the request. Make sure the value of the Authorization header is formed correctly, including the signature. (OpenURI::HTTPError)

Workaround

When redirecting without transferring the Authorization header content, the download succeeds without errors.
This means the Authorization header is retained during redirects.

require 'open-uri'

uri = ENV['ARTIFACT_URL']
access_token = ENV['GITHUB_ACCESS_TOKEN']

headers = {
 "Authorization" => "token #{access_token}",
 redirect: false
}

loop do
 begin
   URI.open(uri, headers)
   break
 rescue OpenURI::HTTPRedirect => redirect
   headers.delete("Authorization")
   pp uri = redirect.uri
 end
end

$ ARTIFACT_URL="https://api.github.com/repos/:owner/sandbox-github-actions-artifacts-v4/actions/artifacts/:artifact_id/zip" \
 GITHUB_ACCESS_TOKEN="your_access_token" \
 ruby download_artifact.rb

#<URI::HTTPS https://productionresultssa11.blob.core.windows.net/actions-results/xxx>

Expected Improvement

We aim to improve security by removing the Authorization header during redirects if necessary, preventing its inadvertent transfer to the redirect destination host.

The Fetch specification recommends removing the Authorization header on cross-origin redirects.

• https://fetch.spec.whatwg.org/#http-redirect-fetch

Supplemental Information

Examples of software or specifications that do not transfer the Authorization header on redirect:

Curl

Curl does not transfer the Authorization header on redirects and this issue is reported as a CVE.

• curl/curl@af32cd3
• https://curl.se/docs/CVE-2018-1000007.html

GO

Go removes the Authorization header when redirecting to different hosts but retains it if the redirect is to the same host.

• https://github.com/golang/go/blob/b3040679ad0eccaaadb825ed8c0704086ecc23eb/src/net/http/client.go#L41-L49
• https://go-review.googlesource.com/c/go/+/28930

Proposed Solutions

To address the Expected Improvement mentioned above and to ensure future flexibility for handling headers beyond just the Authorization header, we propose introducing a customizable solution that allows users to define their own header-handling rules. This approach ensures that users can manage which headers are retained or removed during redirects, providing a flexible and secure implementation.

To provide this customization feature, we introduce the request_specific_fields option. This option offers two methods for handling headers as follows:

• Specify headers only for the initial request.
• Specify headers dynamically for each request during the request lifecycle.

Specify headers for the initial request (Using a Hash)

Users can specify request specific headers for the initial request via a Hash.
These headers will only be applied to the first request and automatically removed on redirects.

URI.open("http://...", request_specific_fields: { "Authorization" => "token dummy" }) {|f| ... }

Specify headers for each request (Using a Proc)

For more dynamic control, users can define a Proc that generates headers based on the URL of each request.
The Proc is evaluated for every request, including those made during redirects, allowing for flexible header management throughout the request lifecycle.

URI.open("http://...", request_specific_fields: lambda { |uri|
  if uri.host == "example.com"
    { "Authorization" => "token dummy" }
  else
    { "Authorization" => nil }
  end
}) {|f| ... }

By implementing the request_specific_fields option in this way, we provide flexibility without hardcoding sensitive fields like the "Authorization" header. Users maintain full control over which headers are sent with each request, allowing them to exclude sensitive headers during redirects if desired. This solution keeps OpenURI secure by design while offering customizable behavior when needed.

[Watson1978]

Rebased with master branch

[otegami]

@akr Thank you very much for your maintenance efforts. When you have time, could you review this PR?

[otegami]

@akr Sorry to bother you while you're busy. If there is any missing information or unclear parts that would help in reviewing this PR, please let me know. I’m more than happy to prepare for anything needed to assist with the review.

[akr]

I understand the motivation. We need to customize header fields for each request.

However, I feel that hard coding the field name "Authorization" is not a good idea.

How about adding a keyword argument to specify request-specific header fields?

uri.open(request_specific_fields: { "Authorization" => "token #{access_token}")

The hash specifies the header fields only before redirecting.

Note that the argument can be extended with procedures.

uri.open(request_specific_fields: -> (uri) do {...} end)

The procedure is called for each request and returns additional header fields for the request.

[otegami]

@akr Thank you for explaining the proposed method to customize header fields for each request using a hash or a block. I think the policy to allow customizable headers is a great approach. Could you help me clarify the specification about the following points?

Specify a Hash

When we pass a hash as request_specific_fields, my understanding is that this configuration is intended only for the initial request and does not persist through redirects. Is it correct to assume that any headers specified, including but not limited to the "Authorization" header, are automatically removed by the library for any subsequent requests?

Specify a Block

Regarding the use of a block, does the library evaluate this block with the new URL for each request, allowing for dynamic header customization at every step of the request process, including during multiple redirections?

Please let me know if my understanding is incorrect. Thank you for helping me.

---

Additionally, I have a suggestion regarding the default behavior for the "Authorization" header. Could you share your thoughts on this?

Suggestion about the Default Behavior for "Authorization" Header

Considering that users might not always remember to specify the "Authorization" header explicitly in request_specific_fields, do you think it would be beneficial to have a default implementation that automatically excludes this header from subsequent requests? This could prevent potential security issues by ensuring sensitive headers are not inadvertently sent across redirects unless explicitly included by the user. If a user wishes to customize this behavior further, they could override this default implementation by specifying request_specific_fields.

[akr]

Your understanding of "Specify a Hash" and "Specify a Block" is correct.
("Block" is not an appropriate word here. "Proc object" would be better, though.)

I don't accept 'Suggestion about the Default Behavior for "Authorization" Header'.
It is the responsibility of the users.

If such a mechanism is introduced in open-uri, someone may report that some fields (such as "Cookie" field) are not treated like "Authorization" field as a security problem.
Since I don't know all HTTP headers in the future, I expect it to occur someday.
I don't want to receive such a security problem report.
Thus, I don't want to introduce the mechanism.

[otegami]

Thank you for clarifying the specification. I also understand your decision not to introduce the default behavior for the Authorization header.

With that in mind, I will proceed to implement request_specific_fields using specify a Hash and specify a Proc object based on the specifications we discussed.

[otegami]

@akr
Thank you for your guidance on the header customization using a Hash and a Proc object. I have implemented the request_specific_fields option.

Could you please review the implementation to ensure it meets our specifications?

[akr]

I think "format" is not an appropriate word here.
Also, inspect should be called for the option value to show it for users.

So, how about making the exception message
"Invalid request_specific_fields: #{options[:request_specific_fields].inspect}"?

[otegami]

fix f89ce51:
Thanks you so much. I completely agree with your suggestion. Adding inspect will indeed provide users with more detailed information when debugging.
I also added the word option to the message to maintain consistency with other option-related error messages.