Skip to content

Handle the blacklisted IP error in the Go middleware

David Fernandez requested to merge 220561-fix-invalid-project-path-response-in-go-middleware into master

🌲 Context

The go middleware adds support of the go-get=1 parameter. This parameter is used when a user $ go get, This middleware will reply the proper text response accordingly: in particular, the response will have the project path so that $ go can clone the repository.

The problem is when authenticated users try to access projects/groups that don't exist: the middleware will reply back with simple_path which is defined as the first two path segments. $ go get will then build the repository url but since the project/group doesn't exist, the repository url will be wrong and $ git clone will fail.

The above scenario is executed by scripts that run that on a loop. So basically, gitlab.com will receive n requests with wrong paths on this middleware. This leads to the core issue of #220561 (closed): authenticated user are rate limited. These scripts will thus trigger a GitLab::Auth::IpBlacklisted error.

Since this error is not handled by the middleware, this will end up in a 500 error response and this counts towards our production SLAs.

🔬 What does this MR do?

🖼 Screenshots

Note that the project used in these examples doesn't exist. In other words, the project path doesn't lead anywhere.

Before this MR

$ go get -v gdk.test/group/subgroup/noproject/subpackage
package gdk.test/group/subgroup/noproject/subpackage: unrecognized import path "gdk.test/group/subgroup/noproject/subpackage": reading https://gdk.test/group/subgroup/noproject/subpackage?go-get=1: 500 Internal Server Error
	server response:
	Gitlab::Auth::IpBlacklisted at group/subgroup/noproject/subpackage
	==================================================================

-> Notice the 500 Internal Server Error

With this MR

$ go get -v gdk.test/group/subgroup/noproject/subpackage
package gdk.test/group/subgroup/noproject/subpackage: unrecognized import path "gdk.test/group/subgroup/noproject/subpackage": reading https://gdk.test/group/subgroup/noproject/subpackage?go-get=1: 403 Forbidden

-> we have now 403 Forbidden

🔮 Other considerations

To be complete here, I don't fully understand this explanation. I also don't get why we reply a successful response when the project is not found. My guess is that unauthenticated access to private projects are replied with the simple_path so that $ go get can build the repository path and $ git clone will use its own saved credentials against the private repository which works if the user has the git access to the private repository.

To me, if the project is not found, we should return bad request and that's it. If users want to use the go get feature on private projects, they should properly set up their credentials first.

Thoughts for a follow up MR I guess.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by David Fernandez

Merge request reports