Geo: Improve security of authentication for API endpoints
Currently, all GitLab Geo API calls rely on a system hook token sent in the X-Gitlab-Token
header of the API call. A similar pattern shows up everywhere in GitLab API calls (e.g. via the query parameter private_token
). The Geo API validation works as the following:
- The current GitLab node looks up its
GeoNode
entry based on the URL/port information of the instance - This
GeoNode
entry has a system hook entry, which has a static API token associated with it - The API call does a string comparison between the value of the
X-Gitlab-Token
and the value stored in the database
However, especially since we are going to make it possible for Geo to copy files via https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1237, I think we need to start moving away from this pattern because it makes the surface attack vector quite large. We are sending static passwords over the network, and we should never do this. A malicious attacker just needs to obtain that single token to be able to access files stored in the system.
A better approach would be to use an HMAC approach, such as ones used in the JSON Web Tokens (JWT) standard.
For Geo, I prototyped a JWT-based approach in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1237. However, the primary key is currently the static API token from the primary GeoNode
. This is not that secure because the value is exchanged over the network in a number of places:
- via PostgreSQL replication in an unencrypted
string
field - via the
X-Gitlab-Token
header in all GitLab Geo API calls handled by the primary node
Using a shared secret that is replicated via PostgeSQL is convenient, but I doubt it is considered a security best practice.
Looking at AWS S3 for inspiration, I think in an ideal world, we would have the primary generate credentials that need to be populated by a system admin separately. S3 uses two items:
- A unique access ID
- Secret access key
I propose we do something similar but use JWT instead. Putting the credentials as a separate step would add yet another step to the already complicated Geo installation, but I think we need to automate this in any case (see #1664 (closed)).
@jacobvosmaer-gitlab, @briann, @brodock, what do you think?
Adding @to1ne and @oswaldo because they are working on breaking changes for API v4. I also suggest we should move to an HMAC-based approach in v5.