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-75011] Use Apache Mina as ssh transport layer, remove trilead #1022

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 8 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ THE SOFTWARE.
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.jenkins.plugins.mina-sshd-api</groupId>
<artifactId>mina-sshd-api-core</artifactId>
</dependency>
<dependency>
<groupId>io.jenkins.plugins.mina-sshd-api</groupId>
<artifactId>mina-sshd-api-scp</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>apache-httpcomponents-client-4-api</artifactId>
Expand Down Expand Up @@ -138,10 +146,6 @@ THE SOFTWARE.
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>ssh-credentials</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>trilead-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.aws-java-sdk</groupId>
<artifactId>aws-java-sdk-ec2</artifactId>
Expand Down
24 changes: 24 additions & 0 deletions src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,22 @@
*/
package hudson.plugins.ec2;

import static org.apache.sshd.client.session.ClientSession.REMOTE_COMMAND_WAIT_EVENTS;

import com.amazonaws.AmazonClientException;
import hudson.model.TaskListener;
import hudson.slaves.ComputerLauncher;
import hudson.slaves.SlaveComputer;
import java.io.IOException;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.channel.ClientChannelEvent;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.scp.client.CloseableScpClient;
import org.apache.sshd.scp.client.ScpClient;
import org.apache.sshd.scp.client.ScpClientCreator;

/**
* {@link ComputerLauncher} for EC2 that wraps the real user-specified {@link ComputerLauncher}.
Expand Down Expand Up @@ -81,4 +90,19 @@
*/
protected abstract void launchScript(EC2Computer computer, TaskListener listener)
throws AmazonClientException, IOException, InterruptedException;

protected int waitCompletion(ClientChannel clientChannel, long timeout) {
Set<ClientChannelEvent> clientChannelEvents = clientChannel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, timeout);
if (clientChannelEvents.contains(ClientChannelEvent.TIMEOUT)) {
return -1;
} else {
return clientChannel.getExitStatus();
}
}

protected CloseableScpClient createScpClient(ClientSession session) {
ScpClientCreator creator = ScpClientCreator.instance();
ScpClient client = creator.createScpClient(session);
return CloseableScpClient.singleSessionInstance(client);

Check warning on line 106 in src/main/java/hudson/plugins/ec2/EC2ComputerLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 95-106 are not covered by tests
}
}
536 changes: 334 additions & 202 deletions src/main/java/hudson/plugins/ec2/ssh/EC2MacLauncher.java

Large diffs are not rendered by default.

535 changes: 333 additions & 202 deletions src/main/java/hudson/plugins/ec2/ssh/EC2UnixLauncher.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
*/
package hudson.plugins.ec2.ssh;

import com.trilead.ssh2.ServerHostKeyVerifier;
import java.security.MessageDigest;
import java.util.logging.Logger;

