Skip to content
This repository was archived by the owner on May 20, 2024. It is now read-only.

Conversation

@kins-dev
Copy link

This brings in a new version of SSH.NET.

Beyond the typical performance improvements and security fixes, this adds:

  • OpenSSH key compatibility
  • ED25519 support
  • Pagent support

The last feature is the most interesting. If you use a plugin such as KeeAgent, you can keep your SSH keys stored in one database that you must open before you can access a second database. For example lets say you have a database kept on premises behind ssh for IT purposes. And then let's imagine that this database is accessible by at least 2 people. (Primary support and backup.) You could share a password but that's not a great security practice. Instead, it is better to create individual ssh keys so access can be revoked at a per user level. Each user can then store their personal ssh key in their own personal database.

@shellster
Copy link
Owner

Thank you. I'll review shortly, and merge.

@kins-dev
Copy link
Author

Sorry, just realized some of the files in here are actual in the SSH.Net core. I was testing client vs core authentication, and left some files in place, give me some time to clean it up.

@kins-dev
Copy link
Author

Yes sorry about that, if you're reviewing the original commit, the first 7 files (the ones not in the sftp namespace) go away with my latest commit.

@shellster
Copy link
Owner

Looks good. Thanks so much for the addition. I'm gonna pull this and test it to verify. Then merge the changes separately so I can bump the versioning and give you credit in the readme. Once I've done that, I'll post back here and then close the pull request.

@shellster
Copy link
Owner

@kins-dev I'm looking at your changes, and I see you have committed a binary for the SSH.NET library. I'm leery of committing an unknown binary lib because of the potential security risks. I'd much rather rebuild it myself after reviewing those changes, just to be more certain of the origin. Looking at SSH.NET, it looks like your Pagent changes to SSH.NET has not yet been merged. Is this true? If so, can you point me to the branch or PR that I should review/build and include with these changes? Can you also write up a brief explanation of how to use the Pagent changes, so I can add them to the README? Last question, I for one, use Keepass with Mono, so it is important to me to make sure that this plugin can at least not interfere with that. Will your Pagent changes work in Linux/Mono? If not, will they at least not cause the rest of the features to be non-functional?

@kins-dev
Copy link
Author

I appreciate your caution.

So pagent with mono isn't going to work out so well, because unless you're running wine, there isn't a pagent provider (Linux uses openssh sockets instead of pagent). I was just mentioning in this PR, the issues around that.

So I might need to put some guards around that in particular.

That said this is a fall back if a password isn't provided and a key file isn't provided, so it shouldn't break mono users. I haven't tested that specifically.

The source I compiled from is from this commit and can be pulled here

@shellster
Copy link
Owner

Closing this merge as the changes are merged (As mentioned above, I rebuilt with versioning changes, updated readme, and rebuilt your branch of SSH.NET after reviewing the changes and testing). Please confirm when you have a chance and reopen if you experience any issues.

@shellster shellster closed this Jul 15, 2020
@shellster
Copy link
Owner

I think I may not have tested as thoroughly as I should have. Latest version of the plugin appears to be failing, at least under linux/mono, for sftp with key and password. I'm gonna try to dig into it and figure it out.

@kins-dev
Copy link
Author

Key text, not a key file? (Looking back at the changes I added that as well.)

Only real difference I see, provided it isn't going down the wrong branch of logic, is the using statement on the memory stream so it get's cleaned up once it should be finished.

I'd be really surprised if PrivateKeyConnectionInfo was somehow attempting to use the closed stream. Do you have a stack trace or anything you can share?

@shellster
Copy link
Owner

I'm on vacation the rest of this week so I may or may not get back to this. I'm not sweating it too bad, as old releases are still available if anyone else runs into the problem. If I don't get a chance to look at it sooner, I'll get to it next week. Doesn't appear to be crashing, just fails similar to if the server wasn't available. Might be that I compiled with the long version of .NET or something as it appears to be working in Windows.

@shellster shellster reopened this Jul 17, 2020
@shellster
Copy link
Owner

Hi @kins-dev ,

Sorry it took me so long to get back to this.
I think I have figured out all the issues that broke mono support when I pulled your Pageant stuff int 2.3.0.
There were a couple minor logic bugs that I've fixed here: https://github.com/shellster/keepass-sftp-sync/tree/2.3.1-Fixes

The big issues appears to be that the latest version of SSH.Net has made some changes that are incompatible with Mono. I haven't been able to fully run them to ground but I think it has to do with some known issues in Mono around System.Net.Webclient.OpenRead and their current implementation.

Bottom line, there does not appear to be an easy way to reconcile this at the moment. I was looking back over your Pageant changes, and it appears to me that all your changes are additions. That is to say, all of your support was implemented as new classes and did not require any changes to the existing SSH.net library. I was wondering if you might be willing to spin a version of your Pageant support as a separate library that just references SSH.net? If you do this, I can easily roll it into SftpSync with an older version of SSH.net that works cross-OS.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants