[FIXED JENKINS-44361] Follow HTTP redirects while initiating CLI conn…

…ection
jenkinsci · May 23, 2017 · 741be05 · 741be05
1 parent 8199ec5
commit 741be05
Show file tree

Hide file tree

Showing 4 changed files with 162 additions and 18 deletions.
diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java
@@ -306,9 +306,7 @@ protected CliPort getCliTcpPort(URL url) throws IOException {
 
         flushURLConnection(head);
         if (p1==null && p2==null) {
-            // we aren't finding headers we are expecting. Is this even running Jenkins?
-            if (head.getHeaderField("X-Hudson")==null && head.getHeaderField("X-Jenkins")==null)
-                throw new IOException("There's no Jenkins running at "+url);
+            verifyJenkinsConnection(head);
 
             throw new IOException("No X-Jenkins-CLI2-Port among " + head.getHeaderFields().keySet());
         }
@@ -317,6 +315,27 @@ protected CliPort getCliTcpPort(URL url) throws IOException {
         else            return new CliPort(new InetSocketAddress(h,Integer.parseInt(p1)),identity,1);
     }
 
+    /**
+     * Make sure the connection is open against Jenkins server.
+     *
+     * @param c The open connection.
+     * @throws IOException in case of communication problem.
+     * @throws NotTalkingToJenkinsException when connection is not made to Jenkins service.
+     */
+    /*package*/ static void verifyJenkinsConnection(URLConnection c) throws IOException {
+        if (c.getHeaderField("X-Hudson")==null && c.getHeaderField("X-Jenkins")==null)
+            throw new NotTalkingToJenkinsException(c);
+    }
+    /*package*/ static final class NotTalkingToJenkinsException extends IOException {
+        public NotTalkingToJenkinsException(String s) {
+            super(s);
+        }
+
+        public NotTalkingToJenkinsException(URLConnection c) {
+            super("There's no Jenkins running at " + c.getURL().toString());
+        }
+    }
+
     /**
      * Flush the supplied {@link URLConnection} input and close the
      * connection nicely.
@@ -405,6 +424,9 @@ public void upgrade() {
     public static void main(final String[] _args) throws Exception {
         try {
             System.exit(_main(_args));
+        } catch (NotTalkingToJenkinsException ex) {
+            System.err.println(ex.getMessage());
+            System.exit(-1);
         } catch (Throwable t) {
             // if the CLI main thread die, make sure to kill the JVM.
             t.printStackTrace();

diff --git a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java
@@ -74,17 +74,20 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio
             throw new IllegalArgumentException(relativeTarget);
         }
 
-        this.base = base;
+        // As this transport mode is using POST, it is necessary to resolve possible redirections using GET first.
+        HttpURLConnection con = (HttpURLConnection) base.openConnection();
+        con.getInputStream().close();
+        this.base = con.getURL();
         this.authorization = authorization;
 
-        URL target = new URL(base, relativeTarget);
+        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
-        HttpURLConnection con = (HttpURLConnection) target.openConnection();
+        con = (HttpURLConnection) target.openConnection();
         con.setDoOutput(true); // request POST to avoid caching
         con.setRequestMethod("POST");
         con.addRequestProperty("Session", uuid.toString());
@@ -99,7 +102,7 @@ public FullDuplexHttpStream(URL base, String relativeTarget, String authorizatio
         input = con.getInputStream();
         // make sure we hit the right URL
         if (con.getHeaderField("Hudson-Duplex") == null) {
-            throw new IOException(target + " does not look like Jenkins, or is not serving the HTTP Duplex transport");
+            throw new CLI.NotTalkingToJenkinsException("There's no Jenkins running at " + target + ", or is not serving the HTTP Duplex transport");
         }
 
         // client->server uses chunked encoded POST for unlimited capacity. 
@@ -158,6 +161,8 @@ private String readData(String dest) throws IOException {
             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();

diff --git a/cli/src/main/java/hudson/cli/SSHCLI.java b/cli/src/main/java/hudson/cli/SSHCLI.java
@@ -63,6 +63,7 @@ static int sshConnection(String jenkinsUrl, String user, List<String> args, Priv
         Logger.getLogger(SecurityUtils.class.getName()).setLevel(Level.WARNING); // suppress: BouncyCastle not registered, using the default JCE provider
         URL url = new URL(jenkinsUrl + "login");
         URLConnection conn = url.openConnection();
+        CLI.verifyJenkinsConnection(conn);
         String endpointDescription = conn.getHeaderField("X-SSH-Endpoint");
 
         if (endpointDescription == null) {

diff --git a/test/src/test/java/hudson/cli/CLITest.java b/test/src/test/java/hudson/cli/CLITest.java
@@ -24,20 +24,26 @@
 
 package hudson.cli;
 
+import com.gargoylesoftware.htmlunit.WebResponse;
 import com.google.common.collect.Lists;
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.Proc;
 import hudson.model.FreeStyleProject;
+import hudson.model.UnprotectedRootAction;
 import hudson.model.User;
+import hudson.security.csrf.CrumbExclusion;
 import hudson.util.StreamTaskListener;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+
+import jenkins.model.GlobalConfiguration;
 import jenkins.model.Jenkins;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
@@ -56,6 +62,17 @@
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockAuthorizationStrategy;
 import org.jvnet.hudson.test.SleepBuilder;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.HttpResponses;
+import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerProxy;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 public class CLITest {
 
@@ -68,9 +85,12 @@ public class CLITest {
     @Rule
     public TemporaryFolder tmp = new TemporaryFolder();
 
+    private File home;
+    private File jar;
+
     /** Sets up a fake {@code user.home} so that tests {@code -ssh} mode does not get confused by the developer’s real {@code ~/.ssh/known_hosts}. */
     private File tempHome() throws IOException {
-        File home = tmp.newFolder();
+        home = tmp.newFolder();
         // Seems it gets created automatically but with inappropriate permissions:
         File known_hosts = new File(new File(home, ".ssh"), "known_hosts");
         assumeTrue(known_hosts.getParentFile().mkdir());
@@ -93,12 +113,12 @@ private File tempHome() throws IOException {
     @Issue("JENKINS-41745")
     @Test
     public void strictHostKey() throws Exception {
-        File home = tempHome();
+        home = tempHome();
+        grabCliJar();
+
         r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
         r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("admin"));
         SSHD.get().setPort(0);
-        File jar = tmp.newFile("jenkins-cli.jar");
-        FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
         File privkey = tmp.newFile("id_rsa");
         FileUtils.copyURLToFile(CLITest.class.getResource("id_rsa"), privkey);
         User.get("admin").addProperty(new UserPropertyImpl(IOUtils.toString(CLITest.class.getResource("id_rsa.pub"))));
@@ -117,25 +137,30 @@ public void strictHostKey() throws Exception {
         assertThat(baos.toString(), containsString("Authenticated as: admin"));
     }
 
+    private void grabCliJar() throws IOException {
+        jar = tmp.newFile("jenkins-cli.jar");
+        FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
+    }
+
     @Issue("JENKINS-41745")
     @Test
     public void interrupt() throws Exception {
-        File home = tempHome();
+        home = tempHome();
+        grabCliJar();
+
         r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
         r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("admin"));
         SSHD.get().setPort(0);
-        File jar = tmp.newFile("jenkins-cli.jar");
-        FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
         File privkey = tmp.newFile("id_rsa");
         FileUtils.copyURLToFile(CLITest.class.getResource("id_rsa"), privkey);
         User.get("admin").addProperty(new UserPropertyImpl(IOUtils.toString(CLITest.class.getResource("id_rsa.pub"))));
         FreeStyleProject p = r.createFreeStyleProject("p");
         p.getBuildersList().add(new SleepBuilder(TimeUnit.MINUTES.toMillis(5)));
-        doInterrupt(jar, home, p, "-remoting", "-i", privkey.getAbsolutePath());
-        doInterrupt(jar, home, p, "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath());
-        doInterrupt(jar, home, p, "-http", "-auth", "admin:admin");
+        doInterrupt(p, "-remoting", "-i", privkey.getAbsolutePath());
+        doInterrupt(p, "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath());
+        doInterrupt(p, "-http", "-auth", "admin:admin");
     }
-    private void doInterrupt(File jar, File home, FreeStyleProject p, String... modeArgs) throws Exception {
+    private void doInterrupt(FreeStyleProject p, String... modeArgs) throws Exception {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         List<String> args = Lists.newArrayList("java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString());
         args.addAll(Arrays.asList(modeArgs));
@@ -149,4 +174,95 @@ private void doInterrupt(File jar, File home, FreeStyleProject p, String... mode
         r.waitForCompletion(p.getLastBuild());
     }
 
+    @Test
+    public void reportNotJenkins() throws Exception {
+        home = tempHome();
+        grabCliJar();
+
+        for (String transport: Arrays.asList("-remoting", "-http", "-ssh")) {
+
+            String url = "http://jenkins.io";
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            int ret = new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+                    "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", url, transport, "-user", "asdf", "who-am-i"
+            ).stdout(baos).stderr(baos).join();
+
+            assertThat(baos.toString(), containsString("There's no Jenkins running at"));
+            assertNotEquals(0, ret);
+        }
+    }
+
+    @Test
+    public void redirectToEndpointShouldBeFollowed() throws Exception {
+        home = tempHome();
+        grabCliJar();
+
+        // Enable CLI over SSH
+        SSHD sshd = GlobalConfiguration.all().get(SSHD.class);
+        sshd.setPort(0); // random
+        sshd.start();
+
+        // Sanity check
+        JenkinsRule.WebClient wc = r.createWebClient();
+        wc.getOptions().setRedirectEnabled(false);
+        wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
+        WebResponse rsp = wc.goTo("cli-proxy/").getWebResponse();
+        assertEquals(rsp.getContentAsString(), 302, rsp.getStatusCode());
+        assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-Jenkins"));
+        assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-Jenkins-CLI-Port"));
+        assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-SSH-Endpoint"));
+
+        for (String transport: Arrays.asList("-remoting", "-http", "-ssh")) {
+
+            String url = r.getURL().toString() + "cli-proxy/";
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            int ret = new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+                    "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", url, transport, "-user", "asdf", "who-am-i"
+            ).stdout(baos).stderr(baos).join();
+
+            //assertThat(baos.toString(), containsString("There's no Jenkins running at"));
+            assertThat(baos.toString(), containsString("Authenticated as: anonymous"));
+            assertEquals(0, ret);
+        }
+    }
+    @TestExtension("redirectToEndpointShouldBeFollowed")
+    public static final class CliProxyAction extends CrumbExclusion implements UnprotectedRootAction, StaplerProxy {
+
+        @Override public String getIconFileName() {
+            return "cli-proxy";
+        }
+
+        @Override public String getDisplayName() {
+            return "cli-proxy";
+        }
+
+        @Override public String getUrlName() {
+            return "cli-proxy";
+        }
+
+        @Override public Object getTarget() {
+            throw doDynamic(Stapler.getCurrentRequest(), Stapler.getCurrentResponse());
+        }
+
+        public HttpResponses.HttpResponseException doDynamic(StaplerRequest req, StaplerResponse rsp) {
+            final String url = req.getRequestURIWithQueryString().replaceFirst("/cli-proxy", "");
+            // Custom written redirect so no traces of Jenkins are present in headers
+            return new HttpResponses.HttpResponseException() {
+                @Override
+                public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+                    rsp.setHeader("Location", url);
+                    rsp.setContentType("text/html");
+                    rsp.setStatus(302);
+                    PrintWriter w = rsp.getWriter();
+                    w.append("Redirect to ").append(url);
+                }
+            };
+        }
+
+        @Override // Permit access to cli-proxy/XXX without CSRF checks
+        public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
+            chain.doFilter(request, response);
+            return true;
+        }
+    }
 }