Skip to content

Verify SSH cert namespace on initial requests only

Igor Drozdov requested to merge id-fix-gitaly-internal-requests-for-ssh-certificates into master

What does this MR do and why?

Related epic: Add support for git access control via SSH cert... (&10662 - closed)

When a user is utilizing SSH certificates for authentication/authorization in order to push into repo, the following sequence of actions is performed:

sequenceDiagram
    participant Git on client
    participant GitLab Shell
    participant Gitaly
    participant Rails

    Note left of Git on client: git push
    Git on client->>+GitLab Shell: ssh git-receive-pack ...
    GitLab Shell->>+Rails: GET /internal/api/authorized_certs?key=fingerprint-of-signing-key&user_identity=username-or-primary-email
    Note right of Rails: Lookup key ID
    Rails-->>-GitLab Shell: 200 OK, command="gitlab-shell upload-pack key_id=1"
    GitLab Shell->>+Rails: GET /internal/api/allowed?action=upload_pack&key_id=1&changes=_any&namespace=namespace
    Note right of Rails: Auth check
    Rails-->>-GitLab Shell: 200 OK, { gitaly: ... }
    GitLab Shell->>+Gitaly: SSHService.SSHReceivePack gRPC request
    Gitaly->>+Rails: GET /internal/api/allowed?action=upload_pack&key_id=1&changes=changes-list
    Note right of Rails: Auth check/Push rules check
    Rails-->>-Gitaly: 200 OK
    Note over Git on client,Gitaly: Bidirectional communication between Git client and Gitaly
    Gitaly -->>-GitLab Shell: SSHService.SSHReceivePack gRPC response
    GitLab Shell-->>-Git on client: ssh git-receive-pack response

When Gitlab Rails checks access to a project in a namespace, it relies on the namespace param sent by Gitlab Shell. However, Gitaly doesn't have the context of SSH certificates, so it doesn't and shouldn't send namespace param. It resulted in a bug when the first /allowed request succeeded, while the second failed.

Let's check the namespace only for the first request. We can identify the first request by the changes params. When Gitlab Shell/Workhorse call this endpoint, the changes param equals _any, while Gitaly sends the actual changes list like:

0000000000000000000000000000000000000000 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/mybranch

Merge request reports