Merge branch 'security-stable-2.7' into security-stable-2.19

core/pom.xml

@@ -591,12 +591,6 @@ THE SOFTWARE.
       <version>1.3.1-jenkins-1</version>
     </dependency>
 
-    <dependency>
-        <groupId>org.mindrot</groupId>
-        <artifactId>jbcrypt</artifactId>
-        <version>0.3m</version>
-    </dependency>
-
     <!-- offline profiler API to put in the classpath if we need it -->
     <!--dependency>
       <groupId>com.yourkit.api</groupId>

core/src/main/java/hudson/diagnosis/HudsonHomeDiskUsageMonitor.java

@@ -32,6 +32,7 @@ import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import java.io.IOException;
 import java.util.List;
@@ -64,6 +65,7 @@ public final class HudsonHomeDiskUsageMonitor extends AdministrativeMonitor {
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
+    @RequirePOST
     public HttpResponse doAct(@QueryParameter String no) throws IOException {
         if(no!=null) {
             disable(true);

core/src/main/java/hudson/diagnosis/OldDataMonitor.java

@@ -52,6 +52,7 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.CheckForNull;
+
 import jenkins.model.Jenkins;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;

core/src/main/java/hudson/diagnosis/ReverseProxySetupMonitor.java

@@ -37,6 +37,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
 import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 /**
  * Looks out for a broken reverse proxy setup that doesn't rewrite the location header correctly.
@@ -84,6 +85,7 @@ public class ReverseProxySetupMonitor extends AdministrativeMonitor {
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
+    @RequirePOST
     public HttpResponse doAct(@QueryParameter String no) throws IOException {
         if(no!=null) { // dismiss
             disable(true);

core/src/main/java/hudson/diagnosis/TooManyJobsButNoView.java

@@ -29,6 +29,7 @@ import hudson.Extension;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import java.io.IOException;
 
@@ -50,6 +51,7 @@ public class TooManyJobsButNoView extends AdministrativeMonitor {
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
+    @RequirePOST
     public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
         if(req.hasParameter("no")) {
             disable(true);

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

@@ -33,8 +33,12 @@ import java.util.Set;
 import java.io.IOException;
 
 import jenkins.model.Jenkins;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.kohsuke.stapler.StaplerProxy;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 /**
  * Checks the health of a subsystem of Jenkins and if there's something
@@ -74,7 +78,7 @@ import org.kohsuke.stapler.StaplerResponse;
  * @see Jenkins#administrativeMonitors
  */
 @LegacyInstancesAreScopedToHudson
-public abstract class AdministrativeMonitor extends AbstractModelObject implements ExtensionPoint {
+public abstract class AdministrativeMonitor extends AbstractModelObject implements ExtensionPoint, StaplerProxy {
     /**
      * Human-readable ID of this monitor, which needs to be unique within the system.
      *
@@ -142,12 +146,21 @@ public abstract class AdministrativeMonitor extends AbstractModelObject implemen
     /**
      * URL binding to disable this monitor.
      */
+    @RequirePOST
     public void doDisable(StaplerRequest req, StaplerResponse rsp) throws IOException {
-        Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
         disable(true);
         rsp.sendRedirect2(req.getContextPath()+"/manage");
     }
 
+    /**
+     * Requires ADMINISTER permission for any operation in here.
+     */
+    @Restricted(NoExternalUse.class)
+    public Object getTarget() {
+        Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
+        return this;
+    }
+
     /**
      * All registered {@link AdministrativeMonitor} instances.
      */

core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

@@ -60,7 +60,6 @@ import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
-import org.mindrot.jbcrypt.BCrypt;
 import org.springframework.dao.DataAccessException;
 
 import javax.servlet.Filter;

core/src/main/java/jenkins/diagnostics/SecurityIsOffMonitor.java

@@ -6,6 +6,7 @@ import jenkins.model.Jenkins;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import java.io.IOException;
 
@@ -28,6 +29,7 @@ public class SecurityIsOffMonitor extends AdministrativeMonitor {
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
+    @RequirePOST
     public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
         if(req.hasParameter("no")) {
             disable(true);

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

@@ -138,7 +138,6 @@ import hudson.security.AccessControlled;
 import hudson.security.AuthorizationStrategy;
 import hudson.security.BasicAuthenticationFilter;
 import hudson.security.FederatedLoginService;
-import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
 import hudson.security.HudsonFilter;
 import hudson.security.LegacyAuthorizationStrategy;
 import hudson.security.LegacySecurityRealm;
@@ -1705,11 +1704,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      */
     @Exported(name="jobs")
     public List<TopLevelItem> getItems() {
-        if (authorizationStrategy instanceof AuthorizationStrategy.Unsecured ||
-            authorizationStrategy instanceof FullControlOnceLoggedInAuthorizationStrategy) {
-            return new ArrayList(items.values());
-        }
-
         List<TopLevelItem> viewableItems = new ArrayList<TopLevelItem>();
         for (TopLevelItem item : items.values()) {
             if (item.hasPermission(Item.READ))

core/src/main/java/jenkins/security/RekeySecretAdminMonitor.java

@@ -12,7 +12,6 @@ import jenkins.model.Jenkins;
 import jenkins.util.io.FileBoolean;
 import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.HttpResponse;
-import org.kohsuke.stapler.StaplerProxy;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
@@ -30,7 +29,7 @@ import java.util.logging.Logger;
  * @author Kohsuke Kawaguchi
  */
 @Extension @Symbol("rekeySecret")
-public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor implements StaplerProxy {
+public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor {
 
     /**
      * Whether we detected a need to run the rewrite program.
@@ -63,14 +62,6 @@ public class RekeySecretAdminMonitor extends AsynchronousAdministrativeMonitor i
             needed.on();
     }
 
-    /**
-     * Requires ADMINISTER permission for any operation in here.
-     */
-    public Object getTarget() {
-        Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
-        return this;
-    }
-
     @Override
     public boolean isActivated() {
         return needed.isOn();

core/src/main/java/jenkins/security/s2m/AdminCallableMonitor.java

@@ -9,6 +9,7 @@ import org.jenkinsci.Symbol;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import javax.inject.Inject;
 import java.io.IOException;
@@ -50,6 +51,7 @@ public class AdminCallableMonitor extends AdministrativeMonitor {
     /**
      * Depending on whether the user said "examin" or "dismiss", send him to the right place.
      */
+    @RequirePOST
     public HttpResponse doAct(@QueryParameter String dismiss) throws IOException {
         if(dismiss!=null) {
             disable(true);

core/src/main/java/jenkins/security/s2m/MasterKillSwitchWarning.java

@@ -5,6 +5,7 @@ import hudson.model.AdministrativeMonitor;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.interceptor.RequirePOST;
 
 import javax.inject.Inject;
 import java.io.IOException;
@@ -28,6 +29,7 @@ public class MasterKillSwitchWarning extends AdministrativeMonitor {
         return rule.getMasterKillSwitch() && config.isRelevant();
     }
 
+    @RequirePOST
     public HttpResponse doAct(@QueryParameter String dismiss) throws IOException {
         if(dismiss!=null) {
             disable(true);

core/src/main/resources/hudson/diagnosis/HudsonHomeDiskUsageMonitor/index.jelly

@@ -24,7 +24,7 @@ THE SOFTWARE.
 
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
-	<l:layout title="${%JENKINS_HOME is almost full}" permission="${app.ADMINISTER}">
+	<l:layout title="${%JENKINS_HOME is almost full}">
 		<l:main-panel>
       <h1>
         <l:icon class="icon-warning icon-xlg"/>

core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly

@@ -24,7 +24,7 @@ THE SOFTWARE.
 
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-<l:layout title="${%Manage Old Data}" permission="${app.ADMINISTER}">
+<l:layout title="${%Manage Old Data}">
   <st:include page="sidepanel.jelly" it="${app}"/>
   <l:main-panel>
     <h1>${%Manage Old Data}</h1>

test/src/test/java/hudson/diagnosis/HudsonHomeDiskUsageMonitorTest.java

 package hudson.diagnosis;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.HttpMethod;
+import com.gargoylesoftware.htmlunit.WebRequest;
+import com.gargoylesoftware.htmlunit.util.NameValuePair;
+import hudson.model.User;
+import hudson.security.GlobalMatrixAuthorizationStrategy;
+import hudson.security.HudsonPrivateSecurityRealm;
+import hudson.security.Permission;
+import jenkins.model.Jenkins;
+import org.acegisecurity.context.SecurityContextHolder;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.xml.sax.SAXException;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
@@ -13,6 +25,7 @@ import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.ElementNotFoundException;
 
 import java.io.IOException;
+import java.util.Collections;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -45,6 +58,48 @@ public class HudsonHomeDiskUsageMonitorTest {
         }
     }
 
+    @Issue("SECURITY-371")
+    @Test
+    public void noAccessForNonAdmin() throws Exception {
+        JenkinsRule.WebClient wc = j.createWebClient();
+
+        // TODO: Use MockAuthorizationStrategy in later versions
+        JenkinsRule.DummySecurityRealm realm = j.createDummySecurityRealm();
+        realm.addGroups("administrator", "admins");
+        realm.addGroups("bob", "users");
+        j.jenkins.setSecurityRealm(realm);
+        GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy();
+        auth.add(Jenkins.ADMINISTER, "admins");
+        auth.add(Jenkins.READ, "users");
+        j.jenkins.setAuthorizationStrategy(auth);
+
+        WebRequest request = new WebRequest(wc.createCrumbedUrl("administrativeMonitor/hudsonHomeIsFull/act"), HttpMethod.POST);
+        NameValuePair param = new NameValuePair("no", "true");
+        request.setRequestParameters(Collections.singletonList(param));
+
+        HudsonHomeDiskUsageMonitor mon = HudsonHomeDiskUsageMonitor.get();
+
+        try {
+            wc.login("bob");
+            wc.getPage(request);
+        } catch (FailingHttpStatusCodeException e) {
+            assertEquals(403, e.getStatusCode());
+        }
+        assertTrue(mon.isEnabled());
+
+        try {
+            WebRequest getIndex = new WebRequest(wc.createCrumbedUrl("administrativeMonitor/hudsonHomeIsFull"), HttpMethod.GET);
+            wc.getPage(getIndex);
+        } catch (FailingHttpStatusCodeException e) {
+            assertEquals(403, e.getStatusCode());
+        }
+
+        wc.login("administrator");
+        wc.getPage(request);
+        assertFalse(mon.isEnabled());
+
+    }
+
     /**
      * Gets the warning form.
      */

test/src/test/java/jenkins/security/Security380Test.java

+package jenkins.security;
+
+import com.gargoylesoftware.htmlunit.Page;
+import hudson.model.UnprotectedRootAction;
+import hudson.security.ACL;
+import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
+import hudson.util.HttpResponses;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+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.TestExtension;
+import org.kohsuke.stapler.HttpResponse;
+
+public class Security380Test {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testGetItemsWithoutAnonRead() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(false);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+
+        j.createFreeStyleProject();
+        ACL.impersonate(Jenkins.ANONYMOUS, new Runnable() {
+            @Override
+            public void run() {
+                Assert.assertEquals("no items", 0, Jenkins.getInstance().getItems().size());
+            }
+        });
+    }
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testGetItems() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(true);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+
+        j.createFreeStyleProject();
+        ACL.impersonate(Jenkins.ANONYMOUS, new Runnable() {
+            @Override
+            public void run() {
+                Assert.assertEquals("one item", 1, Jenkins.getInstance().getItems().size());
+            }
+        });
+    }
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testWithUnprotectedRootAction() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(false);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+        j.createFreeStyleProject();
+
+        JenkinsRule.WebClient wc = j.createWebClient();
+        Page page = wc.goTo("listJobs", "text/plain");
+        Assert.assertEquals("expect 0 items", "0", page.getWebResponse().getContentAsString().trim());
+    }
+
+    @TestExtension
+    public static class JobListingUnprotectedRootAction implements UnprotectedRootAction {
+
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+
+        @Override
+        public String getUrlName() {
+            return "listJobs";
+        }
+
+        public HttpResponse doIndex() throws Exception {
+            return HttpResponses.plainText(Integer.toString(Jenkins.getInstance().getItems().size()));
+        }
+    }
+}