Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for ed25519 authentication #1220

Closed
wants to merge 5 commits into from

Conversation

9072997
Copy link

@9072997 9072997 commented Jun 8, 2021

Description

This adds support for connecting to servers that require ed25519 authentication.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
    • I don't think this is necessary, since it does not appear that the supported authentication methods are listed anywhere
  • Added myself / the copyright holder to the AUTHORS file

Reference implementations:

The ed25519 implementation was taken from here because it was portable C (and therefore easy to make work with cgo) and because it was very close to the code the reference C implementation was derived from.

The biggest issue with this is that it introduces a dependency on cgo. I spent a fair amount of time trying to get this going with the ed25519 implementation from the standard library (crypto/ed25519), but I lack the cryptography knowledge to figure out what is happening here on a high level so I can use the corresponding high-level functions from crypto/ed25519.

If cgo is a deal breaker (and there is not someone else on the team with the knowledge/time to implement this in pure go), would it be worth exploring an API to allow additional authentication methods as "plugins"? I could then implement this as a plugin and only users who needed ed25519 would have to worry about cgo.

@9072997 9072997 marked this pull request as ready for review June 8, 2021 03:32
@9072997
Copy link
Author

9072997 commented Jun 11, 2021

I may have discovered an issue with this compiling under Linux. I will take a look at it this weekend.

@9072997
Copy link
Author

9072997 commented Jun 14, 2021

this compiles under Linux now

@ldanielw1
Copy link

ldanielw1 commented Mar 2, 2022

Thanks so much for working on this! Was wondering if there were any updates on this effort? Would be very interested in seeing this support added into go!

@methane
Copy link
Member

methane commented Mar 2, 2022

I'm sorry, but I am very busy these days so I check only issue tracker.
Merging new features is postponed.

By quick looking, cgo is deal breaker. I won't merge this pull request. Please use @9072997's fork if you really need the ed25519 plugin.

@9072997
Copy link
Author

9072997 commented Mar 3, 2022

Since cgo is a deal breaker I'm going to go ahead and close this PR. I'll open another if I ever manage to get this going in pure go.

@9072997 9072997 closed this Mar 3, 2022
@Gusted Gusted mentioned this pull request Dec 3, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants