Commit 650a7c73 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets Committed by Marin Jankovski

Merge branch 'project-existence-leak' into 'master'

Don't leak information about private project existence via Git-over-SSH/HTTP.

Fixes #2040 and https://gitlab.com/gitlab-org/gitlab-ce/issues/343.

Both `Grack::Auth` (used by Git-over-HTTP) and `Api::Internal /allowed` (used by gitlab-shell/Git-over-SSH) now return a generic "Not Found" error when the project exists but the user doesn't have access to it.

See merge request !1578
parent c0f8b956
...@@ -16,6 +16,17 @@ module API ...@@ -16,6 +16,17 @@ module API
# #
post "/allowed" do post "/allowed" do
status 200 status 200
actor = if params[:key_id]
Key.find_by(id: params[:key_id])
elsif params[:user_id]
User.find_by(id: params[:user_id])
end
unless actor
return Gitlab::GitAccessStatus.new(false, 'No such user or key')
end
project_path = params[:project] project_path = params[:project]
# Check for *.wiki repositories. # Check for *.wiki repositories.
...@@ -32,26 +43,20 @@ module API ...@@ -32,26 +43,20 @@ module API
project = Project.find_with_namespace(project_path) project = Project.find_with_namespace(project_path)
unless project if project
return Gitlab::GitAccessStatus.new(false, 'No such project') status = access.check(
actor,
params[:action],
project,
params[:changes]
)
end end
actor = if params[:key_id] if project && status && status.allowed?
Key.find_by(id: params[:key_id]) status
elsif params[:user_id] else
User.find_by(id: params[:user_id]) Gitlab::GitAccessStatus.new(false, 'No such project')
end
unless actor
return Gitlab::GitAccessStatus.new(false, 'No such user or key')
end end
access.check(
actor,
params[:action],
project,
params[:changes]
)
end end
# #
......
...@@ -10,8 +10,9 @@ module Grack ...@@ -10,8 +10,9 @@ module Grack
@request = Rack::Request.new(env) @request = Rack::Request.new(env)
@auth = Request.new(env) @auth = Request.new(env)
# Need this patch due to the rails mount @gitlab_ci = false
# Need this patch due to the rails mount
# Need this if under RELATIVE_URL_ROOT # Need this if under RELATIVE_URL_ROOT
unless Gitlab.config.gitlab.relative_url_root.empty? unless Gitlab.config.gitlab.relative_url_root.empty?
# If website is mounted using relative_url_root need to remove it first # If website is mounted using relative_url_root need to remove it first
...@@ -22,8 +23,12 @@ module Grack ...@@ -22,8 +23,12 @@ module Grack
@env['SCRIPT_NAME'] = "" @env['SCRIPT_NAME'] = ""
if project auth!
auth!
if project && authorized_request?
@app.call(env)
elsif @user.nil? && !@gitlab_ci
unauthorized
else else
render_not_found render_not_found
end end
...@@ -32,35 +37,30 @@ module Grack ...@@ -32,35 +37,30 @@ module Grack
private private
def auth! def auth!
if @auth.provided? 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 return bad_request unless @auth.basic?
# if valid token passed
if gitlab_ci_request?(login, password)
return @app.call(env)
end
@user = authenticate_user(login, password) # Authentication with username and password
login, password = @auth.credentials
if @user # Allow authentication for GitLab CI service
Gitlab::ShellEnv.set_env(@user) # if valid token passed
@env['REMOTE_USER'] = @auth.username if gitlab_ci_request?(login, password)
end @gitlab_ci = true
return
end end
if authorized_request? @user = authenticate_user(login, password)
@app.call(env)
else if @user
unauthorized Gitlab::ShellEnv.set_env(@user)
@env['REMOTE_USER'] = @auth.username
end end
end end
def gitlab_ci_request?(login, password) def gitlab_ci_request?(login, password)
if login == "gitlab-ci-token" && project.gitlab_ci? if login == "gitlab-ci-token" && project && project.gitlab_ci?
token = project.gitlab_ci_service.token token = project.gitlab_ci_service.token
if token.present? && token == password && git_cmd == 'git-upload-pack' if token.present? && token == password && git_cmd == 'git-upload-pack'
...@@ -107,6 +107,8 @@ module Grack ...@@ -107,6 +107,8 @@ module Grack
end end
def authorized_request? def authorized_request?
return true if @gitlab_ci
case git_cmd case git_cmd
when *Gitlab::GitAccess::DOWNLOAD_COMMANDS when *Gitlab::GitAccess::DOWNLOAD_COMMANDS
if user if user
...@@ -141,7 +143,9 @@ module Grack ...@@ -141,7 +143,9 @@ module Grack
end end
def project def project
@project ||= project_by_path(@request.path_info) return @project if defined?(@project)
@project = project_by_path(@request.path_info)
end end
def project_by_path(path) def project_by_path(path)
......
require "spec_helper"
describe Grack::Auth do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:app) { lambda { |env| [200, {}, "Success!"] } }
let!(:auth) { Grack::Auth.new(app) }
let(:env) {
{
"rack.input" => "",
"REQUEST_METHOD" => "GET",
"QUERY_STRING" => "service=git-upload-pack"
}
}
let(:status) { auth.call(env).first }
describe "#call" do
context "when the project doesn't exist" do
before do
env["PATH_INFO"] = "doesnt/exist.git"
end
context "when no authentication is provided" do
it "responds with status 401" do
expect(status).to eq(401)
end
end
context "when username and password are provided" do
context "when authentication fails" do
before do
env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
end
it "responds with status 401" do
expect(status).to eq(401)
end
end
context "when authentication succeeds" do
before do
env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
end
it "responds with status 404" do
expect(status).to eq(404)
end
end
end
end
context "when the project exists" do
before do
env["PATH_INFO"] = project.path_with_namespace + ".git"
end
context "when the project is public" do
before do
project.update_attribute(:visibility_level, Project::PUBLIC)
end
it "responds with status 200" do
expect(status).to eq(200)
end
end
context "when the project is private" do
before do
project.update_attribute(:visibility_level, Project::PRIVATE)
end
context "when no authentication is provided" do
it "responds with status 401" do
expect(status).to eq(401)
end
end
context "when username and password are provided" do
context "when authentication fails" do
before do
env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
end
it "responds with status 401" do
expect(status).to eq(401)
end
end
context "when authentication succeeds" do
before do
env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
end
context "when the user has access to the project" do
before do
project.team << [user, :master]
end
context "when the user is blocked" do
before do
user.block
project.team << [user, :master]
end
it "responds with status 404" do
expect(status).to eq(404)
end
end
context "when the user isn't blocked" do
it "responds with status 200" do
expect(status).to eq(200)
end
end
end
context "when the user doesn't have access to the project" do
it "responds with status 404" do
expect(status).to eq(404)
end
end
end
end
context "when a gitlab ci token is provided" do
let(:token) { "123" }
before do
gitlab_ci_service = project.build_gitlab_ci_service
gitlab_ci_service.active = true
gitlab_ci_service.token = token
gitlab_ci_service.project_url = "http://google.com"
gitlab_ci_service.save
env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials("gitlab-ci-token", token)
end
it "responds with status 200" do
expect(status).to eq(200)
end
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment