bcpierce00 / unison

Unison file synchronizer
GNU General Public License v3.0
3.9k stars 225 forks source link

unison-gui asks for SSH password without need #917

Closed kolAflash closed 1 year ago

kolAflash commented 1 year ago

Since updating my computers to Debian-12 and Unison-2.52.1 it's asking for the users password on the SSH server when connecting to another Debian-12. (not private key password)

SSH public authentication is enabled. There's no private key password.
So Unison shouldn't ask for a SSH password at all.
I can actually simply press enter without giving a password and it works. With Unison-2.53.3 I can even see the synchronization starting in the other GUI window while the password dialog is in foreground.

Last good version: unison-v2.51.4+ocaml-4.12.0+x86_64.linux.tar.gz
First bad version: unison-v2.51.5+ocaml-4.12.0+x86_64.linux.tar.gz
The current Unison-2.53.3 is also affected.

 

Debug output just says:

Unison 2.52.1 (ocaml 4.13.1) log started at 2023-05-06 at 23:01:24

[remote] Shell connection: ssh (ssh, -l, myuser, 192.168.1.2, -e, none, -C, unison, -server, __new-rpc-mode)
gdt commented 1 year ago

Please read the Reporting Bugs wiki page referenced in the README. We do not accept bug reports for old versions. Besides that general principle, there have been a number of ssh/password fixes. Therefore, please update to the latest release on both sides and test again - I have set the feedback label for 30 days to provide debug info with-up-to-date versions.. Feel free to ask for help on unison-users.

gdt commented 1 year ago

Sorry, I see that you said this happens with 2.53.3, which I didn't notice when looking at the debug output.

We will need a repro recipe. It sounds like you are using pubkey authentication with a key not protected by a passphrase. You didn't give the ssh version.

kolAflash commented 1 year ago

All software versions are standard Debian-12. So SSH is version 9.2p1.

gdt commented 1 year ago

Can you test with an ssh key w/o a passphrase, and one with, in the latter case with ssh-agent set up? If that makes the difference, that will help a lot. Also with each ssh, do something like "ssh remote id" (where id is just a simple command; you can pick something else), and compare the output. I am wondering if we can understand the behavior difference from ssh.

Also, to the extent you can read unison docs and turn on even more debugging, which might need code changes, that wil also help. Please also test the CLI as that will also use ssh but it doesn't try to intercept parts of it and present it via a GUI.

tleedjarv commented 1 year ago

With Unison-2.53.3 I can even see the synchronization starting in the other GUI window while the password dialog is in foreground.

Could you please paste a screenshot showing the password dialog in this situation (you don't need to include the background window). You can clear out any private details like usernames, hostnames, paths.

Meanwhile, I will prepare a debug build for you to try. I'll post a link here once it is ready.

kolAflash commented 1 year ago

@tleedjarv
Here are the screenshots :-)
(used Unison-2.53.3) Again thanks for the help!

 

I created a Key with ssh-keygen -t rsa -b 4096 -f ~/.ssh/id_rsa_test and added it to the ~/.ssh/config:

Host 192.168.x.y
  IdentityFile /home/myuser/.ssh/id_rsa_test

 

