Merge branch 'security-stable-2.176' into security-master

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

@@ -1515,7 +1515,7 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas
                                 if(e.getMessage().contains("Connection timed out")) {
                                     // Google can't be down, so this is probably a proxy issue
                                     connectionStates.put(ConnectionStatus.INTERNET, ConnectionStatus.FAILED);
-                                    statuses.add(Messages.UpdateCenter_Status_ConnectionFailed(connectionCheckUrl));
+                                    statuses.add(Messages.UpdateCenter_Status_ConnectionFailed(Functions.xmlEscape(connectionCheckUrl)));
                                     return;
                                 }
                             }
@@ -1537,12 +1537,12 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas
                 statuses.add(Messages.UpdateCenter_Status_Success());
             } catch (UnknownHostException e) {
                 connectionStates.put(ConnectionStatus.UPDATE_SITE, ConnectionStatus.FAILED);
-                statuses.add(Messages.UpdateCenter_Status_UnknownHostException(e.getMessage()));
+                statuses.add(Messages.UpdateCenter_Status_UnknownHostException(Functions.xmlEscape(e.getMessage())));
                 addStatus(e);
                 error = e;
             } catch (Exception e) {
                 connectionStates.put(ConnectionStatus.UPDATE_SITE, ConnectionStatus.FAILED);
-                statuses.add(Functions.printThrowable(e));
+                addStatus(e);
                 error = e;
             }
             
@@ -1556,7 +1556,7 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas
             }
         }
 
-        private void addStatus(UnknownHostException e) {
+        private void addStatus(Throwable e) {
             statuses.add("<pre>"+ Functions.xmlEscape(Functions.printThrowable(e))+"</pre>");
         }
 

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

@@ -91,7 +91,7 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
                 }
                 if (!EXCLUDE_SESSION_ID) {
                     buffer.append(';');
-                    buffer.append(getSessionId(req));
+                    buffer.append(req.getSession().getId());
                 }
 
                 md.update(buffer.toString().getBytes());
@@ -101,14 +101,6 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
         return null;
     }
 
-    private String getSessionId(@Nonnull HttpServletRequest request) {
-        HttpSession session = request.getSession(false);
-        if (session == null) {
-            return "NO_SESSION";
-        }
-        return session.getId();
-    }
-
     /**
      * {@inheritDoc}
      */

test/src/test/java/hudson/security/csrf/DefaultCrumbIssuerSEC1491Test.java

+package hudson.security.csrf;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+//TODO merge back to DefaultCrumbIssuerTest
+public class DefaultCrumbIssuerSEC1491Test {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Before public void setIssuer() {
+        r.jenkins.setCrumbIssuer(new DefaultCrumbIssuer(false));
+    }
+
+    @Test
+    @Issue("SECURITY-1491")
+    public void sessionIncludedEvenForAnonymousCall() throws Exception {
+        boolean previousValue = DefaultCrumbIssuer.EXCLUDE_SESSION_ID;
+
+        try {
+            r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+
+            // let anonymous user have read access
+            MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
+            authorizationStrategy.grant(Jenkins.ADMINISTER).everywhere().toEveryone();
+            r.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+            DefaultCrumbIssuer issuer = new DefaultCrumbIssuer(true);
+            r.jenkins.setCrumbIssuer(issuer);
+
+            DefaultCrumbIssuer.EXCLUDE_SESSION_ID = true;
+            sameCrumbUsedOnDifferentAnonymousRequest_tokenAreEqual(true, "job_noSession");
+
+            DefaultCrumbIssuer.EXCLUDE_SESSION_ID = false;
+            sameCrumbUsedOnDifferentAnonymousRequest_tokenAreEqual(false, "job_session");
+        } finally {
+            DefaultCrumbIssuer.EXCLUDE_SESSION_ID = previousValue;
+        }
+    }
+
+    private void sameCrumbUsedOnDifferentAnonymousRequest_tokenAreEqual(boolean areEqual, String namePrefix) throws Exception {
+        String responseForCrumb = r.createWebClient().goTo("crumbIssuer/api/xml?xpath=concat(//crumbRequestField,'=',//crumb)", "text/plain")
+                .getWebResponse().getContentAsString();
+        // responseForCrumb = Jenkins-Crumb=xxxx
+        String crumb1 = responseForCrumb.substring(CrumbIssuer.DEFAULT_CRUMB_NAME.length() + "=".length());
+
+        String jobName1 = namePrefix + "-test1";
+        String jobName2 = namePrefix + "-test2";
+
+        WebRequest request1 = createRequestForJobCreation(jobName1);
+        try {
+            r.createWebClient().getPage(request1);
+            fail();
+        } catch (FailingHttpStatusCodeException e) {
+            assertTrue(e.getMessage().contains("No valid crumb"));
+        }
+        // cannot create new job due to missing crumb
+        assertNull(r.jenkins.getItem(jobName1));
+
+        WebRequest request2 = createRequestForJobCreation(jobName2);
+        request2.setAdditionalHeader(CrumbIssuer.DEFAULT_CRUMB_NAME, crumb1);
+        if (areEqual) {
+            r.createWebClient().getPage(request2);
+
+            assertNotNull(r.jenkins.getItem(jobName2));
+        } else {
+            try {
+                r.createWebClient().getPage(request2);
+                fail("Should have failed due to invalid crumb");
+            } catch (FailingHttpStatusCodeException e) {
+                assertEquals(HttpURLConnection.HTTP_FORBIDDEN, e.getStatusCode());
+                // cannot create new job due to invalid crumb
+                assertNull(r.jenkins.getItem(jobName2));
+            }
+        }
+    }
+
+    @Test
+    @Issue("SECURITY-1491")
+    public void twoRequestsWithoutSessionGetDifferentCrumbs() throws Exception {
+        String responseForCrumb = r.createWebClient().goTo("crumbIssuer/api/xml?xpath=concat(//crumbRequestField,'=',//crumb)", "text/plain")
+                .getWebResponse().getContentAsString();
+        // responseForCrumb = Jenkins-Crumb=xxxx
+        String crumb1 = responseForCrumb.substring(CrumbIssuer.DEFAULT_CRUMB_NAME.length() + "=".length());
+
+        responseForCrumb = r.createWebClient().goTo("crumbIssuer/api/xml?xpath=concat(//crumbRequestField,'=',//crumb)", "text/plain")
+                .getWebResponse().getContentAsString();
+        // responseForCrumb = Jenkins-Crumb=xxxx
+        String crumb2 = responseForCrumb.substring(CrumbIssuer.DEFAULT_CRUMB_NAME.length() + "=".length());
+
+        Assert.assertNotEquals("should be different crumbs", crumb1, crumb2);
+    }
+
+    private WebRequest createRequestForJobCreation(String jobName) throws Exception {
+        WebRequest req = new WebRequest(new URL(r.getURL() + "createItem?name=" + jobName), HttpMethod.POST);
+        req.setAdditionalHeader("Content-Type", "application/xml");
+        req.setRequestBody("<project/>");
+        return req;
+    }
+
+    @Test
+    public void anonCanStillPostRequestUsingBrowsers() throws Exception {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+
+        MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
+        authorizationStrategy.grant(Jenkins.ADMINISTER).everywhere().toEveryone();
+        r.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+        DefaultCrumbIssuer issuer = new DefaultCrumbIssuer(true);
+        r.jenkins.setCrumbIssuer(issuer);
+
+        HtmlPage p = r.createWebClient().goTo("configure");
+        r.submit(p.getFormByName("config"));
+    }
+}