Skip to content

[JENKINS-76083] Fix tests for SSH Verifiers + fix regression #1146

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.

Sign up for GitHub

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

Merged
res0nance merged 4 commits into from
Sep 16, 2025
Merged

[JENKINS-76083] Fix tests for SSH Verifiers + fix regression #1146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2eb7919
[JENKINS-76083] Fix tests for SSH Verifiers
Dohbedoh Sep 10, 2025
430f54b
[JENKINS-76083] Fix regression: pass the correct algorithm
Dohbedoh Sep 10, 2025
4d88652
Merge branch 'master' into JENKINS-76083
Dohbedoh Sep 10, 2025
3b3c757
Merge branch 'master' into JENKINS-76083
res0nance Sep 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/main/java/hudson/plugins/ec2/ssh/EC2SSHLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.services.ec2.model.Instance;
import software.amazon.awssdk.services.ec2.model.InstanceStateName;
Expand Down Expand Up @@ -376,10 +378,8 @@ protected ClientSession connectToSsh(
}
}

/**
* Our host key verifier just pick up the right strategy and call its verify method.
*/
static class ServerKeyVerifierImpl implements ServerKeyVerifier {
@Restricted(NoExternalUse.class)
public static class ServerKeyVerifierImpl implements ServerKeyVerifier {
private final EC2Computer computer;
private final TaskListener listener;

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/ec2/ssh/verifiers/HostKeyHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public HostKey getHostKey(@NonNull PublicKey serverKey) throws IOException {
return null;
}
AsymmetricKeyParameter parameters = PublicKeyFactory.createKey(serverKey.getEncoded());
return new HostKey(serverKey.getAlgorithm(), OpenSSHPublicKeyUtil.encodePublicKey(parameters));
return new HostKey(sshAlgorithm, OpenSSHPublicKeyUtil.encodePublicKey(parameters));
}

/**
Expand Down
153 changes: 84 additions & 69 deletions src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,28 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyString;

import hudson.plugins.ec2.EC2Computer;
import hudson.plugins.ec2.HostKeyVerificationStrategyEnum;
import hudson.plugins.ec2.InstanceState;
import hudson.plugins.ec2.MockEC2Computer;
import hudson.plugins.ec2.ssh.EC2SSHLauncher;
import hudson.plugins.ec2.util.ConnectionExtension;
import hudson.util.LogTaskListener;
import java.io.IOException;
import java.net.SocketAddress;
import java.security.PublicKey;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
import org.hamcrest.core.StringContains;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LogRecorder;
import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
import org.testcontainers.containers.Container;

@Disabled("TODO fails, but unclear when it last passed")
@WithJenkins
class SshHostKeyVerificationStrategyTest {

Expand Down Expand Up @@ -61,18 +60,22 @@ private List<StrategyTest> getStrategiesToTest() throws Exception {
List<StrategyTest> strategiesToCheck = new ArrayList<>();

strategiesToCheck.add(forHardStrategyNotPrinted());
strategiesToCheck.add(forHardStrategyPrinted());
strategiesToCheck.add(forHardStrategyPrintedAndChanged());
strategiesToCheck.add(forHardStrategyPrinted("ecdsa"));
strategiesToCheck.add(forHardStrategyPrinted("ed25519"));
strategiesToCheck.add(forHardStrategyPrinted("rsa"));
strategiesToCheck.add(forSoftStrategy());
strategiesToCheck.add(forAceptNewStrategy());
strategiesToCheck.add(forSoftStrategyPrinted("ecdsa"));
strategiesToCheck.add(forSoftStrategyPrinted("ed25519"));
strategiesToCheck.add(forSoftStrategyPrinted("rsa"));
strategiesToCheck.add(forAcceptNewStrategy());
strategiesToCheck.add(forOffStrategy());

return strategiesToCheck;
}

// Check the hard strategy with a key not printed
private StrategyTest forHardStrategyNotPrinted() throws Exception {
return new StrategyTest("-hardStrategyNotPrinted", new CheckNewHardStrategy())
return new StrategyTest("-hardStrategyNotPrinted", HostKeyVerificationStrategyEnum.CHECK_NEW_HARD)
.addConnectionAttempt(builder().setState(InstanceState.PENDING).setMessagesInLog(new String[] {
"is not running, waiting to validate the key against the console",
"The instance console is blank. Cannot check the key"
Expand All @@ -83,53 +86,35 @@ private StrategyTest forHardStrategyNotPrinted() throws Exception {
}))
.addConnectionAttempt(builder()
.setConsole("A console without the key")
.isOfflineByKey(true)
// Uptime calculation may fail, so we don't check the message about waiting 2 minutes
// .isOfflineByKey(true)
.setMessagesInLog(new String[] {
"didn't print the host key. Expected a line starting with",
"presented by the instance has not been found on the instance console"
}));
}

// Check the hard strategy with the key printed
private StrategyTest forHardStrategyPrinted() throws Exception {
return new StrategyTest("-hardStrategyPrinted", new CheckNewHardStrategy())
.addConnectionAttempt(builder().setState(InstanceState.PENDING).setMessagesInLog(new String[] {
"is not running, waiting to validate the key against the console",
"The instance console is blank. Cannot check the key"
}))
.addConnectionAttempt(builder().setMessagesInLog(new String[] {
"has a blank console. Maybe the console is yet not available",
"The instance console is blank. Cannot check the key"
}))
private StrategyTest forHardStrategyPrinted(String algorithm) throws Exception {
return new StrategyTest("-hardStrategyPrinted-" + algorithm, HostKeyVerificationStrategyEnum.CHECK_NEW_HARD)
.addConnectionAttempt(builder()
.setConsole("A text before the key\n" + connection.ED255219_PUB_KEY + "\n a bit more text")
.setMessagesInLog(new String[] {"has been successfully checked against the instance console"}));
}

// Check the hard strategy with the key printed and the host key is changed afterward
private StrategyTest forHardStrategyPrintedAndChanged() throws Exception {
return new StrategyTest("-hardStrategyPrintedAndChanged", new CheckNewHardStrategy())
.addConnectionAttempt(builder().setState(InstanceState.PENDING).setMessagesInLog(new String[] {
"is not running, waiting to validate the key against the console",
"The instance console is blank. Cannot check the key"
}))
.addConnectionAttempt(builder().setMessagesInLog(new String[] {
"has a blank console. Maybe the console is yet not available",
"The instance console is blank. Cannot check the key"
}))
.addConnectionAttempt(builder()
.setConsole("A text before the key\n" + connection.ED255219_PUB_KEY + "\n a bit more text")
.setAlgorithm(algorithm)
.setConsole(
"A text before the key\n" + connection.getPublicKey(algorithm) + "\n a bit more text")
.setMessagesInLog(new String[] {"has been successfully checked against the instance console"}))
.addConnectionAttempt(builder()
.setConsole("The console doesn't matter, the key is already stored. We check against this one")
.isOfflineByKey(true)
.isChangeHostKey(true)
.setMessagesInLog(new String[] {"presented by the instance has changed since first saved "}));
.setMessagesInLog(new String[] {
"presented by the instance has changed since first saved ",
"is closed to prevent a possible man-in-the-middle attack"
}));
}

// Check the soft strategy
private StrategyTest forSoftStrategy() throws Exception {
return new StrategyTest("-softStrategy", new CheckNewSoftStrategy())
return new StrategyTest("-softStrategy", HostKeyVerificationStrategyEnum.CHECK_NEW_SOFT)
.addConnectionAttempt(builder().setState(InstanceState.PENDING).setMessagesInLog(new String[] {
"is not running, waiting to validate the key against the console",
"The instance console is blank. Cannot check the key"
Expand All @@ -138,24 +123,41 @@ private StrategyTest forSoftStrategy() throws Exception {
"has a blank console. Maybe the console is yet not available",
"The instance console is blank. Cannot check the key"
}))

// Allowed and persisted
.addConnectionAttempt(
builder().setConsole("A console without the key").setMessagesInLog(new String[] {
"didn't print the host key. Expected a line starting with",
"Cannot check the key but the connection to ",
" is allowed"
}))

// The key was stored on the previous step, gathered from known_hosts
.addConnectionAttempt(builder()
.setConsole("A console without the key")
.setMessagesInLog(new String[] {"Connection allowed after the host key has been verified"}));
}

// Check the soft strategy
private StrategyTest forSoftStrategyPrinted(String algorithm) throws Exception {
return new StrategyTest("-softStrategyPrinted-" + algorithm, HostKeyVerificationStrategyEnum.CHECK_NEW_SOFT)
// The key was stored on the previous step, gathered from known_hosts
.addConnectionAttempt(builder()
.setAlgorithm(algorithm)
.setConsole(
"A text before the key\n" + connection.getPublicKey(algorithm) + "\n a bit more text")
.setMessagesInLog(new String[] {"has been successfully checked against the instance console"}))
.addConnectionAttempt(builder()
.setConsole("The console doesn't matter, the key is already stored. We check against this one")
.isOfflineByKey(true)
.isChangeHostKey(true)
.setMessagesInLog(new String[] {
"presented by the instance has changed since first saved ",
"is closed to prevent a possible man-in-the-middle attack"
}));
}

// Check the accept-new strategy
private StrategyTest forAceptNewStrategy() throws Exception {
return new StrategyTest("-acceptNewStrategy", new AcceptNewStrategy())
private StrategyTest forAcceptNewStrategy() throws Exception {
return new StrategyTest("-acceptNewStrategy", HostKeyVerificationStrategyEnum.ACCEPT_NEW)
// We don't even check the console
.addConnectionAttempt(builder()
.setState(InstanceState.PENDING)
Expand All @@ -166,7 +168,7 @@ private StrategyTest forAceptNewStrategy() throws Exception {

// Check the off strategy
private StrategyTest forOffStrategy() throws Exception {
return new StrategyTest("-offStrategy", new NonVerifyingKeyVerificationStrategy())
return new StrategyTest("-offStrategy", HostKeyVerificationStrategyEnum.OFF)
.addConnectionAttempt(builder()
.setState(InstanceState.PENDING)
.setMessagesInLog(new String[] {"No SSH key verification"}));
Expand All @@ -183,17 +185,22 @@ private ConnectionAttempt.Builder builder() {
private static class StrategyTest {
List<ConnectionAttempt> connectionAttempts = new ArrayList<>();
MockEC2Computer computer;
ServerHostKeyVerifierImpl verifier;
ServerKeyVerifier verifier;

public void check() throws Exception {
for (ConnectionAttempt connectionAttempt : connectionAttempts) {
connectionAttempt.attempt();
}
}

private StrategyTest(String computerSuffix, SshHostKeyVerificationStrategy strategy) throws Exception {
private StrategyTest(String computerSuffix, HostKeyVerificationStrategyEnum strategy) throws Exception {
computer = MockEC2Computer.createComputer(computerSuffix);
verifier = new ServerHostKeyVerifierImpl(computer, strategy);
jenkins.jenkins.addNode(computer.getNode());
computer.getSlaveTemplate().setHostKeyVerificationStrategy(strategy);
verifier = new EC2SSHLauncher.ServerKeyVerifierImpl(
computer,
new LogTaskListener(
Logger.getLogger(SshHostKeyVerificationStrategyTest.class.getName()), Level.ALL));
}

private StrategyTest addConnectionAttempt(ConnectionAttempt.Builder computerStateBuilder) {
Expand All @@ -208,6 +215,8 @@ private StrategyTest addConnectionAttempt(ConnectionAttempt.Builder computerStat
* verifier should be used to connect to it and the expected state of the computer after the attempt.
*/
private static class ConnectionAttempt {

private String algorithm;
// The console that the computer will have
private String console = null;
// The state the computer is on
Expand Down Expand Up @@ -253,6 +262,29 @@ private void configure() throws IOException, InterruptedException {
assertThat(removeResult.getStdout(), emptyString());
Container.ExecResult regenResult = connection.execInContainer("ssh-keygen", "-A");
assertThat(regenResult.getStderr(), emptyString());

if (algorithm != null) {
// Keep the new key of the algorithm used in this test, restore the rest
Container.ExecResult algorithmResult = connection.execInContainer(
"sh",
"-c",
String.format(
"mv /etc/ssh/ssh_host_%1$s_key /etc/ssh/keep_ssh_host_%1$s_key && rm -f /etc/ssh/ssh_host_* && mv /etc/ssh/keep_ssh_host_%1$s_key.pub /etc/ssh/ssh_host_%1$s_key.pub",
algorithm));
assertThat(algorithmResult.getStderr(), emptyString());
assertThat(algorithmResult.getStdout(), emptyString());
}

} else if (algorithm != null) {
// Restore the original keys
Container.ExecResult algorithmResult = connection.execInContainer(
"sh",
"-c",
String.format(
"rm -f /etc/ssh/ssh_host_* && cp /etc/ssh/originals/ssh_host_%1$s_key* /etc/ssh/",
algorithm));
assertThat(algorithmResult.getStderr(), emptyString());
assertThat(algorithmResult.getStdout(), emptyString());
}
}

Expand Down Expand Up @@ -298,6 +330,11 @@ private void assertState() {
static class Builder {
ConnectionAttempt connectionAttempt;

Builder setAlgorithm(String algorithm) {
connectionAttempt.algorithm = algorithm;
return this;
}

Builder setConsole(String console) {
connectionAttempt.console = console;
return this;
Expand Down Expand Up @@ -335,26 +372,4 @@ private ConnectionAttempt build(MockEC2Computer computer, ServerKeyVerifier veri
}
}
}

// A verifier using the set strategy
private static class ServerHostKeyVerifierImpl implements ServerKeyVerifier {
private final EC2Computer computer;
private final SshHostKeyVerificationStrategy strategy;

public ServerHostKeyVerifierImpl(final EC2Computer computer, final SshHostKeyVerificationStrategy strategy) {
this.computer = computer;
this.strategy = strategy;
}

@Override
public boolean verifyServerKey(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) {
// TODO: change by the verifier defined on the instance template or the default one
try {
return strategy.verify(computer, new HostKey(serverKey.getAlgorithm(), serverKey.getEncoded()), null);
} catch (Exception e) {
e.printStackTrace(); // stack trace swallowed by LoggingUtils otherwise
throw new RuntimeException(e);
}
}
}
}
22 changes: 13 additions & 9 deletions src/test/java/hudson/plugins/ec2/util/ConnectionExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package hudson.plugins.ec2.util;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyString;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.time.Duration;
import java.util.Locale;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.future.ConnectFuture;
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
Expand Down Expand Up @@ -59,9 +61,6 @@ public class ConnectionExtension implements BeforeAllCallback, AfterAllCallback
private static final String publicKey =
"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDfHtD5E3GR6qUMBuixGbu1esUu43V9zJngREsS6Z78ktVJifIEzi/QqEoyRMWzJVeIy6QcJLT+Zid9O+MaV3vqnNh2hdLOdxzzNmu4OcxWDMBShJI/UfnC0vp+3plLVodeFa17lOaQ7gYNeKUzCOuDBaxxSEPpiFqx8OIRW9oI1Wdohi0oyEEtjUxvpky4WeptIxZZn1LYHj1EyT9AvpdLb0t4KqV5OXal3lQQUaJ2GI8GPolZn91k79w2c2+EXfsvlKKo7mUsFNhwK0OBnJexjuN+FQaXnJFsYpxOQ2JGH9uSkpM+IBxcEDHQe/UcMY3qD49X+tyHt0P+sPi1E5EN user@test";

// The public ed-25510 host key of the server
public String ED255219_PUB_KEY;

private final SshClient sshClient = SshClient.setUpDefaultClient();

private ClientSession connection;
Expand All @@ -78,8 +77,6 @@ public ClientSession connect(ServerKeyVerifier verifier) throws Exception {
connection.addPublicKeyIdentity(KeyHelper.decodeKeyPair(privateKey, ""));
connection.auth().await(Duration.ofSeconds(10));

assertTrue(connection.isAuthenticated());

return connection;
}

Expand All @@ -99,16 +96,23 @@ public void beforeAll(ExtensionContext context) throws Exception {

sshClient.start();

// Backup all SSH Host Keys to allow tests to modify them
Container.ExecResult backup = execInContainer(
"sh", "-c", "mkdir -p /etc/ssh/originals/ && cp /etc/ssh/ssh_host_* /etc/ssh/originals/");
assertThat(backup.getStderr(), emptyString());
assertThat(backup.getStdout(), emptyString());

} catch (RuntimeException re) {
throw new TestAbortedException("The container to connect to cannot be started", re);
}

sshContainer.start();
}

public String getPublicKey(String algorithm) throws IOException {
try {
// We get the key after it's generated
ED255219_PUB_KEY = sshContainer
.execInContainer("cat", "/etc/ssh/ssh_host_ed25519_key.pub")
return sshContainer
.execInContainer("cat", "/etc/ssh/ssh_host_" + algorithm.toLowerCase(Locale.ROOT) + "_key.pub")
.getStdout();
} catch (UnsupportedOperationException | IOException | InterruptedException e) {
throw new IOException("Cannot get the public ssh host key from the docker instance", e);
Expand Down