Open mitchcapper opened 8 years ago
I don't recall having flat out rejected Pageant support. I would prefer packaging it separately from the core SSH.NET library, but that's not to say that it can't live under the SSH.NET "umbrella" and receive the same amount of love and support.
Sure, so there are two options I see(but would be happy to hear of others): 1) Modular auth support then we can make an paegent auth plugin dll, but as SSH.NET is a library something on its own this may not be the best way to go 2) As there are separate projects and we are talking about only adding this feature to the .NET35 build (as I think that's probably the only it would work on) it would be segmented out from the central code as it is. In addition the user would still need to load that agent in with a call so it wouldn't just auto-try paegent auth.
I am happy to put a PR together, in whatever form you would prefer it in but I didn't want to waste time on something that wouldn't be accepted as its already working for my solution (https://github.com/mitchcapper/NovaSFTP2).
Do you care to elaborate on why you consider the first option "not the best way to go" ?
Thanks!
Well it requires an additional DLL to be generated increasing the requirements for the user to include both (and another nuget package if not bundled in). Its not a big deal but do you want one dll per additional module (not sure if there are any other modules in mind)?. If not just one "Extras" dll? Do you want to define the interface for auth modules or are you find with a PR including that? Do you want the module part of the official repo or just 3rd party maintained?
I don't have strong preferences on any of these things I just want to avoid doing work that is going to be redone so want a strong direction on what you are looking to accept.
I'd prefer a separate nuget and repo, like I've done for (some of) our crypto stuff. That repo can be part of the sshnet organization, to make it easier to discover and to make it clear that it's the same team providing support for both. Even though I prefer a separate nuget, I'm pretty sure we'll have to release new versions of this nuget package in sync with the main SSH.NET package itself.
I haven't yet checked out your code yet, but I'll definitely want to limit changes to SSH.NET itself to the bare minimum. Once these changes are clearly identified, we can discuss more refactoring or even decide to integrate the authentication module in SSH.NET anyway.
I can't and won't ask for a long term commitment to the SSH.NET project, but I do expect the code to be clear, well-written and - where necessary - documented.
You can start by submitting a PR that defines the interface, but please consider it as a work document. I can't garantee that we won't redo it multiple times.
I'm currently working on finishing the .NET Core support and getting the 2016.0.0 release stable, so I may not be very responsive. Please bear with me :)
Hi, this is note to the first comment because is not up-to-date. WinSSHFS currently use SSH.NET from here with several changes as submodule and you can find it here: https://github.com/Foreveryone-cz/SSH.NET-for-winsshfs Plan is to rebase often to SSH.NET master branch.
To add pageant support to your custom builds shoud be easy with one cherrypick of this commit: https://github.com/Foreveryone-cz/SSH.NET-for-winsshfs/commit/34719699c2d46d389df95080707c8622d9143e82
If I may add a +1 to this. It doesn't really matter to me how it is packaged. But both support for Putty formatted key files and pagent would be really good. Putty formatted keys are [annoyingly] dominant on windows platforms and compatibility is useful. Especially for those users who seek to have a single key per user / machine and not user / machine / application.
This doesn't compile on .NET Core due to lots of missing features.
"Updated" (hacked) the patch here: #536
Going to mention this (https://github.com/sshnet/SSH.NET/compare/develop...kins-dev:develop). For some reason this also got rejected. I think that possibly the best solution would be to roll this patch as a separate library. The main problem is that certain SSH.NET methods and structures are currently too restricted to be accessible. If we could get these marked as public, then this pull could be refactored as an external library. Thoughts @drieseng? Currently I jumped the gun and accepted a pull for pageant support to a SCP plugin for Keepass that is based on this SSH.NET library. At the moment, I'm having to maintain a fork of this library to continue the Pageant support, so I've a vested interest in finding a way to make this work for everyone.
Hey all, is there any buy-in on a path forward for this? I'm happy to help mint a PR if we can get directions from anyone that's part of the SSH.NET team on how they want to approach this. As I mentioned in my previous message, the kins-dev PR opened up access to some SSH.NET internals, but if that is unacceptable, and alternative approach would be to implement more granular hooking of the authentication process in SSH.NET which would potentially allow a lot of similar style solutions in the future. My project which relies on SSH.NET is currently hanging in limbo where I have to pull in copies of SSH.NET and patch and build them for each release, and that really isn't sustainable or smart, long term, so I'm eager for an opportunity to work with the SSH.NET team to figure out how to do this "correctly".
@shellster As I previously mentioned, I'd first like to see what the minimal interface is that we'd need to expose to allow for Pageant support. My preference remains to ship Pageant support (and other OS or TFM specific functionality) separately from the "core" SSH.NET library.
@drieseng Thank you for getting back to me. I will research this, this next weekend, and get back to you with my proposal. Last time I did this, there were only very minimal changes needed, mostly exposing one or two data types, and a couple methods. I want to recheck this though, to make sure that this is still the case.
@shellster Thanks, and sorry if I caused misunderstandings.
Please see the above PR. These are the minimal changes that I need to support Pageant style authentication by simply opening up some protected/internal/private classes. I do not know if this is the "best" way to proceed. For instance, to create a similar Kerberos Authentication plugin would likely require opening up some additional event handlers in Session/ISession.
Also not a big fan of how I currently have to directly write data back and forth to the Session channel: https://github.com/shellster/PageantPlugin/blob/e92f047fc0267ded9d3465c8987d92d106d338fc/AgentAuthenticationMethod.cs#L70
In short, for my needs, the above PR is good enough. For maximal support in this fashion, basically everything in Session/ISession needs to be made public, as well as any other objects and classes that must be exposed as a consequence. Long term, it may make more sense to consider providing a more robust plugin framework, where plugins can return data whenever certain events are fired instead of giving them direction access to Session Channels to read/write.
@drieseng When you get a chance would love to get your thoughts on the previous PR and a possible long term strategy.
@shellster I tried a different approach to extend SSH.NET with OpenSSH Agent Authentication. Instead of reimplementing Pubkey-Auth, it adds own Keys with a DigitalSignature, which asks the Agent to sign data.
Reduces the needed changes in SSH.NET to a new ctor for PrivateKeyFile.
https://github.com/darinkes/SshNet.Agent
IMHO thats the best approach, since Agents are just a storage for PrivateKeys, so reimplementing the whole Publickey-AuthenticationMethod doesn't make sense. As already mentioned its too complicated and also its too much unnecessary duplicated code.
Edit: To be more verbose:
The best approach would be to keep a common object everybody knows and can use. For everything regarding KeyAuthentication that would be PrivateKeyFile.
This way we can add extensions and they are compatibel, cause they use the same Interface/Object.
I'm currently Working on two Extensions: SshNet.Keygen and SshNet.Agent
Both can be used together, for example to Generate a Key, store it in your Agent and use it for Authentication while showing the Key-Fingerprint and PublicKey. Just because both use the base PrivateKeyFile- and Key-Objects.
Example:
using var agent = new Agent();
// Generate the key
var key = SshKey.Generate<ED25519Key>();
// store it in your agent
agent.AddIdentity(key);
// show the public bits
Console.WriteLine(key.ToOpenSshPublicFormat());
Console.WriteLine(key.Fingerprint());
// get the keys from the agent
var keys = agent.RequestIdentities().Select(i => i.Key).ToArray();
// Connect
using var client = new SshClient("ssh.foo.com", "root", keys);
client.Connect();
Talking about other Authentication-Methods thats a whole different story. But in this case its just about Pubkey-Authentication, no matter if its a file, agent, smartcard or something else. And there is no need to expose and reimplement it.
@darinkes I will take a look when I get a chance, but I don't think your solution will work with Pageant, as I understand it, you don't have access to the keys, you have to basically hand off part of the handshake to Pageant to sign and then send that back to the server. So, I think I have no choice but to have access to the actual raw authentication messages.
Looks to me its 100% the same as for OpenSSH Agent:
https://github.com/shellster/PageantPlugin/blob/master/PageantProtocol.cs#L88
vs.
Sorry, was quick typing. Ok, I read the PageantPlugin code and the same approach will also work.
Instead of creating a whole new AuthenticationMethod just to do this: https://github.com/shellster/PageantPlugin/blob/main/AgentAuthenticationMethod.cs#L82
You can create your own DigitalSignature: https://github.com/darinkes/SshNet.Agent/blob/main/SshNet.Agent/Keys/AgentSignature.cs
And create a "PageantKey", which overrides the DigitalSignature of Key: https://github.com/darinkes/SshNet.Agent/blob/main/SshNet.Agent/Keys/RsaAgentKey.cs#L13
Pass your PageantKey to PrivateKeyFile and you should be good to go. SSH.NET will call your Sign() of "PageantDigitalSignature" at the same spot without reimplementing pubkey-auth: https://github.com/darinkes/SSH.NET-1/blob/agent_auth/src/Renci.SshNet/PrivateKeyAuthenticationMethod.cs#L80
For testing you can use this Branch, which already contains a PrivateKeyFile-ctor for direct Key-Setting: https://github.com/darinkes/SSH.NET-1/tree/agent_auth
@shellster I picked up the Code from the PageantPlugin to SshNet.Agent:
https://github.com/darinkes/SshNet.Agent/tree/main#putty-pageant
My test sample also runs with Pageant now. Simply exchange new Agent()
with new Pageant()
.
As expected Pageant implements the Agent Protocol, just like OpenSSH Agent. So basically just needed to switch Streams to make it work.
So its possible to have one SSH.NET Extensions for Agent-Auth, which works for OpenSSH agent and Pageant.
Lets wait for @drieseng if he is OK with this Plan and the needed new ctor for PrivateKeyFile.
@darinkes Thanks for taking this and running with it. I finally had a chance to take a look, and it looks really good. I concur that this is the best path forward. @drieseng I would also like to see @darinkes version and PrivateKeyFile change.
After working on some other SSH.NET Extensions, including a PuTTY Private Key Reader, I now opted for an PrivateKeyFile-Interface. This allows extensions to have a clean and identical interface to SSH.NET, without changing too much in SSH.NET itself.
e.g.:
var key = new PuttyKeyFile("my-key.ppk");
using var client = new SshClient("ssh.foo.com", "root", key);
client.Connect();
See the WIP Extensions:
@drieseng Can we please get the required changes merged so we can use all these new plugins?
Is this going to get merged or is it just dead in the water?
Previously it was rejected to add paegent support. Code like: https://github.com/dimov-cz/win-sshfs has several good changes. Any plans to re-consider supporting those things? PR could be put together now that it is on GitHub.