Libguestfs - Jan 2023 - [PATCH nbdkit v2] ssh: Improve the error message when all authentication methods fail

The current error message:

  nbdkit: ssh[1]: error: all possible authentication methods failed

is confusing and non-actionable.  It's hard even for experts to
understand the relationship between the authentication methods offered
by a server and what we require.

Try to improve the error message in some common situations, especially
where password authentication on the server side is disabled but the
client supplied a password=... parameter.  After this change, you will
see an actionable error:

  nbdkit: ssh[1]: error: the server does not offer password
  authentication but you tried to use a password; if you have root
  access to the server, try editing 'sshd_config' and setting
  'PasswordAuthentication yes'; otherwise try setting up public key
  authentication

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2158300
Thanks: Laszlo Ersek
---
 plugins/ssh/ssh.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/plugins/ssh/ssh.c b/plugins/ssh/ssh.c
index aaa7c2b9f..5a132d8f2 100644
--- a/plugins/ssh/ssh.c
+++ b/plugins/ssh/ssh.c
@@ -361,6 +361,28 @@ authenticate (struct ssh_handle *h)
     if (rc == SSH_AUTH_SUCCESS) return 0;
   }
 
+  /* All compatible methods were tried and none worked.  Come up with
+   * an actionable diagnostic message if we recognise the problem.
+   */
+  if (!(method & SSH_AUTH_METHOD_PUBLICKEY) && password == NULL) {
+    nbdkit_error ("the server does not offer public key authentication;
"
+                  "try using the password=... parameter");
+    return -1;
+  }
+  if ((method & SSH_AUTH_METHOD_PASSWORD) && password != NULL) {
+    nbdkit_error ("password authentication failed, "
+                  "is the username and password correct?");
+    return -1;
+  }
+  if (!(method & SSH_AUTH_METHOD_PASSWORD) && password != NULL) {
+    nbdkit_error ("the server does not offer password authentication
"
+                  "but you tried to use a password; if you have root
access "
+                  "to the server, try editing 'sshd_config' and
setting "
+                  "'PasswordAuthentication yes'; otherwise try
setting up "
+                  "public key authentication");
+    return -1;
+  }
+
   nbdkit_error ("all possible authentication methods failed");
   return -1;
 }
-- 
2.37.3

On 1/5/23 17:17, Richard W.M. Jones wrote:
> The current error message:
> 
>   nbdkit: ssh[1]: error: all possible authentication methods failed
> 
> is confusing and non-actionable.  It's hard even for experts to
> understand the relationship between the authentication methods offered
> by a server and what we require.
> 
> Try to improve the error message in some common situations, especially
> where password authentication on the server side is disabled but the
> client supplied a password=... parameter.  After this change, you will
> see an actionable error:
> 
>   nbdkit: ssh[1]: error: the server does not offer password
>   authentication but you tried to use a password; if you have root
>   access to the server, try editing 'sshd_config' and setting
>   'PasswordAuthentication yes'; otherwise try setting up public key
>   authentication
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2158300
> Thanks: Laszlo Ersek
> ---
>  plugins/ssh/ssh.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/plugins/ssh/ssh.c b/plugins/ssh/ssh.c
> index aaa7c2b9f..5a132d8f2 100644
> --- a/plugins/ssh/ssh.c
> +++ b/plugins/ssh/ssh.c
> @@ -361,6 +361,28 @@ authenticate (struct ssh_handle *h)
>      if (rc == SSH_AUTH_SUCCESS) return 0;
>    }
>  
> +  /* All compatible methods were tried and none worked.  Come up with
> +   * an actionable diagnostic message if we recognise the problem.
> +   */
> +  if (!(method & SSH_AUTH_METHOD_PUBLICKEY) && password ==
NULL) {
> +    nbdkit_error ("the server does not offer public key
authentication; "
> +                  "try using the password=... parameter");
> +    return -1;
> +  }
> +  if ((method & SSH_AUTH_METHOD_PASSWORD) && password != NULL)
{
> +    nbdkit_error ("password authentication failed, "
> +                  "is the username and password correct?");
> +    return -1;
> +  }
> +  if (!(method & SSH_AUTH_METHOD_PASSWORD) && password !=
NULL) {
> +    nbdkit_error ("the server does not offer password authentication
"
> +                  "but you tried to use a password; if you have root
access "
> +                  "to the server, try editing 'sshd_config'
and setting "
> +                  "'PasswordAuthentication yes'; otherwise
try setting up "
> +                  "public key authentication");
> +    return -1;
> +  }
> +
>    nbdkit_error ("all possible authentication methods failed");
>    return -1;
>  }
Nice -- the auth logic is not changed, we're just checking various
frequent problems, and providing matching hints.

Now I wonder if the final error message remains reachable or not; but
the nice thing about this approach is that we need not care! It doesn't
really matter if we've covered *all* possible failures with helpful
hints; the behavior remains safe, we just improve the user information
in some known / frequent cases.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>