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 Ed25519 and EDDSA host keys #51

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -17,7 +17,7 @@

<properties>
<jenkins.version>1.609.1</jenkins.version>
<java.level>6</java.level>
<java.level>7</java.level>
Copy link
Member

Choose a reason for hiding this comment

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

🐛 Jenkins core needs to be bumped to 1.625+ then

<jenkins-test-harness.version>2.18</jenkins-test-harness.version>
</properties>

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/hudson/plugins/sshslaves/verifiers/HostKey.java
Expand Up @@ -49,7 +49,7 @@ public HostKey(String algorithm, byte[] key) {

/**
* Get the algorithm used during key generation.
* @return the agorithm used to generate the key, such as ssh-rsa.
* @return the algorithm used to generate the key, such as ssh-rsa.
*/
public String getAlgorithm() {
return algorithm;
Expand Down Expand Up @@ -94,4 +94,4 @@ public boolean equals(Object obj) {
return false;
return true;
}
}
}
Expand Up @@ -62,7 +62,7 @@ public boolean verify(SlaveComputer computer, HostKey hostKey, TaskListener list
int result = knownHosts.verifyHostkey(((SSHLauncher)computer.getLauncher()).getHost(), hostKey.getAlgorithm(), hostKey.getKey());

if (KnownHosts.HOSTKEY_IS_OK == result) {
listener.getLogger().println(Messages.KnownHostsFileHostKeyVerifier_KeyTrused(SSHLauncher.getTimestamp()));
listener.getLogger().println(Messages.KnownHostsFileHostKeyVerifier_KeyTrusted(SSHLauncher.getTimestamp()));
return true;
} else if (KnownHosts.HOSTKEY_IS_NEW == result) {
listener.getLogger().println(Messages.KnownHostsFileHostKeyVerifier_NewKeyNotTrusted(SSHLauncher.getTimestamp()));
Expand Down
Expand Up @@ -24,6 +24,7 @@
package hudson.plugins.sshslaves.verifiers;

import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.util.StringTokenizer;

import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -92,18 +93,36 @@ private static HostKey parseKey(String key) {
RSASHA1Verify.decodeSSHRSAPublicKey(keyValue);
} else if ("ssh-dss".equals(algorithm)) {
DSASHA1Verify.decodeSSHDSAPublicKey(keyValue);
} else if (isClass("com.trilead.ssh2.signature.ED25519KeyAlgorithm") && "ssh-ed25519".equals(algorithm)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would reorder operations. isClass() is much more heavy than String comparison

decodePublicKey("com.trilead.ssh2.signature.ED25519KeyAlgorithm", keyValue);
} else if (isClass("com.trilead.ssh2.signature.ECDSAKeyAlgorithm") && "ecdsa-sha2-nistp256".equals(algorithm)) {
decodePublicKey("com.trilead.ssh2.signature.ECDSAKeyAlgorithm", keyValue);
} else if (isClass("com.trilead.ssh2.signature.ECDSAKeyAlgorithm") && "ecdsa-sha2-nistp384".equals(algorithm)) {
decodePublicKey("com.trilead.ssh2.signature.ECDSAKeyAlgorithm", keyValue);
} else if (isClass("ECDSAKeyAlgorithm") && "ecdsa-sha2-nistp521".equals(algorithm)) {
decodePublicKey("com.trilead.ssh2.signature.ECDSAKeyAlgorithm", keyValue);
} else {
throw new IllegalArgumentException("Key algorithm should be one of ssh-rsa or ssh-dss");
throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_UnknownKeyAlgorithm());
}
} catch (IOException ex) {
throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex);
} catch (StringIndexOutOfBoundsException ex) {
// can happen in DSASHA1Verifier with certain values (from quick testing)
} catch (IOException | StringIndexOutOfBoundsException | NoSuchMethodException | IllegalAccessException | InvocationTargetException | ClassNotFoundException ex) {
throw new IllegalArgumentException(Messages.ManualKeyProvidedHostKeyVerifier_KeyValueDoesNotParse(algorithm), ex);
}

return new HostKey(algorithm, keyValue);
}

