[SECURITY-54]

Jesse's original patch

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -40,6 +40,7 @@ import hudson.AbortException;
 import hudson.remoting.Launcher;
 import static hudson.slaves.SlaveComputer.LogHolder.SLAVE_LOG_HANDLER;
 import hudson.slaves.OfflineCause.ChannelTermination;
+import hudson.util.Secret;
 
 import java.io.File;
 import java.io.OutputStream;
@@ -58,6 +59,13 @@ import java.util.concurrent.Future;
 import java.security.Security;
 
 import hudson.util.io.ReopenableFileOutputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintWriter;
+import java.security.GeneralSecurityException;
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import javax.servlet.RequestDispatcher;
 import jenkins.model.Jenkins;
 import jenkins.slaves.JnlpSlaveAgentProtocol;
 import org.kohsuke.stapler.StaplerRequest;
@@ -67,7 +75,11 @@ import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpRedirect;
 
 import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletResponse;
+import javax.servlet.ServletOutputStream;
+import javax.servlet.http.HttpServletResponseWrapper;
+import org.kohsuke.stapler.ResponseImpl;
+import org.kohsuke.stapler.WebMethod;
+import org.kohsuke.stapler.compression.FilterServletOutputStream;
 
 /**
  * {@link Computer} for {@link Slave}s.
@@ -129,6 +141,9 @@ public class SlaveComputer extends Computer {
         return acceptingTasks;
     }
 
+    /**
+     * @since 1.498
+     */
     public String getJnlpMac() {
         return JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(getName());
     }
@@ -535,6 +550,40 @@ public class SlaveComputer extends Computer {
         return new Slave.JnlpJar(fileName);
     }
 
+    @WebMethod(name="slave-agent.jnlp")
+    public void doSlaveAgentJnlp(StaplerRequest req, StaplerResponse res) throws IOException, ServletException {
+        RequestDispatcher view = req.getView(this, "slave-agent.jnlp.jelly");
+        if ("true".equals(req.getParameter("encrypt"))) {
+            req.setAttribute("jnlpMac", "SLAVE_SECRET");
+            final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            StaplerResponse temp = new ResponseImpl(req.getStapler(), new HttpServletResponseWrapper(res) {
+                @Override public ServletOutputStream getOutputStream() throws IOException {
+                    return new FilterServletOutputStream(baos);
+                }
+                @Override public PrintWriter getWriter() throws IOException {
+                    throw new IllegalStateException();
+                }
+            });
+            view.forward(req, temp);
+            byte[] jnlpMac = JnlpSlaveAgentProtocol.SLAVE_SECRET.mac(getName().getBytes("UTF-8"));
+            SecretKey key = new SecretKeySpec(jnlpMac, 0, /* export restrictions */ 128 / 8, "AES");
+            byte[] encrypted;
+            try {
+                Cipher c = Secret.getCipher("AES");
+                c.init(Cipher.ENCRYPT_MODE, key);
+                encrypted = c.doFinal(baos.toByteArray());
+            } catch (GeneralSecurityException x) {
+                throw new IOException(x);
+            }
+            res.setContentType("application/octet-stream");
+            res.getOutputStream().write(encrypted);
+        } else {
+            checkPermission(CONNECT);
+            req.setAttribute("jnlpMac", getJnlpMac());
+            view.forward(req, res);
+        }
+    }
+
     @Override
     protected void kill() {
         super.kill();

core/src/main/java/jenkins/model/Jenkins.java

@@ -3567,6 +3567,8 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
             || rest.startsWith("/adjuncts/")
             || rest.startsWith("/signup")
             || rest.startsWith("/tcpSlaveAgentListener")
+            // XXX SlaveComputer.doSlaveAgentJnlp; there should be an annotation to request unprotected access
+            || rest.matches("/computer/.+/slave-agent[.]jnlp") && "true".equals(Stapler.getCurrentRequest().getParameter("encrypt"))
             || rest.startsWith("/cli")
             || rest.startsWith("/federatedLoginService/")
             || rest.startsWith("/securityRealm"))

core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly

@@ -65,7 +65,8 @@ THE SOFTWARE.
                 <p>
                   ${%Run from slave command line:}
                 </p>
-                <pre>java -jar <a href="${rootURL}/jnlpJars/slave.jar">slave.jar</a> -jnlpUrl ${h.inferHudsonURL(request)}${it.url}slave-agent.jnlp -jnlpCredentials ${app.authentication.name}:<a href="${rootURL}/user/${app.authentication.name}/configure" target="_blank">your-API-token</a></pre>
+                <!-- XXX conceal secret w/ JS if possible -->
+                <pre>java -jar <a href="${rootURL}/jnlpJars/slave.jar">slave.jar</a> -jnlpUrl ${h.inferHudsonURL(request)}${it.url}slave-agent.jnlp -secret ${it.jnlpMac}</pre>
               </li>
             </j:otherwise>
           </j:choose>
@@ -85,4 +86,4 @@ THE SOFTWARE.
       </p>
     </j:otherwise>
   </j:choose>
-</j:jelly>
\ No newline at end of file
+</j:jelly>

core/src/main/resources/hudson/slaves/SlaveComputer/slave-agent.jnlp.jelly

@@ -31,8 +31,6 @@ THE SOFTWARE.
   <!--
     See http://www.dallaway.com/acad/webstart/ for obtaining the certificate.
   -->
-  <j:getStatic var="connect" className="hudson.model.Computer" field="CONNECT"/>
-  ${it.checkPermission(connect)}
 
     <!-- See http://java.sun.com/j2se/1.5.0/docs/guide/javaws/developersguide/syntax.html for the syntax -->
     <jnlp spec="1.0+"
@@ -63,7 +61,7 @@ THE SOFTWARE.
       </resources>
 
       <application-desc main-class="hudson.remoting.jnlp.Main">
-        <argument>${it.jnlpMac}</argument>
+        <argument>${jnlpMac}</argument>
         <argument>${it.node.nodeName}</argument>
         <j:if test="${it.launcher.tunnel!=null}">
           <argument>-tunnel</argument>

pom.xml

@@ -187,7 +187,7 @@ THE SOFTWARE.
       <dependency>
         <groupId>org.jenkins-ci.main</groupId>
         <artifactId>remoting</artifactId>
-        <version>2.21</version>
+        <version>2.22</version>
       </dependency>
 
       <dependency>

test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java

@@ -83,7 +83,7 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase {
             assertTrue(jarResource.getWebResponse().getContentType().toLowerCase(Locale.ENGLISH).startsWith("application/"));
         }
 
-
+        // XXX this should be the only part with ANONYMOUS_READONLY
         try {
             jnlp = (XmlPage) jnlpAgent.goTo("computer/test/slave-agent.jnlp", "application/x-java-jnlp-file");
             fail("anonymous users must not be able to get secrets");
@@ -91,4 +91,7 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase {
             assertEquals(HttpURLConnection.HTTP_FORBIDDEN, x.getStatusCode());
         }
     }
+
+    // XXX try to use -secret
+
 }