From e3fa3678404e161be27914b91c69693613b1fcfe Mon Sep 17 00:00:00 2001
From: Philipp Rudo <prudo@redhat.com>
Date: Fri, 25 Mar 2022 15:47:04 +0100
Subject: [PATCH] kdumpctl: simplify propagate_ssh_key

The function has multiple problems:

1) SSH_{USER,SERVER} aren't defined local
2) Weird use of cut and sed to parse the DUMP_TARGET for the user and
   host although check_ssh_config guarantees that it has the format
   <user>@<host>.
3) Unnecessary use of a variable for the return value
4) Weird behavior to first unpack the DUMP_TARGET to SSH_USER and
   SSH_SERVER and then putting it back together again
5) Definition of variable errmsg that is only used once but breaks
   grep-ability of error message.
6) Wrong order when redirecting output of ssh-keygen, see SC2069 [1]

Fix them now.

While at it also improve the error messages in the function.

[1] https://www.shellcheck.net/wiki/SC2069

Signed-off-by: Philipp Rudo <prudo@redhat.com>
Reviewed-by: Tao Liu <ltao@redhat.com>
Reviewed-by: Coiby Xu <coxu@redhat.com>
---
 kdumpctl | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 8ad6e4c..e6d5066 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -755,35 +755,32 @@ check_ssh_target()
 
 propagate_ssh_key()
 {
+	local SSH_USER SSH_SERVER
+
 	if ! check_ssh_config; then
-		derror "No ssh config specified in $KDUMP_CONFIG_FILE.  Can't propagate"
+		derror "No ssh destination defined in $KDUMP_CONFIG_FILE."
+		derror "Please verify that $KDUMP_CONFIG_FILE contains 'ssh <user>@<host>' and that it is properly formatted."
 		exit 1
 	fi
 
 	local KEYFILE=$SSH_KEY_LOCATION
-	local errmsg="Failed to propagate ssh key"
 
 	#Check to see if we already created key, if not, create it.
 	if [[ -f $KEYFILE ]]; then
 		dinfo "Using existing keys..."
 	else
 		dinfo "Generating new ssh keys... "
-		/usr/bin/ssh-keygen -t rsa -f "$KEYFILE" -N "" 2>&1 > /dev/null
+		/usr/bin/ssh-keygen -t rsa -f "$KEYFILE" -N "" &> /dev/null
 		dinfo "done."
 	fi
 
-	#now find the target ssh user and server to contact.
-	SSH_USER=$(echo "$DUMP_TARGET" | cut -d@ -f1)
-	SSH_SERVER=$(echo "$DUMP_TARGET" | sed -e's/\(.*@\)\(.*$\)/\2/')
-
-	#now send the found key to the found server
-	ssh-copy-id -i "$KEYFILE" "$SSH_USER@$SSH_SERVER"
-	RET=$?
-	if [[ $RET == 0 ]]; then
+	SSH_USER=${DUMP_TARGET%@*}
+	SSH_SERVER=${DUMP_TARGET#*@}
+	if ssh-copy-id -i "$KEYFILE" "$DUMP_TARGET"; then
 		dinfo "$KEYFILE has been added to ~$SSH_USER/.ssh/authorized_keys on $SSH_SERVER"
 		return 0
 	else
-		derror "$errmsg, $KEYFILE failed in transfer to $SSH_SERVER"
+		derror "Failed to propagate ssh key, could not transfer $KEYFILE to $SSH_SERVER"
 		exit 1
 	fi
 }