diff --git a/README.md b/README.md index 26a86865..29d88be4 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,8 @@ The vcsrepo module provides a single type with providers to support the followin * [Subversion](#subversion) **Note:** `git` is the only vcs provider officially [supported by Puppet Inc.](https://forge.puppet.com/supported) -**Note:** Release v4.0.1 has been removed from the Puppet Forge and was officially re-released as version v5.0.0 as it contained a breaking change. Details available [here](https://puppetlabs.github.io/iac/team/status/developer/2021/06/04/status-update.html) +**Note:** Release v4.0.1 has been removed from the Puppet Forge and was officially re-released as version v5.0.0 as it contained a breaking change. +Details available [here](https://puppetlabs.github.io/iac/team/status/developer/2021/06/04/status-update.html) ## Setup @@ -813,6 +814,22 @@ The includes parameter is only supported when SVN client version is >= 1.6. For an extensive list of supported operating systems, see [metadata.json](https://github.com/puppetlabs/puppetlabs-vcsrepo/blob/main/metadata.json) +### Response to CVE-2022-24765 + +The vulnerability described in this CVE could impact users working on multi-user machines. +A malicious actor could create a `.git` directory above the current working directory causing all git invocations to occur outside of a repository to read its configuration. + +For a more in-depth description of this vulnerability, check out [this blog post](https://github.blog/2022-04-12-git-security-vulnerability-announced/). + +Fixes were released in Git versions 2.35.2 and 1:2.25.1-1ubuntu3.4 respectively. + +VCSRepo users were impacted when running newer versions of Git and managing repositories that were owned by a user or group that differed from the user executing Git. + +For example, setting the `owner` parameter on a resource would cause Puppet runs to fail with a `Path /destination/path exists and is not the desired repository.` error. + +Impacted users are now advised to use the new `safe_directory` parameter on Git resources. +Explicitily setting the value to `true` will add the current path specified on the resource to the `safe.directory` git configuration for the current user (global scope) allowing the Puppet run to continue without error. + ## Development diff --git a/lib/puppet/provider/vcsrepo/git.rb b/lib/puppet/provider/vcsrepo/git.rb index 9da6135c..f7b16e3c 100644 --- a/lib/puppet/provider/vcsrepo/git.rb +++ b/lib/puppet/provider/vcsrepo/git.rb @@ -6,7 +6,7 @@ desc 'Supports Git repositories' has_features :bare_repositories, :reference_tracking, :ssh_identity, :multiple_remotes, - :user, :depth, :branch, :submodules + :user, :depth, :branch, :submodules, :safe_directory def create check_force @@ -36,6 +36,7 @@ def create end def destroy + remove_safe_directory if safe_directories.include?(@resource.value(:path)) FileUtils.rm_rf(@resource.value(:path)) end @@ -140,6 +141,7 @@ def working_copy_exists? end def exists? + update_safe_directory working_copy_exists? || bare_exists? end @@ -564,6 +566,52 @@ def git_version exec_git('--version').match(%r{[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?})[0] end + # @!visibility private + def safe_directories + args = ['config', '--global', '--get-all', 'safe.directory'] + begin + d = git_with_identity(*args) || '' + d.split('\n') + .reject { |v| v.empty? } + .map { |v| v.chomp } + rescue Puppet::ExecutionFailure + [] + end + end + + # @!visibility private + def update_safe_directory + # If the owner parameter is not set, then we don't need to do anything. + return unless @resource.value(:owner) + + if should_add_safe_directory? + add_safe_directory + else + remove_safe_directory + end + end + + # @!visibility private + def add_safe_directory + notice("Adding '#{@resource.value(:path)}' to safe directory list") + args = ['config', '--global', '--add', 'safe.directory', @resource.value(:path)] + git_with_identity(*args) + end + + # @!visibility private + def remove_safe_directory + notice("Removing '#{@resource.value(:path)}' from safe directory list") + args = ['config', '--global', '--unset', 'safe.directory', @resource.value(:path)] + git_with_identity(*args) + end + + # @!visibility private + def should_add_safe_directory? + (@resource.value(:owner) != @resource.value(:user)) && # user and owner should be different + @resource.value(:safe_directory) && # safe_directory should be true + !safe_directories.include?(@resource.value(:path)) # directory should not already be in the list + end + # @!visibility private def git_with_identity(*args) if @resource.value(:trust_server_cert) == :true @@ -599,10 +647,13 @@ def git_with_identity(*args) # Execute git with the given args, running it as the user specified. def exec_git(*args) - exec_args = { failonfail: true, combine: true } + exec_args = { + failonfail: true, + combine: true, + custom_environment: { 'HOME' => Dir.home }, + } if @resource.value(:user) && @resource.value(:user) != Facter['id'].value - env = Etc.getpwnam(@resource.value(:user)) - exec_args[:custom_environment] = { 'HOME' => env['dir'] } + exec_args[:custom_environment] = { 'HOME' => Dir.home(@resource.value(:user)) } exec_args[:uid] = @resource.value(:user) end Puppet::Util::Execution.execute([:git, args], **exec_args) diff --git a/lib/puppet/type/vcsrepo.rb b/lib/puppet/type/vcsrepo.rb index 15339a03..c99dce1b 100644 --- a/lib/puppet/type/vcsrepo.rb +++ b/lib/puppet/type/vcsrepo.rb @@ -61,6 +61,9 @@ feature :keep_local_changes, 'The provider supports keeping local changes on files tracked by the repository when changing revision' + feature :safe_directory, + 'The provider supports setting a safe directory. This will only be used for newer versions of git.' + ensurable do desc 'Ensure the version control repository.' attr_accessor :latest @@ -112,7 +115,9 @@ def insync?(is) end newvalue :absent do - provider.destroy + if provider.exists? + provider.destroy + end end newvalue :latest, required_features: [:reference_tracking] do @@ -301,6 +306,12 @@ def insync?(is) defaultto :false end + newparam :safe_directory, required_features: [:safe_directory] do + desc 'Marks the current directory specified by the path parameter as a safe directory.' + newvalues(true, false) + defaultto :false + end + autorequire(:package) do ['git', 'git-core', 'mercurial', 'subversion'] end diff --git a/spec/acceptance/clone_repo_spec.rb b/spec/acceptance/clone_repo_spec.rb index 89ada353..8dd1b45e 100644 --- a/spec/acceptance/clone_repo_spec.rb +++ b/spec/acceptance/clone_repo_spec.rb @@ -9,6 +9,7 @@ after(:all) do run_shell("rm -rf #{tmpdir}/testrepo") + run_shell("rm -rf #{tmpdir}/testrepo_owner") run_shell("rm -rf #{tmpdir}/testrepo_mirror_repo") end @@ -225,6 +226,7 @@ provider => git, source => "file://#{tmpdir}/testrepo.git", owner => 'vagrant', + safe_directory => true, } MANIFEST it 'clones a repo' do diff --git a/spec/unit/puppet/provider/vcsrepo/git_spec.rb b/spec/unit/puppet/provider/vcsrepo/git_spec.rb index d6983109..2091dffd 100644 --- a/spec/unit/puppet/provider/vcsrepo/git_spec.rb +++ b/spec/unit/puppet/provider/vcsrepo/git_spec.rb @@ -193,6 +193,7 @@ def branch_a_list(include_branch = nil?) expect(provider).to receive(:path_exists?).and_return(true) expect(provider).to receive(:path_empty?).and_return(false) provider.destroy + expect(provider).to receive(:exec_git).with('config', '--global', '--get-all', 'safe.directory') expect(provider).to receive(:exec_git).with('clone', resource.value(:source), resource.value(:path)) expect(provider).to receive(:update_submodules) expect(provider).to receive(:update_remote_url).with('origin', resource.value(:source)).and_return false