DevHeads.net

Review Request: More kio_sftp login related fixes

Review request for KDE Runtime and Andreas Schneider.

Description
This is the last one of the sftp login fixes series and addresses the following problems:

#1. Correctly handle login failure that results from a different username being used when setting the
SSH_OPTIONS_USER option and calling ssh_userauth_password. I think this might have been due to
a regression caused by my previous patch. Nonetheless, this patch addresses it.

#2. Changed public key authentication so that incorrect public key passwords generate a retry dialog
instead of simply continuing to the next available authentication method.

Diffs
kioslave/sftp/kio_sftp.h f497c0b
kioslave/sftp/kio_sftp.cpp e38c629

Diff: <a href="http://git.reviewboard.kde.org/r/104642/diff/" title="http://git.reviewboard.kde.org/r/104642/diff/">http://git.reviewboard.kde.org/r/104642/diff/</a>

Testing
Testing for #1:
===========
1.) Make sure a ssh server is running on your system.
2.) Attempt to login into your system: sftp://127.0.0.1.
3.) When prompted for credentials, enter a user name other than the currently logged in user.

Current Behavior:
Login attempt will simply fail eventually and and error page is displayed.

New (Fixed) Behavior:
Successfully log into the server with the specified user name.

Testing for #2:
===========
1.) Create a ssh key with password protection and add it to the authorized_keys file.
2.) Make sure the ssh public key is in your .ssh directory.
3.) Attempt to login into your system: sftp://127.0.0.1
4.) When prompted for the passpharse for the key, enter a bogus password.

Current behavior:
No retry dialog is ever shown for an invalid or improper ssh key passphrase and the process simply moves on to the next authentication method.

New(Fixed) behavior:
Show a retry dialog if the failure is due to invalid password. If the user then presses cancel on the retry dialog, simply behave the same way as if the cancel button is pressed on the password dialog. That is continue onto the next authentication method.

Thanks,

Dawit Alemayehu

Comments

Re: Review Request: More kio_sftp login related fixes

By Dawit A at 04/25/2012 - 22:42

(Updated April 26, 2012, 3:42 a.m.)

Review request for KDE Runtime and Andreas Schneider.

Changes
* Renamed sftpOpen to sftpLogin.

* Added a new sftpOpenConnection function and moved the portion of the code that performs connection and creation of a new ssh session there from openConnection. This makes it much easier to disconnect and reconnect without having to retry all the potential authentication methods.

Description
This is the last one of the sftp login fixes series and addresses the following problems:

#1. Correctly handle login failure that results from a different username being used when setting the
SSH_OPTIONS_USER option and calling ssh_userauth_password. I think this might have been due to
a regression caused by my previous patch. Nonetheless, this patch addresses it.

#2. Changed public key authentication so that incorrect public key passwords generate a retry dialog
instead of simply continuing to the next available authentication method.

Diffs (updated)
kioslave/sftp/kio_sftp.h f497c0b
kioslave/sftp/kio_sftp.cpp e38c629

Diff: <a href="http://git.reviewboard.kde.org/r/104642/diff/" title="http://git.reviewboard.kde.org/r/104642/diff/">http://git.reviewboard.kde.org/r/104642/diff/</a>

Testing
Testing for #1:
===========
1.) Make sure a ssh server is running on your system.
2.) Attempt to login into your system: sftp://127.0.0.1.
3.) When prompted for credentials, enter a user name other than the currently logged in user.

Current Behavior:
Login attempt will simply fail eventually and and error page is displayed.

New (Fixed) Behavior:
Successfully log into the server with the specified user name.

Testing for #2:
===========
1.) Create a ssh key with password protection and add it to the authorized_keys file.
2.) Make sure the ssh public key is in your .ssh directory.
3.) Attempt to login into your system: sftp://127.0.0.1
4.) When prompted for the passpharse for the key, enter a bogus password.

Current behavior:
No retry dialog is ever shown for an invalid or improper ssh key passphrase and the process simply moves on to the next authentication method.

New(Fixed) behavior:
Show a retry dialog if the failure is due to invalid password. If the user then presses cancel on the retry dialog, simply behave the same way as if the cancel button is pressed on the password dialog. That is continue onto the next authentication method.

Thanks,

Dawit Alemayehu

Re: Review Request: More kio_sftp login related fixes

By Commit Hook at 07/09/2012 - 04:27

This review has been submitted with commit 939d5388085267fcf052260d03d81b3e9c6ba023 by Dawit Alemayehu to branch KDE/4.9.

- Commit Hook

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Commit Hook at 07/06/2012 - 16:42

This review has been submitted with commit 51d580a019c67a7bf6751db8391738aa27c8ef66 by Dawit Alemayehu to branch master.

- Commit Hook

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 05/28/2012 - 08:24

Ship it!

Ship It!

- Andreas Schneider

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 06/12/2012 - 03:45

git clone git://git.libssh.org/projects/libssh.git
cd libssh/build
cmake -DWITH_SERVER=ON ..
make

after that you should have examples/samplesshd-kbdint

You need to run it as root that it can read the dsa and rsa key.

sudo examples/samplesshd-kbdint -p 2222

then connect with

sftp://libssh@localhost:2222

the daemon stops after a successful authentication. The password is libssh. You can also reach me on freenode: /query gladiac

- Andreas

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Dawit A at 06/11/2012 - 10:24

