Create Faraday adapter which respects GitLab's security settings
Problem to solve
GitLab accepts user URLs for a variety of reasons. When we can, we use Gitlab::HTTP
which is secure by default and protects against SSRF and DNS Rebind attacks 1.
In some places, we use Faraday instead of Gitlab::HTTP
. We also use third-party ruby gems which depend on Faraday. This creates a potential for vulnerabilities, and these vulnerabilities have been exploitable in the past.
The problem is: how can we keep using Faraday / third party gems, while also protecting ourselves against attacks?
Note: There are no exploitable vulnerabilities; this is a defense in depth measure.
Proposal
-
(In-progress) Create a Faraday adapter which uses Gitlab::URLBlocker
to resolve a DNS record to an IP address, and validate that the IP address is allowed- Using
UrlBlocker
andGitlab::CurrentSettings.
we can honour the network settings defined by admins, incl. "allow localhost", "allow local network", and "ip allow lists" 2 - MR here: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2298
- Using
-
Review by staff engineer to identify far-reaching impacts - e.g. Elastic, Prometheus, and other places an administrator might have configured to point at local network resources
- To the best of my knowledge there aren't any
-
Transfer ownership to a group who can create a rollout issue, monitor during deployment, own the feature flag, etc - AppSec contributions are treated as Community contribution, so I can't own this :/
Our own code using Faraday
Code | URL | Comments |
---|---|---|
ObjectStorage |
https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/uploaders/object_storage.rb#L286-288 | |
ContainerRegistry::BaseClient |
https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/container_registry/base_client.rb | Seems like Container Registry base URL is defined as a static config variable, so no risk there. |
QA's Environments::Base
|
https://gitlab.com/gitlab-org/gitlab/-/blob/master/qa/contracts/provider/environments/base.rb | |
Tooling::MergeRequest |
https://gitlab.com/gitlab-org/gitlab/-/blob/master/tooling/merge_request.rb |
Gems using Faraday
Gem | Comments |
---|---|
acme-client |
|
asana |
|
azure-storage-common |
/!\ Also depends on net-http-persistent
|
danger |
/!\ Also depends on faraday-http-cache
|
elasticsearch-transport |
/!\ Do we need to allow code-based overrides of configured network settings? Elastic is frequently hosted on local networks |
google-cloud-env |
|
googleauth |
|
google-api-client -> signet
|
|
graphlient |
https://github.com/ashkan18/graphlient#swapping-the-http-stack |
ms_rest |
|
ms_rest_azure |
|
oauth2 |
Big impact (see below) |
octokit |
|
sawyer |
|
sentry-raven |
|
signet |
|
test_file_finder |
|
omniauth-atlassian-oauth2 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-auth0 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-authentiq -> ominiauth-oauth2 -> oauth2
|
|
omniauth-azure-activedirectory-v2 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-azure-oauth2 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-dingtalk-oauth2 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-facebook -> ominiauth-oauth2 -> oauth2
|
|
omniauth-github -> ominiauth-oauth2 -> oauth2
|
|
omniauth-gitlab -> ominiauth-oauth2 -> oauth2
|
|
omniauth-google-oauth2 -> ominiauth-oauth2 -> oauth2
|
|
omniauth-oauth2-generic -> ominiauth-oauth2 -> oauth2
|
|
omniauth-salesforce -> ominiauth-oauth2 -> oauth2
|
As this issue does not relate to exploitable vulnerabilities, and the class of vulnerability is widely known, and this is defense in depth, I'll make this public.
Edited by Nick Malcolm