Create secured private dirs (yadm-dev#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.
daiglej · Aug 23, 2017 · ebc90bf · ebc90bf
1 parent c20973c
commit ebc90bf
Show file tree

Hide file tree

Showing 5 changed files with 314 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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() {