[JENKINS-26580] Updated Jnlp3 implementation and added tests.

jenkinsci · Apr 23, 2015 · b0233c3 · b0233c3
1 parent e044831
commit b0233c3
Show file tree

Hide file tree

Showing 22 changed files with 928 additions and 281 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,7 @@
 *.iml
 *.ipr
 *.iws
+.idea
 target
 /.classpath
 /.project

diff --git a/pom.xml b/pom.xml
@@ -126,19 +126,19 @@ THE SOFTWARE.
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-all</artifactId>
-      <version>1.9.5</version>
+      <version>1.10.19</version>
       <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.powermock</groupId>
       <artifactId>powermock-module-junit4</artifactId>
-      <version>1.5</version>
+      <version>1.6.2</version>
       <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.powermock</groupId>
       <artifactId>powermock-api-mockito</artifactId>
-      <version>1.5</version>
+      <version>1.6.2</version>
       <scope>test</scope>
     </dependency>
     <dependency>

diff --git a/src/main/java/org/jenkinsci/remoting/engine/EngineUtil.java b/src/main/java/org/jenkinsci/remoting/engine/EngineUtil.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  * 
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  * 
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  * 
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  * 
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  *
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -27,60 +27,70 @@
 import hudson.remoting.ChannelBuilder;
 import hudson.remoting.EngineListenerSplitter;
 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 javax.crypto.CipherInputStream;
 import javax.crypto.CipherOutputStream;
 import java.io.BufferedInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
-import java.math.BigInteger;
 import java.net.Socket;
