Skip to content

Agent auth and Keygen - #794

Merged
drieseng merged 5 commits into
sshnet:developfrom
darinkes:agent_auth
Mar 5, 2022
Merged

Agent auth and Keygen#794
drieseng merged 5 commits into
sshnet:developfrom
darinkes:agent_auth

Conversation

@darinkes

@darinkesdarinkes commented Mar 9, 2021

Copy link
Copy Markdown
Collaborator

Its based on the openssh_format_rsa branch, so ignore the first three commits.

"Allow to set PrivateKeyFile Key directly" and "Make ECDSA Key Bits accessible" Commits are needed for the WIP SSH.NET Extensions https://github.com/darinkes/SshNet.Keygen and https://github.com/darinkes/SshNet.Agent.

"Enable netstandard2.1 build" is a build fix, which respect netstandard versions newer than 2.0 for ECDSA Keys

@darinkes
darinkesforce-pushed the agent_auth branch 3 times, most recently from be269df to ce48377CompareMarch 14, 2021 18:09
@darinkes
darinkesforce-pushed the agent_auth branch 2 times, most recently from d1a1cd5 to f7fad26CompareAugust 16, 2021 15:20
@darinkes

darinkes commented Aug 16, 2021

Copy link
Copy Markdown
CollaboratorAuthor

Hey @drieseng,

Would be awesome if you could take a look here. A merge and release of SSH.NET would allow me to finalize my planned SSH.NET Extensions.

Thanks in advance

IgorMilavec
IgorMilavec reviewed Feb 20, 2022
View reviewed changes
Comment threadsrc/Renci.SshNet/Security/Cryptography/EcdsaDigitalSignature.cs Outdated
@IgorMilavec

IgorMilavec commented Feb 20, 2022•
edited
Loading

Copy link
Copy Markdown
Collaborator

Apart from minor conditionals change (see above) this code looks great to me.
I have found some other private key related PRs: #468, #595, #577, #468 and #179. Could you have a look if they affect IPrivateKeyFile design in any way?

Edit:
Found #705 and #781 which tackle Pageant support. Do you think we could have an extensibility interface that would support not only files (with direct access to key bytes) but also agents?

@darinkes

darinkes commented Feb 22, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Apart from minor conditionals change (see above) this code looks great to me. I have found some other private key related PRs: #468, #595, #577, #468 and #179. Could you have a look if they affect IPrivateKeyFile design in any way?

Can't see any conflict.

Edit: Found #705 and #781 which tackle Pageant support. Do you think we could have an extensibility interface that would support not only files (with direct access to key bytes) but also agents?

This extension uses the same interface: https://github.com/darinkes/SshNet.Agent (Includes SSH-Agent und Pageant)
For SSH there is no difference between a Keyfile or Agent.

@IgorMilavec

IgorMilavec commented Feb 22, 2022

Copy link
Copy Markdown
Collaborator

Great! Can you please change the #if so we can ask Gert if this could be merged before the final release with legacy targets?

@darinkes

darinkes commented Feb 22, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Great! Can you please change the #if so we can ask Gert if this could be merged before the final release with legacy targets?

I actually think it should be an extra issue and PR. Or is there a reason we need it now?

@IgorMilavec

IgorMilavec commented Feb 22, 2022

Copy link
Copy Markdown
Collaborator

With the current condition, the implementation of EcdsaDigitalSignature will revert to pre-netstandard implementation when/if we target net5.0 or greater. Of course when we raise the target, we should check all the conditional compiles if they are affected. So I guess in the end it's a matter of personal preference. :)

@IgorMilavec

IgorMilavec commented Feb 22, 2022

Copy link
Copy Markdown
Collaborator

@drieseng can you please have a look at this PR? It provides a great extensibility point and would either close or support closing many issues.

@drieseng

drieseng commented Feb 24, 2022

Copy link
Copy Markdown
Member

@IgorMilavec I'll try to find time for this sometime this week. Sorry for the lag.

drieseng
drieseng requested changes Feb 27, 2022
View reviewed changes
Comment threadsrc/Renci.SshNet/Security/Cryptography/EcdsaDigitalSignature.cs Outdated
Comment threadsrc/Renci.SshNet/Security/Cryptography/EcdsaKey.cs Outdated
Comment threadsrc/Renci.SshNet/PrivateKeyFile.cs Outdated
Comment threadsrc/Renci.SshNet/PrivateKeyFile.cs Outdated
/// </para>
/// </remarks>
public class PrivateKeyFile : IDisposable
public class PrivateKeyFile : IPrivateKeyFile, IDisposable

@driesengdriesengFeb 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to make this one sealed to allow the JIT to devirtualize calls?
Not a big deal.

Comment threadsrc/Renci.SshNet/IPrivateKeyFile.cs Outdated
/// <summary>
/// Represents private key file interface.
/// </summary>
public interface IPrivateKeyFile

@driesengdriesengFeb 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing file-related in this interface. Do we want to extend it with file related properties or methods, or change the name of the interface?

@darinkesdarinkesFeb 28, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe IPrivateKeySource?

@darinkes
Allow to set PrivateKeyFile Key directly
5b1edbd
So you can add your own Key-Classes to SSH.NET
Add ED25519 ctor for just pub key part.
@darinkes
Make ECDSA Key Bits accessible
7e972ce
You cant export imported CngKeys. To be able to export
them to agent or Key-Files make the private bits also
accessible.
@darinkes
Better NETFRAMEWORK vs NETSTANDARD handling
080a268
@darinkes
Add Comment Property to Key
526707f
@darinkes
Add IPrivateKeySource
1ac1a28
So Extension can add own PrivateKeyFiles, e.g. PuttyKeyFile.
@drieseng
drieseng merged commit bc99ada into sshnet:developMar 5, 2022
@drieseng

drieseng commented Mar 5, 2022

Copy link
Copy Markdown
Member

@darinkes Thanks!

@darinkes

darinkes commented Mar 5, 2022

Copy link
Copy Markdown
CollaboratorAuthor

@drieseng also thanks! I will update my extensions ASAP :)

This was referenced Mar 23, 2022
Closed
Closed
@Kim-SSiKim-SSi mentioned this pull request Apr 26, 2023
Closed
drieseng pushed a commit that referenced this pull request May 24, 2023
@darinkes@drieseng
Agent auth and Keygen (#794)
e727f4a
* Allow to set PrivateKeyFile Key directly
So you can add your own Key-Classes to SSH.NET
* Add ED25519 ctor for just pub key part.
* Make ECDSA Key Bits accessible
You cant export imported CngKeys. To be able to export them to agent or Key-Files make the private bits also accessible.
* Better NETFRAMEWORK vs NETSTANDARD handling
* Add Comment Property to Key
* Add IPrivateKeySource
So Extension can add own PrivateKeyFiles, e.g. PuttyKeyFile.
@WojciechNagorskiWojciechNagorski added this to the vNext milestone Sep 13, 2023
This was referenced Aug 12, 2026
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Open
Open
Merged
Open
Merged
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@driesengdriesengdrieseng approved these changes
+1 more reviewer
@IgorMilavecIgorMilavecIgorMilavec left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2023.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@darinkes@IgorMilavec@drieseng@WojciechNagorski@stevesobol@zybexXL