From a2bacd80963670daccb557174c962ba68f2232a7 Mon Sep 17 00:00:00 2001
From: Flavio Poletti <flavio@polettix.it>
Date: Sat, 15 Oct 2016 23:55:43 +0200
Subject: [PATCH] Patch GH #2446 "Adding a file with two ssh-key..."

This is a proposed change for addressing GH #2446 "Adding a file with
two ssh-keys can result in an error" (see
https://github.com/dokku/dokku/issues/2446).

The following checks are performed:

- validity of key contents when read from a file
- key contents only contains one single line/key
- authorized_keys file is ultimately valid

The last test is an extension over the previous implementation in that
all lines in the file are checked.

The commit also includes extensions to the relevant test file to
excercise the new checks. It also takes care to save and then restore
the authorized_keys file.
---
 plugins/ssh-keys/functions       |  6 ++++-
 plugins/ssh-keys/subcommands/add |  4 +++
 tests/unit/30_ssh_keys.bats      | 43 ++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/plugins/ssh-keys/functions b/plugins/ssh-keys/functions
index 32e7e971e..24b5e6807 100755
--- a/plugins/ssh-keys/functions
+++ b/plugins/ssh-keys/functions
@@ -5,7 +5,11 @@ source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions"
 verify_ssh_key_file() {
   declare desc="Test that public key is valid"
   [[ -s ${DOKKU_ROOT}/.ssh/authorized_keys ]] || dokku_log_fail "No public keys found."
-  ssh-keygen -l -f "${DOKKU_ROOT}/.ssh/authorized_keys" &> /dev/null || dokku_log_fail "${DOKKU_ROOT}/.ssh/authorized_keys failed ssh-keygen check."
+  local key line=0
+  while read key ; do
+    line=$((line + 1))
+    ssh-keygen -l -f /dev/stdin <<< "$key" &> /dev/null || dokku_log_fail "${DOKKU_ROOT}/.ssh/authorized_keys line $line failed ssh-keygen check."
+  done <"${DOKKU_ROOT}/.ssh/authorized_keys"
 }
 
 verify_ssh_key_exists() {
diff --git a/plugins/ssh-keys/subcommands/add b/plugins/ssh-keys/subcommands/add
index 165993a25..dac80e909 100755
--- a/plugins/ssh-keys/subcommands/add
+++ b/plugins/ssh-keys/subcommands/add
@@ -14,10 +14,14 @@ add_keys() {
     key_contents="$key_from_pipe"
   elif [[ -n "$key_file" ]]; then
     key_contents="$(cat "$key_file")"
+    ssh-keygen -lf /dev/stdin <<< "$key_contents" &> /dev/null || dokku_log_fail "Key from file is not a valid ssh public key"
   fi
   [[ -n "$name" && -n "$key_contents" ]] || dokku_log_fail "Two arguments are required if not piping, ie: dokku ssh-keys:add <NAME> <KEY_FILE>"
+  local count="$(wc -l <<< "$key_contents")"
+  [ "$count" -eq 1 ] || dokku_log_fail 'Too many keys provided, set one per invocation of dokku ssh-keys:add <NAME> <KEY_FILE>'
   verify_ssh_key_exists
   echo "$key_contents" | sshcommand acl-add dokku "$name" || dokku_log_fail "sshcommand returned an error: $?"
+  verify_ssh_key_file
 }
 
 add_keys "$@"
diff --git a/tests/unit/30_ssh_keys.bats b/tests/unit/30_ssh_keys.bats
index d3f157c04..6f4b3e287 100644
--- a/tests/unit/30_ssh_keys.bats
+++ b/tests/unit/30_ssh_keys.bats
@@ -4,10 +4,31 @@ load test_helper
 
 setup() {
   global_setup
+
+  # create a temporary key and save in a variable
   create_key
+  local KEY="$(cat /tmp/testkey.pub)"
+  destroy_key
+
+  # now create key that will be really used
+  create_key
+
+  # the temporary key is useful for adding in the file with two keys
+  # useful for a negative test
+  { cat /tmp/testkey.pub ; echo "$KEY" ; } > /tmp/testkey-double.pub
+
+  # another negative test input
+  echo 'invalid!' > /tmp/testkey-invalid.pub
+
+  # save current authorized_keys to remove all changes afterwards
+  cp "${DOKKU_ROOT:-/home/dokku}/.ssh/authorized_keys" /tmp/testkey-authorized_keys
 }
 
 teardown() {
+  # restore authorized_keys to its contents before the tests
+  cp /tmp/testkey-authorized_keys "${DOKKU_ROOT:-/home/dokku}/.ssh/authorized_keys"
+
+  # normal cleanup after here
   destroy_key
   global_teardown
 }
@@ -37,4 +58,26 @@ teardown() {
   echo "output: "$output
   echo "status: "$status
   assert_success
+  run /bin/bash -c "dokku ssh-keys:add name3 /tmp/testkey-double.pub"
+  echo "output: "$output
+  echo "status: "$status
+  assert_failure
+  run /bin/bash -c "dokku ssh-keys:add name4 /tmp/testkey-invalid.pub"
+  echo "output: "$output
+  echo "status: "$status
+  assert_failure
+  run /bin/bash -c "dokku ssh-keys:add name4 /tmp/testkey.pub"
+  echo "output: "$output
+  echo "status: "$status
+  assert_success
+
+  # leave this as the last test in the sequence! It introduces an error in authorized_keys
+  run /bin/bash -c 'echo invalid >> "${DOKKU_ROOT:-/home/dokku}/.ssh/authorized_keys"'
+  echo "output: "$output
+  echo "status: "$status
+  assert_success
+  run /bin/bash -c "dokku ssh-keys:add name5 /tmp/testkey.pub"
+  echo "output: "$output
+  echo "status: "$status
+  assert_failure
 }