[SECURITY-1114]

This commit is contained in:
Daniel Beck committed 2018-09-27 11:38:33 +02:00
1 parent 41f74e2a82
commit ba33bd67cd
4 files changed
+150 -2

No files matched your search

@@ -56,6 +56,7 @@ import jenkins.util.xml.XMLUtils;
import org.apache.tools.ant.taskdefs.Copy;
import org.apache.tools.ant.types.FileSet;
import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.WebMethod;
import org.kohsuke.stapler.export.Exported;
@@ -91,6 +92,8 @@ import javax.xml.transform.stream.StreamSource;
import static hudson.model.queue.Executables.getParentOf;
import hudson.model.queue.SubTask;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import org.apache.commons.io.FileUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -841,6 +844,14 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
@Restricted(NoExternalUse.class)
public Object getTarget() {
if (!SKIP_PERMISSION_CHECK) {
if (!getACL().hasPermission(Item.DISCOVER)) {
// work around Stapler bug when returning null from getTarget()
try {
Stapler.getCurrentResponse().sendError(SC_NOT_FOUND); // send same response body Stapler would send
} catch (IOException ex) {
throw HttpResponses.notFound();
}
}
getACL().checkPermission(Item.READ);
}
return this;
+30 -1
View File
@@ -93,6 +93,8 @@ import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
import static java.util.logging.Level.*;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
@@ -120,8 +122,10 @@ import org.apache.commons.lang.ArrayUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.export.Exported;
@@ -141,7 +145,7 @@ import org.kohsuke.stapler.interceptor.RequirePOST;
*/
@ExportedBean
public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,RunT>>
extends Actionable implements ExtensionPoint, Comparable<RunT>, AccessControlled, PersistenceRoot, DescriptorByNameOwner, OnMaster {
extends Actionable implements ExtensionPoint, Comparable<RunT>, AccessControlled, PersistenceRoot, DescriptorByNameOwner, OnMaster, StaplerProxy {
/**
* The original {@link Queue.Item#getId()} has not yet been mapped onto the {@link Run} instance.
@@ -2480,6 +2484,31 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
return returnedResult;
}
@Override
@Restricted(NoExternalUse.class)
public Object getTarget() {
if (!SKIP_PERMISSION_CHECK) {
// This is a bit weird, but while the Run's PermissionScope does not have READ, delegate to the parent
if (!getParent().hasPermission(Item.DISCOVER)) {
// work around Stapler bug when returning null from getTarget()
try {
Stapler.getCurrentResponse().sendError(SC_NOT_FOUND); // send same response body Stapler would send
} catch (IOException ex) {
throw HttpResponses.notFound();
}
}
getParent().checkPermission(Item.READ);
}
return this;
}
/**
* Escape hatch for StaplerProxy-based access control
*/
@Restricted(NoExternalUse.class)
public static /* Script Console modifiable */ boolean SKIP_PERMISSION_CHECK = Boolean.getBoolean(Run.class.getName() + ".skipPermissionCheck");
public static class RedirectUp {
public void doDynamic(StaplerResponse rsp) throws IOException {
// Compromise to handle both browsers (auto-redirect) and programmatic access
+12 -1
View File
@@ -97,6 +97,8 @@ import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
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;
@@ -105,6 +107,8 @@ import org.kohsuke.stapler.export.ExportedBean;
import org.kohsuke.stapler.interceptor.RequirePOST;
import org.springframework.dao.DataAccessException;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
/**
* Represents a user.
*
@@ -1065,7 +1069,14 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
@Restricted(NoExternalUse.class)
public Object getTarget() {
if (!SKIP_PERMISSION_CHECK) {
Jenkins.getInstance().checkPermission(Jenkins.READ);
if (!Jenkins.getInstance().hasPermission(Jenkins.READ)) {
// work around Stapler bug when returning null from getTarget()
try {
Stapler.getCurrentResponse().sendError(SC_NOT_FOUND); // send same response body Stapler would send
} catch (IOException ex) {
throw HttpResponses.notFound();
}
}
}
return this;
}
@@ -0,0 +1,97 @@
package hudson;
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import hudson.model.AbstractItem;
import hudson.model.Item;
import hudson.model.RootAction;
import hudson.security.ACL;
import hudson.security.ACLContext;
import jenkins.model.Jenkins;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.For;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.TestExtension;
import javax.annotation.CheckForNull;
public class AbstractItemSecurity1114Test {
@Rule
public JenkinsRule j = new JenkinsRule();
@Test
@Issue("SECURITY-1114")
@For(AbstractItem.class)
public void testAccess() throws Exception {
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
authorizationStrategy.grant(Jenkins.READ).onRoot().toEveryone();
authorizationStrategy.grant(Item.DISCOVER).everywhere().to("alice");
authorizationStrategy.grant(Item.READ).everywhere().to("bob");
j.jenkins.setAuthorizationStrategy(authorizationStrategy);
j.createFreeStyleProject("myproject");
// alice can discover project
JenkinsRule.WebClient wc = j.createWebClient().login("alice");
try {
wc.goTo("bypass/myproject");
Assert.fail("expected exception");
} catch (FailingHttpStatusCodeException e) {
Assert.assertEquals("alice can discover", 403, e.getStatusCode());
}
// bob can read project
wc = j.createWebClient().login("bob");
wc.goTo("bypass/myproject"); // success
// carol has no permissions
wc = j.createWebClient().login("carol");
try {
wc.goTo("bypass/nonexisting");
Assert.fail("expected exception");
} catch (FailingHttpStatusCodeException e) {
Assert.assertEquals("carol gets 404 for nonexisting project", 404, e.getStatusCode());
}
try {
wc.goTo("bypass/myproject");
Assert.fail("expected exception");
} catch (FailingHttpStatusCodeException e) {
Assert.assertEquals("carol gets 404 for invisible project", 404, e.getStatusCode());
}
}
@TestExtension
public static class BypassAccess implements RootAction {
public Item getDynamic(String name) {
Item item;
try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
item = Jenkins.getInstance().getItemByFullName(name);
}
return item;
}
@CheckForNull
@Override
public String getIconFileName() {
return null;
}
@CheckForNull
@Override
public String getDisplayName() {
return null;
}
@CheckForNull
@Override
public String getUrlName() {
return "bypass";
}
}
}