[JENKINS-18114] Fixing CLI crumbs

cli/src/main/java/hudson/cli/CLI.java

@@ -549,7 +549,7 @@ public boolean verify(String s, SSLSession sslSession) {
                 for (Handler h : Logger.getLogger("").getHandlers()) {
                     h.setLevel(level);
                 }
-                for (Logger logger : new Logger[] {LOGGER, PlainCLIProtocol.LOGGER, Logger.getLogger("org.apache.sshd")}) { // perhaps also Channel
+                for (Logger logger : new Logger[] {LOGGER, FullDuplexHttpStream.LOGGER, PlainCLIProtocol.LOGGER, Logger.getLogger("org.apache.sshd")}) { // perhaps also Channel
                     logger.setLevel(level);
                 }
                 args = args.subList(2, args.size());

cli/src/main/java/hudson/cli/FullDuplexHttpStream.java

@@ -1,9 +1,7 @@
 package hudson.cli;
 
-import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
@@ -16,7 +14,7 @@
 /**
  * Creates a capacity-unlimited bi-directional {@link InputStream}/{@link OutputStream} pair over
  * HTTP, which is a request/response protocol.
- *
+ * {@code FullDuplexHttpService} is the counterpart on the server side.
  * @author Kohsuke Kawaguchi
  */
 public class FullDuplexHttpStream {
@@ -29,10 +27,18 @@ public class FullDuplexHttpStream {
     private final OutputStream output;
     private final InputStream input;
 
+    /**
+     * A way to get data from the server.
+     * There will be an initial zero byte used as a handshake which you should expect and ignore.
+     */
     public InputStream getInputStream() {
         return input;
     }
 
+    /**
+     * A way to upload data to the server.
+     * You will need to write to this and {@link OutputStream#flush} it to finish establishing a connection.
+     */
     public OutputStream getOutputStream() {
         return output;
     }
@@ -79,11 +85,10 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio
 
         URL target = new URL(this.base, relativeTarget);
 
-        CrumbData crumbData = new CrumbData();
-
         UUID uuid = UUID.randomUUID(); // so that the server can correlate those two connections
 
         // server->client
+        LOGGER.fine("establishing download side");
         HttpURLConnection con = (HttpURLConnection) target.openConnection();
         con.setDoOutput(true); // request POST to avoid caching
         con.setRequestMethod("POST");
@@ -92,17 +97,16 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio
         if (authorization != null) {
             con.addRequestProperty("Authorization", authorization);
         }
-        if(crumbData.isValid) {
-            con.addRequestProperty(crumbData.crumbName, crumbData.crumb);
-        }
         con.getOutputStream().close();
         input = con.getInputStream();
-        // make sure we hit the right URL
+        // make sure we hit the right URL; no need for CLI.verifyJenkinsConnection here
         if (con.getHeaderField("Hudson-Duplex") == null) {
             throw new CLI.NotTalkingToJenkinsException("There's no Jenkins running at " + target + ", or is not serving the HTTP Duplex transport");
         }
+        LOGGER.fine("established download side"); // calling getResponseCode or getHeaderFields breaks everything
 
-        // client->server uses chunked encoded POST for unlimited capacity. 
+        // client->server uses chunked encoded POST for unlimited capacity.
+        LOGGER.fine("establishing upload side");
         con = (HttpURLConnection) target.openConnection();
         con.setDoOutput(true); // request POST
         con.setRequestMethod("POST");
@@ -113,11 +117,8 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio
         if (authorization != null) {
         	con.addRequestProperty ("Authorization", authorization);
         }
-
-        if(crumbData.isValid) {
-            con.addRequestProperty(crumbData.crumbName, crumbData.crumb);
-        }
         output = con.getOutputStream();
+        LOGGER.fine("established upload side");
     }
 
     // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first.
@@ -141,59 +142,4 @@ private URL tryToResolveRedirects(URL base, String authorization) {
     static final int BLOCK_SIZE = 1024;
     static final Logger LOGGER = Logger.getLogger(FullDuplexHttpStream.class.getName());
 
-    private final class CrumbData {
-    	String crumbName;
-    	String crumb;
-    	boolean isValid;
-
-    	private CrumbData() {
-            this.crumbName = "";
-            this.crumb = "";
-            this.isValid = false;
-            getData();
-    	}
-
-    	private void getData() {
-            try {
-                String base = createCrumbUrlBase();
-                String[] pair = readData(base + "?xpath=concat(//crumbRequestField,\":\",//crumb)").split(":", 2);
-                crumbName = pair[0];
-                crumb = pair[1];
-                isValid = true;
-                LOGGER.fine("Crumb data: "+crumbName+"="+crumb);
-            } catch (IOException e) {
-                // presumably this Hudson doesn't use crumb 
-                LOGGER.log(Level.FINE,"Failed to get crumb data",e);
-            }
-    	}
-
-    	private String createCrumbUrlBase() {
-            return base + "crumbIssuer/api/xml/";
-    	}
-
-    	private String readData(String dest) throws IOException {
-            HttpURLConnection con = (HttpURLConnection) new URL(dest).openConnection();
-            if (authorization != null) {
-                con.addRequestProperty("Authorization", authorization);
-            }
-            CLI.verifyJenkinsConnection(con);
-
-            try (BufferedReader reader = new BufferedReader(new InputStreamReader(con.getInputStream()))) {
-                String line = reader.readLine();
-                String nextLine = reader.readLine();
-                if (nextLine != null) {
-                    System.err.println("Warning: received junk from " + dest);
-                    System.err.println(line);
-                    System.err.println(nextLine);
-                    while ((nextLine = reader.readLine()) != null) {
-                        System.err.println(nextLine);
-                    }
-                }
-                return line;
-            }
-            finally {
-                con.disconnect();
-            }
-        }
-    }
 }

core/src/main/java/hudson/model/Api.java

@@ -53,6 +53,8 @@
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * Used to expose remote access API for ".../api/"
@@ -228,7 +230,8 @@ private boolean permit(StaplerRequest req) {
         return false;
     }
 
-    private void setHeaders(StaplerResponse rsp) {
+    @Restricted(NoExternalUse.class)
+    protected void setHeaders(StaplerResponse rsp) {
         rsp.setHeader("X-Jenkins", Jenkins.VERSION);
         rsp.setHeader("X-Jenkins-Session", Jenkins.SESSION_HASH);
     }

core/src/main/java/hudson/security/csrf/CrumbIssuer.java

@@ -193,6 +193,7 @@ public static class RestrictedApi extends Api {
         }
 
         @Override public void doXml(StaplerRequest req, StaplerResponse rsp, @QueryParameter String xpath, @QueryParameter String wrapper, @QueryParameter String tree, @QueryParameter int depth) throws IOException, ServletException {
+            setHeaders(rsp);
             String text;
             CrumbIssuer ci = (CrumbIssuer) bean;
             if ("/*/crumbRequestField/text()".equals(xpath)) { // old FullDuplexHttpStream

core/src/main/java/jenkins/util/FullDuplexHttpService.java

@@ -24,6 +24,8 @@
 package jenkins.util;
 
 import hudson.cli.FullDuplexHttpStream;
+import hudson.model.RootAction;
+import hudson.security.csrf.CrumbExclusion;
 import hudson.util.ChunkedInputStream;
 import hudson.util.ChunkedOutputStream;
 import java.io.IOException;
@@ -44,6 +46,8 @@
 
 /**
  * Server-side counterpart to {@link FullDuplexHttpStream}.
+ * <p>
+ * To use, bind this to an endpoint with {@link RootAction} (you will also need a {@link CrumbExclusion}).
  * @since 2.54
  */
 public abstract class FullDuplexHttpService {

test/src/test/java/hudson/cli/CLIActionTest.java

@@ -1,9 +1,5 @@
 package hudson.cli;
 
-import com.gargoylesoftware.htmlunit.HttpMethod;
-import com.gargoylesoftware.htmlunit.Page;
-import com.gargoylesoftware.htmlunit.WebRequest;
-import com.gargoylesoftware.htmlunit.WebResponse;
 import com.google.common.collect.Lists;
 import hudson.Functions;
 import hudson.Launcher;
@@ -26,7 +22,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.UUID;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -122,16 +117,6 @@ public void serveCliActionToAnonymousUserWithoutPermissions() throws Exception {
         JenkinsRule.WebClient wc = j.createWebClient();
         // The behavior changed due to SECURITY-192. index page is no longer accessible to anonymous
         wc.assertFails("cli", HttpURLConnection.HTTP_FORBIDDEN);
-        // so we check the access by emulating the CLI connection post request
-        WebRequest settings = new WebRequest(new URL(j.getURL(), "cli"));
-        settings.setHttpMethod(HttpMethod.POST);
-        settings.setAdditionalHeader("Session", UUID.randomUUID().toString());
-        settings.setAdditionalHeader("Side", "download"); // We try to download something to init the duplex channel
-
-        Page page = wc.getPage(settings);
-        WebResponse webResponse = page.getWebResponse();
-        assertEquals("We expect that the proper POST request from CLI gets processed successfully",
-            200, webResponse.getStatusCode());
     }
 
     @Test

test/src/test/java/jenkins/util/FullDuplexHttpServiceTest.java

@@ -0,0 +1,112 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2017 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
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.util;
+
+import hudson.cli.FullDuplexHttpStream;
+import hudson.model.RootAction;
+import hudson.security.csrf.CrumbExclusion;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.junit.Rule;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
+
+public class FullDuplexHttpServiceTest {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Rule
+    public LoggerRule logging = new LoggerRule().record(FullDuplexHttpService.class, Level.FINE).record(FullDuplexHttpStream.class, Level.FINE);
+
+    @Test
+    public void smokes() throws Exception {
+        logging.record("org.eclipse.jetty", Level.ALL);
+        FullDuplexHttpStream con = new FullDuplexHttpStream(r.getURL(), "test/", null);
+        InputStream is = con.getInputStream();
+        OutputStream os = con.getOutputStream();
+        os.write(33);
+        os.flush();
+        Logger.getLogger(FullDuplexHttpServiceTest.class.getName()).info("uploaded initial content");
+        assertEquals(0, is.read()); // see FullDuplexHttpStream.getInputStream
+        assertEquals(66, is.read());
+    }
+    @TestExtension("smokes")
+    public static class Endpoint implements RootAction {
+        private transient final Map<UUID, FullDuplexHttpService> duplexServices = new HashMap<>();
+        @Override
+        public String getUrlName() {
+            return "test";
+        }
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+        public HttpResponse doIndex() {
+            return new FullDuplexHttpService.Response(duplexServices) {
+                @Override
+                protected FullDuplexHttpService createService(StaplerRequest req, UUID uuid) throws IOException, InterruptedException {
+                    return new FullDuplexHttpService(uuid) {
+                        @Override
+                        protected void run(InputStream upload, OutputStream download) throws IOException, InterruptedException {
+                            int x = upload.read();
+                            download.write(x * 2);
+                        }
+                    };
+                }
+            };
+        }
+    }
+    @TestExtension("smokes")
+    public static class EndpointCrumbExclusion extends CrumbExclusion {
+        @Override
+        public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
+            if ("/test/".equals(request.getPathInfo())) {
+                chain.doFilter(request, response);
+                return true;
+            }
+            return false;
+        }
+    }
+
+}