Re: Should ssh_userauth_none() produce an error?

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Mon, 2018-08-27 at 09:32 +0200, Andreas Schneider wrote:
> On Sunday, 26 August 2018 22:30:29 CEST Jakub Jelen wrote:
> > Hello,
> 
> Hi Jakub,
> 
> > at this moment, I am debugging application, that is using the above
> > method to figure out the list of supported authentication
> > mechanisms
> > and is failing later without any specific error message (my other
> > email
> > from today). Then we ask for the last error, we are getting the
> > error
> > from the above authentication method, which should not be an error
> > at
> > all (actually confirmed from the server log -- the only failed
> > method
> > was the "none" one probing for the supported authentication
> > methods):
> > 
> > read failed: Access denied. Authentication that can continue:
> > publickey,gssapi-keyex,gssapi-with-mic,password (libssh error code:
> > 1,
> > sftp error code: 0)
> > 
> > Reading through the tests, they make sure this function returns
> > this
> > error, but I don't think this is a very good idea, because almost
> > any
> > other failure, which will not set its own error will see this error
> > message.
> > 
> > It is also confusing, that this type of message does not list what
> > authentication method was used (none) so this is something that
> > should
> > get fixed at least.
> > 
> > I understand, this might be harder to change, since this might be
> > considered as part of API, but this email is mostly to start a
> > discussion if others might see this as a problem as well or what
> > can be
> > done about such lurking errors in the code.
> 
> Thanks for bringing this up, what do you think about the following
> branch?
> 
> https://gitlab.com/cryptomilk/libssh-mirror/commits/master-auth

Putting authentication-related variables in the session struct makes it
more clear what the members do. The function for resetting error was
something I was already searching for so I believe this will come handy
also in other cases. Resetting the error on the successful
authentication sounds good place to me. As well as the improved logging
makes it more clear what failed. This looks like you managed to handle
this without API change which is indeed good.

I noticed only a copy&paste error in ssh_userauth_password(), which
says SSH_AUTH_METHOD_PUBLICKEY instead of SSH_AUTH_METHOD_PASSWORD.

Additionally, what confused me is also that doc/mainpage.dox lists
hostbased authentication as supported, while there is no such method
implemented in auth.c -- the documentation should be probably updated
(attached trivial patch).


Thanks,
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.

From 0c8a2bdcbbe6c39029ca072cee77bfad252c42b8 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@xxxxxxxxxx>
Date: Mon, 27 Aug 2018 10:01:25 +0200
Subject: [PATCH] doc: There is no hostbased authentication implemented

Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx>
---
 doc/mainpage.dox | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/mainpage.dox b/doc/mainpage.dox
index a65caf9b..bf9a1e41 100644
--- a/doc/mainpage.dox
+++ b/doc/mainpage.dox
@@ -24,7 +24,7 @@ The libssh library provides:
  - <strong>Ciphers</strong>: <i>aes256-ctr, aes192-ctr, aes128-ctr</i>, aes256-cbc (rijndael-cbc@xxxxxxxxxxxxxx), aes192-cbc, aes128-cbc, 3des-cbc, blowfish-cbc, none
  - <strong>Compression Schemes</strong>: zlib, <i>zlib@xxxxxxxxxxx</i>, none
  - <strong>MAC hashes</strong>: hmac-sha1, hmac-sha2-256, hmac-sha2-384, hmac-sha2-512, hmac-md5, none
- - <strong>Authentication</strong>: none, password, public-key, hostbased, keyboard-interactive, <i>gssapi-with-mic</i>
+ - <strong>Authentication</strong>: none, password, public-key, keyboard-interactive, <i>gssapi-with-mic</i>
  - <strong>Channels</strong>: shell, exec (incl. SCP wrapper), direct-tcpip, subsystem, <i>auth-agent-req@xxxxxxxxxxx</i>
  - <strong>Global Requests</strong>: tcpip-forward, forwarded-tcpip
  - <strong>Channel Requests</strong>: x11, pty, <i>exit-status, signal, exit-signal, keepalive@xxxxxxxxxxx, auth-agent-req@xxxxxxxxxxx</i>
-- 
2.17.1