Skip to content

Commit 9c08b9f

Browse files
Pldi23Pldi23
committed
SECURITY-2189
1 parent 40947f2 commit 9c08b9f

File tree

3 files changed

+150
-7
lines changed

3 files changed

+150
-7
lines changed

‎src/main/java/com/cloudbees/jenkins/plugins/sshagent/SSHAgentBuildWrapper.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import hudson.model.Queue;
4545
import hudson.model.queue.Tasks;
4646
import hudson.security.ACL;
47+
import hudson.security.AccessControlled;
4748
import hudson.tasks.BuildWrapper;
4849
import hudson.tasks.BuildWrapperDescriptor;
4950
import hudson.util.IOException2;
@@ -63,8 +64,9 @@
6364
import javax.annotation.Nonnull;
6465
import jenkins.model.Jenkins;
6566
import org.apache.commons.lang.StringUtils;
67+
import org.kohsuke.stapler.AncestorInPath;
6668
import org.kohsuke.stapler.DataBoundConstructor;
67-
import org.kohsuke.stapler.Stapler;
69+
import org.kohsuke.stapler.QueryParameter;
6870

6971
/**
7072
* A build wrapper that provides an SSH agent using supplied credentials
@@ -496,8 +498,11 @@ public String getDisplayName() {
496498
* @return the list box model.
497499
*/
498500
@SuppressWarnings("unused") // used by stapler
499-
public ListBoxModel doFillIdItems() {
500-
Item item = Stapler.getCurrentRequest().findAncestorObject(Item.class);
501+
public ListBoxModel doFillIdItems(@AncestorInPath Item item) {
502+
AccessControlled contextToCheck = item == null ? Jenkins.get() : item;
503+
if (!contextToCheck.hasPermission(CredentialsProvider.VIEW)) {
504+
return new StandardUsernameListBoxModel();
505+
}
501506
return new StandardUsernameListBoxModel()
502507
.includeMatchingAs(
503508
item instanceof Queue.Task ? Tasks.getAuthenticationOf((Queue.Task) item) : ACL.SYSTEM,
@@ -507,7 +512,6 @@ public ListBoxModel doFillIdItems() {
507512
SSHAuthenticator.matcher()
508513
);
509514
}
510-
511515
}
512516
}
513517

‎src/main/java/com/cloudbees/jenkins/plugins/sshagent/SSHAgentStep.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@
1010
import hudson.model.Queue;
1111
import hudson.model.queue.Tasks;
1212
import hudson.security.ACL;
13+
import hudson.security.AccessControlled;
1314
import hudson.util.ListBoxModel;
15+
import jenkins.model.Jenkins;
1416
import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
1517
import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
18+
import org.kohsuke.stapler.AncestorInPath;
1619
import org.kohsuke.stapler.DataBoundConstructor;
1720
import org.kohsuke.stapler.DataBoundSetter;
18-
import org.kohsuke.stapler.Stapler;
21+
import org.kohsuke.stapler.QueryParameter;
1922

