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

[Jenkins-42959] support ECDSA keys #12

Merged
merged 1 commit into from Apr 10, 2017
Merged

Conversation

@mc1arke
Copy link
Member

mc1arke commented Apr 2, 2017

Trilead currently silently ignores any ECDSA public key entries found in known_hosts, doesn't support them for identifying a remote host, and can't use ECDSA keys in user public key authentication. This change provides this support, as well as various other commits to allow some abstraction and extension of provision of key types.

@mc1arke

This comment has been minimized.

Copy link
Member Author

mc1arke commented Apr 2, 2017

@paladox
paladox approved these changes Apr 3, 2017
@paladox

This comment has been minimized.

Copy link
Contributor

paladox commented Apr 4, 2017

Would this be able to be merged please?

@mc1arke

This comment has been minimized.

Copy link
Member Author

mc1arke commented Apr 4, 2017

I'd ideally like a second review from someone with knowledge of SSH and/or Trilead before I merge this.

@mc1arke mc1arke force-pushed the JENKINS-42959-support-ECDSA-keys branch 2 times, most recently from 3d27802 to a5548c8 Apr 5, 2017
@paladox

This comment has been minimized.

Copy link
Contributor

paladox commented Apr 5, 2017

@mc1arke ok :), should we wait 24 hours before merging this?

@mc1arke

This comment has been minimized.

Copy link
Member Author

mc1arke commented Apr 6, 2017

A minimum of 24 hours wait is just a rough guide I follow for anything other than a simple/obvious change. For a change where I've refactored code and introduced abstractions, as well as introduced new features, I'd generally like to give as much time as possible for people to chip in. However it looks like no-one from @jenkinsci/code-reviewers it wanting to provide any particular input, and I'm unlikely to get someone just wander by and have a look so I'm likely to just proceed with merging the outstanding requests when I have time to focus on it. For instance, this pull request will break the KnownHostsTest in one of the other pull requests if I don't make changes between merges, so I need to re-test each pull before I merge them.

@paladox

This comment has been minimized.

Copy link
Contributor

paladox commented Apr 6, 2017

Ok thanks :)

@MarkEWaite

This comment has been minimized.

Copy link

MarkEWaite commented Apr 6, 2017

I'd love to provide input, but I have other items which are more pressing right now. I won't be able to experiment with this change until next week at the earliest.

I am not an ssh expert, though I do have a wide variation of systems which I can use for various tests.

@mc1arke mc1arke force-pushed the JENKINS-42959-support-ECDSA-keys branch from a5548c8 to c2962b4 Apr 10, 2017
@mc1arke mc1arke merged commit d0178c2 into master Apr 10, 2017
1 check passed
1 check passed
Jenkins This pull request looks good
Details
@paladox

This comment has been minimized.

Copy link
Contributor

paladox commented Apr 10, 2017

:)

@mc1arke mc1arke deleted the JENKINS-42959-support-ECDSA-keys branch Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.