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

Resolve ssh hostname aliases before running ssh-keyscan #889

Merged
merged 9 commits into from
Jan 11, 2019

Conversation

ticky
Copy link
Contributor

@ticky ticky commented Jan 10, 2019

This is a PoC to improve compatibility with ssh aliases which don’t conform to our special-casing behaviour.

For any incoming ssh host, this asks ssh for what host it will connect to. This allows ssh-keyscan to connect to that host, rather than whatever the alias name is, which may or may not match.

It also supersedes the older behaviour wherein our special-case aliases were stripped, and on agents which are already correctly configured, should work the same.

I haven’t been able to get the test suite working to confirm this works, but I figured putting the sketch up would be better than doing nothing! 😃

I’ve updated this to pass, and add new, tests, and it’s looking good!

For reference, ssh -G appears to have been added in OpenSSH in 6.0 (released April 22, 2012, sadly no source link as this took gripping multiple tgz files to bisect), but it appears to be turned off in some OS’ distributions of 6.x (one of our virtual servers is running a version from 2014, but it’s missing) so the existence of the fallback remains potentially important.

@ticky ticky requested a review from lox January 10, 2019 03:38
bootstrap/git.go Outdated Show resolved Hide resolved
Co-Authored-By: ticky <hello@jessicastokes.net>
bootstrap/knownhosts.go Outdated Show resolved Hide resolved
Co-Authored-By: ticky <hello@jessicastokes.net>
@lox
Copy link
Contributor

lox commented Jan 10, 2019

This looks awesome @ticky! I was wondering when that github alias stuff would bite us.

@ticky
Copy link
Contributor Author

ticky commented Jan 10, 2019

I’m sort of delighted that my new code has basically not needed an edit, it was only supporting stuff and tests 😃

@lox
Copy link
Contributor

lox commented Jan 10, 2019

You are also the first other human to use bintest :)

@lox
Copy link
Contributor

lox commented Jan 10, 2019

Looks good to go to me @ticky!

return fmt.Sprintf("%s:%s", hostname, port)
}

// if we got here, either the `-G` flag was unsupported, or ssh -G
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we should emit a warning here… 🧐

@ticky ticky merged commit d147148 into master Jan 11, 2019
@keithduncan keithduncan deleted the git-ssh-alias-resolution branch September 22, 2021 03:51
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

2 participants