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

[Richard W.M. Jones]

On Fri, Jan 06, 2023 at 08:29:06AM +0100, Laszlo Ersek wrote:
> 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>

Thanks - commits c93a8957efcc26652b31f5bc359dfd3c4019b4f8 (the earlier
clean-up) and bea88cff5ac9c42f1a068ad24d43d5ed0506edaa.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW