1
0
Fork 0
mirror of https://github.com/capistrano/capistrano synced 2023-03-27 23:21:18 -04:00

Security: randomize path of git wrapper script

Before, the `:git_wrapper_path` was a somewhat predictable value and
located in `/tmp` by default, which is world-writable. That meant that
there was a chance (albeit very small) that another process could guess
the path and overwrite it with something malicious.

Fix by randomly generating a path name so that the git wrapper script
location cannot be predicted.

This change should be transparent to capistrano users since the
`:git_wrapper_path` is only intended to be used internally. If you need
a predictable value for this path, set a custom value for
`:git_wrapper_path` in your `deploy.rb` file.
This commit is contained in:
Matt Brictson 2020-12-31 10:50:39 -08:00
parent c676e64899
commit 9c92ad51ed
No known key found for this signature in database
GPG key ID: 2F279EAD1F2ACFAF
4 changed files with 9 additions and 12 deletions

View file

@ -5,7 +5,7 @@ Then(/^references in the remote repo are listed$/) do
end end
Then(/^git wrapper permissions are 0700$/) do Then(/^git wrapper permissions are 0700$/) do
permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path.shellescape}) == "700" ]) permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path_glob}) == "700" ])
_stdout, _stderr, status = vagrant_cli_command("ssh -c #{permissions_test.shellescape}") _stdout, _stderr, status = vagrant_cli_command("ssh -c #{permissions_test.shellescape}")
expect(status).to be_success expect(status).to be_success

View file

@ -1,5 +1,6 @@
require "capistrano/scm/plugin" require "capistrano/scm/plugin"
require "cgi" require "cgi"
require "securerandom"
require "shellwords" require "shellwords"
require "uri" require "uri"
@ -7,10 +8,9 @@ class Capistrano::SCM::Git < Capistrano::SCM::Plugin
def set_defaults def set_defaults
set_if_empty :git_shallow_clone, false set_if_empty :git_shallow_clone, false
set_if_empty :git_wrapper_path, lambda { set_if_empty :git_wrapper_path, lambda {
# Try to avoid permissions issues when multiple users deploy the same app # Use a unique name that won't collide with other deployments, and
# by using different file names in the same dir for each deployer and stage. # that cannot be guessed by other processes that have access to /tmp.
suffix = %i(application stage local_user).map { |key| fetch(key).to_s }.join("-") "#{fetch(:tmp_dir)}/git-ssh-#{SecureRandom.hex(10)}.sh"
"#{fetch(:tmp_dir)}/git-ssh-#{suffix}.sh"
} }
set_if_empty :git_environmental_variables, lambda { set_if_empty :git_environmental_variables, lambda {
{ {

View file

@ -28,13 +28,10 @@ module Capistrano
end end
describe "#set_defaults" do describe "#set_defaults" do
it "makes git_wrapper_path using application, stage, and local_user" do it "makes git_wrapper_path using a random hex value" do
env.set(:tmp_dir, "/tmp") env.set(:tmp_dir, "/tmp")
env.set(:application, "my_app")
env.set(:stage, "staging")
env.set(:local_user, "(Git Web User) via ShipIt")
subject.set_defaults subject.set_defaults
expect(env.fetch(:git_wrapper_path)).to eq("/tmp/git-ssh-my_app-staging-(Git Web User) via ShipIt.sh") expect(env.fetch(:git_wrapper_path)).to match(%r{/tmp/git-ssh-\h{20}\.sh})
end end
it "makes git_max_concurrent_connections" do it "makes git_max_concurrent_connections" do

View file

@ -185,8 +185,8 @@ module TestApp
FileUtils.mv(config_path, location) FileUtils.mv(config_path, location)
end end
def git_wrapper_path def git_wrapper_path_glob
"/tmp/git-ssh-my_app_name-#{stage}-#{current_user}.sh" "/tmp/git-ssh-*.sh"
end end
def with_clean_bundler_env(&block) def with_clean_bundler_env(&block)