Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-08-01

GITLAB_SHELL_VERSION

-7.2.0
+8.0.0

app/controllers/jwt_controller.rb

@@ -54,6 +54,22 @@ class JwtController < ApplicationController
   end
 
   def auth_params
-    params.permit(:service, :scope, :account, :client_id)
+    params.permit(:service, :account, :client_id)
+          .merge(additional_params)
+  end
+
+  def additional_params
+    { scopes: scopes_param }.compact
+  end
+
+  # We have to parse scope here, because Docker Client does not send an array of scopes,
+  # but rather a flat list and we loose second scope when being processed by Rails:
+  # scope=scopeA&scope=scopeB
+  #
+  # This method makes to always return an array of scopes
+  def scopes_param
+    return unless params[:scope].present?
+
+    Array(Rack::Utils.parse_query(request.query_string)['scope'])
   end
 end

app/services/auth/container_registry_authentication_service.rb

@@ -9,11 +9,11 @@ module Auth
 
       return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled
 
-      unless scope || current_user || project
+      unless scopes.any? || current_user || project
         return error('DENIED', status: 403, message: 'access forbidden')
       end
 
-      { token: authorized_token(scope).encoded }
+      { token: authorized_token(*scopes).encoded }
     end
 
     def self.full_access_token(*names)
@@ -47,10 +47,12 @@ module Auth
       end
     end
 
-    def scope
-      return unless params[:scope]
+    def scopes
+      return [] unless params[:scopes]
 
-      @scope ||= process_scope(params[:scope])
+      @scopes ||= params[:scopes].map do |scope|
+        process_scope(scope)
+      end.compact
     end
 
     def process_scope(scope)

changelogs/unreleased/34572-ssh-certificates.yml

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

changelogs/unreleased/fix-multiple-scopes.yml

+---
+title: Support multiple scopes when authing container registry scopes
+merge_request: 20617
+author:
+type: fixed

changelogs/unreleased/rails5-fix-flaky-spec-user-uses-shortcuts.yml

+---
+title: 'Rails5: fix flaky spec'
+merge_request: 20953
+author: Jasper Maes
+type: fixed

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

@@ -15,4 +15,7 @@ that to prioritize important jobs.
 to restart Sidekiq.
 - [Extra Sidekiq operations](extra_sidekiq_processes.md): Configure an extra set of Sidekiq processes to ensure certain queues always have dedicated workers, no matter the amount of jobs that need to be processed. **[STARTER ONLY]**
 - [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).

doc/ci/examples/test-and-deploy-python-application-to-heroku.md

@@ -46,7 +46,7 @@ This project has three jobs:
 
 ## Store API keys
 
-You'll need to create two variables in `Project > Variables`:
+You'll need to create two variables in `Settings > CI/CD > Variables` on your GitLab project settings:
 1. `HEROKU_STAGING_API_KEY` - Heroku API key used to deploy staging app,
 2. `HEROKU_PRODUCTION_API_KEY` - Heroku API key used to deploy production app.
 

doc/install/installation.md

@@ -12,7 +12,7 @@ Since installations from source don't have Runit, Sidekiq can't be terminated an
 
 ## Select Version to Install
 
-Make sure you view [this installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md) from the branch (version) of GitLab you would like to install (e.g., `11-1-stable`).
+Make sure you view [this installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md) from the branch (version) of GitLab you would like to install (e.g., `11-2-stable`).
 You can select the branch in the version dropdown in the top left corner of GitLab (below the menu bar).
 
 If the highest number stable branch is unclear please check the [GitLab Blog](https://about.gitlab.com/blog/) for installation guide links by version.
@@ -300,9 +300,9 @@ sudo usermod -aG redis git
 ### Clone the Source
 
     # Clone GitLab repository
-    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 11-1-stable gitlab
+    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 11-2-stable gitlab
 
-**Note:** You can change `11-1-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server!
+**Note:** You can change `11-2-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server!
 
 ### Configure It
 

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/features/projects/user_uses_shortcuts_spec.rb

@@ -9,6 +9,8 @@ describe 'User uses shortcuts', :js do
     sign_in(user)
 
     visit(project_path(project))
+
+    wait_for_requests
   end
 
   context 'when navigating to the Project pages' do

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

spec/requests/jwt_controller_spec.rb

@@ -70,6 +70,25 @@ describe JwtController do
 
       it { expect(service_class).to have_received(:new).with(nil, user, parameters) }
 
+      context 'when passing a flat array of scopes' do
+        # We use this trick to make rails to generate a query_string:
+        # scope=scope1&scope=scope2
+        # It works because :scope and 'scope' are the same as string, but different objects
+        let(:parameters) do
+          {
+            :service => service_name,
+            :scope => 'scope1',
+            'scope' => 'scope2'
+          }
+        end
+
+        let(:service_parameters) do
+          { service: service_name, scopes: %w(scope1 scope2) }
+        end
+
+        it { expect(service_class).to have_received(:new).with(nil, user, service_parameters) }
+      end
+
       context 'when user has 2FA enabled' do
         let(:user) { create(:user, :two_factor) }