Skip to content

Commit

Permalink
[JENKINS-18114] Fixing CLI crumbs (#3019)
Browse files Browse the repository at this point in the history
* [JENKINS-18114] The CLI client already asks for a crumb; we just to actually serve it, from CrumbIssuer.RestrictedApi.

* serveCliActionToAnonymousUserWithoutPermissions() was checking some stuff better checked by authentication(), and was failing since this fake client was not passing a crumb.

* Bring back CliCrumbExclusion, needed for anonymous use with no read access, and just let the client stop asking for a crumb.

* Added FullDuplexHttpServiceTest; useful to have a simple, self-contained demonstration of the HTTP Duplex transport.
  • Loading branch information
jglick authored and oleg-nenashev committed Sep 22, 2017
1 parent 3f84d30 commit 5e84c54
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 86 deletions.
2 changes: 1 addition & 1 deletion cli/src/main/java/hudson/cli/CLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
84 changes: 15 additions & 69 deletions cli/src/main/java/hudson/cli/FullDuplexHttpStream.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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.
Expand All @@ -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();
}
}
}
}
5 changes: 4 additions & 1 deletion core/src/main/java/hudson/model/Api.java
Original file line number Diff line number Diff line change
Expand Up @@ -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/"
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/hudson/security/csrf/CrumbIssuer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/jenkins/util/FullDuplexHttpService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
15 changes: 0 additions & 15 deletions test/src/test/java/hudson/cli/CLIActionTest.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -121,16 +116,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
Expand Down
112 changes: 112 additions & 0 deletions test/src/test/java/jenkins/util/FullDuplexHttpServiceTest.java
Original file line number Diff line number Diff line change
@@ -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;
}
}

}

0 comments on commit 5e84c54

Please sign in to comment.