Unbreak SSH agent support for SSH VPN tunnels.
ClosedPublic

Authored by catherinez on Jul 10 2018, 10:49 PM.

Details

Summary

Right now selecting SSH agent as authentication method for SSH VPNs
results in a password dialog being shown and then a failure no matter
what is entered. This is because the agent authentication method
does not expect a password to be returned but instead expects
a path to the agent socket to be sent. The upstream nm-ssh implements
this, but KDE's plasma-nm does not.

This change implements the behavior that nm-ssh-service expects
from the frontend, and allows using SSH agent authentication with
SSH VPNs set up by plasma-nm.

I fully admit that this change is a bit hacky in that it hardcodes nm-ssh
specific functionality in the core of plasma-nm, but I feel it could be
fine for the following reasons:

It fixes completely broken functionality at a relatively low cost.
There is similar hardcoded behavior already e.g. for OpenConnect
in PasswordDialog::initializeUi().
Doing this properly requires a major refactor of plasma-nm, that is
pulling VpnUiPlugin creation into SecretAgent instead of
PasswordDialog where it is now, and I have neither time nor
grasp of plasma-nm codebase to do this.

Fixes https://github.com/danfruehauf/NetworkManager-ssh/issues/37.
Fixes https://github.com/danfruehauf/NetworkManager-ssh/issues/54.

Diff Detail

Repository
R116 Plasma Network Management Applet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
catherinez created this revision.Jul 10 2018, 10:49 PM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptJul 10 2018, 10:49 PM
catherinez requested review of this revision.Jul 10 2018, 10:49 PM
jgrulich added inline comments.Jul 13 2018, 1:31 PM
kded/secretagent.cpp
363

Use QLatin1String()

367

When SSH_AUTH_SOCK is empty, it shouldn't continue asking for the password, you should instead report an error.

372

Use QLatin1String()

catherinez updated this revision to Diff 38850.Jul 31 2018, 3:24 PM

Added QLatin1String() where requested, handled the case of empty SSH_AUTH_SOCK.

catherinez marked 3 inline comments as done.Jul 31 2018, 3:24 PM

Done.

jgrulich requested changes to this revision.Aug 1 2018, 5:30 AM
jgrulich added inline comments.
kded/secretagent.cpp
370

You should also return true to indicate that the request is completed, otherwise it will still continue.

This revision now requires changes to proceed.Aug 1 2018, 5:30 AM
catherinez updated this revision to Diff 39036.Aug 3 2018, 8:24 PM

Complete request when SSH_AUTH_SOCK is not present.

catherinez marked an inline comment as done.Aug 3 2018, 8:24 PM

Done.

jgrulich accepted this revision.Aug 6 2018, 6:05 AM
This revision is now accepted and ready to land.Aug 6 2018, 6:05 AM

Thank you for your contribution.

This revision was automatically updated to reflect the committed changes.