-import java.security.SecureRandom;
 import java.util.Properties;
 
 /**
  * Implementation of the JNLP3-connect protocol.
  *
  * <p>This protocol aims to provide a basic level of security for JNLP based
- * slaves. The handshake process uses a
- * <a href="http://en.wikipedia.org/wiki/Challenge%E2%80%93response_authentication#Cryptographic_techniques">
- * challenge-response authentication</a> process so both master and slave can
- * authenticate each other using a shared secret that isn't transmitted. Once
- * they have authenticated each other an encrypted {@link Channel} is setup
- * for further communication.
+ * slaves. Both the master and the slave securely authenticate each other and
+ * then setup an encrypted {@link Channel}.
  *
- * <p>The handshake details are as follows:
+ * <p>The slave secret is never exchanged, but instead used as a shared secret
+ * to generate symmetric key {@link javax.crypto.Cipher}s that can be used to
+ * perform a secure handshake. During the handshake both the slave and the
+ * master send each other challenge phrases which can only be decrypted with
+ * the correct cipher created with the slave secret. Once decrypted the SHA-256
+ * hash of the challenge is computed and sent back to authenticate.
  *
- * <ul>
- * <li>Slave constructs symmetric key {@link javax.crypto.Cipher}s using the
- * slave name, slave secret and a randomly generated
- * {@link javax.crypto.spec.IvParameterSpec}. These ciphers will be used
- * during the handshake process and will be referred to the handshake ciphers.
- * <li>Slave initiates handshake with the master sending it the slave name,
- * generated IvParameterSpec and an encrypted challenge phrase which consists
- * of a small fixed part and a large random
- * part.
- * <li>If the master is genuine it can lookup the slave secret for the slave
- * name and construct a matching ciphers using the slave name, slave secret
- * and given IvParameterSpec.
- * <li>The master will decrypt the challenge phrase and check the fixed part
- * matches to verify the identity of the slave. It will then reverse the random
- * part to construct a challenge response. It will encrypt the challenge
- * response and send it to the slave.
- * <li>The slave will decrypt the challenge response to verify the identity
- * of the master. It will then generate a new pair of symmetric key ciphers
- * and send them to the master encrypted using the handshake ciphers.
- * These new ciphers will be used for constructing an encrypted
- * {@link Channel} for future communication by both sides.
- * </ul>
+ * <p>Once the handshake is successful another pair of symmetric key ciphers is
+ * created using random keys that are exchanged. These ciphers are used to
+ * create an encrypted channel.
  *
- * <p>The handshake does not require the slave secrets be sent over the wire,
- * instead the challenge-response process leverages that if both parties are
- * genuine then they both should have the slave secret which can be used to
- * construct ciphers.
+ * <p>The following goes over the handshake in more detail:
+ * <pre>
+ * Client                                                                Master
+ *           handshake ciphers = createFrom(slave name, slave secret)
+ *
+ *   |                                                                     |
+ *   |      initiate(slave name, encrypt(challenge), encrypt(cookie))      |
+ *   |  -------------------------------------------------------------->>>  |
+ *   |                                                                     |
+ *   |                       encrypt(hash(challenge))                      |
+ *   |  <<<--------------------------------------------------------------  |
+ *   |                                                                     |
+ *   |                          GREETING_SUCCESS                           |
+ *   |  -------------------------------------------------------------->>>  |
+ *   |                                                                     |
+ *   |                         encrypt(challenge)                          |
+ *   |  <<<--------------------------------------------------------------  |
+ *   |                                                                     |
+ *   |                       encrypt(hash(challenge))                      |
+ *   |  -------------------------------------------------------------->>>  |
+ *   |                                                                     |
+ *   |                          GREETING_SUCCESS                           |
+ *   |  <<<--------------------------------------------------------------  |
+ *   |                                                                     |
+ *   |                          encrypt(cookie)                            |
+ *   |  <<<--------------------------------------------------------------  |
+ *   |                                                                     |
+ *   |                  encrypt(AES key) + encrypt(IvSpec)                 |
+ *   |  -------------------------------------------------------------->>>  |
+ *   |                                                                     |
+ *
+ *              channel ciphers = createFrom(AES key, IvSpec)
+ *           channel = channelBuilder.createWith(channel ciphers)
+ * </pre>
  *
  * <p>The entire process assumes the slave secret has not been leaked
  * beforehand and the slave obtains it in a secure manner.
@@ -116,50 +126,31 @@ String getCookie() {
         return cookie;
     }
 
+    ChannelCiphers getChannelCiphers() {
+        return channelCiphers;
+    }
+
     @Override
     boolean performHandshake(DataOutputStream outputStream,
             BufferedInputStream inputStream) throws IOException {
-        HandshakeCiphers handshakeCiphers = null;
-        try {
-            handshakeCiphers = HandshakeCiphers.create(slaveName, slaveSecret, null);
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to create handshake ciphers", e);
-            return false;
-        }
-
-        String challenge = generateChallenge();
-        if (!initiateHandshakeWithChallenge(challenge, outputStream, handshakeCiphers)) {
-            return false;
-        }
+        HandshakeCiphers handshakeCiphers = HandshakeCiphers.create(slaveName, slaveSecret);
 
-        if (!verifyChallengeResponse(challenge, inputStream, handshakeCiphers)) {
+        // Authenticate the master.
+        if (!initiateAndValidateMaster(inputStream, outputStream, handshakeCiphers)) {
             return false;
         }
 
-        // Master authenticated, send encryption keys to use for Channel.
-        try {
-            channelCiphers = ChannelCiphers.create();
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to create channel ciphers", e);
-            return false;
-        }
-        outputStream.writeUTF(GREETING_SUCCESS);
-        try {
-            outputStream.writeUTF(handshakeCiphers.encrypt(
-                    CipherUtils.keyToString(channelCiphers.getAesKey())));
-            outputStream.writeUTF(handshakeCiphers.encrypt(
-                    CipherUtils.keyToString(channelCiphers.getSpecKey())));
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to encrypt channel ciphers", e);
+        // The master now wants to authenticate us.
+        if (!authenticateToMaster(inputStream, outputStream, handshakeCiphers)) {
             return false;
         }
 
-        try {
-            cookie = handshakeCiphers.decrypt(EngineUtil.readLine(inputStream));
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to decrypt cookie", e);
-            return false;
-        }
+        // Authentication complete, send encryption keys to use for Channel.
+        channelCiphers = ChannelCiphers.create();
+        outputStream.writeUTF(handshakeCiphers.encrypt(
+                Jnlp3Util.keyToString(channelCiphers.getAesKey())));
+        outputStream.writeUTF(handshakeCiphers.encrypt(
+                Jnlp3Util.keyToString(channelCiphers.getSpecKey())));
 
         return true;
     }
@@ -172,80 +163,58 @@ Channel buildChannel(Socket socket, ChannelBuilder channelBuilder) throws IOExce
         );
     }
 
-    private boolean initiateHandshakeWithChallenge(
-            String challenge, DataOutputStream outputStream,
-            HandshakeCiphers handshakeCiphers) throws IOException {
-        String encryptedChallenge = null;
-        try {
-            encryptedChallenge = handshakeCiphers.encrypt(challenge);
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to create encrypted challenge", e);
-            return false;
-        }
+    private boolean initiateAndValidateMaster(BufferedInputStream inputStream,
+            DataOutputStream outputStream, HandshakeCiphers handshakeCiphers) throws IOException {
+        String challenge = Jnlp3Util.generateChallenge();
 
+        // Send initial information which includes the encrypted challenge.
         Properties props = new Properties();
         props.put(SLAVE_NAME_KEY, slaveName);
-        props.put(HANDSHAKE_SPEC_KEY, CipherUtils.keyToString(handshakeCiphers.getSpecKey()));
-        props.put(CHALLENGE_KEY, encryptedChallenge);
-
-        // If there is a cookie send that as well.
+        props.put(CHALLENGE_KEY, handshakeCiphers.encrypt(challenge));
         if (cookie != null) {
-            props.put(COOKIE_KEY, cookie);
+            props.put(COOKIE_KEY, handshakeCiphers.encrypt(cookie));
         }
         ByteArrayOutputStream o = new ByteArrayOutputStream();
         props.store(o, null);
-
         outputStream.writeUTF(PROTOCOL_PREFIX + NAME);
         outputStream.writeUTF(o.toString("UTF-8"));
-        return true;
-    }
 
-    private boolean verifyChallengeResponse(
-            String challenge, BufferedInputStream inputStream,
-            HandshakeCiphers handshakeCiphers) throws IOException {
+        // Validate challenge response.
         Integer challengeResponseLength = Integer.parseInt(EngineUtil.readLine(inputStream));
         String encryptedChallengeResponse = EngineUtil.readChars(
                 inputStream, challengeResponseLength);
-        String challengeResponse = null;
-        try {
-            challengeResponse = handshakeCiphers.decrypt(encryptedChallengeResponse);
-        } catch (Exception e) {
-            events.status(NAME + ": Failed to decrypt response from master", e);
+        String challengeResponse = handshakeCiphers.decrypt(encryptedChallengeResponse);
+        if (!Jnlp3Util.validateChallengeResponse(challenge, challengeResponse)) {
+            events.status(NAME + ": Incorrect challenge response from master");
             return false;
         }
 
-        if (!challengeResponse.startsWith(CHALLENGE_PREFIX)) {
-            events.status("Response from master did not start with challenge prefix");
-            return false;
-        }
+        outputStream.writeUTF(GREETING_SUCCESS);
+        return true;
+    }
+
+    private boolean authenticateToMaster(BufferedInputStream inputStream,
+            DataOutputStream outputStream, HandshakeCiphers handshakeCiphers) throws IOException {
+        // Read the master challenge.
+        Integer challengeLength = Integer.parseInt(EngineUtil.readLine(inputStream));
+        String encryptedChallenge = EngineUtil.readChars(inputStream, challengeLength);
+        String masterChallenge = handshakeCiphers.decrypt(encryptedChallenge);
 
-        // The master should have reversed the challenge phrase (minus the prefix).
-        if (!challenge.substring(CHALLENGE_PREFIX.length()).equals(
-                new StringBuilder(challengeResponse.substring(CHALLENGE_PREFIX.length()))
-                        .reverse().toString())) {
-            events.status("Master authentication failed");
+        // Send the response.
+        String challengeResponse = Jnlp3Util.createChallengeResponse(masterChallenge);
+        String encryptedChallengeResponse = handshakeCiphers.encrypt(challengeResponse);
+        outputStream.writeUTF(encryptedChallengeResponse);
+
+        // See if the master accepted us.
+        if (!GREETING_SUCCESS.equals(EngineUtil.readLine(inputStream))) {
             return false;
         }
-
+        cookie = handshakeCiphers.decrypt(EngineUtil.readLine(inputStream));
         return true;
     }
 
-    /**
-     * Generate a challenge for the master to respond to. The challenge will
-     * consist of 2 parts, a small fixed prefix and a large random part.
-     *
-     * <p>When the master decrypts the challenge it will check to see it starts
-     * with the fixed prefix to verify the identity of the slave.
-     */
-    private String generateChallenge() {
-        String randomString = new BigInteger(10400, new SecureRandom()).toString(32);
-        return CHALLENGE_PREFIX + randomString;
-    }
-
     public static final String CHALLENGE_KEY = "Challenge";
-    public static final String CHALLENGE_PREFIX = "JNLP3";
     public static final String COOKIE_KEY = "Cookie";
-    public static final String HANDSHAKE_SPEC_KEY = "Handshake-Spec";
     public static final String NAME = "JNLP3-connect";
     public static final String SLAVE_NAME_KEY = "Node-Name";
 }
diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocolFactory.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpProtocolFactory.java
@@ -1,7 +1,7 @@
 /*
  * The MIT License
  * 
- * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
+ * Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi, CloudBees, Inc.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -51,7 +51,7 @@ public class JnlpProtocolFactory {
     public static List<JnlpProtocol> createProtocols(String slaveName, String slaveSecret,
             EngineListenerSplitter events) {
         return Arrays.asList(
-            new JnlpProtocol3(slaveName, slaveSecret, events),
+            new JnlpProtocol2(slaveName, slaveSecret, events),
             new JnlpProtocol1(slaveName, slaveSecret, events)
         );
     }