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

plumbing/transport/ssh: do not override HostKeyCallback if set #698

Closed

Conversation

SVilgelm
Copy link

@SVilgelm SVilgelm commented Mar 3, 2023

We use the custom Auth method that returns ClientConfig with custom HostKeyCallback to check a host against known fingerprints (kind of white list), but the SetConfigHostKeyFields function always overrides the HostKeyCallback to read the fingerprints from the files and etc...

Added a simple check fixes the issue by calling the SetConfigHostKeyFields only if the HostKeyCallback is nil.

We use the custom Auth method that returns `ClientConfig` with custom `HostKeyCallback` to check a host against known fingerprints (kind of white list),
but the `SetConfigHostKeyFields` function always overrides the `HostKeyCallback` to read the fingerprints from the files and etc...

Added a simple check fixes the issue by calling the `SetConfigHostKeyFields` only if the `HostKeyCallback` is `nil`.
@hiddeco
Copy link
Member

hiddeco commented Mar 3, 2023

Can you maybe extend the tests in plumbing/transport/ssh/common_test.go to cover for this scenario? Other than this, it looks good to me.

@pjbgf
Copy link
Member

pjbgf commented Mar 5, 2023

Duplicate of #655.

@SVilgelm
Copy link
Author

SVilgelm commented Mar 5, 2023

cool, then I close this in favor of #655, that implementation looks better than mine, thank you!

@SVilgelm SVilgelm closed this Mar 5, 2023
@SVilgelm SVilgelm deleted the do-not-override-HostKeyCallback branch March 5, 2023 17:08
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.

None yet

3 participants