Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-26580] Updated implementation of Jnlp3 protocol
  • Loading branch information
akshayabd committed Apr 23, 2015
1 parent 9bb35a7 commit e9f5caa
Showing 1 changed file with 64 additions and 55 deletions.
119 changes: 64 additions & 55 deletions core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol3.java
Expand Up @@ -13,8 +13,8 @@
import org.jenkinsci.remoting.engine.JnlpProtocol;
import org.jenkinsci.remoting.engine.JnlpProtocol3;
import org.jenkinsci.remoting.engine.jnlp3.ChannelCiphers;
import org.jenkinsci.remoting.engine.jnlp3.CipherUtils;
import org.jenkinsci.remoting.engine.jnlp3.HandshakeCiphers;
import org.jenkinsci.remoting.engine.jnlp3.Jnlp3Util;
import org.jenkinsci.remoting.nio.NioChannelHub;

import javax.crypto.CipherInputStream;
Expand All @@ -28,6 +28,7 @@
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.net.Socket;
import java.nio.charset.Charset;
import java.security.SecureRandom;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
Expand All @@ -38,6 +39,8 @@
* Master-side implementation for JNLP3-connect protocol.
*
* <p>@see {@link JnlpProtocol3} for more details.
*
* @author Akshay Dayal
*/
@Extension
public class JnlpSlaveAgentProtocol3 extends AgentProtocol {
Expand All @@ -59,45 +62,41 @@ static class Handler extends JnlpSlaveHandshake {
public Handler(NioChannelHub hub, Socket socket) throws IOException {
super(hub,socket,
new DataInputStream(socket.getInputStream()),
new PrintWriter(new BufferedWriter(
new OutputStreamWriter(socket.getOutputStream(), "UTF-8")), true));
new PrintWriter(new BufferedWriter(new OutputStreamWriter(
socket.getOutputStream(), Charset.forName("UTF-8"))), true));
}

protected void run() throws IOException, InterruptedException {
request.load(new ByteArrayInputStream(in.readUTF().getBytes("UTF-8")));
// Get initiation information from slave.
request.load(new ByteArrayInputStream(in.readUTF().getBytes(Charset.forName("UTF-8"))));
String nodeName = request.getProperty(JnlpProtocol3.SLAVE_NAME_KEY);
String encryptedChallenge = request.getProperty(JnlpProtocol3.CHALLENGE_KEY);
byte[] handshakeSpecKey = CipherUtils.keyFromString(
request.getProperty(JnlpProtocol3.HANDSHAKE_SPEC_KEY));
String cookie = request.getProperty(JnlpProtocol3.COOKIE_KEY);

// Create handshake ciphers.
SlaveComputer computer = (SlaveComputer) Jenkins.getInstance().getComputer(nodeName);
if(computer == null) {
error("Slave trying to register for invalid node: " + nodeName);
return;
}
String slaveSecret = computer.getJnlpMac();
HandshakeCiphers handshakeCiphers = HandshakeCiphers.create(nodeName, slaveSecret);

HandshakeCiphers handshakeCiphers = null;
try {
handshakeCiphers = HandshakeCiphers.create(nodeName, slaveSecret, handshakeSpecKey);
} catch (Exception e) {
error("Failed to create handshake ciphers for node: " + nodeName);
// Authenticate to the slave.
if (!authenticateToSlave(handshakeCiphers)) {
return;
}

String challenge = null;
try {
challenge = handshakeCiphers.decrypt(encryptedChallenge);
} catch (Exception e) {
throw new IOException("Unable to decrypt challenge", e);
// If there is a cookie decrypt it.
String cookie = null;
if (request.getProperty(JnlpProtocol3.COOKIE_KEY) != null) {
cookie = handshakeCiphers.decrypt(request.getProperty(JnlpProtocol3.COOKIE_KEY));
}
if (!challenge.startsWith(JnlpProtocol3.CHALLENGE_PREFIX)) {
error("Received invalid challenge");

// Validate the slave.
if (!validateSlave(handshakeCiphers)) {
return;
}

// At this point the slave looks legit, check if we think they are already connected.
// The slave is authenticated, see if its already connected.
Channel oldChannel = computer.getChannel();
if(oldChannel != null) {
if (cookie != null && cookie.equals(oldChannel.getProperty(COOKIE_NAME))) {
Expand All @@ -121,50 +120,60 @@ protected void run() throws IOException, InterruptedException {
}
}

// Send challenge response.
String challengeReverse = new StringBuilder(
challenge.substring(JnlpProtocol3.CHALLENGE_PREFIX.length()))
.reverse().toString();
String challengeResponse = JnlpProtocol3.CHALLENGE_PREFIX + challengeReverse;
String encryptedChallengeResponse = null;
try {
encryptedChallengeResponse = handshakeCiphers.encrypt(challengeResponse);
} catch (Exception e) {
throw new IOException("Error encrypting challenge response", e);
}
out.println(encryptedChallengeResponse.getBytes("UTF-8").length);
// Send greeting and new cookie.
out.println(JnlpProtocol.GREETING_SUCCESS);
String newCookie = generateCookie();
out.println(handshakeCiphers.encrypt(newCookie));

// Now get the channel cipher information.
String aesKeyString = handshakeCiphers.decrypt(in.readUTF());
String specKeyString = handshakeCiphers.decrypt(in.readUTF());
ChannelCiphers channelCiphers = ChannelCiphers.create(
Jnlp3Util.keyFromString(aesKeyString),
Jnlp3Util.keyFromString(specKeyString));

Channel establishedChannel = jnlpConnect(computer, channelCiphers);
establishedChannel.setProperty(COOKIE_NAME, newCookie);
}

private boolean authenticateToSlave(HandshakeCiphers handshakeCiphers) throws IOException {
String challenge = handshakeCiphers.decrypt(
request.getProperty(JnlpProtocol3.CHALLENGE_KEY));

// Send slave challenge response.
String challengeResponse = Jnlp3Util.createChallengeResponse(challenge);
String encryptedChallengeResponse = handshakeCiphers.encrypt(challengeResponse);
out.println(encryptedChallengeResponse.getBytes(Charset.forName("UTF-8")).length);
out.print(encryptedChallengeResponse);
out.flush();

// If the slave accepted our challenge response it will send channel cipher keys.
// If the slave accepted our challenge response send our challenge.
String challengeVerificationMessage = in.readUTF();
if (!challengeVerificationMessage.equals(JnlpProtocol.GREETING_SUCCESS)) {
error("Slave did not accept our challenge response");
return;
}
String encryptedAesKeyString = in.readUTF();
String encryptedSpecKeyString = in.readUTF();
ChannelCiphers channelCiphers = null;
try {
String aesKeyString = handshakeCiphers.decrypt(encryptedAesKeyString);
String specKeyString = handshakeCiphers.decrypt(encryptedSpecKeyString);
channelCiphers = ChannelCiphers.create(
CipherUtils.keyFromString(aesKeyString),
CipherUtils.keyFromString(specKeyString));
} catch (Exception e) {
error("Failed to decrypt channel cipher keys");
return;
return false;
}

String newCookie = generateCookie();
try {
out.println(handshakeCiphers.encrypt(newCookie));
} catch (Exception e) {
throw new IOException("Error encrypting cookie", e);
return true;
}

private boolean validateSlave(HandshakeCiphers handshakeCiphers) throws IOException {
String masterChallenge = Jnlp3Util.generateChallenge();
String encryptedMasterChallenge = handshakeCiphers.encrypt(masterChallenge);
out.println(encryptedMasterChallenge.getBytes(Charset.forName("UTF-8")).length);
out.print(encryptedMasterChallenge);
out.flush();

// Verify the challenge response from the slave.
String encryptedMasterChallengeResponse = in.readUTF();
String masterChallengeResponse = handshakeCiphers.decrypt(
encryptedMasterChallengeResponse);
if (!Jnlp3Util.validateChallengeResponse(masterChallenge, masterChallengeResponse)) {
error("Incorrect master challenge response from slave");
return false;
}

Channel establishedChannel = jnlpConnect(computer, channelCiphers);
establishedChannel.setProperty(COOKIE_NAME, newCookie);
return true;
}

protected Channel jnlpConnect(
Expand Down

11 comments on commit e9f5caa

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

I've updated to 1.655 on both server and slave. When I start the slave, I get this:

Mar 28, 2016 3:07:05 PM hudson.remoting.jnlp.Main$CuiListener status
INFO: Server didn't accept the handshake: Unknown protocol:Protocol:JNLP3-connect

Other than updating to 1.655, is there anything else I need to do to get JNLP3 working?

@oleg-nenashev
Copy link
Member

Choose a reason for hiding this comment

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

@jeffkayser2
I think it's due to this piece of code:

static {
String propName = JnlpSlaveAgentProtocol3.class.getName() + ".enabled";
if (System.getProperties().containsKey(propName))
ENABLED = Boolean.getBoolean(propName);
else {
byte hash = Util.fromHexString(Jenkins.getActiveInstance().getLegacyInstanceId())[0];
ENABLED = (hash%10)==0;
}
}

Maybe it makes sense to explicitly set jenkins.slaves.JnlpSlaveAgentProtocol3.enabled on the server side

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

Not sure how to do what you are suggesting. I saw the code that you referenced, but it was not clear to me how to enable it. Is there a command line option that you have to set explicitly for JNLP3 to work?

@oleg-nenashev
Copy link
Member

Choose a reason for hiding this comment

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

Yes, there is a system property.
You would get it set up by starting Jenkins with following settings:

java -Djenkins.slaves.JnlpSlaveAgentProtocol3.enabled=true -jar jenkins.war ...

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

Thanks for the clarification. That worked great!

Mar 28, 2016 10:58:20 PM hudson.remoting.jnlp.Main$CuiListener status
INFO: Trying protocol: JNLP3-connect
Mar 28, 2016 10:58:21 PM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connected

Thank you so much.

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

I'm running into an issue with JNLP3. I have three slaves communicating through a proxy, so by the time the traffic gets to the master, they are coming from the same IP address. With JNLP3, I can't get more than one slave connected at a time. Other slaves connect, but then I get a message:

WARNING: Making {slave name} offline because it’s not responding
Mar 28, 2016 11:35:31 PM hudson.node_monitors.ResponseTimeMonitor$1 monitor

If I remove the "-Djenkins.slaves.JnlpSlaveAgentProtocol3.enabled=true" switch (reverting to JNLP2), I can connect multiple slaves at the same time, no issues.

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

Clarification: the three slaves have the same IP address (from the perspective of the master), but the remote source ports are all different (they are ephemeral ports).

@oleg-nenashev
Copy link
Member

Choose a reason for hiding this comment

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

@jeffkayser2 It makes sense to create a bug for it. JNLP3 is a sensitive security feature, so I think it will be a high priority if it gets confirmed.
CC @daniel-beck @kohsuke @akshayabd

@jeffkayser2
Copy link

Choose a reason for hiding this comment

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

I created issue SECURITY-259: Can only connect one JNLP3 slave per IP address. Sorry if that is the wrong category.

@akshayabd
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm interesting use-case, let me see if its possible to reproduce the issue when multiple JNLP3 nodes run on the same machine, ie. can we reproduce the issue without a proxy first.

@oleg-nenashev
Copy link
Member

Choose a reason for hiding this comment

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

@akshayabd Thanks for looking into it! FYI we've moved the issue to the common tracker. Now it's JENKINS-33886

Please sign in to comment.