private static void decodePublicKey(String className, byte[] keyValue) throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException {
Copy link
Member

Choose a reason for hiding this comment

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

I would rather handle these exceptions within the method

Class.forName(className).getMethod("decodePublicKey", byte[].class).invoke(keyValue);
Copy link
Member

Choose a reason for hiding this comment

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

Some logging on the FINE/FINEST level would be really useful. Just to know when the new algorithm decoding fails

}

private static boolean isClass(String className) {
try {
Class.forName(className);
return true;
} catch (ClassNotFoundException ex) {
return false;
}
}

@Extension
public static class ManuallyProvidedKeyVerificationStrategyDescriptor extends SshHostKeyVerificationStrategyDescriptor {
Expand Down
Expand Up @@ -93,7 +93,7 @@ else if (!existingHostKey.equals(hostKey)) {
return false;
}
else {
listener.getLogger().println(Messages.ManualTrustingHostKeyVerifier_KeyTrused(SSHLauncher.getTimestamp()));
listener.getLogger().println(Messages.ManualTrustingHostKeyVerifier_KeyTrusted(SSHLauncher.getTimestamp()));
return true;
}
}
Expand Down Expand Up @@ -135,4 +135,4 @@ public String getDisplayName() {

}

}
}
Expand Up @@ -39,7 +39,7 @@ SSHLauncher.PortLessThanZero=The port value must be greater than 0
SSHLauncher.PortMoreThan65535=The port value must be less than 65536
ManualTrustingHostKeyVerifier.KeyNotTrusted={0} [SSH] WARNING: The SSH key for this host is not currently trusted. Connections will be denied until this new key is authorised.
ManualTrustingHostKeyVerifier.KeyAutoTrusted={0} [SSH] The SSH key with fingerprint {1} has been automatically trusted for connections to this machine.
ManualTrustingHostKeyVerifier.KeyTrused={0} [SSH] SSH host key matches key seen previously for this host. Connection will be allowed.
ManualTrustingHostKeyVerifier.KeyTrusted={0} [SSH] SSH host key matches key seen previously for this host. Connection will be allowed.
Copy link
Member

Choose a reason for hiding this comment

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

Translations are OK since there is no translations of this field

ManualTrustingHostKeyVerifier.DescriptorDisplayName=Manually trusted key Verification Strategy
NonVerifyingHostKeyVerifier.NoVerificationWarning={0} [SSH] WARNING: SSH Host Keys are not being verified. Man-in-the-middle attacks may be possible against this connection.
NonVerifyingHostKeyVerifier.DescriptorDisplayName=Non verifying Verification Strategy
Expand All @@ -49,9 +49,10 @@ ManualKeyProvidedHostKeyVerifier.KeyTrusted={0} [SSH] SSH host key matched the k
ManualKeyProvidedHostKeyVerifier.TwoPartKey=Key should be 2 parts: algorithm and Base 64 encoded key value.
ManualKeyProvidedHostKeyVerifier.Base64EncodedKeyValueRequired=The value part of the key should be a Base64 encoded value.
ManualKeyProvidedHostKeyVerifier.KeyValueDoesNotParse=Key value does not parse into a valid {0} key
ManualKeyProvidedHostKeyVerifier.UnknownKeyAlgorithm=Key algorithm should be one of ssh-rsa, ssh-dss, ssh-ed25519, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, or ecdsa-sha2-nistp521.
ManualKeyProvidedHostKeyVerifier.DisplayName=Manually provided key Verification Strategy
KnownHostsFileHostKeyVerifier.DisplayName=Known hosts file Verification Strategy
KnownHostsFileHostKeyVerifier.NewKeyNotTrusted={0} [SSH] WARNING: No entry currently exists in the Known Hosts file for this host. Connections will be denied until this new host and its associated key is added to the Known Hosts file.
KnownHostsFileHostKeyVerifier.ChangedKeyNotTrusted={0} [SSH] The SSH key presented by the remote host does not match the key saved in the Known Hosts file against this host. Connections to this host will be denied until the two keys match.
KnownHostsFileHostKeyVerifier.KeyTrused={0} [SSH] SSH host key matches key in Known Hosts file. Connection will be allowed.
KnownHostsFileHostKeyVerifier.KeyTrusted={0} [SSH] SSH host key matches key in Known Hosts file. Connection will be allowed.
KnownHostsFileHostKeyVerifier.NoKnownHostsFile={0} [SSH] No Known Hosts file was found at {0}. Please ensure one is created at this path and that Jenkins can read it.