From ebc90bfc9822759298bf4deaccb43a537c355038 Mon Sep 17 00:00:00 2001 From: Tim Byrne Date: Mon, 17 Jul 2017 08:52:06 -0500 Subject: [PATCH] Create secured private dirs (#74) Directories are created prior to merge during clone, and prior to any Git command run. This directly addresses CVE-2017-11353. When cloning a repo which includes data in a .ssh or .gnupg directory, if those directories do not exist at the time of cloning, yadm will create the directories with mask 0700 prior to merging the fetched data into the work-tree. When running a Git command and .ssh or .gnupg directories do not exist, create those directories with mask 0700 prior to running the Git command. However, do not create those directories if yadm.auto-private-dirs is false. --- test/105_accept_clone.bats | 137 +++++++++++++++++++++++ test/115_accept_introspect.bats | 2 +- test/118_accept_assert_private_dirs.bats | 102 +++++++++++++++++ yadm | 44 ++++++++ yadm.1 | 38 +++++-- 5 files changed, 314 insertions(+), 9 deletions(-) create mode 100644 test/118_accept_assert_private_dirs.bats diff --git a/test/105_accept_clone.bats b/test/105_accept_clone.bats index e25623c..42750b8 100644 --- a/test/105_accept_clone.bats +++ b/test/105_accept_clone.bats @@ -440,3 +440,140 @@ EOF remote_output=$(GIT_DIR="$T_DIR_REPO" git remote show) [ "$remote_output" = "origin" ] } + +@test "Command 'clone' (local insecure .ssh and .gnupg data, no related data in repo)" { + echo " + Local .ssh/.gnupg data exists and is insecure + but yadm repo contains no .ssh/.gnupg data + local insecure data should remain accessible + (yadm is hands-off) + " + #; setup scenario + rm -rf "$T_DIR_WORK" "$T_DIR_REPO" + mkdir -p "$T_DIR_WORK/.ssh" + mkdir -p "$T_DIR_WORK/.gnupg" + touch "$T_DIR_WORK/.ssh/testfile" + touch "$T_DIR_WORK/.gnupg/testfile" + find "$T_DIR_WORK" -exec chmod a+rw '{}' ';' + + #; run clone (with debug on) + run "${T_YADM_Y[@]}" clone -d -w "$T_DIR_WORK" "$REMOTE_URL" + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ Initialized ]] + [[ "$output" =~ initial\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + [[ "$output" =~ initial\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + # standard perms still apply afterwards unless disabled with auto.perms + test_perms "$T_DIR_WORK/.gnupg" "drwx------" + test_perms "$T_DIR_WORK/.ssh" "drwx------" + +} + +@test "Command 'clone' (local insecure .gnupg data, related data in repo)" { + echo " + Local .gnupg data exists and is insecure + and yadm repo contains .gnupg data + .gnupg dir should be secured post merge + " + #; setup scenario + IN_REPO=(.bash_profile .vimrc .gnupg/gpg.conf) + setup + rm -rf "$T_DIR_WORK" "$T_DIR_REPO" + mkdir -p "$T_DIR_WORK/.gnupg" + touch "$T_DIR_WORK/.gnupg/testfile" + find "$T_DIR_WORK" -exec chmod a+rw '{}' ';' + + #; run clone (with debug on) + run "${T_YADM_Y[@]}" clone -d -w "$T_DIR_WORK" "$REMOTE_URL" + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ Initialized ]] + [[ "$output" =~ initial\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwxrwxrwx.+\.gnupg ]] + test_perms "$T_DIR_WORK/.gnupg" "drwx------" +} + +@test "Command 'clone' (local insecure .ssh data, related data in repo)" { + echo " + Local .ssh data exists and is insecure + and yadm repo contains .ssh data + .ssh dir should be secured post merge + " + #; setup scenario + IN_REPO=(.bash_profile .vimrc .ssh/config) + setup + rm -rf "$T_DIR_WORK" "$T_DIR_REPO" + mkdir -p "$T_DIR_WORK/.ssh" + touch "$T_DIR_WORK/.ssh/testfile" + find "$T_DIR_WORK" -exec chmod a+rw '{}' ';' + + #; run clone (with debug on) + run "${T_YADM_Y[@]}" clone -d -w "$T_DIR_WORK" "$REMOTE_URL" + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ Initialized ]] + [[ "$output" =~ initial\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwxrwxrwx.+\.ssh ]] + test_perms "$T_DIR_WORK/.ssh" "drwx------" +} + +@test "Command 'clone' (no existing .gnupg, .gnupg data tracked in repo)" { + echo " + Local .gnupg does not exist + and yadm repo contains .gnupg data + .gnupg dir should be created and secured prior to merge + tracked .gnupg data should be user accessible only + " + #; setup scenario + IN_REPO=(.bash_profile .vimrc .gnupg/gpg.conf) + setup + rm -rf "$T_DIR_WORK" + mkdir -p "$T_DIR_WORK" + rm -rf "$T_DIR_REPO" + + #; run clone (with debug on) + run "${T_YADM_Y[@]}" clone -d -w "$T_DIR_WORK" "$REMOTE_URL" + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ Initialized ]] + [[ ! "$output" =~ initial\ private\ dir\ perms ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwx------.+\.gnupg ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwx------.+\.gnupg ]] + test_perms "$T_DIR_WORK/.gnupg" "drwx------" +} + +@test "Command 'clone' (no existing .ssh, .ssh data tracked in repo)" { + echo " + Local .ssh does not exist + and yadm repo contains .ssh data + .ssh dir should be created and secured prior to merge + tracked .ssh data should be user accessible only + " + #; setup scenario + IN_REPO=(.bash_profile .vimrc .ssh/config) + setup + rm -rf "$T_DIR_WORK" + mkdir -p "$T_DIR_WORK" + rm -rf "$T_DIR_REPO" + + #; run clone (with debug on) + run "${T_YADM_Y[@]}" clone -d -w "$T_DIR_WORK" "$REMOTE_URL" + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ Initialized ]] + [[ ! "$output" =~ initial\ private\ dir\ perms ]] + [[ "$output" =~ pre-merge\ private\ dir\ perms\ drwx------.+\.ssh ]] + [[ "$output" =~ post-merge\ private\ dir\ perms\ drwx------.+\.ssh ]] + test_perms "$T_DIR_WORK/.ssh" "drwx------" +} diff --git a/test/115_accept_introspect.bats b/test/115_accept_introspect.bats index 0d1922b..56131f2 100644 --- a/test/115_accept_introspect.bats +++ b/test/115_accept_introspect.bats @@ -73,7 +73,7 @@ function count_introspect() { Exit with 0 " - count_introspect "configs" 0 12 'yadm\.auto-alt' + count_introspect "configs" 0 13 'yadm\.auto-alt' } @test "Command 'introspect' (repo)" { diff --git a/test/118_accept_assert_private_dirs.bats b/test/118_accept_assert_private_dirs.bats new file mode 100644 index 0000000..151a2e0 --- /dev/null +++ b/test/118_accept_assert_private_dirs.bats @@ -0,0 +1,102 @@ +load common +load_fixtures +status=;output=; #; populated by bats run() + +IN_REPO=(.bash_profile .vimrc) + +setup() { + destroy_tmp + build_repo "${IN_REPO[@]}" + rm -rf "$T_DIR_WORK" + mkdir -p "$T_DIR_WORK" +} + +@test "Private dirs (private dirs missing)" { + echo " + When a git command is run + And private directories are missing + Create private directories prior to command + " + + #; confirm directories are missing at start + [ ! -e "$T_DIR_WORK/.gnupg" ] + [ ! -e "$T_DIR_WORK/.ssh" ] + + #; run status + export DEBUG=yes + run "${T_YADM_Y[@]}" status + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ On\ branch\ master ]] + + #; confirm private directories are created + [ -d "$T_DIR_WORK/.gnupg" ] + test_perms "$T_DIR_WORK/.gnupg" "drwx------" + [ -d "$T_DIR_WORK/.ssh" ] + test_perms "$T_DIR_WORK/.ssh" "drwx------" + + #; confirm directories are created before command is run + [[ "$output" =~ Creating.+/.gnupg/.+Creating.+/.ssh/.+Running\ git\ command\ git\ status ]] +} + +@test "Private dirs (private dirs missing / yadm.auto-private-dirs=false)" { + echo " + When a git command is run + And private directories are missing + But auto-private-dirs is false + Do not create private dirs + " + + #; confirm directories are missing at start + [ ! -e "$T_DIR_WORK/.gnupg" ] + [ ! -e "$T_DIR_WORK/.ssh" ] + + #; set configuration + run "${T_YADM_Y[@]}" config --bool "yadm.auto-private-dirs" "false" + + #; run status + run "${T_YADM_Y[@]}" status + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ On\ branch\ master ]] + + #; confirm private directories are not created + [ ! -e "$T_DIR_WORK/.gnupg" ] + [ ! -e "$T_DIR_WORK/.ssh" ] +} + +@test "Private dirs (private dirs exist / yadm.auto-perms=false)" { + echo " + When a git command is run + And private directories exist + And yadm is configured not to auto update perms + Do not alter directories + " + + #shellcheck disable=SC2174 + mkdir -m 0777 -p "$T_DIR_WORK/.gnupg" "$T_DIR_WORK/.ssh" + + #; confirm directories are preset and open + [ -d "$T_DIR_WORK/.gnupg" ] + test_perms "$T_DIR_WORK/.gnupg" "drwxrwxrwx" + [ -d "$T_DIR_WORK/.ssh" ] + test_perms "$T_DIR_WORK/.ssh" "drwxrwxrwx" + + #; set configuration + run "${T_YADM_Y[@]}" config --bool "yadm.auto-perms" "false" + + #; run status + run "${T_YADM_Y[@]}" status + + #; validate status and output + [ "$status" -eq 0 ] + [[ "$output" =~ On\ branch\ master ]] + + #; confirm directories are still preset and open + [ -d "$T_DIR_WORK/.gnupg" ] + test_perms "$T_DIR_WORK/.gnupg" "drwxrwxrwx" + [ -d "$T_DIR_WORK/.ssh" ] + test_perms "$T_DIR_WORK/.ssh" "drwxrwxrwx" +} diff --git a/yadm b/yadm index 1b8608f..2ec5d9f 100755 --- a/yadm +++ b/yadm @@ -292,6 +292,8 @@ function clone() { shift done + [ -n "$DEBUG" ] && display_private_perms "initial" + #; clone will begin with a bare repo local empty= init $empty @@ -310,6 +312,15 @@ function clone() { rm -rf "$YADM_REPO" error_out "Unable to fetch origin ${clone_args[0]}" } + debug "Determining if repo tracks private directories" + for private_dir in .ssh/ .gnupg/; do + found_log=$("$GIT_PROGRAM" log -n 1 origin/master -- "$private_dir" 2>/dev/null) + if [ -n "$found_log" ]; then + debug "Private directory $private_dir is tracked by repo" + assert_private_dirs "$private_dir" + fi + done + [ -n "$DEBUG" ] && display_private_perms "pre-merge" debug "Doing an initial merge of origin/master" "$GIT_PROGRAM" merge origin/master || { debug "Merge failed, doing a reset and stashing conflicts." @@ -351,6 +362,8 @@ EOF fi } + [ -n "$DEBUG" ] && display_private_perms "post-merge" + CHANGES_POSSIBLE=1 } @@ -513,9 +526,18 @@ function git_command() { set -- "config" "${@:2}" fi + #; ensure private .ssh and .gnupg directories exist first + #; TODO: consider restricting this to only commands which modify the work-tree + + auto_private_dirs=$(config --bool yadm.auto-private-dirs) + if [ "$auto_private_dirs" != "false" ] ; then + assert_private_dirs .gnupg/ .ssh/ + fi + CHANGES_POSSIBLE=1 #; pass commands through to git + debug "Running git command $GIT_PROGRAM $*" "$GIT_PROGRAM" "$@" return "$?" } @@ -613,6 +635,7 @@ local.os local.user yadm.auto-alt yadm.auto-perms +yadm.auto-private-dirs yadm.cygwin-copy yadm.git-program yadm.gpg-perms @@ -906,6 +929,27 @@ function invoke_hook() { } +function assert_private_dirs() { + work=$(unix_path "$("$GIT_PROGRAM" config core.worktree)") + for private_dir in "$@"; do + if [ ! -d "$work/$private_dir" ]; then + debug "Creating $work/$private_dir" + #shellcheck disable=SC2174 + mkdir -m 0700 -p "$work/$private_dir" >/dev/null 2>&1 + fi + done +} + +function display_private_perms() { + when="$1" + for private_dir in .ssh .gnupg; do + if [ -d "$YADM_WORK/$private_dir" ]; then + private_perms=$(ls -ld "$YADM_WORK/$private_dir") + debug "$when" private dir perms "$private_perms" + fi + done +} + #; ****** Auto Functions ****** function auto_alt() { diff --git a/yadm.1 b/yadm.1 index ff0e65b..e919901 100644 --- a/yadm.1 +++ b/yadm.1 @@ -350,6 +350,9 @@ If disabled, you may still run manually to update permissions. This feature is enabled by default. .TP +.B yadm.auto-private-dirs +Disable the automatic creating of private directories described in the section PERMISSIONS. +.TP .B yadm.ssh-perms Disable the permission changes to .IR $HOME/.ssh/* . @@ -608,12 +611,10 @@ It is recommended that you use a private repository when keeping confidential files, even though they are encrypted. .SH PERMISSIONS When files are checked out of a Git repository, their initial permissions are -dependent upon the user's umask. This can result in confidential files with lax permissions. - -To prevent this, +dependent upon the user's umask. Because of this, .B yadm -will automatically update the permissions of confidential files. -The "group" and "others" permissions will be removed from the following files: +will automatically update the permissions of some file paths. The "group" and +"others" permissions will be removed from the following files: .RI - " $HOME/.yadm/files.gpg @@ -629,11 +630,32 @@ The "group" and "others" permissions will be removed from the following files: .B yadm will automatically update permissions by default. This can be disabled using the .I yadm.auto-perms -configuration. -Even if disabled, permissions can be manually updated by running +configuration. Even if disabled, permissions can be manually updated by running .BR yadm\ perms . -The SSH directory processing can be disabled using the +The +.I .ssh +directory processing can be disabled using the .I yadm.ssh-perms +configuration. The +.I .gnupg +directory processing can be disabled using the +.I yadm.gpg-perms +configuration. + +When cloning a repo which includes data in a +.IR .ssh " or " .gnupg +directory, if those directories do not exist at the time of cloning, +.B yadm +will create the directories with mask 0700 prior to merging the fetched data +into the work-tree. + +When running a Git command and +.IR .ssh " or " .gnupg +directories do not exist, +.B yadm +will create those directories with mask 0700 prior to running the Git command. +This can be disabled using the +.I yadm.auto-private-dirs configuration. .SH HOOKS For every command