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-75214] Added prioritization of SSH host key algorithms #1048

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

Conversation

jmdesprez
Copy link
Contributor

Fix for https://issues.jenkins.io/browse/JENKINS-75214

When a HostKey already exists for the Computer, the algorithm is used as the preferred one to initiate the connection.

🚨 ssh-ed25519 support

Please note that, as for now, mina doesn't support ssh-ed25519 out of the box. This support will be added in a future release: apache/mina-sshd#657.

In the meantime, it is require to install https://plugins.jenkins.io/eddsa-api/ for ssh-ed25519 to be supported.

Testing done

Note: I moved MockEC2Computer to use it in SSHClientHelperTest

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@@ -0,0 +1,143 @@
package hudson.plugins.ec2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: refactoring, I extracted this class

@@ -337,126 +327,6 @@ private ConnectionAttempt build(MockEC2Computer computer, ServerKeyVerifier veri
}
}

// A mock ec2 computer returning the data we want
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: refactoring, I extracted this class

}

// Keep only supported algorithms
return NamedFactory.setUpBuiltinFactories(true, preferred);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@PereBueno PereBueno left a comment

Choose a reason for hiding this comment

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

change LGTM

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.

2 participants