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

Directly expose the SSH server KEXT, MAC and Cipher algorithms #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

belak
Copy link
Collaborator

@belak belak commented Nov 2, 2018

I believe this is one of the last pieces needed for go-gitea/gitea#3896

There may be a better interface for this, but it looks like this was designed so you can change the server config in the middle of running so there's a little extra code to make sure the slice gets copied and not just referenced.

@belak belak requested a review from progrium November 2, 2018 18:43
@belak
Copy link
Collaborator Author

belak commented Nov 2, 2018

Note that I may end up dropping this if we don't need this for gitea.

Copy link
Member

@shazow shazow left a comment

Choose a reason for hiding this comment

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

In x/crypto/ssh, these are buried/bundled under another struct layer of Config: https://godoc.org/golang.org/x/crypto/ssh#Config

Would it be a good idea to do that here as well? Especially since most people won't need them.

Would it make sense to use the original ssh.Config type and pass it along?

@belak
Copy link
Collaborator Author

belak commented Nov 2, 2018

I did consider exposing the raw SSH config. I suppose that makes more sense since it should only be needed in very specific cases.

Do you have any opinions on copying the config vs just referencing it? I find it a little odd that we create a new server config for every connection.

Thanks for your comments!

@shazow
Copy link
Member

shazow commented Nov 2, 2018

I don't have strong feelings either way, just an idea. :)

@progrium
Copy link
Contributor

progrium commented Nov 2, 2018

I could go either way. At the moment at least.

@belak
Copy link
Collaborator Author

belak commented Nov 3, 2018

I'd like to clean up some of the config handling and make it per call to listen, but that doesn't need to happen right now.

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