public class HostKeyVerifierImpl implements ServerHostKeyVerifier {
public class HostKeyVerifierImpl {

Choose a reason for hiding this comment

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

Should we refactor the class name as it is no longer implementing the Interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept the name as this is a public class.

private static final Logger LOGGER = Logger.getLogger(HostKeyVerifierImpl.class.getName());

private final String console;
Expand All @@ -51,7 +50,6 @@ private String getFingerprint(byte[] serverHostKey) throws Exception {
return buf.toString();
}

@Override
public boolean verifyServerHostKey(String hostname, int port, String serverHostKeyAlgorithm, byte[] serverHostKey)
throws Exception {
String fingerprint = getFingerprint(serverHostKey);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package hudson.plugins.ec2.ssh.proxy;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.Base64;
import org.apache.sshd.client.session.ClientProxyConnector;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;

/**
* {@link ClientProxyConnector} that issue an HTTP CONNECT to connect through an HTTP proxy.
*/
public class ProxyCONNECTListener implements ClientProxyConnector {

private static final long timeout = Duration.ofSeconds(10).toMillis();

public final String targetHost;
public final int targetPort;
public final String proxyUser;
public final String proxyPass;

Check warning

Code scanning / Jenkins Security Scan

Jenkins: Plaintext password storage Warning

Field should be reviewed whether it stores a password and is serialized to disk: proxyPass

public ProxyCONNECTListener(String targetHost, int targetPort, String proxyUser, String proxyPass) {
this.targetHost = targetHost;
this.targetPort = targetPort;
this.proxyUser = proxyUser;
this.proxyPass = proxyPass;
}

@Override
public void sendClientProxyMetadata(ClientSession session) throws Exception {
proxyCONNECT(session.getIoSession());
}

public void proxyCONNECT(IoSession ioSession) {
StringBuilder connectRequest = new StringBuilder();

// Based on https://www.rfc-editor.org/rfc/rfc7231#section-4.3.6
connectRequest
.append("CONNECT ")
.append(targetHost)
.append(':')
.append(targetPort)
.append(" HTTP/1.0\r\n");
// Host should be included https://datatracker.ietf.org/doc/html/rfc2616#section-14.23
connectRequest
.append("Host: ")
.append(targetHost)
.append(':')
.append(targetPort)
.append("\r\n");

if ((proxyUser != null) && (proxyPass != null)) {
String credentials = proxyUser + ":" + proxyPass;
String encoded = Base64.getEncoder().encodeToString(credentials.getBytes(StandardCharsets.ISO_8859_1));
connectRequest.append("Proxy-Authorization: Basic ");
connectRequest.append(encoded);
connectRequest.append("\r\n");
}

// End of the header
connectRequest.append("\r\n");

try {
ioSession
.writeBuffer(new ByteArrayBuffer(connectRequest.toString().getBytes(StandardCharsets.US_ASCII)))
.await(timeout);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

Check warning on line 72 in src/main/java/hudson/plugins/ec2/ssh/proxy/ProxyCONNECTListener.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 17-72 are not covered by tests
}
11 changes: 9 additions & 2 deletions src/main/java/hudson/plugins/ec2/ssh/verifiers/HostKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@
*/
package hudson.plugins.ec2.ssh.verifiers;

import com.trilead.ssh2.KnownHosts;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.Serializable;
import java.util.Arrays;
import org.apache.sshd.common.digest.BuiltinDigests;
import org.apache.sshd.common.digest.DigestUtils;
import org.apache.sshd.common.util.buffer.BufferUtils;

/**
* A representation of the SSH key provided by a remote host to verify itself
Expand Down Expand Up @@ -69,7 +71,12 @@
}

public String getFingerprint() {
return KnownHosts.createHexFingerprint(getAlgorithm(), getKey());
try {
byte[] rawFingerprint = DigestUtils.getRawFingerprint(BuiltinDigests.md5.get(), getKey());
return BufferUtils.toHex(':', rawFingerprint).toLowerCase();
} catch (Exception e) {
return "";

Check warning on line 78 in src/main/java/hudson/plugins/ec2/ssh/verifiers/HostKey.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 77-78 are not covered by tests
}
}

@Override
Expand Down
161 changes: 161 additions & 0 deletions src/main/java/hudson/plugins/ec2/util/KeyHelper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package hudson.plugins.ec2.util;

import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.io.StringReader;
import java.security.KeyFactory;
import java.security.KeyPair;
import java.security.NoSuchAlgorithmException;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.interfaces.DSAParams;
import java.security.interfaces.DSAPrivateKey;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPrivateCrtKey;
import java.security.spec.DSAPublicKeySpec;
import java.security.spec.ECField;
import java.security.spec.ECParameterSpec;
import java.security.spec.EllipticCurve;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.RSAPublicKeySpec;
import java.security.spec.X509EncodedKeySpec;
import org.bouncycastle.asn1.ASN1BitString;
import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
import org.bouncycastle.jcajce.interfaces.EdDSAPrivateKey;
import org.bouncycastle.openssl.PEMEncryptedKeyPair;
import org.bouncycastle.openssl.PEMKeyPair;
import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
import org.bouncycastle.openssl.jcajce.JcePEMDecryptorProviderBuilder;

/**
* Utility class to parse PEM.
*/
public abstract class KeyHelper {
private KeyHelper() {}

/**
* Decodes a PEM-encoded key pair into a {@link KeyPair} object. This method supports
* various types of PEM input such as encrypted private keys, public keys, and key pairs.
*
* @param pem The PEM-formatted string containing the key data.
* @param password The password used to decrypt encrypted key pairs, if applicable. Can be null if no password is required.
* @return A {@link KeyPair} containing the public and private keys. If a public key is provided without a matching private key,
* the private key in the returned {@link KeyPair} will be null.
* @throws IOException If an error occurs during parsing or decryption of the PEM input.
* @throws IllegalArgumentException If the provided PEM input cannot be parsed or is of an unsupported type.
*/
public static KeyPair decodeKeyPair(@NonNull String pem, @NonNull String password) throws IOException {
try (org.bouncycastle.openssl.PEMParser pemParser =
new org.bouncycastle.openssl.PEMParser(new StringReader(pem))) {
Object object = pemParser.readObject();
if (object == null) {
throw new IllegalArgumentException("Failed to parse PEM input");
}
JcaPEMKeyConverter converter = new JcaPEMKeyConverter();

if (object instanceof PEMEncryptedKeyPair) {
PEMKeyPair decryptedKeyPair = ((PEMEncryptedKeyPair) object)
.decryptKeyPair(new JcePEMDecryptorProviderBuilder().build(password.toCharArray()));
Copy link

@PereBueno PereBueno Jan 20, 2025

Choose a reason for hiding this comment

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

Might throw a NPE when password is null (e.g.: https://github.com/jenkinsci/ec2-plugin/pull/1022/files#diff-1d5f872d1bd42e798ad1690b14df1b4a4f79926b27a7be424148228e9073ac5aR76) let's add a @NonNull annotation or perform a null check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added: 9abf818

Choose a reason for hiding this comment

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

since the password can be null, it would be better to remove the @NonNull annotation and add a null check?

PrivateKey privateKey = converter.getPrivateKey(decryptedKeyPair.getPrivateKeyInfo());
PublicKey publicKey = converter.getPublicKey(decryptedKeyPair.getPublicKeyInfo());
return new KeyPair(publicKey, privateKey);
} else if (object instanceof PrivateKeyInfo) {
PrivateKeyInfo privateKeyInfo = (PrivateKeyInfo) object;
PrivateKey privateKey = converter.getPrivateKey(privateKeyInfo);
PublicKey publicKey = generatePublicKeyFromPrivateKey(privateKeyInfo, privateKey);
return new KeyPair(publicKey, privateKey);
} else if (object instanceof SubjectPublicKeyInfo) {
PublicKey publicKey = converter.getPublicKey((SubjectPublicKeyInfo) object);
return new KeyPair(publicKey, null);
} else if (object instanceof PEMKeyPair) {
SubjectPublicKeyInfo publicKeyInfo = ((PEMKeyPair) object).getPublicKeyInfo();
PrivateKeyInfo privateKeyInfo = ((PEMKeyPair) object).getPrivateKeyInfo();
return new KeyPair(converter.getPublicKey(publicKeyInfo), converter.getPrivateKey(privateKeyInfo));
} else {
throw new IllegalArgumentException(
"Unsupported PEM object type: " + object.getClass().getName());
}
} catch (Exception e) {
throw new IOException("Failed to parse PEM input", e);

Check warning on line 81 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 50-81 are not covered by tests
}
}

/* visible for testing */
/**
* Extract a {@link PublicKey} from the given {@link PrivateKey}
* @param privateKey the private key to extract from
* @return the corresponding public key or null if the extraction is not possible
*/
static PublicKey generatePublicKeyFromPrivateKey(PrivateKeyInfo privateKeyInfo, @NonNull PrivateKey privateKey) {
try {
if (privateKey instanceof RSAPrivateCrtKey)
return KeyFactory.getInstance("RSA")
.generatePublic(new RSAPublicKeySpec(
((RSAPrivateCrtKey) privateKey).getModulus(),
((RSAPrivateCrtKey) privateKey).getPublicExponent()));
else if (privateKey instanceof DSAPrivateKey) {

Check warning on line 98 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 98 is only partially covered, one branch is missing
DSAParams dsaParams = ((DSAPrivateKey) privateKey).getParams();
return KeyFactory.getInstance("DSA")
.generatePublic(new DSAPublicKeySpec(
dsaParams.getG().modPow(((DSAPrivateKey) privateKey).getX(), dsaParams.getP()),
dsaParams.getP(),
dsaParams.getQ(),
dsaParams.getG()));

Check warning on line 105 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 99-105 are not covered by tests
} else if (privateKey instanceof ECPrivateKey) {
ASN1BitString asn1BitString = org.bouncycastle.asn1.sec.ECPrivateKey.getInstance(
privateKeyInfo.getPrivateKey().getOctets())
.getPublicKey();
return KeyFactory.getInstance("EC")
.generatePublic(new X509EncodedKeySpec(
new SubjectPublicKeyInfo(privateKeyInfo.getPrivateKeyAlgorithm(), asn1BitString)
.getEncoded()));
} else if (privateKey instanceof EdDSAPrivateKey) {
return ((EdDSAPrivateKey) privateKey).getPublicKey();
} else {

Choose a reason for hiding this comment

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

Are we only supporting two algorithms?

Copy link
Contributor Author

@jmdesprez jmdesprez Jan 20, 2025

Choose a reason for hiding this comment

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

I rewrote it in a more robust way. Anyway, I think that this is not used because we get a PrivateKeyInfo.
4e9d26d

return null;
}
} catch (NoSuchAlgorithmException | IOException | InvalidKeySpecException e) {
return null;

Check warning on line 120 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 119-120 are not covered by tests
}
}

/**
* Determines the SSH algorithm identifier corresponding to the given server public key.
* This method matches the key type to the appropriate SSH algorithm string.
* When an {@link ECPublicKey} is given, an NIST curse will be assumed.
*
* @param serverKey The server's {@link PublicKey} object for which the SSH algorithm identifier
* needs to be determined.
* @return A {@code String} representing the SSH algorithm identifier for the given server key,
* or {@code null} if the key type is unsupported or cannot be determined.
*/
public static String getSshAlgorithm(@NonNull PublicKey serverKey) {
switch (serverKey.getAlgorithm()) {
case "RSA":
return "ssh-rsa";
case "EC":
if (serverKey instanceof ECPublicKey) {

Check warning on line 139 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 139 is only partially covered, one branch is missing
ECPublicKey ecPublicKey = (ECPublicKey) serverKey;
ECParameterSpec params = ecPublicKey.getParams();
if (params != null) {

Check warning on line 142 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 142 is only partially covered, one branch is missing

EllipticCurve curve = params.getCurve();
ECField field = (curve != null) ? curve.getField() : null;

Check warning on line 145 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 145 is only partially covered, one branch is missing
if (field != null) {

Check warning on line 146 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 146 is only partially covered, one branch is missing
int fieldSize = field.getFieldSize();
// Assume NIST curve
return "ecdsa-sha2-nistp" + fieldSize;
}
}
}
return null;

Check warning on line 153 in src/main/java/hudson/plugins/ec2/util/KeyHelper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 153 is not covered by tests
case "EdDSA":
case "Ed25519":
return "ssh-ed25519";
default:
return null;
}
}
}
Loading
Loading