Skip to content

Commit

Permalink
SECURITY-2246: Prevent unauthorized access to activity details
Browse files Browse the repository at this point in the history
  • Loading branch information
@olivergondza
olivergondza committed Mar 20, 2021
1 parent 6c75484 commit 07dd3da
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 26 deletions.
9 changes: 8 additions & 1 deletion src/main/java/org/jenkinsci/plugins/cloudstats/CloudStatistics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.StaplerProxy;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
Expand All @@ -72,7 +73,7 @@
* Statistics of provisioning activities.
*/
@Extension
public class CloudStatistics extends ManagementLink implements Saveable {
public class CloudStatistics extends ManagementLink implements Saveable, StaplerProxy {

private static final Logger LOGGER = Logger.getLogger(CloudStatistics.class.getName());

Expand Down Expand Up @@ -150,6 +151,12 @@ public String getIconFileName() {
return SystemReadPermission.SYSTEM_READ;
}

@Override
public Object getTarget() {
Jenkins.get().checkPermission(getRequiredPermission());
return this;
}

private boolean isEmpty() {
synchronized (active) {
return log.isEmpty() && active.isEmpty();
Expand Down
85 changes: 60 additions & 25 deletions src/test/java/org/jenkinsci/plugins/cloudstats/CloudStatisticsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,17 @@
import hudson.model.queue.QueueTaskFuture;
import hudson.security.AuthorizationStrategy;
import hudson.slaves.NodeProvisioner;
import jenkins.model.Jenkins;
import jenkins.model.NodeListener;
import org.jenkinsci.plugins.cloudstats.CloudStatistics.ProvisioningListener;
import org.jenkinsci.plugins.cloudstats.PhaseExecutionAttachment.ExceptionAttachment;
import org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Id;
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 org.jvnet.hudson.test.recipes.LocalData;

import javax.annotation.Nonnull;
Expand All @@ -63,13 +68,16 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
import static org.jenkinsci.plugins.cloudstats.CloudStatistics.ProvisioningListener.get;
import static org.jenkinsci.plugins.cloudstats.CloudStatistics.get;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Phase.COMPLETED;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Phase.LAUNCHING;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Phase.OPERATING;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Phase.PROVISIONING;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Status.FAIL;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Status.OK;
import static org.jenkinsci.plugins.cloudstats.ProvisioningActivity.Status.WARN;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -124,7 +132,7 @@ public void provisionAndFail() throws Exception {

PhaseExecution prov = activity.getPhaseExecution(PROVISIONING);
assertEquals(FAIL, activity.getStatus());
PhaseExecutionAttachment.ExceptionAttachment attachment = prov.getAttachments(PhaseExecutionAttachment.ExceptionAttachment.class).get(0);
ExceptionAttachment attachment = prov.getAttachments(ExceptionAttachment.class).get(0);
assertEquals(Functions.printThrowable(TestCloud.ThrowException.EXCEPTION), attachment.getText());
assertEquals(FAIL, attachment.getStatus());
assertEquals(FAIL, activity.getStatus());
Expand Down Expand Up @@ -202,15 +210,15 @@ public void ui() throws Exception {

final String EXCEPTION_MESSAGE = "Something bad happened. Something bad happened. Something bad happened. Something bad happened. Something bad happened. Something bad happened.";
CloudStatistics cs = CloudStatistics.get();
CloudStatistics.ProvisioningListener provisioningListener = CloudStatistics.ProvisioningListener.get();
ProvisioningListener provisioningListener = ProvisioningListener.get();

// When

ProvisioningActivity.Id failId = new ProvisioningActivity.Id("MyCloud", "broken-template");
Id failId = new Id("MyCloud", "broken-template");
provisioningListener.onStarted(failId);
provisioningListener.onFailure(failId, new Exception(EXCEPTION_MESSAGE));

ProvisioningActivity.Id warnId = new ProvisioningActivity.Id("PickyCloud", null, "agent");
Id warnId = new Id("PickyCloud", null, "agent");
provisioningListener.onStarted(warnId);
Node slave = TrackedAgent.create(warnId, j);
ProvisioningActivity a = provisioningListener.onComplete(warnId, slave);
Expand All @@ -219,7 +227,7 @@ public void ui() throws Exception {

slave.toComputer().waitUntilOnline();

ProvisioningActivity.Id okId = new ProvisioningActivity.Id("MyCloud", "working-template", "future-agent");
Id okId = new Id("MyCloud", "working-template", "future-agent");
provisioningListener.onStarted(okId);
slave = TrackedAgent.create(okId, j);
provisioningListener.onComplete(okId, slave);
Expand All @@ -238,7 +246,7 @@ public void ui() throws Exception {
assertNotNull(failedToProvision.getPhaseExecution(COMPLETED));
PhaseExecution failedProvisioning = failedToProvision.getPhaseExecution(PROVISIONING);
assertEquals(FAIL, failedProvisioning.getStatus());
PhaseExecutionAttachment.ExceptionAttachment exception = (PhaseExecutionAttachment.ExceptionAttachment) failedProvisioning.getAttachments().get(0);
ExceptionAttachment exception = (ExceptionAttachment) failedProvisioning.getAttachments().get(0);
assertEquals(EXCEPTION_MESSAGE, exception.getTitle());
assertThat(exception.getText(), startsWith("java.lang.Exception: " + EXCEPTION_MESSAGE));

Expand Down Expand Up @@ -285,10 +293,10 @@ public void ui() throws Exception {
@Test
public void renameActivity() throws Exception {
CloudStatistics cs = CloudStatistics.get();
CloudStatistics.ProvisioningListener l = CloudStatistics.ProvisioningListener.get();
ProvisioningListener l = ProvisioningListener.get();

ProvisioningActivity.Id fixup = new ProvisioningActivity.Id("Cloud", "template", "incorrectName");
ProvisioningActivity.Id assign = new ProvisioningActivity.Id("Cloud", "template");
Id fixup = new Id("Cloud", "template", "incorrectName");
Id assign = new Id("Cloud", "template");
ProvisioningActivity fActivity = l.onStarted(fixup);
ProvisioningActivity aActivity = l.onStarted(assign);

Expand Down Expand Up @@ -332,8 +340,8 @@ public void migrateToV03() {
@Test @Issue("JENKINS-41037")
public void modifiedWhileSerialized() throws Exception {
final CloudStatistics cs = CloudStatistics.get();
final CloudStatistics.ProvisioningListener l = CloudStatistics.ProvisioningListener.get();
final ProvisioningActivity activity = l.onStarted(new ProvisioningActivity.Id("Cloud", "template", "PAOriginal"));
final ProvisioningListener l = ProvisioningListener.get();
final ProvisioningActivity activity = l.onStarted(new Id("Cloud", "template", "PAOriginal"));
final StatsModifyingAttachment blocker = new StatsModifyingAttachment(OK, "Blocker");
Computer.threadPoolForRemoting.submit(new Callable<Object>() {
@Override public Object call() {
Expand All @@ -357,21 +365,21 @@ public void modifiedWhileSerialized() throws Exception {
@Test
public void multipleAttachmentsForPhase() throws Exception {
CloudStatistics cs = CloudStatistics.get();
CloudStatistics.ProvisioningListener provisioningListener = CloudStatistics.ProvisioningListener.get();
ProvisioningListener provisioningListener = ProvisioningListener.get();

ProvisioningActivity.Id pid = new ProvisioningActivity.Id("cloud", "template");
Id pid = new Id("cloud", "template");
ProvisioningActivity pa = provisioningListener.onStarted(pid);
cs.attach(pa, PROVISIONING, new PhaseExecutionAttachment.ExceptionAttachment(OK, new Error("OKmsg")));
cs.attach(pa, PROVISIONING, new PhaseExecutionAttachment.ExceptionAttachment(WARN, new Error("WARNmsg")));
cs.attach(pa, PROVISIONING, new ExceptionAttachment(OK, new Error("OKmsg")));
cs.attach(pa, PROVISIONING, new ExceptionAttachment(WARN, new Error("WARNmsg")));

pa.enter(LAUNCHING);

cs.attach(pa, LAUNCHING, new PhaseExecutionAttachment.ExceptionAttachment(WARN, new Error("WARNmsg")));
cs.attach(pa, LAUNCHING, new PhaseExecutionAttachment.ExceptionAttachment(FAIL, new Error("FAILmsg")));
cs.attach(pa, LAUNCHING, new ExceptionAttachment(WARN, new Error("WARNmsg")));
cs.attach(pa, LAUNCHING, new ExceptionAttachment(FAIL, new Error("FAILmsg")));

// Attaching failure caused the activity to complete
cs.attach(pa, COMPLETED, new PhaseExecutionAttachment.ExceptionAttachment(OK, new Error("OKmsg1")));
cs.attach(pa, COMPLETED, new PhaseExecutionAttachment.ExceptionAttachment(OK, new Error("OKmsg2")));
cs.attach(pa, COMPLETED, new ExceptionAttachment(OK, new Error("OKmsg1")));
cs.attach(pa, COMPLETED, new ExceptionAttachment(OK, new Error("OKmsg2")));

// All 6 attachments can be navigated to
JenkinsRule.WebClient wc = j.createWebClient();
Expand Down Expand Up @@ -407,6 +415,33 @@ public void multipleAttachmentsForPhase() throws Exception {
assertEquals(cs.getRetainedActivities(), cs.getNotCompletedActivities());
}

@Test @Issue("SECURITY-2246")
public void denyAccessToStatsDetails() throws Exception {
CloudStatistics cs = CloudStatistics.get();
ProvisioningListener provisioningListener = ProvisioningListener.get();

Id pid = new Id("cloud", "template");
ProvisioningActivity pa = provisioningListener.onStarted(pid);
cs.attach(pa, PROVISIONING, new ExceptionAttachment(WARN, new Error("WARNmsg")));

j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
j.jenkins.setAuthorizationStrategy(
new MockAuthorizationStrategy()
.grant(Jenkins.READ).everywhere().to("user")
.grant(Jenkins.ADMINISTER).everywhere().to("boss")
);

PhaseExecution phaseExecution = pa.getPhaseExecution(PROVISIONING);
String url = cs.getUrl(pa, phaseExecution, phaseExecution.getAttachment("exception")).substring(1);

JenkinsRule.WebClient adminWc = j.createWebClient().login("boss", "boss");
adminWc.goTo(url);

JenkinsRule.WebClient userWc = j.createWebClient().login("user", "user");
userWc.setThrowExceptionOnFailingStatusCode(false);
assertEquals(403, userWc.goTo(url).getWebResponse().getStatusCode());
}

@Test
@LocalData
@Issue("JENKINS-41037")
Expand All @@ -428,12 +463,12 @@ public void migrateToV010() throws Exception {
public void migrateToV013() throws Exception {
CloudStatistics cs = CloudStatistics.get();
ProvisioningActivity activity = cs.getActivities().iterator().next();
List<PhaseExecutionAttachment.ExceptionAttachment> attachments = activity.getPhaseExecution(PROVISIONING).getAttachments(PhaseExecutionAttachment.ExceptionAttachment.class);
PhaseExecutionAttachment.ExceptionAttachment partial = attachments.get(0);
List<ExceptionAttachment> attachments = activity.getPhaseExecution(PROVISIONING).getAttachments(ExceptionAttachment.class);
ExceptionAttachment partial = attachments.get(0);
assertThat(partial.getDisplayName(), equalTo("EXCEPTION_MESSAGE"));
assertThat(partial.getText(), equalTo("Plugin was unable to deserialize the exception from version 0.12 or older"));

PhaseExecutionAttachment.ExceptionAttachment full = attachments.get(1);
ExceptionAttachment full = attachments.get(1);

final String EX_MSG = "java.lang.NullPointerException";
assertThat(full.getDisplayName(), equalTo(EX_MSG));
Expand Down Expand Up @@ -464,7 +499,7 @@ public void testConcurrentModificationException() throws Exception {
public void run() {
for (;;) {
try {
ProvisioningActivity activity = CloudStatistics.ProvisioningListener.get().onStarted(new ProvisioningActivity.Id("test1", null, "test1"));
ProvisioningActivity activity = ProvisioningListener.get().onStarted(new Id("test1", null, "test1"));
activity.enterIfNotAlready(LAUNCHING);
Thread.sleep(new Random().nextInt(50));
activity.enterIfNotAlready(OPERATING);
Expand Down Expand Up @@ -538,8 +573,8 @@ private Object writeReplace() throws ObjectStreamException {
try {
// Avoid saving as it is a) not related to test and b) spins infinite recursion of saving
BulkChange bc = new BulkChange(CloudStatistics.get());
final CloudStatistics.ProvisioningListener l = CloudStatistics.ProvisioningListener.get();
l.onStarted(new ProvisioningActivity.Id("Cloud", "template", "PAModifying"));
final ProvisioningListener l = ProvisioningListener.get();
l.onStarted(new Id("Cloud", "template", "PAModifying"));
bc.abort();
} catch (Throwable e) {
return e;
Expand Down

0 comments on commit 07dd3da

Please sign in to comment.