Commit 1803bc26 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch 'fix/export-project-file-permissions' into 'security'

Fix export project file permissions issue

Fixes security concerns of https://gitlab.com/gitlab-org/gitlab-ce/issues/22757

I have just added the permissions 0700 to the creation of any of the export paths, as @jacobvosmaer suggested in https://gitlab.com/gitlab-org/gitlab-ce/issues/22757#note_16197616

After this has fixed, it could take up to 24 hours in the worse case scenario for old archives to be completely safe - This is the time `ImportExportProjectCleanupWorker` may take to remove the folders. The temporary folders will be 0700 straight away for new installations.

See merge request !2003
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 04def4d8
...@@ -11,6 +11,7 @@ v 8.12.4 (unreleased) ...@@ -11,6 +11,7 @@ v 8.12.4 (unreleased)
- Restrict failed login attempts for users with 2FA enabled. !6668 - Restrict failed login attempts for users with 2FA enabled. !6668
- Fix failed project deletion when feature visibility set to private. !6688 - Fix failed project deletion when feature visibility set to private. !6688
- Prevent claiming associated model IDs via import. - Prevent claiming associated model IDs via import.
- Set GitLab project exported file permissions to owner only
v 8.12.3 v 8.12.3
- Update Gitlab Shell to support low IO priority for storage moves - Update Gitlab Shell to support low IO priority for storage moves
......
module Gitlab module Gitlab
module ImportExport module ImportExport
module CommandLineUtil module CommandLineUtil
DEFAULT_MODE = 0700
def tar_czf(archive:, dir:) def tar_czf(archive:, dir:)
tar_with_options(archive: archive, dir: dir, options: 'czf') tar_with_options(archive: archive, dir: dir, options: 'czf')
end end
...@@ -21,6 +23,11 @@ module Gitlab ...@@ -21,6 +23,11 @@ module Gitlab
execute(%W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args) execute(%W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args)
end end
def mkdir_p(path)
FileUtils.mkdir_p(path, mode: DEFAULT_MODE)
FileUtils.chmod(DEFAULT_MODE, path)
end
private private
def tar_with_options(archive:, dir:, options:) def tar_with_options(archive:, dir:, options:)
...@@ -45,7 +52,7 @@ module Gitlab ...@@ -45,7 +52,7 @@ module Gitlab
# if we are copying files, create the destination folder # if we are copying files, create the destination folder
destination_folder = File.file?(source) ? File.dirname(destination) : destination destination_folder = File.file?(source) ? File.dirname(destination) : destination
FileUtils.mkdir_p(destination_folder) mkdir_p(destination_folder)
FileUtils.copy_entry(source, destination) FileUtils.copy_entry(source, destination)
true true
end end
......
...@@ -15,7 +15,7 @@ module Gitlab ...@@ -15,7 +15,7 @@ module Gitlab
end end
def import def import
FileUtils.mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
wait_for_archived_file do wait_for_archived_file do
decompress_archive decompress_archive
......
module Gitlab module Gitlab
module ImportExport module ImportExport
class ProjectTreeSaver class ProjectTreeSaver
include Gitlab::ImportExport::CommandLineUtil
attr_reader :full_path attr_reader :full_path
def initialize(project:, shared:) def initialize(project:, shared:)
...@@ -10,7 +12,7 @@ module Gitlab ...@@ -10,7 +12,7 @@ module Gitlab
end end
def save def save
FileUtils.mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
File.write(full_path, project_json_tree) File.write(full_path, project_json_tree)
true true
......
...@@ -12,7 +12,7 @@ module Gitlab ...@@ -12,7 +12,7 @@ module Gitlab
def restore def restore
return true unless File.exist?(@path_to_bundle) return true unless File.exist?(@path_to_bundle)
FileUtils.mkdir_p(path_to_repo) mkdir_p(path_to_repo)
git_unbundle(repo_path: path_to_repo, bundle_path: @path_to_bundle) && repo_restore_hooks git_unbundle(repo_path: path_to_repo, bundle_path: @path_to_bundle) && repo_restore_hooks
rescue => e rescue => e
......
...@@ -20,7 +20,7 @@ module Gitlab ...@@ -20,7 +20,7 @@ module Gitlab
private private
def bundle_to_disk def bundle_to_disk
FileUtils.mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
git_bundle(repo_path: path_to_repo, bundle_path: @full_path) git_bundle(repo_path: path_to_repo, bundle_path: @full_path)
rescue => e rescue => e
@shared.error(e) @shared.error(e)
......
module Gitlab module Gitlab
module ImportExport module ImportExport
class VersionSaver class VersionSaver
include Gitlab::ImportExport::CommandLineUtil
def initialize(shared:) def initialize(shared:)
@shared = shared @shared = shared
end end
def save def save
FileUtils.mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
File.write(version_file, Gitlab::ImportExport.version, mode: 'w') File.write(version_file, Gitlab::ImportExport.version, mode: 'w')
rescue => e rescue => e
......
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
end end
def bundle_to_disk(full_path) def bundle_to_disk(full_path)
FileUtils.mkdir_p(@shared.export_path) mkdir_p(@shared.export_path)
git_bundle(repo_path: path_to_repo, bundle_path: full_path) git_bundle(repo_path: path_to_repo, bundle_path: full_path)
rescue => e rescue => e
@shared.error(e) @shared.error(e)
......
...@@ -47,6 +47,8 @@ feature 'Import/Export - project export integration test', feature: true, js: tr ...@@ -47,6 +47,8 @@ feature 'Import/Export - project export integration test', feature: true, js: tr
expect(page).to have_content('Download export') expect(page).to have_content('Download export')
expect(file_permissions(project.export_path)).to eq(0700)
in_directory_with_expanded_export(project) do |exit_status, tmpdir| in_directory_with_expanded_export(project) do |exit_status, tmpdir|
expect(exit_status).to eq(0) expect(exit_status).to eq(0)
......
...@@ -130,4 +130,8 @@ module ExportFileHelper ...@@ -130,4 +130,8 @@ module ExportFileHelper
(parsed_model_attributes - parent.keys - excluded_attributes).empty? (parsed_model_attributes - parent.keys - excluded_attributes).empty?
end end
def file_permissions(file)
File.stat(file).mode & 0777
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