Skip to content

Fix Gitlab::HTTP_V2 allowed_internal_uris https scheme

Moaz Khalifa requested to merge 442928-fix-Gitlab-HTTP_V2-allowed_internal_uris-scheme into master

What does this MR do and why?

A customer reported that they are facing an error trying to publish their NuGet packages to the NuGet Repository. After investigation, it turned out that the error is because of the public_url validation on the project_url field in the Packages::Nuget::Metadatum model.

The PublicUrlValidator inherets from AddressableUrlValidator which uses Gitlab::HTTP_V2::UrlBlocker to validate the public URLs.

The project_url value is the domain of the self-managed GitLab instance, which should be valid since it's an internal URI. The validation of the internal URIs is done in this method in Gitlab::HTTP_V2::UrlBlocker.

However, the validation is failing. After more investigation, it turned out that Gitlab::HTTP_V2.configuration.allowed_internal_uris is returning the allowed internal URLs, but without taking into consideration the correct scheme of the http URL.

Gitlab::HTTP_V2.configuration.allowed_internal_uris are constructed in this initializer. However, the used URI::HTTP class isn't returning https scheme even if the scheme passed to it is https.

URI::HTTP.build(scheme: 'https', host: 'git.leetdevelopers.com', port: 443)
#<URI::HTTP http://git.leetdevelopers.com:443>

Notice that even with https scheme, URI::HTTP.build will always return a URI::HTTP with http scheme.

To fix this, we need to use URI::HTTPS instead:

URI::HTTPS.build(scheme: 'https', host: 'git.leetdevelopers.com', port: 443)
#<URI::HTTPS https://git.leetdevelopers.com>

By having the correct scheme of the internal URI, this method in Gitlab::HTTP_V2::UrlBlocker will pass the validation of the URL. Otherwise, if the schemes are not matching, the validation fails.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

In rails console, run the following:

# check the allowed internal URIs
Gitlab::HTTP_V2.configuration.allowed_internal_uris
=> [#<URI::HTTP http://gdk.test:3000>, #<URI::Generic ssh://gdk.test:2222>]

# now we need to stub the protocol setting of gitlab config to be `https`
Gitlab.config.gitlab.protocol = 'https'
=> "https"

# we need to load the initialozer so it can catch the new protocol value:
load Rails.root.join('config/initializers/7_gitlab_http.rb')
=> true

# check the allowed internal URIs again, now we see that the URI::HTTPS has https scheme
Gitlab::HTTP_V2.configuration.allowed_internal_uris
=> [#<URI::HTTPS https://gdk.test:3000>, #<URI::Generic ssh://gdk.test:2222>]

Repeating the above on master wouldn't result in the https scheme in the last allowed internal URIs check

Related to #442928 (closed)

Edited by Moaz Khalifa

Merge request reports