Skip to content

Commit f2eb8eb

Browse files
daniel-beckjenkinsci-cert-ci
authored andcommitted
[SECURITY-3495][SECURITY-3496]
1 parent 0f8f5a2 commit f2eb8eb

File tree

5 files changed

+204
-4
lines changed

5 files changed

+204
-4
lines changed

core/src/main/java/hudson/cli/GetNodeCommand.java

+13-1
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@
2727
import hudson.Extension;
2828
import hudson.model.Computer;
2929
import hudson.model.Node;
30+
import java.io.ByteArrayOutputStream;
3031
import java.io.IOException;
32+
import java.nio.charset.StandardCharsets;
3133
import jenkins.model.Jenkins;
34+
import jenkins.security.ExtendedReadRedaction;
3235
import org.kohsuke.args4j.Argument;
3336

3437
/**
@@ -52,7 +55,16 @@ protected int run() throws IOException {
5255

5356
node.checkPermission(Computer.EXTENDED_READ);
5457

55-
Jenkins.XSTREAM2.toXMLUTF8(node, stdout);
58+
if (node.hasPermission(Computer.CONFIGURE)) {
59+
Jenkins.XSTREAM2.toXMLUTF8(node, stdout);
60+
} else {
61+
var baos = new ByteArrayOutputStream();
62+
Jenkins.XSTREAM2.toXMLUTF8(node, baos);
63+
String xml = baos.toString(StandardCharsets.UTF_8);
64+
65+
xml = ExtendedReadRedaction.applyAll(xml);
66+
org.apache.commons.io.IOUtils.write(xml, stdout, StandardCharsets.UTF_8);
67+
}
5668

5769
return 0;
5870
}

core/src/main/java/hudson/cli/GetViewCommand.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626

2727
import hudson.Extension;
2828
import hudson.model.View;
29+
import java.io.ByteArrayOutputStream;
30+
import java.nio.charset.StandardCharsets;
31+
import jenkins.security.ExtendedReadRedaction;
2932
import org.kohsuke.args4j.Argument;
3033

3134
/**
@@ -48,7 +51,17 @@ public String getShortDescription() {
4851
protected int run() throws Exception {
4952

5053
view.checkPermission(View.READ);
51-
view.writeXml(stdout);
54+
55+
if (view.hasPermission(View.CONFIGURE)) {
56+
view.writeXml(stdout);
57+
} else {
58+
var baos = new ByteArrayOutputStream();
59+
view.writeXml(baos);
60+
String xml = baos.toString(StandardCharsets.UTF_8);
61+
62+
xml = ExtendedReadRedaction.applyAll(xml);
63+
org.apache.commons.io.IOUtils.write(xml, stdout, StandardCharsets.UTF_8);
64+
}
5265

5366
return 0;
5467
}

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import hudson.util.RemotingDiagnostics.HeapDump;
7575
import hudson.util.RunList;
7676
import jakarta.servlet.ServletException;
77+
import java.io.ByteArrayOutputStream;
7778
import java.io.File;
7879
import java.io.IOException;
7980
import java.io.InputStream;
@@ -83,6 +84,7 @@
8384
import java.net.InetAddress;
8485
import java.net.NetworkInterface;
8586
import java.nio.charset.Charset;
87+
import java.nio.charset.StandardCharsets;
8688
import java.nio.file.Files;
8789
import java.nio.file.InvalidPathException;
8890
import java.nio.file.StandardCopyOption;
@@ -110,6 +112,7 @@
110112
import jenkins.model.IComputer;
111113
import jenkins.model.IDisplayExecutor;
112114
import jenkins.model.Jenkins;
115+
import jenkins.security.ExtendedReadRedaction;
113116
import jenkins.security.ImpersonatingExecutorService;
114117
import jenkins.security.MasterToSlaveCallable;
115118
import jenkins.security.stapler.StaplerDispatchable;
@@ -1526,7 +1529,16 @@ public void doConfigDotXml(StaplerRequest2 req, StaplerResponse2 rsp)
15261529
if (node == null) {
15271530
throw HttpResponses.notFound();
15281531
}
1529-
Jenkins.XSTREAM2.toXMLUTF8(node, rsp.getOutputStream());
1532+
if (hasPermission(CONFIGURE)) {
1533+
Jenkins.XSTREAM2.toXMLUTF8(node, rsp.getOutputStream());
1534+
} else {
1535+
var baos = new ByteArrayOutputStream();
1536+
Jenkins.XSTREAM2.toXMLUTF8(node, baos);
1537+
String xml = baos.toString(StandardCharsets.UTF_8);
1538+
1539+
xml = ExtendedReadRedaction.applyAll(xml);
1540+
org.apache.commons.io.IOUtils.write(xml, rsp.getOutputStream(), StandardCharsets.UTF_8);
1541+
}
15301542
return;
15311543
}
15321544
if (req.getMethod().equals("POST")) {

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

+12-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import jakarta.servlet.http.HttpServletResponse;
6363
import java.io.BufferedInputStream;
6464
import java.io.ByteArrayInputStream;
65+
import java.io.ByteArrayOutputStream;
6566
import java.io.IOException;
6667
import java.io.InputStream;
6768
import java.io.OutputStream;
@@ -94,6 +95,7 @@
9495
import jenkins.model.item_category.Categories;
9596
import jenkins.model.item_category.Category;
9697
import jenkins.model.item_category.ItemCategory;
98+
import jenkins.security.ExtendedReadRedaction;
9799
import jenkins.security.stapler.StaplerNotDispatchable;
98100
import jenkins.util.xml.XMLUtils;
99101
import jenkins.widgets.HasWidgets;
@@ -983,7 +985,16 @@ private HttpResponse doConfigDotXmlImpl(StaplerRequest2 req) throws IOException
983985
@Override
984986
public void generateResponse(StaplerRequest2 req, StaplerResponse2 rsp, Object node) throws IOException, ServletException {
985987
rsp.setContentType("application/xml");
986-
View.this.writeXml(rsp.getOutputStream());
988+
if (hasPermission(CONFIGURE)) {
989+
View.this.writeXml(rsp.getOutputStream());
990+
} else {
991+
var baos = new ByteArrayOutputStream();
992+
View.this.writeXml(baos);
993+
String xml = baos.toString(StandardCharsets.UTF_8);
994+
995+
xml = ExtendedReadRedaction.applyAll(xml);
996+
org.apache.commons.io.IOUtils.write(xml, rsp.getOutputStream(), StandardCharsets.UTF_8);
997+
}
987998
}
988999
};
9891000
}

test/src/test/java/lib/form/PasswordTest.java

+152
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
package lib.form;
2626

27+
import static java.nio.file.Files.readString;
2728
import static org.hamcrest.MatcherAssert.assertThat;
2829
import static org.hamcrest.Matchers.containsString;
2930
import static org.hamcrest.Matchers.is;
@@ -40,6 +41,8 @@
4041
import hudson.Launcher;
4142
import hudson.cli.CopyJobCommand;
4243
import hudson.cli.GetJobCommand;
44+
import hudson.cli.GetNodeCommand;
45+
import hudson.cli.GetViewCommand;
4346
import hudson.model.AbstractProject;
4447
import hudson.model.Action;
4548
import hudson.model.Computer;
@@ -48,25 +51,35 @@
4851
import hudson.model.Job;
4952
import hudson.model.JobProperty;
5053
import hudson.model.JobPropertyDescriptor;
54+
import hudson.model.ListView;
55+
import hudson.model.Node;
5156
import hudson.model.RootAction;
5257
import hudson.model.Run;
5358
import hudson.model.TaskListener;
5459
import hudson.model.User;
60+
import hudson.model.View;
61+
import hudson.model.ViewProperty;
62+
import hudson.security.ACL;
63+
import hudson.slaves.DumbSlave;
64+
import hudson.slaves.NodeProperty;
5565
import hudson.tasks.BuildStepDescriptor;
5666
import hudson.tasks.Builder;
5767
import hudson.util.FormValidation;
5868
import hudson.util.Secret;
5969
import java.io.ByteArrayOutputStream;
70+
import java.io.File;
6071
import java.io.IOException;
6172
import java.io.PrintStream;
6273
import java.util.Arrays;
6374
import java.util.Collection;
6475
import java.util.List;
6576
import java.util.Locale;
77+
import java.util.Map;
6678
import java.util.regex.Pattern;
6779
import jenkins.model.GlobalConfiguration;
6880
import jenkins.model.Jenkins;
6981
import jenkins.model.TransientActionFactory;
82+
import jenkins.security.ExtendedReadRedaction;
7083
import jenkins.security.ExtendedReadSecretRedaction;
7184
import jenkins.tasks.SimpleBuildStep;
7285
import org.htmlunit.Page;
@@ -124,6 +137,145 @@ public String getUrlName() {
124137
}
125138
}
126139

140+
@For({ExtendedReadRedaction.class, ExtendedReadSecretRedaction.class})
141+
@Issue("SECURITY-3495")
142+
@Test
143+
public void testNodeSecrets() throws Exception {
144+
Computer.EXTENDED_READ.setEnabled(true);
145+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
146+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("alice").grant(Jenkins.READ, Computer.EXTENDED_READ).everywhere().to("bob"));
147+
148+
final DumbSlave onlineSlave = j.createOnlineSlave();
149+
final String secretText = "t0ps3cr3td4t4_node";
150+
final Secret encryptedSecret = Secret.fromString(secretText);
151+
final String encryptedSecretText = encryptedSecret.getEncryptedValue();
152+
153+
onlineSlave.getNodeProperties().add(new NodePropertyWithSecret(encryptedSecret));
154+
onlineSlave.save();
155+
156+
assertThat(readString(new File(onlineSlave.getRootDir(), "config.xml").toPath()), containsString(encryptedSecretText));
157+
158+
159+
{ // admin can see encrypted value
160+
GetNodeCommand command = new GetNodeCommand();
161+
try (JenkinsRule.WebClient wc = j.createWebClient().login("alice")) {
162+
final Page page = wc.goTo(onlineSlave.getComputer().getUrl() + "config.xml", "application/xml");
163+
final String content = page.getWebResponse().getContentAsString();
164+
165+
assertThat(content, not(containsString(secretText)));
166+
assertThat(content, containsString(encryptedSecretText));
167+
assertThat(content, containsString("<secret>" + encryptedSecretText + "</secret>"));
168+
169+
var baos = new ByteArrayOutputStream();
170+
try (var unused = ACL.as(User.get("alice", true, Map.of()))) {
171+
command.setTransportAuth2(Jenkins.getAuthentication2());
172+
command.main(List.of(onlineSlave.getNodeName()), Locale.US, System.in, new PrintStream(baos), System.err);
173+
}
174+
assertEquals(content, baos.toString(page.getWebResponse().getContentCharset()));
175+
}
176+
}
177+
178+
{ // extended reader gets only redacted value
179+
GetNodeCommand command = new GetNodeCommand();
180+
try (JenkinsRule.WebClient wc = j.createWebClient().login("bob")) {
181+
final Page page = wc.goTo(onlineSlave.getComputer().getUrl() + "config.xml", "application/xml");
182+
final String content = page.getWebResponse().getContentAsString();
183+
184+
assertThat(content, not(containsString(secretText)));
185+
assertThat(content, not(containsString(encryptedSecretText)));
186+
assertThat(content, containsString("<secret>********</secret>"));
187+
188+
var baos = new ByteArrayOutputStream();
189+
try (var unused = ACL.as(User.get("bob", true, Map.of()))) {
190+
command.setTransportAuth2(Jenkins.getAuthentication2());
191+
command.main(List.of(onlineSlave.getNodeName()), Locale.US, System.in, new PrintStream(baos), System.err);
192+
}
193+
assertEquals(content, baos.toString(page.getWebResponse().getContentCharset()));
194+
}
195+
}
196+
}
197+
198+
public static class NodePropertyWithSecret extends NodeProperty<Node> {
199+
private final Secret secret;
200+
201+
public NodePropertyWithSecret(Secret secret) {
202+
this.secret = secret;
203+
}
204+
205+
public Secret getSecret() {
206+
return secret;
207+
}
208+
}
209+
210+
@For({ExtendedReadRedaction.class, ExtendedReadSecretRedaction.class})
211+
@Issue("SECURITY-3496")
212+
@Test
213+
public void testViewSecrets() throws Exception {
214+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
215+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("alice").grant(Jenkins.READ, View.READ).everywhere().to("bob"));
216+
217+
final String secretText = "t0ps3cr3td4t4_view";
218+
final Secret encryptedSecret = Secret.fromString(secretText);
219+
final String encryptedSecretText = encryptedSecret.getEncryptedValue();
220+
221+
final ListView v = new ListView("security-3496");
222+
v.getProperties().add(new ViewPropertyWithSecret(encryptedSecret));
223+
j.jenkins.addView(v);
224+
225+
assertThat(readString(new File(j.jenkins.getRootDir(), "config.xml").toPath()), containsString(encryptedSecretText));
226+
227+
228+
{ // admin can see encrypted value
229+
var command = new GetViewCommand();
230+
try (JenkinsRule.WebClient wc = j.createWebClient().login("alice")) {
231+
final Page page = wc.goTo(v.getUrl() + "config.xml", "application/xml");
232+
final String content = page.getWebResponse().getContentAsString();
233+
234+
assertThat(content, not(containsString(secretText)));
235+
assertThat(content, containsString(encryptedSecretText));
236+
assertThat(content, containsString("<secret>" + encryptedSecretText + "</secret>"));
237+
238+
var baos = new ByteArrayOutputStream();
239+
try (var unused = ACL.as(User.get("alice", true, Map.of()))) {
240+
command.setTransportAuth2(Jenkins.getAuthentication2());
241+
command.main(List.of(v.getViewName()), Locale.US, System.in, new PrintStream(baos), System.err);
242+
}
243+
assertEquals(content, baos.toString(page.getWebResponse().getContentCharset()));
244+
}
245+
}
246+
247+
{ // extended reader gets only redacted value
248+
var command = new GetViewCommand();
249+
try (JenkinsRule.WebClient wc = j.createWebClient().login("bob")) {
250+
final Page page = wc.goTo(v.getUrl() + "config.xml", "application/xml");
251+
final String content = page.getWebResponse().getContentAsString();
252+
253+
assertThat(content, not(containsString(secretText)));
254+
assertThat(content, not(containsString(encryptedSecretText)));
255+
assertThat(content, containsString("<secret>********</secret>"));
256+
257+
var baos = new ByteArrayOutputStream();
258+
try (var unused = ACL.as(User.get("bob", true, Map.of()))) {
259+
command.setTransportAuth2(Jenkins.getAuthentication2());
260+
command.main(List.of(v.getViewName()), Locale.US, System.in, new PrintStream(baos), System.err);
261+
}
262+
assertEquals(content, baos.toString(page.getWebResponse().getContentCharset()));
263+
}
264+
}
265+
}
266+
267+
public static class ViewPropertyWithSecret extends ViewProperty {
268+
private final Secret secret;
269+
270+
public ViewPropertyWithSecret(Secret secret) {
271+
this.secret = secret;
272+
}
273+
274+
public Secret getSecret() {
275+
return secret;
276+
}
277+
}
278+
127279
@Issue({"SECURITY-266", "SECURITY-304"})
128280
@Test
129281
@For(ExtendedReadSecretRedaction.class)

0 commit comments

Comments
 (0)