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

@@ -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() {

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 "$@"

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
 }