2023
import java.io.Serializable;
2124
import java.util.Collections;
@@ -76,8 +79,11 @@ public boolean takesImplicitBlockArgument() {
7679
* @return the list box model.
7780
*/
7881
@SuppressWarnings("unused") // used by stapler
79-
public ListBoxModel doFillCredentialsItems() {
80-
Item item = Stapler.getCurrentRequest().findAncestorObject(Item.class);
82+
public ListBoxModel doFillCredentialsItems(@AncestorInPath Item item) {
83+
AccessControlled contextToCheck = item == null ? Jenkins.get() : item;
84+
if (!contextToCheck.hasPermission(CredentialsProvider.VIEW)) {
85+
return new StandardUsernameListBoxModel();
86+
}
8187
return new StandardUsernameListBoxModel()
8288
.includeMatchingAs(
8389
item instanceof Queue.Task ? Tasks.getAuthenticationOf((Queue.Task)item) : ACL.SYSTEM,

‎src/test/java/com/cloudbees/jenkins/plugins/sshagent/Security2189Test.java‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package com.cloudbees.jenkins.plugins.sshagent;
2+
3+
import com.cloudbees.jenkins.plugins.sshcredentials.SSHUserPrivateKey;
4+
import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey;
5+
import com.cloudbees.plugins.credentials.CredentialsScope;
6+
import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
7+
import hudson.model.FreeStyleProject;
8+
import hudson.model.Item;
9+
import hudson.model.User;
10+
import hudson.security.ACL;
11+
import hudson.security.ACLContext;
12+
import hudson.util.ListBoxModel;
13+
import jenkins.model.Jenkins;
14+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
15+
import org.junit.Rule;
16+
import org.junit.Test;
17+
import org.jvnet.hudson.test.Issue;
18+
import org.jvnet.hudson.test.JenkinsRule;
19+
import org.jvnet.hudson.test.MockAuthorizationStrategy;
20+
21+
import java.io.IOException;
22+
23+
import static org.hamcrest.CoreMatchers.is;
24+
import static org.hamcrest.CoreMatchers.not;
25+
import static org.hamcrest.MatcherAssert.assertThat;
26+
import static org.hamcrest.Matchers.empty;
27+
import static org.hamcrest.Matchers.hasSize;
28+
29+
/**
30+
* Test if there is any information disclosure
31+
*/
32+
public class Security2189Test {
33+
34+
private static final String SECURE_DATA = "SecureData";
35+
private static final String TEST_NAME = "test";
36+
private static final String ADMINISTER_NAME = "administer";
37+
private static final String WITHOUT_ANY_PERMISSION_USER_NAME = "WithoutAnyPermissionUser";
38+
39+
@Rule
40+
public JenkinsRule r = new JenkinsRule();
41+
42+
private WorkflowJob job;
43+
private FreeStyleProject project;
44+
45+
@Issue("SECURITY-2189")
46+
@Test
47+
public void doFillCredentialsItemsWhenAdminThenListPopulatedWithNameAndId() throws IOException {
48+
setUpAuthorizationAndWorkflowJob();
49+
initCredentials(SECURE_DATA, TEST_NAME);
50+
51+
try(ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(ADMINISTER_NAME))) {
52+
SSHAgentStep.DescriptorImpl descriptor = (SSHAgentStep.DescriptorImpl) Jenkins.get().getDescriptorOrDie(SSHAgentStep.class);
53+
ListBoxModel secureData = descriptor.doFillCredentialsItems(job);
54+
ListBoxModel expected = new ListBoxModel(new ListBoxModel.Option(TEST_NAME, SECURE_DATA));
55+
56+
assertListBoxModel(secureData, expected);
57+
}
58+
}
59+
60+
@Issue("SECURITY-2189")
61+
@Test
62+
public void doFillCredentialsItemsWhenUserWithoutAnyCredentialsThenListNotPopulated() throws Exception {
63+
setUpAuthorizationAndWorkflowJob();
64+
initCredentials(SECURE_DATA, TEST_NAME);
65+
66+
try(ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(WITHOUT_ANY_PERMISSION_USER_NAME))) {
67+
SSHAgentStep.DescriptorImpl descriptor = (SSHAgentStep.DescriptorImpl) Jenkins.get().getDescriptorOrDie(SSHAgentStep.class);
68+
ListBoxModel secureData = descriptor.doFillCredentialsItems(job);
69+
70+
assertThat(secureData, is(empty()));
71+
}
72+
}
73+
74+
@Issue("SECURITY-2189")
75+
@Test
76+
public void doFillIdItemsWhenAdminThenListPopulated() throws IOException {
77+
setUpAuthorizationAndFreestyleProject();
78+
initCredentials(SECURE_DATA, TEST_NAME);
79+
80+
try(ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(ADMINISTER_NAME))) {
81+
SSHAgentBuildWrapper.CredentialHolder.DescriptorImpl descriptor = (SSHAgentBuildWrapper.CredentialHolder.DescriptorImpl) Jenkins.get().getDescriptorOrDie(SSHAgentBuildWrapper.CredentialHolder.class);
82+
ListBoxModel secureData = descriptor.doFillIdItems(project);
83+
ListBoxModel expected = new ListBoxModel(new ListBoxModel.Option(TEST_NAME, SECURE_DATA));
84+
85+
assertListBoxModel(secureData, expected);
86+
}
87+
}
88+
89+
@Issue("SECURITY-2189")
90+
@Test
91+
public void doFillIdItemsWhenUserWithoutAnyPermissionThenListNotPopulated() throws Exception {
92+
setUpAuthorizationAndWorkflowJob();
93+
initCredentials(SECURE_DATA, TEST_NAME);
94+
95+
try(ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(WITHOUT_ANY_PERMISSION_USER_NAME))) {
96+
SSHAgentBuildWrapper.CredentialHolder.DescriptorImpl descriptor = (SSHAgentBuildWrapper.CredentialHolder.DescriptorImpl) Jenkins.get().getDescriptorOrDie(SSHAgentBuildWrapper.CredentialHolder.class);
97+
ListBoxModel secureData = descriptor.doFillIdItems(project);
98+
99+
assertThat(secureData, is(empty()));
100+
}
101+
}
102+
103+
private void setUpAuthorizationAndWorkflowJob() throws IOException {
104+
job = r.jenkins.createProject(WorkflowJob.class, "j");
105+
setUpAuthorization(job);
106+
}
107+
108+
private void setUpAuthorizationAndFreestyleProject() throws IOException {
109+
project = r.jenkins.createProject(FreeStyleProject.class, "p");
110+
setUpAuthorization(project);
111+
}
112+
113+
private void setUpAuthorization(Item... items) {
114+
r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
115+
r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
116+
.grant(Jenkins.ADMINISTER).everywhere().to(ADMINISTER_NAME)
117+
.grant().onItems(items).to(WITHOUT_ANY_PERMISSION_USER_NAME));
118+
}
119+
120+
private static void assertListBoxModel(ListBoxModel actual, ListBoxModel expected) {
121+
assertThat(actual, is(not(empty())));
122+
assertThat(actual, hasSize(expected.size()));
123+
assertThat(actual.get(0).name, is(expected.get(0).name));
124+
assertThat(actual.get(0).value, is(expected.get(0).value));
125+
}
126+
127+
private static void initCredentials(String credentialsId, String name) throws IOException {
128+
SSHUserPrivateKey key = new BasicSSHUserPrivateKey(CredentialsScope.GLOBAL, credentialsId, "cloudbees",
129+
null, "* .*", name);
130+
SystemCredentialsProvider.getInstance().getCredentials().add(key);
131+
SystemCredentialsProvider.getInstance().save();
132+
}
133+
}

0 commit comments

Comments
 (0)