From 0d9752446d8e2b3b4fdb37eb8ec75c5e5f996f1c Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 20 Jul 2016 18:41:26 +0200 Subject: Add LFS controllers --- .../projects/git_http_client_controller.rb | 110 +++++++++++++++++++++ app/controllers/projects/git_http_controller.rb | 107 +------------------- app/controllers/projects/lfs_api_controller.rb | 94 ++++++++++++++++++ app/controllers/projects/lfs_storage_controller.rb | 106 ++++++++++++++++++++ app/helpers/lfs_helper.rb | 66 +++++++++++++ config/routes.rb | 20 +++- spec/requests/lfs_http_spec.rb | 33 +++++-- 7 files changed, 420 insertions(+), 116 deletions(-) create mode 100644 app/controllers/projects/git_http_client_controller.rb create mode 100644 app/controllers/projects/lfs_api_controller.rb create mode 100644 app/controllers/projects/lfs_storage_controller.rb create mode 100644 app/helpers/lfs_helper.rb diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb new file mode 100644 index 00000000000..7c21bd181dc --- /dev/null +++ b/app/controllers/projects/git_http_client_controller.rb @@ -0,0 +1,110 @@ +# This file should be identical in GitLab Community Edition and Enterprise Edition + +class Projects::GitHttpClientController < Projects::ApplicationController + include ActionController::HttpAuthentication::Basic + include KerberosSpnegoHelper + + attr_reader :user + + # Git clients will not know what authenticity token to send along + skip_before_action :verify_authenticity_token + skip_before_action :repository + before_action :authenticate_user + before_action :ensure_project_found! + + private + + def authenticate_user + if project && project.public? && download_request? + return # Allow access + end + + if allow_basic_auth? && basic_auth_provided? + login, password = user_name_and_password(request) + auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) + + if auth_result.type == :ci && download_request? + @ci = true + elsif auth_result.type == :oauth && !download_request? + # Not allowed + else + @user = auth_result.user + end + + if ci? || user + return # Allow access + end + elsif allow_kerberos_spnego_auth? && spnego_provided? + @user = find_kerberos_user + + if user + send_final_spnego_response + return # Allow access + end + end + + send_challenges + render plain: "HTTP Basic: Access denied\n", status: 401 + end + + def basic_auth_provided? + has_basic_credentials?(request) + end + + def send_challenges + challenges = [] + challenges << 'Basic realm="GitLab"' if allow_basic_auth? + challenges << spnego_challenge if allow_kerberos_spnego_auth? + headers['Www-Authenticate'] = challenges.join("\n") if challenges.any? + end + + def ensure_project_found! + render_not_found if project.blank? + end + + def project + return @project if defined?(@project) + + project_id, _ = project_id_with_suffix + if project_id.blank? + @project = nil + else + @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}") + end + end + + # This method returns two values so that we can parse + # params[:project_id] (untrusted input!) in exactly one place. + def project_id_with_suffix + id = params[:project_id] || '' + + %w[.wiki.git .git].each do |suffix| + if id.end_with?(suffix) + # Be careful to only remove the suffix from the end of 'id'. + # Accidentally removing it from the middle is how security + # vulnerabilities happen! + return [id.slice(0, id.length - suffix.length), suffix] + end + end + + # Something is wrong with params[:project_id]; do not pass it on. + [nil, nil] + end + + def repository + _, suffix = project_id_with_suffix + if suffix == '.wiki.git' + project.wiki.repository + else + project.repository + end + end + + def render_not_found + render plain: 'Not Found', status: :not_found + end + + def ci? + @ci.present? + end +end diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 40a8b7940d9..be73a4c0d2c 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -1,17 +1,6 @@ # This file should be identical in GitLab Community Edition and Enterprise Edition -class Projects::GitHttpController < Projects::ApplicationController - include ActionController::HttpAuthentication::Basic - include KerberosSpnegoHelper - - attr_reader :user - - # Git clients will not know what authenticity token to send along - skip_before_action :verify_authenticity_token - skip_before_action :repository - before_action :authenticate_user - before_action :ensure_project_found! - +class Projects::GitHttpController < Projects::GitHttpClientController # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull) # GET /foo/bar.git/info/refs?service=git-receive-pack (git push) def info_refs @@ -46,81 +35,8 @@ class Projects::GitHttpController < Projects::ApplicationController private - def authenticate_user - if project && project.public? && upload_pack? - return # Allow access - end - - if allow_basic_auth? && basic_auth_provided? - login, password = user_name_and_password(request) - auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) - - if auth_result.type == :ci && upload_pack? - @ci = true - elsif auth_result.type == :oauth && !upload_pack? - # Not allowed - else - @user = auth_result.user - end - - if ci? || user - return # Allow access - end - elsif allow_kerberos_spnego_auth? && spnego_provided? - @user = find_kerberos_user - - if user - send_final_spnego_response - return # Allow access - end - end - - send_challenges - render plain: "HTTP Basic: Access denied\n", status: 401 - end - - def basic_auth_provided? - has_basic_credentials?(request) - end - - def send_challenges - challenges = [] - challenges << 'Basic realm="GitLab"' if allow_basic_auth? - challenges << spnego_challenge if allow_kerberos_spnego_auth? - headers['Www-Authenticate'] = challenges.join("\n") if challenges.any? - end - - def ensure_project_found! - render_not_found if project.blank? - end - - def project - return @project if defined?(@project) - - project_id, _ = project_id_with_suffix - if project_id.blank? - @project = nil - else - @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}") - end - end - - # This method returns two values so that we can parse - # params[:project_id] (untrusted input!) in exactly one place. - def project_id_with_suffix - id = params[:project_id] || '' - - %w[.wiki.git .git].each do |suffix| - if id.end_with?(suffix) - # Be careful to only remove the suffix from the end of 'id'. - # Accidentally removing it from the middle is how security - # vulnerabilities happen! - return [id.slice(0, id.length - suffix.length), suffix] - end - end - - # Something is wrong with params[:project_id]; do not pass it on. - [nil, nil] + def download_request? + upload_pack? end def upload_pack? @@ -143,27 +59,10 @@ class Projects::GitHttpController < Projects::ApplicationController render json: Gitlab::Workhorse.git_http_ok(repository, user) end - def repository - _, suffix = project_id_with_suffix - if suffix == '.wiki.git' - project.wiki.repository - else - project.repository - end - end - - def render_not_found - render plain: 'Not Found', status: :not_found - end - def render_not_allowed render plain: download_access.message, status: :forbidden end - def ci? - @ci.present? - end - def upload_pack_allowed? return false unless Gitlab.config.gitlab_shell.upload_pack diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb new file mode 100644 index 00000000000..694ce5a0aa1 --- /dev/null +++ b/app/controllers/projects/lfs_api_controller.rb @@ -0,0 +1,94 @@ +class Projects::LfsApiController < Projects::GitHttpClientController + include LfsHelper + + before_action :lfs_enabled! + before_action :lfs_check_access!, except: [:deprecated] + + def batch + unless objects.present? + render_lfs_not_found + return + end + + if download_request? + render json: { objects: download_objects! } + elsif upload_request? + render json: { objects: upload_objects! } + else + raise "Never reached" + end + end + + def deprecated + render( + json: { + message: 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + status: 501 + ) + end + + private + + def objects + (params[:objects] || []).to_a + end + + def existing_oids + @existing_oids ||= begin + storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid) + end + end + + def download_objects! + objects.each do |object| + if existing_oids.include?(object[:oid]) + object[:actions] = download_actions(object) + else + object[:error] = { + code: 404, + message: "Object does not exist on the server or you don't have permissions to access it", + } + end + end + objects + end + + def upload_objects! + objects.each do |object| + object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid]) + end + objects + end + + def download_actions(object) + { + download: { + href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}", + header: { + Authorization: request.headers['Authorization'] + }.compact + } + } + end + + def upload_actions(object) + { + upload: { + href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}", + header: { + Authorization: request.headers['Authorization'] + }.compact + } + } + end + + def download_request? + params[:operation] == 'download' + end + + def upload_request? + params[:operation] == 'upload' + end +end diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb new file mode 100644 index 00000000000..3aa08bcd4ae --- /dev/null +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -0,0 +1,106 @@ +class Projects::LfsStorageController < Projects::GitHttpClientController + include LfsHelper + + before_action :lfs_enabled! + before_action :lfs_check_access! + + def download + lfs_object = LfsObject.find_by_oid(oid) + unless lfs_object && lfs_object.file.exists? + render_lfs_not_found + return + end + + send_file lfs_object.file.path, content_type: "application/octet-stream" + end + + def upload_authorize + render( + json: { + StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload", + LfsOid: oid, + LfsSize: size, + }, + content_type: 'application/json; charset=utf-8' + ) + end + + def upload_finalize + unless tmp_filename + render_lfs_forbidden + return + end + + if store_file(oid, size, tmp_filename) + head 200 + else + render plain: 'Unprocessable entity', status: 422 + end + end + + private + + def download_request? + action_name == 'download' + end + + def upload_request? + %w[upload_authorize upload_finalize].include? action_name + end + + def oid + params[:oid].to_s + end + + def size + params[:size].to_i + end + + def tmp_filename + name = request.headers['X-Gitlab-Lfs-Tmp'] + if name.present? + name.gsub!(/^.*(\\|\/)/, '') + name = name.match(/[0-9a-f]{73}/) + name[0] if name + else + nil + end + end + + def store_file(oid, size, tmp_file) + tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) + + object = LfsObject.find_or_create_by(oid: oid, size: size) + if object.file.exists? + success = true + else + success = move_tmp_file_to_storage(object, tmp_file_path) + end + + if success + success = link_to_project(object) + end + + success + ensure + # Ensure that the tmp file is removed + FileUtils.rm_f(tmp_file_path) + end + + def move_tmp_file_to_storage(object, path) + File.open(path) do |f| + object.file = f + end + + object.file.store! + object.save + end + + def link_to_project(object) + if object && !object.projects.exists?(storage_project.id) + object.projects << storage_project + object.save + end + end +end + diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb new file mode 100644 index 00000000000..6146a2ad849 --- /dev/null +++ b/app/helpers/lfs_helper.rb @@ -0,0 +1,66 @@ +module LfsHelper + def lfs_enabled! + return if Gitlab.config.lfs.enabled + + render( + json: { + message: 'Git LFS is not enabled on this GitLab server, contact your admin.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + status: 501 + ) + end + + def lfs_check_access! + return if download_request? && lfs_download_access? + return if upload_request? && lfs_upload_access? + + if project.public? || (user && user.can?(:read_project, project)) + render_lfs_forbidden + else + render_lfs_not_found + end + end + + def lfs_download_access? + project.public? || ci? || (user && user.can?(:download_code, project)) + end + + def lfs_upload_access? + user && user.can?(:push_code, project) + end + + def render_lfs_forbidden + render( + json: { + message: 'Access forbidden. Check your access level.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + content_type: "application/vnd.git-lfs+json", + status: 403 + ) + end + + def render_lfs_not_found + render( + json: { + message: 'Not found.', + documentation_url: "#{Gitlab.config.gitlab.url}/help", + }, + content_type: "application/vnd.git-lfs+json", + status: 404 + ) + end + + def storage_project + @storage_project ||= begin + result = project + + while result.forked? do + result = result.forked_from_project + end + + result + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 21f3585bacd..47599441cad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -85,9 +85,6 @@ Rails.application.routes.draw do # Health check get 'health_check(/:checks)' => 'health_check#index', as: :health_check - # Enable Grack support (for LFS only) - mount Grack::AuthSpawner, at: '/', constraints: lambda { |request| /[-\/\w\.]+\.git\/(info\/lfs|gitlab-lfs)/.match(request.path_info) }, via: [:get, :post, :put] - # Help get 'help' => 'help#index' get 'help/shortcuts' => 'help#shortcuts' @@ -483,11 +480,26 @@ Rails.application.routes.draw do end scope module: :projects do - # Git HTTP clients ('git clone' etc.) scope constraints: { id: /.+\.git/, format: nil } do + # Git HTTP clients ('git clone' etc.) get '/info/refs', to: 'git_http#info_refs' post '/git-upload-pack', to: 'git_http#git_upload_pack' post '/git-receive-pack', to: 'git_http#git_receive_pack' + + # Git LFS API (metadata) + post '/info/lfs/objects/batch', to: 'lfs_api#batch' + post '/info/lfs/objects', to: 'lfs_api#deprecated' + get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated' + + # GitLab LFS object storage + scope constraints: { oid: /[a-f0-9]{64}/ } do + get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download' + + scope constraints: { size: /[0-9]+/ } do + put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize' + put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize' + end + end end # Allow /info/refs, /info/refs?service=git-upload-pack, and diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 93d2bc160cc..d6fe4935635 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -31,6 +31,7 @@ describe Gitlab::Lfs::Router do 'operation' => 'upload' } end + let(:authorization) { authorize_user } before do allow(Gitlab.config.lfs).to receive(:enabled).and_return(false) @@ -71,6 +72,7 @@ describe Gitlab::Lfs::Router do end context 'when handling lfs request using deprecated API' do + let(:authorization) { authorize_user } before do post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers end @@ -118,8 +120,8 @@ describe Gitlab::Lfs::Router do project.lfs_objects << lfs_object end - it 'responds with status 403' do - expect(response).to have_http_status(403) + it 'responds with status 404' do + expect(response).to have_http_status(404) end end @@ -147,8 +149,8 @@ describe Gitlab::Lfs::Router do context 'without required headers' do let(:authorization) { authorize_user } - it 'responds with status 403' do - expect(response).to have_http_status(403) + it 'responds with status 404' do + expect(response).to have_http_status(404) end end end @@ -304,10 +306,10 @@ describe Gitlab::Lfs::Router do end context 'when user does is not member of the project' do - let(:role) { :guest } + let(:update_user_permissions) { nil } - it 'responds with 403' do - expect(response).to have_http_status(403) + it 'responds with 404' do + expect(response).to have_http_status(404) end end @@ -510,6 +512,7 @@ describe Gitlab::Lfs::Router do describe 'unsupported' do let(:project) { create(:empty_project) } + let(:authorization) { authorize_user } let(:body) do { 'operation' => 'other', 'objects' => [ @@ -557,7 +560,7 @@ describe Gitlab::Lfs::Router do end it 'does not recognize it as a valid lfs command' do - expect(response).to have_http_status(403) + expect(response).to have_http_status(401) end end end @@ -582,6 +585,16 @@ describe Gitlab::Lfs::Router do expect(response).to have_http_status(403) end end + + context 'and request is sent with a malformed headers' do + before do + put_finalize('cat /etc/passwd') + end + + it 'does not recognize it as a valid lfs command' do + expect(response).to have_http_status(403) + end + end end describe 'to one project' do @@ -627,6 +640,10 @@ describe Gitlab::Lfs::Router do end describe 'and user does not have push access' do + before do + project.team << [user, :reporter] + end + it_behaves_like 'forbidden' end end -- cgit v1.2.3 From df3ca41e62efb7ccbc1ea82d5676527e8eeca530 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 22 Jul 2016 15:45:38 +0200 Subject: Remove obsolete code --- config/initializers/5_backend.rb | 3 - lib/gitlab/backend/grack_auth.rb | 163 ------------------- lib/gitlab/lfs/response.rb | 329 --------------------------------------- lib/gitlab/lfs/router.rb | 98 ------------ spec/requests/lfs_http_spec.rb | 2 +- 5 files changed, 1 insertion(+), 594 deletions(-) delete mode 100644 lib/gitlab/backend/grack_auth.rb delete mode 100644 lib/gitlab/lfs/response.rb delete mode 100644 lib/gitlab/lfs/router.rb diff --git a/config/initializers/5_backend.rb b/config/initializers/5_backend.rb index e026151a032..ed88c8ee1b8 100644 --- a/config/initializers/5_backend.rb +++ b/config/initializers/5_backend.rb @@ -1,6 +1,3 @@ -# GIT over HTTP -require_dependency Rails.root.join('lib/gitlab/backend/grack_auth') - # GIT over SSH require_dependency Rails.root.join('lib/gitlab/backend/shell') diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb deleted file mode 100644 index ab94abeda77..00000000000 --- a/lib/gitlab/backend/grack_auth.rb +++ /dev/null @@ -1,163 +0,0 @@ -module Grack - class AuthSpawner - def self.call(env) - # Avoid issues with instance variables in Grack::Auth persisting across - # requests by creating a new instance for each request. - Auth.new({}).call(env) - end - end - - class Auth < Rack::Auth::Basic - attr_accessor :user, :project, :env - - def call(env) - @env = env - @request = Rack::Request.new(env) - @auth = Request.new(env) - - @ci = false - - # Need this patch due to the rails mount - # Need this if under RELATIVE_URL_ROOT - unless Gitlab.config.gitlab.relative_url_root.empty? - # If website is mounted using relative_url_root need to remove it first - @env['PATH_INFO'] = @request.path.sub(Gitlab.config.gitlab.relative_url_root, '') - else - @env['PATH_INFO'] = @request.path - end - - @env['SCRIPT_NAME'] = "" - - auth! - - lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call - return lfs_response unless lfs_response.nil? - - if @user.nil? && !@ci - unauthorized - else - render_not_found - end - end - - private - - def auth! - return unless @auth.provided? - - return bad_request unless @auth.basic? - - # Authentication with username and password - login, password = @auth.credentials - - # Allow authentication for GitLab CI service - # if valid token passed - if ci_request?(login, password) - @ci = true - return - end - - @user = authenticate_user(login, password) - end - - def ci_request?(login, password) - matched_login = /(?^[a-zA-Z]*-ci)-token$/.match(login) - - if project && matched_login.present? - underscored_service = matched_login['s'].underscore - - if underscored_service == 'gitlab_ci' - return project && project.valid_build_token?(password) - elsif Service.available_services_names.include?(underscored_service) - service_method = "#{underscored_service}_service" - service = project.send(service_method) - - return service && service.activated? && service.valid_token?(password) - end - end - - false - end - - def oauth_access_token_check(login, password) - if login == "oauth2" && git_cmd == 'git-upload-pack' && password.present? - token = Doorkeeper::AccessToken.by_token(password) - token && token.accessible? && User.find_by(id: token.resource_owner_id) - end - end - - def authenticate_user(login, password) - user = Gitlab::Auth.find_with_user_password(login, password) - - unless user - user = oauth_access_token_check(login, password) - end - - # If the user authenticated successfully, we reset the auth failure count - # from Rack::Attack for that IP. A client may attempt to authenticate - # with a username and blank password first, and only after it receives - # a 401 error does it present a password. Resetting the count prevents - # false positives from occurring. - # - # Otherwise, we let Rack::Attack know there was a failed authentication - # attempt from this IP. This information is stored in the Rails cache - # (Redis) and will be used by the Rack::Attack middleware to decide - # whether to block requests from this IP. - config = Gitlab.config.rack_attack.git_basic_auth - - if config.enabled - if user - # A successful login will reset the auth failure count from this IP - Rack::Attack::Allow2Ban.reset(@request.ip, config) - else - banned = Rack::Attack::Allow2Ban.filter(@request.ip, config) do - # Unless the IP is whitelisted, return true so that Allow2Ban - # increments the counter (stored in Rails.cache) for the IP - if config.ip_whitelist.include?(@request.ip) - false - else - true - end - end - - if banned - Rails.logger.info "IP #{@request.ip} failed to login " \ - "as #{login} but has been temporarily banned from Git auth" - end - end - end - - user - end - - def git_cmd - if @request.get? - @request.params['service'] - elsif @request.post? - File.basename(@request.path) - else - nil - end - end - - def project - return @project if defined?(@project) - - @project = project_by_path(@request.path_info) - end - - def project_by_path(path) - if m = /^([\w\.\/-]+)\.git/.match(path).to_a - path_with_namespace = m.last - path_with_namespace.gsub!(/\.wiki$/, '') - - path_with_namespace[0] = '' if path_with_namespace.start_with?('/') - Project.find_with_namespace(path_with_namespace) - end - end - - def render_not_found - [404, { "Content-Type" => "text/plain" }, ["Not Found"]] - end - end -end diff --git a/lib/gitlab/lfs/response.rb b/lib/gitlab/lfs/response.rb deleted file mode 100644 index a1ee1aa81ff..00000000000 --- a/lib/gitlab/lfs/response.rb +++ /dev/null @@ -1,329 +0,0 @@ -module Gitlab - module Lfs - class Response - def initialize(project, user, ci, request) - @origin_project = project - @project = storage_project(project) - @user = user - @ci = ci - @env = request.env - @request = request - end - - def render_download_object_response(oid) - render_response_to_download do - if check_download_sendfile_header? - render_lfs_sendfile(oid) - else - render_not_found - end - end - end - - def render_batch_operation_response - request_body = JSON.parse(@request.body.read) - case request_body["operation"] - when "download" - render_batch_download(request_body) - when "upload" - render_batch_upload(request_body) - else - render_not_found - end - end - - def render_storage_upload_authorize_response(oid, size) - render_response_to_push do - [ - 200, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'StoreLFSPath' => "#{Gitlab.config.lfs.storage_path}/tmp/upload", - 'LfsOid' => oid, - 'LfsSize' => size - })] - ] - end - end - - def render_storage_upload_store_response(oid, size, tmp_file_name) - return render_forbidden unless tmp_file_name - - render_response_to_push do - render_lfs_upload_ok(oid, size, tmp_file_name) - end - end - - def render_unsupported_deprecated_api - [ - 501, - { "Content-Type" => "application/json; charset=utf-8" }, - [JSON.dump({ - 'message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - private - - def render_not_enabled - [ - 501, - { - "Content-Type" => "application/json; charset=utf-8", - }, - [JSON.dump({ - 'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_unauthorized - [ - 401, - { - 'Content-Type' => 'text/plain' - }, - ['Unauthorized'] - ] - end - - def render_not_found - [ - 404, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Not found.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_forbidden - [ - 403, - { - "Content-Type" => "application/vnd.git-lfs+json" - }, - [JSON.dump({ - 'message' => 'Access forbidden. Check your access level.', - 'documentation_url' => "#{Gitlab.config.gitlab.url}/help", - })] - ] - end - - def render_lfs_sendfile(oid) - return render_not_found unless oid.present? - - lfs_object = object_for_download(oid) - - if lfs_object && lfs_object.file.exists? - [ - 200, - { - # GitLab-workhorse will forward Content-Type header - "Content-Type" => "application/octet-stream", - "X-Sendfile" => lfs_object.file.path - }, - [] - ] - else - render_not_found - end - end - - def render_batch_upload(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_push do - response = build_upload_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_batch_download(body) - return render_not_found if body.empty? || body['objects'].nil? - - render_response_to_download do - response = build_download_batch_response(body['objects']) - [ - 200, - { - "Content-Type" => "application/json; charset=utf-8", - "Cache-Control" => "private", - }, - [JSON.dump(response)] - ] - end - end - - def render_lfs_upload_ok(oid, size, tmp_file) - if store_file(oid, size, tmp_file) - [ - 200, - { - 'Content-Type' => 'text/plain', - 'Content-Length' => 0 - }, - [] - ] - else - [ - 422, - { 'Content-Type' => 'text/plain' }, - ["Unprocessable entity"] - ] - end - end - - def render_response_to_download - return render_not_enabled unless Gitlab.config.lfs.enabled - - unless @project.public? - return render_unauthorized unless @user || @ci - return render_forbidden unless user_can_fetch? - end - - yield - end - - def render_response_to_push - return render_not_enabled unless Gitlab.config.lfs.enabled - return render_unauthorized unless @user - return render_forbidden unless user_can_push? - - yield - end - - def check_download_sendfile_header? - @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile" - end - - def user_can_fetch? - # Check user access against the project they used to initiate the pull - @ci || @user.can?(:download_code, @origin_project) - end - - def user_can_push? - # Check user access against the project they used to initiate the push - @user.can?(:push_code, @origin_project) - end - - def storage_project(project) - if project.forked? - storage_project(project.forked_from_project) - else - project - end - end - - def store_file(oid, size, tmp_file) - tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) - - object = LfsObject.find_or_create_by(oid: oid, size: size) - if object.file.exists? - success = true - else - success = move_tmp_file_to_storage(object, tmp_file_path) - end - - if success - success = link_to_project(object) - end - - success - ensure - # Ensure that the tmp file is removed - FileUtils.rm_f(tmp_file_path) - end - - def object_for_download(oid) - @project.lfs_objects.find_by(oid: oid) - end - - def move_tmp_file_to_storage(object, path) - File.open(path) do |f| - object.file = f - end - - object.file.store! - object.save - end - - def link_to_project(object) - if object && !object.projects.exists?(@project.id) - object.projects << @project - object.save - end - end - - def select_existing_objects(objects) - objects_oids = objects.map { |o| o['oid'] } - @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set - end - - def build_upload_batch_response(objects) - selected_objects = select_existing_objects(objects) - - upload_hypermedia_links(objects, selected_objects) - end - - def build_download_batch_response(objects) - selected_objects = select_existing_objects(objects) - - download_hypermedia_links(objects, selected_objects) - end - - def download_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - if existing_objects.include?(object['oid']) - object['actions'] = { - 'download' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - else - object['error'] = { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - end - end - - { 'objects' => all_objects } - end - - def upload_hypermedia_links(all_objects, existing_objects) - all_objects.each do |object| - # generate actions only for non-existing objects - next if existing_objects.include?(object['oid']) - - object['actions'] = { - 'upload' => { - 'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}", - 'header' => { - 'Authorization' => @env['HTTP_AUTHORIZATION'] - }.compact - } - } - end - - { 'objects' => all_objects } - end - end - end -end diff --git a/lib/gitlab/lfs/router.rb b/lib/gitlab/lfs/router.rb deleted file mode 100644 index f2a76a56b8f..00000000000 --- a/lib/gitlab/lfs/router.rb +++ /dev/null @@ -1,98 +0,0 @@ -module Gitlab - module Lfs - class Router - attr_reader :project, :user, :ci, :request - - def initialize(project, user, ci, request) - @project = project - @user = user - @ci = ci - @env = request.env - @request = request - end - - def try_call - return unless @request && @request.path.present? - - case @request.request_method - when 'GET' - get_response - when 'POST' - post_response - when 'PUT' - put_response - else - nil - end - end - - private - - def get_response - path_match = @request.path.match(/\/(info\/lfs|gitlab-lfs)\/objects\/([0-9a-f]{64})$/) - return nil unless path_match - - oid = path_match[2] - return nil unless oid - - case path_match[1] - when "info/lfs" - lfs.render_unsupported_deprecated_api - when "gitlab-lfs" - lfs.render_download_object_response(oid) - else - nil - end - end - - def post_response - post_path = @request.path.match(/\/info\/lfs\/objects(\/batch)?$/) - return nil unless post_path - - # Check for Batch API - if post_path[0].ends_with?("/info/lfs/objects/batch") - lfs.render_batch_operation_response - elsif post_path[0].ends_with?("/info/lfs/objects") - lfs.render_unsupported_deprecated_api - else - nil - end - end - - def put_response - object_match = @request.path.match(/\/gitlab-lfs\/objects\/([0-9a-f]{64})\/([0-9]+)(|\/authorize){1}$/) - return nil if object_match.nil? - - oid = object_match[1] - size = object_match[2].try(:to_i) - return nil if oid.nil? || size.nil? - - # GitLab-workhorse requests - # 1. Try to authorize the request - # 2. send a request with a header containing the name of the temporary file - if object_match[3] && object_match[3] == '/authorize' - lfs.render_storage_upload_authorize_response(oid, size) - else - tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP']) - lfs.render_storage_upload_store_response(oid, size, tmp_file_name) - end - end - - def lfs - return unless @project - - Gitlab::Lfs::Response.new(@project, @user, @ci, @request) - end - - def sanitize_tmp_filename(name) - if name.present? - name.gsub!(/^.*(\\|\/)/, '') - name = name.match(/[0-9a-f]{73}/) - name[0] if name - else - nil - end - end - end - end -end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index d6fe4935635..7692f986d27 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Lfs::Router do +describe 'Git LFS API and storage' do let(:user) { create(:user) } let!(:lfs_object) { create(:lfs_object, :with_file) } -- cgit v1.2.3 From d199b3cdd7a43d46d88a6386b95b48c0b40b8315 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 22 Jul 2016 15:56:10 +0200 Subject: Better cache when modifying in-place --- app/controllers/projects/lfs_api_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index 694ce5a0aa1..1fa9cd8f47f 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -32,7 +32,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController private def objects - (params[:objects] || []).to_a + @objects ||= (params[:objects] || []).to_a end def existing_oids -- cgit v1.2.3 From 23425401d1b574dd87babfffda4d59b9f91d1538 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 22 Jul 2016 16:40:22 +0200 Subject: Rubocop --- app/controllers/projects/lfs_storage_controller.rb | 13 ++++++------- app/helpers/lfs_helper.rb | 3 ++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index 3aa08bcd4ae..d0895e1324c 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -16,12 +16,12 @@ class Projects::LfsStorageController < Projects::GitHttpClientController def upload_authorize render( - json: { - StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload", - LfsOid: oid, - LfsSize: size, - }, - content_type: 'application/json; charset=utf-8' + json: { + StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload", + LfsOid: oid, + LfsSize: size, + }, + content_type: 'application/json; charset=utf-8' ) end @@ -103,4 +103,3 @@ class Projects::LfsStorageController < Projects::GitHttpClientController end end end - diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 6146a2ad849..ae230ee1878 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -56,7 +56,8 @@ module LfsHelper @storage_project ||= begin result = project - while result.forked? do + loop do + break unless result.forked? result = result.forked_from_project end -- cgit v1.2.3 From 71952d057d5edad0697d7da76f5da034689e0f4a Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 22 Jul 2016 17:23:35 +0200 Subject: Handle custom Git LFS content type --- config/initializers/mime_types.rb | 7 +++++++ spec/requests/lfs_http_spec.rb | 10 +++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb index 3e553120205..f498732feca 100644 --- a/config/initializers/mime_types.rb +++ b/config/initializers/mime_types.rb @@ -12,3 +12,10 @@ Mime::Type.register_alias "text/html", :md Mime::Type.register "video/mp4", :mp4, [], [:m4v, :mov] Mime::Type.register "video/webm", :webm Mime::Type.register "video/ogg", :ogv + +middlewares = Gitlab::Application.config.middleware +middlewares.swap(ActionDispatch::ParamsParser, ActionDispatch::ParamsParser, { + Mime::Type.lookup('application/vnd.git-lfs+json') => lambda do |body| + ActiveSupport::JSON.decode(body) + end +}) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 7692f986d27..26572699bf8 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -35,7 +35,7 @@ describe 'Git LFS API and storage' do before do allow(Gitlab.config.lfs).to receive(:enabled).and_return(false) - post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers end it 'responds with 501' do @@ -74,7 +74,7 @@ describe 'Git LFS API and storage' do context 'when handling lfs request using deprecated API' do let(:authorization) { authorize_user } before do - post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers end it_behaves_like 'a deprecated' @@ -164,7 +164,7 @@ describe 'Git LFS API and storage' do enable_lfs update_lfs_permissions update_user_permissions - post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers end describe 'download' do @@ -775,8 +775,8 @@ describe 'Git LFS API and storage' do Projects::ForkService.new(project, user, {}).execute end - def post_json(url, body = nil, headers = nil) - post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/json')) + def post_lfs_json(url, body = nil, headers = nil) + post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json')) end def json_response -- cgit v1.2.3 From 7dff0946a7ea8c47cf531067ac752cf900927c44 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 9 Aug 2016 12:35:36 +0200 Subject: Remove duplicate method reintroduced by merge --- app/controllers/projects/git_http_controller.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index a10dca6afaf..b4373ef89ef 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -59,10 +59,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController render json: Gitlab::Workhorse.git_http_ok(repository, user) end - def render_not_found - render plain: 'Not Found', status: :not_found - end - def render_http_not_allowed render plain: access_check.message, status: :forbidden end -- cgit v1.2.3 From 0012de8c8a6642db40c13273f32c396afaa629eb Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 10 Aug 2016 16:48:21 +0200 Subject: Rename lfs_enabled helper method --- app/controllers/projects/lfs_api_controller.rb | 2 +- app/controllers/projects/lfs_storage_controller.rb | 2 +- app/helpers/lfs_helper.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index 1fa9cd8f47f..ece49dcd922 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -1,7 +1,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController include LfsHelper - before_action :lfs_enabled! + before_action :require_lfs_enabled! before_action :lfs_check_access!, except: [:deprecated] def batch diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index d0895e1324c..6bde0a527ff 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -1,7 +1,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController include LfsHelper - before_action :lfs_enabled! + before_action :require_lfs_enabled! before_action :lfs_check_access! def download diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index ae230ee1878..eb651e3687e 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -1,5 +1,5 @@ module LfsHelper - def lfs_enabled! + def require_lfs_enabled! return if Gitlab.config.lfs.enabled render( -- cgit v1.2.3 From f817eecb22517ece0344977d00ecc7ddfff30594 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 10 Aug 2016 16:49:23 +0200 Subject: Use && and || instead of if --- app/controllers/projects/lfs_storage_controller.rb | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index 6bde0a527ff..a80fa525631 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -68,22 +68,13 @@ class Projects::LfsStorageController < Projects::GitHttpClientController end def store_file(oid, size, tmp_file) + # Define tmp_file_path early because we use it in "ensure" tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file) object = LfsObject.find_or_create_by(oid: oid, size: size) - if object.file.exists? - success = true - else - success = move_tmp_file_to_storage(object, tmp_file_path) - end - - if success - success = link_to_project(object) - end - - success + file_exists = object.file.exists? || move_tmp_file_to_storage(object, tmp_file_path) + file_exists && link_to_project(object) ensure - # Ensure that the tmp file is removed FileUtils.rm_f(tmp_file_path) end -- cgit v1.2.3 From 26b98bfff8c5bb7048bcbec46e028e30c46bccc5 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 10 Aug 2016 17:40:20 +0200 Subject: Improve validation of X-Gitlab-Lfs-Tmp header --- app/controllers/projects/lfs_storage_controller.rb | 10 +++------- spec/requests/lfs_http_spec.rb | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index a80fa525631..69066cb40e6 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -58,13 +58,9 @@ class Projects::LfsStorageController < Projects::GitHttpClientController def tmp_filename name = request.headers['X-Gitlab-Lfs-Tmp'] - if name.present? - name.gsub!(/^.*(\\|\/)/, '') - name = name.match(/[0-9a-f]{73}/) - name[0] if name - else - nil - end + return if name.include?('/') + return unless oid.present? && name.start_with?(oid) + name end def store_file(oid, size, tmp_file) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 26572699bf8..9db282a9f3d 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -556,7 +556,7 @@ describe 'Git LFS API and storage' do context 'and request is sent with a malformed headers' do before do - put_finalize('cat /etc/passwd') + put_finalize('/etc/passwd') end it 'does not recognize it as a valid lfs command' do @@ -588,7 +588,7 @@ describe 'Git LFS API and storage' do context 'and request is sent with a malformed headers' do before do - put_finalize('cat /etc/passwd') + put_finalize('/etc/passwd') end it 'does not recognize it as a valid lfs command' do @@ -636,6 +636,18 @@ describe 'Git LFS API and storage' do it 'lfs object is linked to the project' do expect(lfs_object.projects.pluck(:id)).to include(project.id) end + en + + context 'invalid tempfiles' do + it 'rejects slashes in the tempfile name (path traversal' do + put_finalize('foo/bar') + expect(response).to have_http_status(403) + end + + it 'rejects tempfile names that do not start with the oid' do + put_finalize("foo#{sample_oid}") + expect(response).to have_http_status(403) + end end end -- cgit v1.2.3 From 6e6ad3e2fc386044134cbb33395cceb97e913fa0 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 11 Aug 2016 15:05:49 +0200 Subject: Fix typo --- spec/requests/lfs_http_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 9db282a9f3d..4c9b4a8ba42 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -636,7 +636,7 @@ describe 'Git LFS API and storage' do it 'lfs object is linked to the project' do expect(lfs_object.projects.pluck(:id)).to include(project.id) end - en + end context 'invalid tempfiles' do it 'rejects slashes in the tempfile name (path traversal' do -- cgit v1.2.3