Feat: Provision for password entry in SSH

[chunky]

Summary For both "password" and "keyboard-interative" authentication types, this provides the ability to ask the user for a password just-in-time, avoiding the need for either configuration changes or secret storage.

In support of need for fallback options when SSH_AUTH_SOCK isn't set (Issue #1059) and needing password authentication more generally (Issue #294), I propose this change.

It does two things:

1. Adds an event handler for "keyboard-interactive", and, if a password is being required by the server, prompts user for a password
2. Modifies the event handler for connection error so that if it's an authentication error, then the user is given a chance to enter a password then the config is modified appropriately and the connection is re-initiated

Extra Notes about the "password" fallback The "password" authentication method, when requested by a server, is usually at the end of the priority list. But the "password" field is on the configuration passed to "connect()" on the connection object. It's not currently possible to delay asking for a password until needed with the SSH2 API [*]. So we have a choice:

1. Ask for password, every connection, unnecessarily [User Hostile]
2. Add something to the SSHProfile object that points to a password [eg by incorporating secrets management, or having a boolean "request_password" parameter] [User unfriendly, and potential attack surface]
3. Explicitly recognise when there was a connection error due to authentication, and let the user have another chance to enter a password.

I chose option 3, for this pull request.

[*] I have put in a feature request with SSH2 for this, here: https://github.com/mscdex/ssh2/issues/1400 . But, I have a relatively immediate need for this feature to work. If SSH2 implements my proposed change, then this module can be modified to reflect that, later. Until then, I believe this is an appropriate fix.

Testing These code changes are only in play when actually connecting to a remote server, and observing events that are triggered. I have tested this on my local system: {Keyboard-Interactive Enabled on server: On, Off} X {Password enabled on server: On, Off} X {Public Key Auth set up Properly: Yes, No} X {Entering a valid password: Yes, No}.

I'm unclear how to configure tests where interaction with the remote server is key [and none of the other SSH tests are set up to do this either]

Signed-off-by: Gary Briggs chunky@icculus.org

[chunky]

@smorrisj this is a really meaningful patch in terms of improving usability for SAS where I work. If it's possible for this to be reviewed and, ideally, merged before the next release, that would be a big deal for us in terms of adoption. Please let me know what specifically I can do to move it along, if anything.

[smorrisj]

@chunky thanks for the PR. Apologies for the delay, I was out of the office last week.

I think that the best solution with regards to setup friction and ease of use would be option 2 in the long term. VSCode has a secret storage api that we make use of to store secrets for the other connection types for viya and IOM. I think that ideally we use the same JIT approach to prompt for the password that we're currently using for IOM. We created #294 to track this a while back. Typically, in the ssh2 library, the authHandler property is used to control the different types of auth names that are supported during handshake. The obvious downside (which it looks like you ran into on the ssh2 issue) is that it requires writing custom code to define the names for them, or a custom function that can use the supported callbacks.

I think that if we can implement first class support for #294, then we should also rollout explicit fallback support. It seems like there could be value in allowing advanced users to determine what that order looks like? The default chain is: None -> Password -> Private Key -> Agent (-> keyboard-interactive if tryKeyboard is true) -> Hostbased

Tagging @snlwih for awareness and as input to current conversations around enhancing SSH Connection support.

[chunky]

I tried to create something that did the password entry JIT with secrets manager, and couldn't get it to work. Something was awry, and I know it's my fault, but I couldn't figure out what.

I also coded something that uses the authhandler, based on feedback from the ssh2 maintainer. You can find the code in a separate branch here: https://github.com/sassoftware/vscode-sas-extension/commit/d276e0c27026fa18afb9ebbb1fb61824016af10d The flow is a little awkward [and almost impossible to meaningfully test], but it does get the job done. If you want to implement that, then I would also recommend rolling in something for unshielded identity files since [as you see in the code there], it can work but it's commented out.

I guess another way to put this is... what can I do, or say, or code, that culminates in something getting into any of the next few releases that means my windows users don't need to deal with SSH_AUTH_SOCK? We cannot install the windows SSH stuff on our laptops as described, and overall for windows the whole thing is a bit too user-hostile for most of my potential users.

[smorrisj]

I guess another way to put this is... what can I do, or say, or code, that culminates in something getting into any of the next few releases that means my windows users don't need to deal with SSH_AUTH_SOCK? We cannot install the windows SSH stuff on our laptops as described, and overall for windows the whole thing is a bit too user-hostile for most of my potential users.

Improving the SSH (remote) connection type is high on our list of items for the next few releases (even before these PRs came in). Inside of that enhancement effort, it's fair to say that adding proper username/password support is the highest item we'd want to get in so that we can ease the setup friction for this connection type and reach more users.

If it's agreeable, can we close this out, and open a separate PR to implement option 2 above? I can open it. The auth handler changes look interesting. Agreed though that the testing effort might get tricky. I think there are some good changes here though, that we should consider cannibalizing into the other PR. Thoughts?

[chunky]

I'm absolutely not precious about my code, or PRs. If you want to butcher/cannibalise whatever you can from what I've done, please, feel free. I'm flattered you might find it useful.

For our users, I also note that unshielded private keys are in prolific use for day to day ssh, as the next step beyond password. Handling that, in addition to password, would really give good reach across the community.

My time issue is that I currently have a SAS infrastructure project to roll out viya. This extension is so good, I also want to get our 9.4 / grid users using it, and I'm sneaking this engagement in to the viya thing. But once the viya project completes, I'll have to move on.

[smorrisj]

Opened #1126 to implement username/password support for SSH. I put some initial thoughts in TODOs based on conversations here. @chunky do you mind double checking those comments to ensure we've captured the different items that were discussed here?

[snlwih]

@chunky , now that we're planning for user/pass support for SSH connections (#1126), which also includes some slight internal rearchitecture, can we close this PR?

@smorrisj , do we need a dedicated feature request to ask for fallback to user/pass when key-based authentication fails for SSH, which could be implemented once #1126 is in place? You mention it in one of your earlier comments (https://github.com/sassoftware/vscode-sas-extension/pull/1081#issuecomment-2242857277) in this PR.

[smorrisj]

@smorrisj , do we need a dedicated feature request to ask for fallback to user/pass when key-based authentication fails for SSH, which could be implemented once #1126 is in place? You mention it in one of your earlier comments (#1081 (comment)) in this PR.

I dont think so. The plan was to implement this refactor to use fallback methods as part of #1126. The goal would be to get the list of supported auth methods that the server can support as part of the handshake with the server, and then diff that list against what we can support in the extension. As part of that PR we would also add back in support for specifying identify files. It turns out that the ssh2 library we use has a built-in function for detecting encrypted private keys, so in my draft of changes on #1126, I've added in an extra feature that would detect the encryption and prompt the user to enter the passphrase in the scenario that a private key is specified in the SAS Profile Connection, but it has a passphrase on it. In setting up the auth handler this way, we could support users that:

1. Want to use the ssh agent for all key setup and auth (current method that we support, can setup passphrase or no passphrased private keys with the agent).
2. Do not want to use the ssh agent, and instead specify their private key in the SAS Connection Profile json. This method supports passphrased and non passphrased private key files. This is something that @chunky mentioned above that would ease setup friction.
3. Want to not use any of item 1 or item 2 above, and just want to use a simple username and password to authenticate.

[chunky]

@snlwih Yes, please feel free to close this PR. I'm not precious about my code - I just want this capability, and since you're implementing it elsewhere, then you can close this.

[snlwih]

As discussed above, closing this as the capability will be implemented as part of pull request #1126