non-pw_protected_key.png ![non-pw_protected_key](https://user-images.githubusercontent.com/3355089/236669998-8f05dcc0-0146-484c-a05c-1b3a809cbb99.png)
pw_protected_locked_key.png Here the key is still locked (NOT unlocked by ssh-agent). Unison correctly starts the synchronisation AFTER entering the private key password. ![pw_protected_locked_key](https://user-images.githubusercontent.com/3355089/236670012-dc348846-7ab8-4338-a7d0-f263f24a31a6.png)
pw_protected_agent-unlocked_key.png Unlocked the key before using: `ssh-add ~/.ssh/id_rsa_test` ![pw_protected_agent-unlocked_key](https://user-images.githubusercontent.com/3355089/236670007-bab7c058-bd37-40d7-8fcc-cb43aee75df5.png)
tleedjarv commented 1 year ago

Thank you for this very detailed info.

I have now located the cause of the issue. There are two separate moments here.

First is that you get the output from ssh at all. This is because you probably have VisualHostKey in your ssh configuration. ssh outputs this on stderr. Previous versions of Unison basically discarded stderr output from ssh (to be fair, it was displayed in the terminal if you started the GUI from a terminal). With more recent versions, all output from ssh, both stdout and stderr is displayed in the GUI dialog window. This is not a bug, but in future it could be made smoother for the user (so that you don't have to click through OK, for example).

Then there is this issue that you're requested by Unison for a password even though the password is not requested by ssh. This is a bug, which was partially fixed in the latest version, but one case was overlooked. With this bug fixed, you will not be asked for a password but you will still see the host key visual and need to click OK (but this will not stop update detection starting in the background).

Would you still like to try out a debug build where I can include a proof-of-concept fix for the bug? So that you can see how that will look like the way it's intended to work.

kolAflash commented 1 year ago

With this bug fixed, you will not be asked for a password but you will still see the host key visual and need to click OK (but this will not stop update detection starting in the background).

I liked it the way it was until 2.51.4 with no need to click OK.
_(except the host is not in known_keys)_

... maybe I'll just configure Unison with sshargs = -o VisualHostKey=no.
(Just tested and works fine! 😃)

 

Would you still like to try out a debug build where I can include a proof-of-concept fix for the bug? So that you can see how that will look like the way it's intended to work.

Sure!
Would be great if you can precompile it.

tleedjarv commented 1 year ago

I liked it the way it was until 2.51.4 with no need to click OK. _(except the host is not in known_keys)_

Just a little background: The way it was until 2.51.4 was hiding all errors from the user. For example: errors executing ssh, connection errors, authentication errors, known_hosts warnings, the MITM notice when host key changes, key file not found errors, errors from remote machine (e.g. unison not found), etc. (so at least 3 levels of errors: errors executing ssh, errors from ssh, errors from remote machine). The only thing user would get is "Lost connection with server" with no explanation of what actually went wrong. Unless you so happened to run the GUI from a terminal and looked at terminal output. The new functionality will capture and display all these errors (and warnings and information) in the GUI. Since there is no "log" in GUI then this information is put in the dialog box and the user has to click OK.

... maybe I'll just configure Unison with sshargs = -o VisualHostKey=no. (Just tested and works fine! smiley)

Yes, this will work (and glad it worked for you). This is the best way to go, unless you need to see the output from ssh.

Would you still like to try out a debug build where I can include a proof-of-concept fix for the bug? So that you can see how that will look like the way it's intended to work.

Sure! Would be great if you can precompile it.

If you want to try with VisualHostKey then you can get a compiled binary at https://github.com/tleedjarv/unison/actions/runs/4908531727 (scroll to end of page to see downloads). But even with this fix, the VisualHostKey=no option remains unchanged and is a smoother way for you.

kolAflash commented 1 year ago

If you want to try with VisualHostKey then you can get a compiled binary at https://github.com/tleedjarv/unison/actions/runs/4908531727 (scroll to end of page to see downloads). But even with this fix, the VisualHostKey=no option remains unchanged and is a smoother way for you.

Tested unison-9a46c232.ocaml-5.0.0.ubuntu-22.04 with VisualHostKey=yes set.
It shows the password dialog for about one second. Then it switches to the "OK dialog".

It would be nicer if the password dialog wouldn't even appear. But for me the the sshargs = -o VisualHostKey=no solution is sufficient.

tleedjarv commented 1 year ago

Tested unison-9a46c232.ocaml-5.0.0.ubuntu-22.04 with VisualHostKey=yes set. It shows the password dialog for about one second. Then it switches to the "OK dialog".

That's because it is not possible to tell when ssh is waiting for user input*. So the password entry is provided when any output from ssh is received. Once the connection is established the password entry is removed with the assumption that ssh is no longer waiting for input, but the dialog itself is kept to provide the information to user. That unfortunately means that the password entry is displayed for the duration of receiving output from ssh to the connection having been established.

[*] There is a theoretical way to at least guess what ssh is expecting, but that would probably not be foolproof and not portable between implementations (sshcmd preference can basically define anything)**.

[**] Not that there are any plans for it, just mentioning it here for completeness, there's always the possibility of not using an external ssh process at all, instead opting for something like libssh.

It would be nicer if the password dialog wouldn't even appear.

That is an UI improvement, but as explained above, it would still flash for a second if establishing the connection is not quick enough.

But for me the the sshargs = -o VisualHostKey=no solution is sufficient.

This is perfect. Please leave this ticket open because there still is a bug to be fixed.

tleedjarv commented 1 year ago

I have now opened a PR with a fix (of course, only needed if you keep VisualHostKey=yes).

It would be nicer if the password dialog wouldn't even appear.

That is an UI improvement, but as explained above, it would still flash for a second if establishing the connection is not quick enough.

To add to this, at least in this case, the information is explicitly requested by the user (by setting VisualHostKey=yes) so it would be wrong to hide it completely.

kolAflash commented 1 year ago

[...] To add to this, at least in this case, the information is explicitly requested by the user (by setting VisualHostKey=yes) so it would be wrong to hide it completely.

I need to see the visual key only when making a first connection to a server and the public key is not known to the client yet. I don't need to see it on every successive connection. But you can just either switch it on or off completly.

On the shell the ssh client also just asks you to acknowledge the key when it's not already known to the client. On successive connections it's just being shown without the need to acknowledge it.

gdt commented 1 year ago

ssh issues should be addressed to ssh :-) Sorry, but I don't want to add workarounds to unison to make VisualHostKey do what you want.

But the good news is that the fix is merged.

kolAflash commented 1 year ago

Thanks for your help! 😀