[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Should ssh_userauth_none() produce an error?
- Subject: Re: Should ssh_userauth_none() produce an error?
- From: Jakub Jelen <jjelen@xxxxxxxxxx>
- Reply-to: libssh@xxxxxxxxxx
- Date: Mon, 27 Aug 2018 10:03:38 +0200
- To: Andreas Schneider <asn@xxxxxxxxxxxxxx>, libssh@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
|Re: Should ssh_userauth_none() produce an error?||Andreas Schneider <asn@xxxxxxxxxxxxxx>|