From 9a0fdc706d2862d79b8bc1c4c63df87049f4a71c Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Tue, 16 May 2017 21:18:56 +0100 Subject: [PATCH 01/10] [FIXED JENKINS-42959] Specify preferred host keys during connect --- pom.xml | 11 ++- .../hudson/plugins/sshslaves/SSHLauncher.java | 1 + .../JenkinsTrilead9VersionSupport.java | 39 +++++++++++ ...KnownHostsFileKeyVerificationStrategy.java | 11 +++ ...nuallyProvidedKeyVerificationStrategy.java | 32 ++++----- ...anuallyTrustedKeyVerificationStrategy.java | 20 ++++++ .../SshHostKeyVerificationStrategy.java | 6 ++ .../TrileadVersionSupportManager.java | 70 +++++++++++++++++++ ...lyProvidedKeyVerificationStrategyTest.java | 40 +++++++++++ .../TrileadVersionSupportManagerTest.java | 44 ++++++++++++ 10 files changed, 254 insertions(+), 20 deletions(-) create mode 100644 src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java create mode 100644 src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java create mode 100644 src/test/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategyTest.java create mode 100644 src/test/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManagerTest.java diff --git a/pom.xml b/pom.xml index bab45d5b..59bea059 100644 --- a/pom.xml +++ b/pom.xml @@ -16,8 +16,8 @@ Allows to launch agents over SSH, using a Java implementation of the SSH protocol - 1.609.1 - 6 + 1.625 + 7 2.18 @@ -62,6 +62,13 @@ + + org.jenkins-ci + trilead-ssh2 + build217-jenkins-9 + provided + + org.jenkins-ci.plugins diff --git a/src/main/java/hudson/plugins/sshslaves/SSHLauncher.java b/src/main/java/hudson/plugins/sshslaves/SSHLauncher.java index ffdfbcab..0c914dcb 100644 --- a/src/main/java/hudson/plugins/sshslaves/SSHLauncher.java +++ b/src/main/java/hudson/plugins/sshslaves/SSHLauncher.java @@ -786,6 +786,7 @@ public synchronized void launch(final SlaveComputer computer, final TaskListener public Boolean call() throws InterruptedException { Boolean rval = Boolean.FALSE; try { + connection.setServerHostKeyAlgorithms(sshHostKeyVerificationStrategy.getPreferredKeyAlgorithms(computer)); openConnection(listener, computer); diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java new file mode 100644 index 00000000..34f64ea5 --- /dev/null +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java @@ -0,0 +1,39 @@ +package hudson.plugins.sshslaves.verifiers; + +import com.trilead.ssh2.signature.KeyAlgorithm; +import com.trilead.ssh2.signature.KeyAlgorithmManager; +import hudson.plugins.sshslaves.Messages; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +/** + * @author Michael Clarke + */ +class JenkinsTrilead9VersionSupport extends TrileadVersionSupportManager.TrileadVersionSupport { + + @Override + public String[] getSupportedAlgorithms() { + List algorithms = new ArrayList<>(); + for (KeyAlgorithm algorithm : KeyAlgorithmManager.getSupportedAlgorithms()) { + algorithms.add(algorithm.getKeyFormat()); + } + return algorithms.toArray(new String[algorithms.size()]); + } + + @Override + public HostKey parseKey(String algorithm, byte[] keyValue) { + for (KeyAlgorithm keyAlgorithm : KeyAlgorithmManager.getSupportedAlgorithms()) { + try { + if (keyAlgorithm.getKeyFormat().equals(algorithm)) { + keyAlgorithm.decodePublicKey(keyValue); + return new HostKey(algorithm, keyValue); + } + } catch (IOException ex) { + throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); + } + } + throw new IllegalArgumentException("Unexpected key algorithm " + algorithm); + } +} diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java index 673efb03..631abd03 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java @@ -24,6 +24,7 @@ package hudson.plugins.sshslaves.verifiers; import java.io.File; +import java.io.IOException; import org.kohsuke.stapler.DataBoundConstructor; @@ -73,6 +74,16 @@ public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener list } } + + @Override + public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOException { + if (!KNOWN_HOSTS_FILE.exists()) { + return super.getPreferredKeyAlgorithms(computer); + } + + KnownHosts knownHosts = new KnownHosts(KNOWN_HOSTS_FILE); + return knownHosts.getPreferredServerHostkeyAlgorithmOrder(((SSHLauncher) computer.getLauncher()).getHost()); + } @Extension public static class KnownHostsFileKeyVerificationStrategyDescriptor extends SshHostKeyVerificationStrategyDescriptor { diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java index 6fba0c0d..808081a0 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java @@ -24,13 +24,14 @@ package hudson.plugins.sshslaves.verifiers; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.StringTokenizer; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; -import com.trilead.ssh2.signature.DSASHA1Verify; -import com.trilead.ssh2.signature.RSASHA1Verify; import hudson.Extension; import hudson.model.TaskListener; @@ -75,6 +76,16 @@ public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener list return false; } } + + @Override + public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOException { + List sortedAlgorithms = new ArrayList<>(Arrays.asList(super.getPreferredKeyAlgorithms(computer))); + + sortedAlgorithms.remove(key.getAlgorithm()); + sortedAlgorithms.add(0, key.getAlgorithm()); + + return sortedAlgorithms.toArray(new String[sortedAlgorithms.size()]); + } private static HostKey parseKey(String key) { if (!key.contains(" ")) { @@ -87,22 +98,7 @@ private static HostKey parseKey(String key) { throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_Base64EncodedKeyValueRequired()); } - try { - if ("ssh-rsa".equals(algorithm)) { - RSASHA1Verify.decodeSSHRSAPublicKey(keyValue); - } else if ("ssh-dss".equals(algorithm)) { - DSASHA1Verify.decodeSSHDSAPublicKey(keyValue); - } else { - throw new IllegalArgumentException("Key algorithm should be one of ssh-rsa or ssh-dss"); - } - } catch (IOException ex) { - throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); - } catch (StringIndexOutOfBoundsException ex) { - // can happen in DSASHA1Verifier with certain values (from quick testing) - throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); - } - - return new HostKey(algorithm, keyValue); + return TrileadVersionSupportManager.getTrileadSupport().parseKey(algorithm, keyValue); } @Extension diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyTrustedKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyTrustedKeyVerificationStrategy.java index c45b74c8..63831ec3 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyTrustedKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyTrustedKeyVerificationStrategy.java @@ -33,6 +33,8 @@ import hudson.slaves.SlaveComputer; import java.io.IOException; import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -98,6 +100,24 @@ else if (!existingHostKey.equals(hostKey)) { } } + @Override + public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOException { + String[] algorithms = super.getPreferredKeyAlgorithms(computer); + + HostKey hostKey = HostKeyHelper.getInstance().getHostKey(computer); + + if (null != hostKey) { + List sortedAlgorithms = new ArrayList<>(Arrays.asList(algorithms)); + + sortedAlgorithms.remove(hostKey.getAlgorithm()); + sortedAlgorithms.add(0, hostKey.getAlgorithm()); + + algorithms = sortedAlgorithms.toArray(new String[sortedAlgorithms.size()]); + } + + return algorithms; + } + /** TODO replace with {@link Computer#addAction} after core baseline picks up JENKINS-42969 fix */ private static void addAction(@Nonnull Computer c, @Nonnull Action a) { try { diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/SshHostKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/SshHostKeyVerificationStrategy.java index 29340119..a3b900b1 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/SshHostKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/SshHostKeyVerificationStrategy.java @@ -29,6 +29,8 @@ import hudson.slaves.SlaveComputer; import jenkins.model.Jenkins; +import java.io.IOException; + /** * A method for verifying the host key provided by the remote host during the * initiation of each connection. @@ -53,6 +55,10 @@ public SshHostKeyVerificationStrategyDescriptor getDescriptor() { * @since 1.12 */ public abstract boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener listener) throws Exception; + + public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOException { + return TrileadVersionSupportManager.getTrileadSupport().getSupportedAlgorithms(); + } public static abstract class SshHostKeyVerificationStrategyDescriptor extends Descriptor { diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java new file mode 100644 index 00000000..7a367ad6 --- /dev/null +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -0,0 +1,70 @@ +package hudson.plugins.sshslaves.verifiers; + +import com.trilead.ssh2.signature.DSASHA1Verify; +import com.trilead.ssh2.signature.RSASHA1Verify; +import hudson.plugins.sshslaves.Messages; + +import java.io.IOException; + +/** + * @author Michael Clarke + * @since 1.18 + */ +final class TrileadVersionSupportManager { + + static TrileadVersionSupport getTrileadSupport() { + try { + Thread.currentThread().getContextClassLoader().loadClass("com.trilead.ssh2.signature.KeyAlgorithmManager"); + return createaVersion9Instance(); + } catch (ReflectiveOperationException e) { + // KeyAlgorithmManager doesn't exist, fall back to legacy trilead handler + return new LegacyTrileadVersionSupport(); + } + } + + private static TrileadVersionSupport createaVersion9Instance() { + try { + return (TrileadVersionSupport) Class.forName("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); + } catch (ReflectiveOperationException e) { + throw new IllegalArgumentException("Could not create Trilead support class", e); + } + + } + + public abstract static class TrileadVersionSupport { + + /*package*/ TrileadVersionSupport() { + super(); + } + + public abstract String[] getSupportedAlgorithms(); + + public abstract HostKey parseKey(String algorithm, byte[] keyValue); + } + + private static class LegacyTrileadVersionSupport extends TrileadVersionSupport { + + @Override + public String[] getSupportedAlgorithms() { + return new String[]{"ssh-rsa", "ssh-dss"}; + } + + @Override + public HostKey parseKey(String algorithm, byte[] keyValue) { + try { + if ("ssh-rsa".equals(algorithm)) { + RSASHA1Verify.decodeSSHRSAPublicKey(keyValue); + } else if ("ssh-dss".equals(algorithm)) { + DSASHA1Verify.decodeSSHDSAPublicKey(keyValue); + } else { + throw new IllegalArgumentException("Key algorithm should be one of ssh-rsa or ssh-dss"); + } + } catch (IOException | StringIndexOutOfBoundsException ex) { + throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); + } + + return new HostKey(algorithm, keyValue); + } + } + +} diff --git a/src/test/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategyTest.java b/src/test/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategyTest.java new file mode 100644 index 00000000..e653738f --- /dev/null +++ b/src/test/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategyTest.java @@ -0,0 +1,40 @@ +package hudson.plugins.sshslaves.verifiers; + +import org.apache.commons.io.IOUtils; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.assertArrayEquals; + +/** + * @author Michael Clarke + */ +public class ManuallyProvidedKeyVerificationStrategyTest { + + @Test + public void testRsa() throws IOException { + ManuallyProvidedKeyVerificationStrategy testCase = new ManuallyProvidedKeyVerificationStrategy("ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAQEAtqwn/v4+sYBD0e5UT59zGjQ+iBOJvKbqVX22vt4hFIVrbwmB+HKJGwOINe1gnc/syPGj/5c6yoOnjTdpI/xerip6RjVPRTQVh2nNjsbXIS5epi/39nnPFZ/0hE3ozOtQ1j9OS5bXVBD770ha1UFnCql4DfcWj+y1QVYvm53p2fID+an0HNunnZjq+r2UJgt138lkZN2K7S42U/apqOHStFGVPxF+gmK1fI021QI+QjxfKOoyGNCpbAaMM6jzikqCJOE8M7jpSZgHMO2x+wvjMK8p2uXAaZlYJeUlEqUVGa9jjkdEiTPabFJyrKORrTWX7Ahs6C4vCAgWmNZzOmOvnw== rsa-key-20170516"); + assertArrayEquals(new String[]{"ssh-rsa", "ssh-ed25519", "ecdsa-sha2-nistp521", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp256", "ssh-dss"}, testCase.getPreferredKeyAlgorithms(null)); + } + + @Test + public void testEd25519() throws IOException { + ManuallyProvidedKeyVerificationStrategy testCase = new ManuallyProvidedKeyVerificationStrategy("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMQPcXch45Uak9iiHt1puffR6LHZxZsHU0iyeyUnf5qW ed25519-key-20170516"); + assertArrayEquals(new String[]{"ssh-ed25519", "ecdsa-sha2-nistp521", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp256", "ssh-rsa", "ssh-dss"}, testCase.getPreferredKeyAlgorithms(null)); + } + + + @Test + public void testEcdsa() throws IOException { + ManuallyProvidedKeyVerificationStrategy testCase = new ManuallyProvidedKeyVerificationStrategy("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBMQMVHTpplIuqEcOR8j7wzydDUzXF0Fl82WluEJphpo2JKbJ4DNaL3Zu6bfeDQGuH3hWtG1H0r4ntoDtN940GGA= ecdsa-key-20170516"); + assertArrayEquals(new String[]{"ecdsa-sha2-nistp256", "ssh-ed25519", "ecdsa-sha2-nistp521", "ecdsa-sha2-nistp384", "ssh-rsa", "ssh-dss"}, testCase.getPreferredKeyAlgorithms(null)); + } + + @Test + public void testDsa() throws IOException { + ManuallyProvidedKeyVerificationStrategy testCase = new ManuallyProvidedKeyVerificationStrategy("ssh-dss AAAAB3NzaC1kc3MAAAAhAOD3H2nbagBMaZ7XDnGUBO3vuqi3McIC9A+smJH9lsnzAAAAFQD3lLxlCXN8K4CeNCJdHeXEpeE7vwAAACBtZ3osIr0OtX6uKFumP6ybXGrfiy7otYqmSPwS+A2MywAAACEA34SUyAprA9HHPmRqZnJ6Acgq6KKRrh4SKTPUdJa8aBc= dsa-key-20170516"); + assertArrayEquals(new String[]{"ssh-dss", "ssh-ed25519", "ecdsa-sha2-nistp521", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp256", "ssh-rsa"}, testCase.getPreferredKeyAlgorithms(null)); + } + +} diff --git a/src/test/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManagerTest.java b/src/test/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManagerTest.java new file mode 100644 index 00000000..d48b6122 --- /dev/null +++ b/src/test/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManagerTest.java @@ -0,0 +1,44 @@ +package hudson.plugins.sshslaves.verifiers; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author Michael Clarke + */ +public class TrileadVersionSupportManagerTest { + + @Test + public void testLegacyInstance() { + ClassLoader loader = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader(new BlockingClassloader(Thread.currentThread().getContextClassLoader())); + String name = TrileadVersionSupportManager.getTrileadSupport().getClass().getName(); + assertEquals("hudson.plugins.sshslaves.verifiers.TrileadVersionSupportManager$LegacyTrileadVersionSupport", name); + } finally { + Thread.currentThread().setContextClassLoader(loader); + } + } + + @Test + public void testCurrentInstance() { + assertEquals(JenkinsTrilead9VersionSupport.class, TrileadVersionSupportManager.getTrileadSupport().getClass()); + } + + + private static class BlockingClassloader extends ClassLoader { + + public BlockingClassloader(ClassLoader parent) { + super(parent); + } + + public Class loadClass(String className) throws ClassNotFoundException { + if ("com.trilead.ssh2.signature.KeyAlgorithmManager".equals(className)) { + throw new ClassNotFoundException(className); + } + return super.loadClass(className); + } + + } +} From cf8510581c5fcb932a54e4f3e1fd00afd1b01e91 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Wed, 17 May 2017 20:55:03 +0100 Subject: [PATCH 02/10] Overcome JENKINS-44120 during test case --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 59bea059..9612ef32 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ org.jenkins-ci trilead-ssh2 - build217-jenkins-9 + build-217-jenkins-11 provided From 51709349da52dd47aaff7699f2e03dd871dc993c Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:20:32 +0100 Subject: [PATCH 03/10] Use checked exception on failure parsing keys --- .../verifiers/JenkinsTrilead9VersionSupport.java | 6 +++--- .../sshslaves/verifiers/KeyParseException.java | 16 ++++++++++++++++ .../ManuallyProvidedKeyVerificationStrategy.java | 12 ++++++++---- .../verifiers/TrileadVersionSupportManager.java | 8 ++++---- 4 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 src/main/java/hudson/plugins/sshslaves/verifiers/KeyParseException.java diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java index 34f64ea5..50b9d14b 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java @@ -23,7 +23,7 @@ public String[] getSupportedAlgorithms() { } @Override - public HostKey parseKey(String algorithm, byte[] keyValue) { + public HostKey parseKey(String algorithm, byte[] keyValue) throws KeyParseException { for (KeyAlgorithm keyAlgorithm : KeyAlgorithmManager.getSupportedAlgorithms()) { try { if (keyAlgorithm.getKeyFormat().equals(algorithm)) { @@ -31,9 +31,9 @@ public HostKey parseKey(String algorithm, byte[] keyValue) { return new HostKey(algorithm, keyValue); } } catch (IOException ex) { - throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); + throw new KeyParseException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); } } - throw new IllegalArgumentException("Unexpected key algorithm " + algorithm); + throw new KeyParseException("Unexpected key algorithm: " + algorithm); } } diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/KeyParseException.java b/src/main/java/hudson/plugins/sshslaves/verifiers/KeyParseException.java new file mode 100644 index 00000000..9c821f1a --- /dev/null +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/KeyParseException.java @@ -0,0 +1,16 @@ +package hudson.plugins.sshslaves.verifiers; + +/** + * @author Michael Clarke + * @since 1.18 + */ +public class KeyParseException extends Exception { + + public KeyParseException(String message) { + super(message); + } + + public KeyParseException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java index 808081a0..74bacbd0 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/ManuallyProvidedKeyVerificationStrategy.java @@ -55,7 +55,11 @@ public class ManuallyProvidedKeyVerificationStrategy extends SshHostKeyVerificat @DataBoundConstructor public ManuallyProvidedKeyVerificationStrategy(String key) { super(); - this.key = parseKey(key); + try { + this.key = parseKey(key); + } catch (KeyParseException e) { + throw new IllegalArgumentException("Invalid key: " + e.getMessage(), e); + } } public String getKey() { @@ -87,7 +91,7 @@ public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOExcep return sortedAlgorithms.toArray(new String[sortedAlgorithms.size()]); } - private static HostKey parseKey(String key) { + private static HostKey parseKey(String key) throws KeyParseException { if (!key.contains(" ")) { throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_TwoPartKey()); } @@ -95,7 +99,7 @@ private static HostKey parseKey(String key) { String algorithm = tokenizer.nextToken(); byte[] keyValue = Base64.decode(tokenizer.nextToken()); if (null == keyValue) { - throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_Base64EncodedKeyValueRequired()); + throw new KeyParseException(Messages.ManualKeyProvidedHostKeyVerifier_Base64EncodedKeyValueRequired()); } return TrileadVersionSupportManager.getTrileadSupport().parseKey(algorithm, keyValue); @@ -113,7 +117,7 @@ public FormValidation doCheckKey(@QueryParameter String key) { try { ManuallyProvidedKeyVerificationStrategy.parseKey(key); return FormValidation.ok(); - } catch (IllegalArgumentException ex) { + } catch (KeyParseException ex) { return FormValidation.error(ex.getMessage()); } } diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 7a367ad6..69664b75 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -39,7 +39,7 @@ public abstract static class TrileadVersionSupport { public abstract String[] getSupportedAlgorithms(); - public abstract HostKey parseKey(String algorithm, byte[] keyValue); + public abstract HostKey parseKey(String algorithm, byte[] keyValue) throws KeyParseException; } private static class LegacyTrileadVersionSupport extends TrileadVersionSupport { @@ -50,17 +50,17 @@ public String[] getSupportedAlgorithms() { } @Override - public HostKey parseKey(String algorithm, byte[] keyValue) { + public HostKey parseKey(String algorithm, byte[] keyValue) throws KeyParseException { try { if ("ssh-rsa".equals(algorithm)) { RSASHA1Verify.decodeSSHRSAPublicKey(keyValue); } else if ("ssh-dss".equals(algorithm)) { DSASHA1Verify.decodeSSHDSAPublicKey(keyValue); } else { - throw new IllegalArgumentException("Key algorithm should be one of ssh-rsa or ssh-dss"); + throw new KeyParseException("Key algorithm should be one of ssh-rsa or ssh-dss"); } } catch (IOException | StringIndexOutOfBoundsException ex) { - throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); + throw new KeyParseException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex); } return new HostKey(algorithm, keyValue); From ee2da493a637ca6aea3b49c936a9ae8458a4d67b Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:27:50 +0100 Subject: [PATCH 04/10] Access restrictions to prevent classes becoming public API --- .../sshslaves/verifiers/JenkinsTrilead9VersionSupport.java | 3 +++ .../sshslaves/verifiers/TrileadVersionSupportManager.java | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java index 50b9d14b..e8586c32 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/JenkinsTrilead9VersionSupport.java @@ -3,6 +3,8 @@ import com.trilead.ssh2.signature.KeyAlgorithm; import com.trilead.ssh2.signature.KeyAlgorithmManager; import hudson.plugins.sshslaves.Messages; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import java.io.IOException; import java.util.ArrayList; @@ -11,6 +13,7 @@ /** * @author Michael Clarke */ +@Restricted(NoExternalUse.class) class JenkinsTrilead9VersionSupport extends TrileadVersionSupportManager.TrileadVersionSupport { @Override diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 69664b75..33bfa95d 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -3,6 +3,8 @@ import com.trilead.ssh2.signature.DSASHA1Verify; import com.trilead.ssh2.signature.RSASHA1Verify; import hudson.plugins.sshslaves.Messages; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import java.io.IOException; @@ -10,6 +12,7 @@ * @author Michael Clarke * @since 1.18 */ +@Restricted(NoExternalUse.class) final class TrileadVersionSupportManager { static TrileadVersionSupport getTrileadSupport() { @@ -33,6 +36,7 @@ private static TrileadVersionSupport createaVersion9Instance() { public abstract static class TrileadVersionSupport { + @Restricted(NoExternalUse.class) /*package*/ TrileadVersionSupport() { super(); } From e84557ea464ec3689f443d70bf6739243fd7c409 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:28:19 +0100 Subject: [PATCH 05/10] Fix typo in method name --- .../sshslaves/verifiers/TrileadVersionSupportManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 33bfa95d..12f126c3 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -18,14 +18,14 @@ final class TrileadVersionSupportManager { static TrileadVersionSupport getTrileadSupport() { try { Thread.currentThread().getContextClassLoader().loadClass("com.trilead.ssh2.signature.KeyAlgorithmManager"); - return createaVersion9Instance(); + return createVersion9Instance(); } catch (ReflectiveOperationException e) { // KeyAlgorithmManager doesn't exist, fall back to legacy trilead handler return new LegacyTrileadVersionSupport(); } } - private static TrileadVersionSupport createaVersion9Instance() { + private static TrileadVersionSupport createVersion9Instance() { try { return (TrileadVersionSupport) Class.forName("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); } catch (ReflectiveOperationException e) { From ac0c6398ff7ee4f441c3b0534f314eae5cc05b0c Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:37:09 +0100 Subject: [PATCH 06/10] Consistently select the ClassLoader to use for reflective class loading --- .../sshslaves/verifiers/TrileadVersionSupportManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 12f126c3..5a6bf9ef 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -27,7 +27,7 @@ static TrileadVersionSupport getTrileadSupport() { private static TrileadVersionSupport createVersion9Instance() { try { - return (TrileadVersionSupport) Class.forName("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); + return (TrileadVersionSupport) Thread.currentThread().getContextClassLoader().loadClass("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); } catch (ReflectiveOperationException e) { throw new IllegalArgumentException("Could not create Trilead support class", e); } From b0859b99f5d96396e1c8a67459998b9694e7bacd Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:51:18 +0100 Subject: [PATCH 07/10] Broader error handling of Reflective operations --- .../TrileadVersionSupportManager.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 5a6bf9ef..9d37ecd7 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -7,6 +7,8 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; import java.io.IOException; +import java.util.logging.Level; +import java.util.logging.Logger; /** * @author Michael Clarke @@ -15,22 +17,32 @@ @Restricted(NoExternalUse.class) final class TrileadVersionSupportManager { + private static final Logger LOGGER = Logger.getLogger(TrileadVersionSupportManager.class.getName()); + static TrileadVersionSupport getTrileadSupport() { try { - Thread.currentThread().getContextClassLoader().loadClass("com.trilead.ssh2.signature.KeyAlgorithmManager"); - return createVersion9Instance(); - } catch (ReflectiveOperationException e) { - // KeyAlgorithmManager doesn't exist, fall back to legacy trilead handler - return new LegacyTrileadVersionSupport(); + if (isAfterTrilead8()) { + return createVersion9Instance(); + } + } catch (Exception e) { + LOGGER.log(Level.WARNING, "Could not create Trilead support class. Using legacy Trilead features", e); } + // We're on an old version of Triilead or couldn't create a new handler, fall back to legacy trilead handler + return new LegacyTrileadVersionSupport(); } - private static TrileadVersionSupport createVersion9Instance() { + private static boolean isAfterTrilead8() { try { - return (TrileadVersionSupport) Thread.currentThread().getContextClassLoader().loadClass("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); - } catch (ReflectiveOperationException e) { - throw new IllegalArgumentException("Could not create Trilead support class", e); + Thread.currentThread().getContextClassLoader().loadClass("com.trilead.ssh2.signature.KeyAlgorithmManager"); + } catch (ClassNotFoundException ex) { + return false; } + return true; + } + + private static TrileadVersionSupport createVersion9Instance() throws ReflectiveOperationException { + return (TrileadVersionSupport) Thread.currentThread().getContextClassLoader() + .loadClass("hudson.plugins.sshslaves.verifiers.JenkinsTrilead9VersionSupport").newInstance(); } From 1bf0cbb601a376039dc1f664a11af6d325d1b137 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 21:56:56 +0100 Subject: [PATCH 08/10] Check instance type before performing narrowing cast --- .../verifiers/KnownHostsFileKeyVerificationStrategy.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java index 631abd03..7bc28d80 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java @@ -26,6 +26,7 @@ import java.io.File; import java.io.IOException; +import hudson.slaves.ComputerLauncher; import org.kohsuke.stapler.DataBoundConstructor; import com.trilead.ssh2.KnownHosts; @@ -53,6 +54,10 @@ public KnownHostsFileKeyVerificationStrategy() { @Override public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener listener) throws Exception { + ComputerLauncher launcher = computer.getLauncher(); + if (!(launcher instanceof SSHLauncher)) { + return false; + } if (!KNOWN_HOSTS_FILE.exists()) { listener.getLogger().println(Messages.KnownHostsFileHostKeyVerifier_NoKnownHostsFile(KNOWN_HOSTS_FILE.getAbsolutePath())); @@ -60,7 +65,7 @@ public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener list } KnownHosts knownHosts = new KnownHosts(KNOWN_HOSTS_FILE); - int result = knownHosts.verifyHostkey(((SSHLauncher)computer.getLauncher()).getHost(), hostKey.getAlgorithm(), hostKey.getKey()); + int result = knownHosts.verifyHostkey(((SSHLauncher)launcher).getHost(), hostKey.getAlgorithm(), hostKey.getKey()); if (KnownHosts.HOSTKEY_IS_OK == result) { listener.getLogger().println(Messages.KnownHostsFileHostKeyVerifier_KeyTrused(SSHLauncher.getTimestamp())); From d871c1d1920dc85fa6f07f7c11cdaf9c76dd55ba Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 22:07:56 +0100 Subject: [PATCH 09/10] Add Javadoc to entry points or visible API --- .../TrileadVersionSupportManager.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java index 9d37ecd7..7061c174 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/TrileadVersionSupportManager.java @@ -11,6 +11,7 @@ import java.util.logging.Logger; /** + * An abstraction layer to allow handling of feature changes (e.g. new key types) between different Trilead versions. * @author Michael Clarke * @since 1.18 */ @@ -19,6 +20,12 @@ final class TrileadVersionSupportManager { private static final Logger LOGGER = Logger.getLogger(TrileadVersionSupportManager.class.getName()); + /** + * Craetes an instance of TrileadVersionSupport that can provide functionality relevant to the version of Trilead + * available in the current executing instance of Jenkins. + * @return an instance of TrileadVersionSupport that provides functionality relevant for the version of Trilead + * currently on the classpath + */ static TrileadVersionSupport getTrileadSupport() { try { if (isAfterTrilead8()) { @@ -53,8 +60,19 @@ public abstract static class TrileadVersionSupport { super(); } + /** + * Returns an array of all Key algorithms supported by Yrilead, e.g. ssh-rsa, ssh-dsa, ssh-eds25519 + * @return an array containing all the key algorithms the version of Trilead in use can support. + */ public abstract String[] getSupportedAlgorithms(); + /** + * Parses a raw key into a {@link HostKey} for later storage or comparison. + * @param algorithm the algorithm the key has been generated with, e.h. ssh-rsa, ssh-dss, ssh-ed25519 + * @param keyValue the value of the key, typically encoded in PEM format. + * @return the input key in a format that can be compared to other keys + * @throws KeyParseException on any failure parsing the key, such as an unknown algorithm or invalid keyValue + */ public abstract HostKey parseKey(String algorithm, byte[] keyValue) throws KeyParseException; } From 12903d2231cc3cd9d2f2ca302b6dec005ed2a128 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Thu, 18 May 2017 22:12:01 +0100 Subject: [PATCH 10/10] Remove unchecked cast to prevent possible ClassCastException --- .../verifiers/KnownHostsFileKeyVerificationStrategy.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java index 7bc28d80..58c38cbe 100644 --- a/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java +++ b/src/main/java/hudson/plugins/sshslaves/verifiers/KnownHostsFileKeyVerificationStrategy.java @@ -82,13 +82,16 @@ public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener list @Override public String[] getPreferredKeyAlgorithms(SlaveComputer computer) throws IOException { - if (!KNOWN_HOSTS_FILE.exists()) { + ComputerLauncher launcher = computer.getLauncher(); + + if (!(launcher instanceof SSHLauncher) || !KNOWN_HOSTS_FILE.exists()) { return super.getPreferredKeyAlgorithms(computer); } KnownHosts knownHosts = new KnownHosts(KNOWN_HOSTS_FILE); - return knownHosts.getPreferredServerHostkeyAlgorithmOrder(((SSHLauncher) computer.getLauncher()).getHost()); + return knownHosts.getPreferredServerHostkeyAlgorithmOrder(((SSHLauncher) launcher).getHost()); } + @Extension public static class KnownHostsFileKeyVerificationStrategyDescriptor extends SshHostKeyVerificationStrategyDescriptor {