I plan to push into master for KDE 4.9, but first I have to fix at least issue #2. Issue #1 can be a TODO to be resolved. I wish I had the time to address it, but unfortunately I have not yet been able to figure out how to generate kbdinit to test it. It does not seem to get generated when one simply compiles libssh from source. At least not as far as I can see. Anyhow, I will fix issue #1 now that I know my changes caused the problem I stated above and push the change before the next 4.9 beta cycle.

- Dawit

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 06/11/2012 - 05:27

I'm fine if you push this to the repo.

- Andreas

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 04/21/2012 - 04:05

Did you also test if keyboard-interactive still works correctly?

- Andreas Schneider

On April 17, 2012, 7:16 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 05/13/2012 - 15:16

To #1: I didn't know that it triggers a reply. I needed to ask questions without a password field so I used the standard dialog. Maybe it is more advanced since 4.0. However if you compile libssh from source there is a kbdint server example which asks questions about hitchhikers guide to the galaxy ;)
Maybe this changed since I wrote it. I looked for a way to ask the user the questions.

To #2: The idea was to prompt the user for a password and then to into kbding and password userauth. So if he entered a password already and kbdint asks for Password then we just use the one he already entered. I think after all the great work you've to this doesn't work as before.

Could you try the libssh kbdint server example? If something doesn't work, let me know and I will change or fix the example.

Thanks for your work and help on this!

- Andreas

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 05/12/2012 - 08:42

Keyboard interactive can ask for the username and 10 questions about Star Wars before it will ask you for the password. You don't know how the password question will look but in most of the cases it is "Password:". So there should be a case handling username + "Password:" question.

You can also simply extend keyboard interactive. Ask for password + a one time password from a token. This way it is pretty flexible. The 'echo' flag is just for your if you should echo to the console what the server asks or not. If echo is set we should use a Password field instead of a normal input field.

I don't know what you mean with the errorMsg. I don't see something in the authenticateKeyboardInteractive() function.

- Andreas

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Dawit A at 05/12/2012 - 09:26

Right. I gathered keyboard interactive can ask multiple questions before it asks for the password. What I am baffled about is how it is currently being handled in authenticateKeyboardInteractive. Perhaps it would have been clearer if I simply stated the problems that I have after testing and reading the code in the "authenticateKeyboardInteractive" function. Here are the two issues I have:

#1. The first call to openPasswordDialog, the one whose first parameter is "infoKbdInt", always sets an error message. The text that says "Use the username input field to answer this question." is the error message and will result in the "retry" dialog being shown (See SlaveBase::openPasswordDialog docs). Unfortunately, I am not able to test that code path because the call to ssh_userauth_kbdint_getprompt returns sets the echo value 0 on my machine. Since I have not yet figured out how to configure my ssh server to do otherwise, I am not able to literally test it. Still, from looking at the code, it is obvious that the retry dialog is used to display the aforementioned "error message" before the user is prompted for login name and a password. Was that done on purpose ?

#2. When testing keyboard interactive, the call to "ssh_userauth_kbdint_getprompt" in authenticateKeyboardInteractive returns "echo = 0" and "prompt = Password" on my machine. As a result I never get prompted to enter a password because the code path for this scenario simply set the "answer" variable to whatever mPassword happens to be set to (including nothing). Hence, keyboard interactive always fails for me. What I do not understand is why when the "prompt" is set to "Password", there is no prompting the user for a password ?

- Dawit

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Dawit A at 04/25/2012 - 22:43

Well I finally had the time to figure out how to activate keyboard interactive and completely cleaned up the sftp login code to make it more readable. I removed the usage of the go to statement to avoid iterating through all the authentication methods just to retry password authentication. However, for the life of me I cannot figure out how the authenticateKeyboardInteractive is supposed to work. For me it does not work at all because it does not prompt me for the password. I read documentation you provided at <a href="http://api.libssh.org/stable/libssh_tutor_authentication.html" title="http://api.libssh.org/stable/libssh_tutor_authentication.html">http://api.libssh.org/stable/libssh_tutor_authentication.html</a>.

What I gathered from reading the documentation is that, in keyboard interactive mode, the server can send a challenge for which the user is supposed to provide an answer. The server indicates that it is sending such challenge prompt by setting the echo parameter to 1. Is that correct ? If it is, how that is then handled is rather befuddling to me. For example, the errorMsg parameter of the openPassword function is always set to the same text. That will cause the retry dialog to be shown with the "Do you want to retry?" question mark. Was that intentional ? What is being achieved by that. I would have tested it myself I had known what option to set in sshd_config to make the server send such challenge. I am sure if

The scenario I was able to test on my system is where the echo parameter is set to 0 and the prompt is set to "Password". In that case for some reason, the user is never prompted to enter the password. Instead the value of mPassword is assigned to it. Was that because the other password dialog (one used for authentication) was used to prompt for username and password already ? Anyhow, I have attached the latest incarnation which cleans up all the previous authentication related changes I made. It is much cleaner and easier to understand the flow of the authentication code now.

- Dawit

On April 26, 2012, 3:42 a.m., Dawit Alemayehu wrote:

Re: Review Request: More kio_sftp login related fixes

By Andreas Schneider at 04/21/2012 - 07:21

Normally password authentication is turned off in the ssh server (default for openssh) and keyboard interactive is used. There are some flaws in password authentication and keyboard-interactive is a much more flexible way. So if you have current Linux distribution then password authentication is turned of and you have keyboard-interactive authentication connecting to localhost.

Thanks for all the fixes. I don't have time for libssh and kio_sftp lately. There are other projects I need to drive forward right now :)

- Andreas

On April 17, 2012, 7:16 a.m., Dawit Alemayehu wrote: