Merge branch 'user-argument-2/upstream' into 'master'

Add support for ssh certificates (internal API)

Closes #34572

See merge request gitlab-org/gitlab-ce!19911

GITLAB_SHELL_VERSION

-7.2.0
+8.0.0

changelogs/unreleased/34572-ssh-certificates.yml

+---
+title: Add support for SSH certificate authentication
+merge_request: 19911
+author: Ævar Arnfjörð Bjarmason
+type: added

doc/administration/operations/fast_ssh_key_lookup.md

+# Consider using SSH certificates instead of, or in addition to this
+
+This document describes a drop-in replacement for the
+`authorized_keys` file for normal (non-deploy key) users. Consider
+using [ssh certificates](ssh_certificates.md), they are even faster,
+but are not is not a drop-in replacement.
+
 # Fast lookup of authorized SSH keys in the database
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/1631) in 

doc/administration/operations/index.md

@@ -14,4 +14,7 @@ that to prioritize important jobs.
 - [Sidekiq MemoryKiller](sidekiq_memory_killer.md): Configure Sidekiq MemoryKiller
 to restart Sidekiq.
 - [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer.
-- [Speed up SSH operations](fast_ssh_key_lookup.md): Authorize SSH users via a fast, indexed lookup to the GitLab database.
+- Speed up SSH operations by [Authorizing SSH users via a fast,
+indexed lookup to the GitLab database](fast_ssh_key_lookup.md), and/or
+by [doing away with user SSH keys stored on GitLab entirely in favor
+of SSH certificates](ssh_certificates.md).

doc/administration/operations/ssh_certificates.md

+# User lookup via OpenSSH's AuthorizedPrincipalsCommand
+
+> [Available in](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19911) GitLab
+> Community Edition 11.2.
+
+GitLab's default SSH authentication requires users to upload their ssh
+public keys before they can use the SSH transport.
+
+In centralized (e.g. corporate) environments this can be a hassle
+operationally, particularly if the SSH keys are temporary keys issued
+to the user, e.g. ones that expire 24 hours after issuing.
+
+In such setups some external automated process is needed to constantly
+upload the new keys to GitLab.
+
+> **Warning:** OpenSSH version 6.9+ is required because that version
+introduced the `AuthorizedPrincipalsCommand` configuration option. If
+using CentOS 6, you can [follow these
+instructions](fast_ssh_key_lookup.html#compiling-a-custom-version-of-openssh-for-centos-6)
+to compile an up-to-date version.
+
+## Why use OpenSSH certificates?
+
+By using OpenSSH certificates all the information about what user on
+GitLab owns the key is encoded in the key itself, and OpenSSH itself
+guarantees that users can't fake this, since they'd need to have
+access to the private CA signing key.
+
+When correctly set up, this does away with the requirement of
+uploading user SSH keys to GitLab entirely.
+
+## Setting up SSH certificate lookup via GitLab Shell
+
+How to fully setup SSH certificates is outside the scope of this
+document. See [OpenSSH's
+PROTOCOL.certkeys](https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?annotate=HEAD)
+for how it works, and e.g. [RedHat's documentation about
+it](https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/sec-using_openssh_certificate_authentication).
+
+We assume that you already have SSH certificates set up, and have
+added the `TrustedUserCAKeys` of your CA to your `sshd_config`, e.g.:
+
+```
+TrustedUserCAKeys /etc/security/mycompany_user_ca.pub
+```
+
+Usually `TrustedUserCAKeys` would not be scoped under a `Match User
+git` in such a setup, since it would also be used for system logins to
+the GitLab server itself, but your setup may vary. If the CA is only
+used for GitLab consider putting this in the `Match User git` section
+(described below).
+
+The SSH certificates being issued by that CA **MUST** have a "key id"
+corresponding to that user's username on GitLab, e.g. (some output
+omitted for brevity):
+
+```
+$ ssh-add -L | grep cert | ssh-keygen -L -f -
+(stdin):1:
+        Type: ssh-rsa-cert-v01@openssh.com user certificate
+        Public key: RSA-CERT SHA256:[...]
+        Signing CA: RSA SHA256:[...]
+        Key ID: "aearnfjord"
+        Serial: 8289829611021396489
+        Valid: from 2018-07-18T09:49:00 to 2018-07-19T09:50:34
+        Principals:
+                sshUsers
+                [...]
+        [...]
+```
+
+Technically that's not strictly true, e.g. it could be
+`prod-aearnfjord` if it's a SSH certificate you'd normally log in to
+servers as the `prod-aearnfjord` user, but then you must specify your
+own `AuthorizedPrincipalsCommand` to do that mapping instead of using
+our provided default.
+
+The important part is that the `AuthorizedPrincipalsCommand` must be
+able to map from the "key id" to a GitLab username in some way, the
+default command we ship assumes there's a 1=1 mapping between the two,
+since the whole point of this is to allow us to extract a GitLab
+username from the key itself, instead of relying on something like the
+default public key to username mapping.
+
+Then, in your `sshd_config` set up `AuthorizedPrincipalsCommand` for
+the `git` user. Hopefully you can use the default one shipped with
+GitLab:
+
+```
+Match User git
+    AuthorizedPrincipalsCommandUser root
+    AuthorizedPrincipalsCommand /opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell-authorized-principals-check %i sshUsers
+```
+
+This command will emit output that looks something like:
+
+```
+command="/opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL}
+```
+
+Where `{KEY_ID}` is the `%i` argument passed to the script
+(e.g. `aeanfjord`), and `{PRINCIPAL}` is the principal passed to it
+(e.g. `sshUsers`).
+
+You will need to customize the `sshUsers` part of that. It should be
+some principal that's guaranteed to be part of the key for all users
+who can log in to GitLab, or you must provide a list of principals,
+one of which is going to be present for the user, e.g.:
+
+```
+    [...]
+    AuthorizedPrincipalsCommand /opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell-authorized-principals-check %i sshUsers windowsUsers
+```
+
+## Principals and security
+
+You can supply as many principals as you want, these will be turned
+into multiple lines of `authorized_keys` output, as described in the
+`AuthorizedPrincipalsFile` documentation in `sshd_config(5)`.
+
+Normally when using the `AuthorizedKeysCommand` with OpenSSH the
+principal is some "group" that's allowed to log into that
+server. However with GitLab it's only used to appease OpenSSH's
+requirement for it, we effectively only care about the "key id" being
+correct. Once that's extracted GitLab will enforce its own ACLs for
+that user (e.g. what projects the user can access).
+
+So it's OK to e.g. be overly generous in what you accept, since if the
+user e.g. has no access to GitLab at all it'll just error out with a
+message about this being an invalid user.
+
+## Interaction with the `authorized_keys` file
+
+SSH certificates can be used in conjunction with the `authorized_keys`
+file, and if setup as configured above the `authorized_keys` file will
+still serve as a fallback.
+
+This is because if the `AuthorizedPrincipalsCommand` can't
+authenticate the user, OpenSSH will fall back on
+`~/.ssh/authorized_keys` (or the `AuthorizedKeysCommand`).
+
+Therefore there may still be a reason to use the ["Fast lookup of
+authorized SSH keys in the database"](fast_ssh_key_lookup.html) method
+in conjunction with this. Since you'll be using SSH certificates for
+all your normal users, and relying on the `~/.ssh/authorized_keys`
+fallback for deploy keys, if you make use of those.
+
+But you may find that there's no reason to do that, since all your
+normal users will use the fast `AuthorizedPrincipalsCommand` path, and
+only automated deployment key access will fall back on
+`~/.ssh/authorized_keys`, or that you have a lot more keys for normal
+users (especially if they're renewed) than you have deploy keys.
+
+## Other security caveats
+
+Users can still bypass SSH certificate authentication by manually
+uploading an SSH public key to their profile, relying on the
+`~/.ssh/authorized_keys` fallback to authenticate it. There's
+currently no feature to prevent this, [but there's an open request for
+adding it](https://gitlab.com/gitlab-org/gitlab-ce/issues/49218).
+
+Such a restriction can currently be hacked in by e.g. providing a
+custom `AuthorizedKeysCommand` which checks if the discovered key-ID
+returned from `gitlab-shell-authorized-keys-check` is a deploy key or
+not (all non-deploy keys should be refused).

lib/api/internal.rb

@@ -11,7 +11,8 @@ module API
       #
       # Params:
       #   key_id - ssh key id for Git over SSH
-      #   user_id - user id for Git over HTTP
+      #   user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode
+      #   username - user name for Git over SSH in keyless SSH cert mode
       #   protocol - Git access protocol being used, e.g. HTTP or SSH
       #   project - project full_path (not path on disk)
       #   action - git action (git-upload-pack or git-receive-pack)
@@ -28,6 +29,8 @@ module API
             Key.find_by(id: params[:key_id])
           elsif params[:user_id]
             User.find_by(id: params[:user_id])
+          elsif params[:username]
+            User.find_by_username(params[:username])
           end
 
         protocol = params[:protocol]
@@ -58,6 +61,7 @@ module API
         {
           status: true,
           gl_repository: gl_repository,
+          gl_id: Gitlab::GlId.gl_id(user),
           gl_username: user&.username,
 
           # This repository_path is a bogus value but gitlab-shell still requires
@@ -71,10 +75,17 @@ module API
       post "/lfs_authenticate" do
         status 200
 
-        key = Key.find(params[:key_id])
-        key.update_last_used_at
+        if params[:key_id]
+          actor = Key.find(params[:key_id])
+          actor.update_last_used_at
+        elsif params[:user_id]
+          actor = User.find_by(id: params[:user_id])
+          raise ActiveRecord::RecordNotFound.new("No such user id!") unless actor
+        else
+          raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!")
+        end
 
-        token_handler = Gitlab::LfsToken.new(key)
+        token_handler = Gitlab::LfsToken.new(actor)
 
         {
           username: token_handler.actor_name,
@@ -100,7 +111,7 @@ module API
       end
 
       #
-      # Discover user by ssh key or user id
+      # Discover user by ssh key, user id or username
       #
       get "/discover" do
         if params[:key_id]
@@ -108,6 +119,8 @@ module API
           user = key.user
         elsif params[:user_id]
           user = User.find_by(id: params[:user_id])
+        elsif params[:username]
+          user = User.find_by(username: params[:username])
         end
 
         present user, with: Entities::UserSafe
@@ -141,22 +154,30 @@ module API
       post '/two_factor_recovery_codes' do
         status 200
 
-        key = Key.find_by(id: params[:key_id])
+        if params[:key_id]
+          key = Key.find_by(id: params[:key_id])
 
-        if key
-          key.update_last_used_at
-        else
-          break { 'success' => false, 'message' => 'Could not find the given key' }
-        end
+          if key
+            key.update_last_used_at
+          else
+            break { 'success' => false, 'message' => 'Could not find the given key' }
+          end
 
-        if key.is_a?(DeployKey)
-          break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' }
-        end
+          if key.is_a?(DeployKey)
+            break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' }
+          end
+
+          user = key.user
 
-        user = key.user
+          unless user
+            break { success: false, message: 'Could not find a user for the given key' }
+          end
+        elsif params[:user_id]
+          user = User.find_by(id: params[:user_id])
 
-        unless user
-          break { success: false, message: 'Could not find a user for the given key' }
+          unless user
+            break { success: false, message: 'Could not find the given user' }
+          end
         end
 
         unless user.two_factor_enabled?

spec/requests/api/internal_spec.rb

@@ -152,7 +152,7 @@ describe API::Internal do
 
     context 'user key' do
       it 'returns the correct information about the key' do
-        lfs_auth(key.id, project)
+        lfs_auth_key(key.id, project)
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['username']).to eq(user.username)
@@ -161,8 +161,30 @@ describe API::Internal do
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
 
+      it 'returns the correct information about the user' do
+        lfs_auth_user(user.id, project)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['username']).to eq(user.username)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).token)
+
+        expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
+      end
+
+      it 'returns a 404 when no key or user is provided' do
+        lfs_auth_project(project)
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
       it 'returns a 404 when the wrong key is provided' do
-        lfs_auth(nil, project)
+        lfs_auth_key(key.id + 12345, project)
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
+      it 'returns a 404 when the wrong user is provided' do
+        lfs_auth_user(user.id + 12345, project)
 
         expect(response).to have_gitlab_http_status(404)
       end
@@ -172,7 +194,7 @@ describe API::Internal do
       let(:key) { create(:deploy_key) }
 
       it 'returns the correct information about the key' do
-        lfs_auth(key.id, project)
+        lfs_auth_key(key.id, project)
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
@@ -183,13 +205,29 @@ describe API::Internal do
   end
 
   describe "GET /internal/discover" do
-    it do
+    it "finds a user by key id" do
       get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
 
       expect(response).to have_gitlab_http_status(200)
 
       expect(json_response['name']).to eq(user.name)
     end
+
+    it "finds a user by user id" do
+      get(api("/internal/discover"), user_id: user.id, secret_token: secret_token)
+
+      expect(response).to have_gitlab_http_status(200)
+
+      expect(json_response['name']).to eq(user.name)
+    end
+
+    it "finds a user by username" do
+      get(api("/internal/discover"), username: user.username, secret_token: secret_token)
+
+      expect(response).to have_gitlab_http_status(200)
+
+      expect(json_response['name']).to eq(user.name)
+    end
   end
 
   describe "GET /internal/authorized_keys" do
@@ -871,7 +909,15 @@ describe API::Internal do
     )
   end
 
-  def lfs_auth(key_id, project)
+  def lfs_auth_project(project)
+    post(
+      api("/internal/lfs_authenticate"),
+      secret_token: secret_token,
+      project: project.full_path
+    )
+  end
+
+  def lfs_auth_key(key_id, project)
     post(
       api("/internal/lfs_authenticate"),
       key_id: key_id,
@@ -879,4 +925,13 @@ describe API::Internal do
       project: project.full_path
     )
   end
+
+  def lfs_auth_user(user_id, project)
+    post(
+      api("/internal/lfs_authenticate"),
+      user_id: user_id,
+      secret_token: secret_token,
+      project: project.full_path
+    )
+  end
 end