Skip to content

Commit c3d776a

Browse files
daniel-beckjenkinsci-cert-ci
daniel-beck
authored and
jenkinsci-cert-ci
committed
1 parent f332d7b commit c3d776a

File tree

6 files changed

+293
-5
lines changed

6 files changed

+293
-5
lines changed

‎core/src/main/java/hudson/model/BuildAuthorizationToken.java‎

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,22 @@
2525
package hudson.model;
2626

2727
import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
28+
import hudson.Extension;
2829
import hudson.Util;
30+
import hudson.diagnosis.OldDataMonitor;
31+
import hudson.model.listeners.ItemListener;
2932
import hudson.security.ACL;
33+
import hudson.util.Secret;
3034
import jakarta.servlet.http.HttpServletResponse;
3135
import java.io.IOException;
36+
import java.nio.charset.StandardCharsets;
37+
import java.security.MessageDigest;
38+
import java.util.logging.Level;
39+
import java.util.logging.Logger;
40+
import jenkins.model.Jenkins;
41+
import jenkins.model.ParameterizedJobMixIn;
42+
import org.kohsuke.accmod.Restricted;
43+
import org.kohsuke.accmod.restrictions.NoExternalUse;
3244
import org.kohsuke.stapler.HttpResponses;
3345
import org.kohsuke.stapler.StaplerRequest;
3446
import org.kohsuke.stapler.StaplerRequest2;
@@ -47,10 +59,25 @@
4759
*/
4860
@Deprecated
4961
public final class BuildAuthorizationToken {
50-
private final String token;
62+
private final Secret token;
5163

64+
private transient boolean fromPlaintext;
65+
66+
/**
67+
* @deprecated since TODO
68+
*/
69+
@Deprecated
5270
public BuildAuthorizationToken(String token) {
71+
this.token = Secret.fromString(token);
72+
this.fromPlaintext = true;
73+
}
74+
75+
/**
76+
* @since TODO
77+
*/
78+
public BuildAuthorizationToken(Secret token) {
5379
this.token = token;
80+
this.fromPlaintext = false;
5481
}
5582

5683
/**
@@ -85,7 +112,7 @@ public static void checkPermission(Job<?, ?> project, BuildAuthorizationToken to
85112
if (token != null && token.token != null) {
86113
//check the provided token
87114
String providedToken = req.getParameter("token");
88-
if (providedToken != null && providedToken.equals(token.token))
115+
if (providedToken != null && MessageDigest.isEqual(providedToken.getBytes(StandardCharsets.UTF_8), token.getToken().getBytes(StandardCharsets.UTF_8)))
89116
return;
90117
if (providedToken != null)
91118
throw new AccessDeniedException(Messages.BuildAuthorizationToken_InvalidTokenProvided());
@@ -110,7 +137,15 @@ public static void checkPermission(Job<?, ?> project, BuildAuthorizationToken to
110137
checkPermission(project, token, StaplerRequest.toStaplerRequest2(req), StaplerResponse.toStaplerResponse2(rsp));
111138
}
112139

140+
@Deprecated
113141
public String getToken() {
142+
return token.getPlainText();
143+
}
144+
145+
/**
146+
* @since TODO
147+
*/
148+
public Secret getEncryptedToken() {
114149
return token;
115150
}
116151

@@ -122,12 +157,44 @@ public boolean canConvert(Class type) {
122157

123158
@Override
124159
public Object fromString(String str) {
125-
return new BuildAuthorizationToken(str);
160+
if (Secret.decrypt(str) == null) {
161+
return new BuildAuthorizationToken(str);
162+
}
163+
return new BuildAuthorizationToken(Secret.fromString(str));
126164
}
127165

128166
@Override
129167
public String toString(Object obj) {
130-
return ((BuildAuthorizationToken) obj).token;
168+
final BuildAuthorizationToken bat = (BuildAuthorizationToken) obj;
169+
// We assume this only gets called when re-saving to its usual destination, so let's clear the in-memory state:
170+
bat.fromPlaintext = false;
171+
return bat.token.getEncryptedValue();
172+
}
173+
}
174+
175+
@Extension
176+
@Restricted(NoExternalUse.class)
177+
public static class ItemListenerImpl extends ItemListener {
178+
private static final Logger LOGGER = Logger.getLogger(ItemListenerImpl.class.getName());
179+
180+
@Override
181+
public void onUpdated(Item item) {
182+
if (item instanceof ParameterizedJobMixIn.ParameterizedJob job) {
183+
BuildAuthorizationToken bat = job.getAuthToken();
184+
if (bat != null) {
185+
if (bat.fromPlaintext) {
186+
OldDataMonitor.report(item, "2.528.3"); // TODO Finalize version number
187+
LOGGER.log(Level.FINE, "Reporting " + item.getFullName());
188+
} else {
189+
LOGGER.log(Level.FINE, "Skipping reporting of " + item.getFullName());
190+
}
191+
}
192+
}
193+
}
194+
195+
@Override
196+
public void onLoaded() {
197+
Jenkins.get().getAllItems(ParameterizedJobMixIn.ParameterizedJob.class).forEach(this::onUpdated);
131198
}
132199
}
133200
}

‎core/src/main/resources/hudson/model/BuildAuthorizationToken/config.jelly‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ THE SOFTWARE.
3535
title="${%Trigger builds remotely} (${%e.g., from scripts})"
3636
checked="${it.authToken!=null}">
3737
<f:entry title="${%Authentication Token}">
38-
<f:textbox name="authToken" value="${it.authToken.token}" />
38+
<f:password name="authToken" value="${it.authToken.encryptedToken}" />
39+
<div class="clearfix"/><!-- f:password in readOnlyMode only has a span, not a div, so the following text would be on the same line as "****" -->
3940
${%Use the following URL to trigger build remotely:}
4041
<code>JENKINS_URL</code>/${it.url}build?token=<code>TOKEN_NAME</code>
4142
${%or}

‎test/src/test/java/hudson/model/BuildAuthorizationTokenMigrationTest.java‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package hudson.model;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.containsString;
5+
import static org.hamcrest.Matchers.equalTo;
6+
import static org.hamcrest.Matchers.not;
7+
import static org.hamcrest.Matchers.notNullValue;
8+
9+
import hudson.util.Secret;
10+
import jakarta.servlet.ServletRequest;
11+
import java.net.URL;
12+
import java.nio.charset.StandardCharsets;
13+
import jenkins.model.Jenkins;
14+
import org.apache.commons.io.IOUtils;
15+
import org.htmlunit.HttpMethod;
16+
import org.htmlunit.Page;
17+
import org.htmlunit.WebRequest;
18+
import org.htmlunit.html.HtmlForm;
19+
import org.htmlunit.html.HtmlPage;
20+
import org.htmlunit.xml.XmlPage;
21+
import org.junit.jupiter.api.BeforeEach;
22+
import org.junit.jupiter.api.Test;
23+
import org.jvnet.hudson.test.Issue;
24+
import org.jvnet.hudson.test.JenkinsRule;
25+
import org.jvnet.hudson.test.MockAuthorizationStrategy;
26+
import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
27+
import org.jvnet.hudson.test.recipes.LocalData;
28+
29+
@SuppressWarnings("deprecation")
30+
@Issue("SECURITY-783")
31+
@WithJenkins
32+
public class BuildAuthorizationTokenMigrationTest {
33+
private static final String OLDTOKEN = "oldtoken";
34+
private static final String ADMIN_USERNAME = "alice";
35+
private static final String VERSION_NUMBER = "2.528.3"; // TODO Finalize version number
36+
private static final String JOB_NAME = "my_freestyle_job";
37+
private static final String OLD_DATA_PAGE_URL = "administrativeMonitor/OldData/manage";
38+
private static final String JOB_WITHOUT_TOKEN_NAME = "job_without_token";
39+
public static final String EXTENDED_READER_USERNAME = "bob";
40+
41+
// This should be a JenkinsSessionRule / RealJenkinsRule, to ensure no monitor after startup,
42+
// but it looks like neither works (anymore) with @LocalData/@WithLocalData.
43+
private JenkinsRule j;
44+
45+
@BeforeEach
46+
public void setUp(JenkinsRule j) {
47+
this.j = j;
48+
}
49+
50+
@Test
51+
@LocalData
52+
void basicMigration() throws Throwable {
53+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
54+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
55+
.grant(Jenkins.READ).everywhere().toEveryone()
56+
.grant(Item.READ, Item.EXTENDED_READ).everywhere().to(EXTENDED_READER_USERNAME)
57+
.grant(Jenkins.ADMINISTER).everywhere().to(ADMIN_USERNAME));
58+
j.jenkins.save();
59+
60+
final FreeStyleProject fs = j.jenkins.getItemByFullName(JOB_NAME, FreeStyleProject.class);
61+
62+
{
63+
// inspect original migrated token
64+
assertThat(fs.getConfigFile().asString(), containsString("<authToken>" + OLDTOKEN + "</authToken>"));
65+
66+
final BuildAuthorizationToken authToken = fs.getAuthToken();
67+
assertThat(authToken, notNullValue());
68+
assertThat(authToken.getToken(), equalTo(OLDTOKEN));
69+
assertThat(authToken.getEncryptedToken().getPlainText(), equalTo(OLDTOKEN));
70+
71+
// Not redacted yet
72+
try (JenkinsRule.WebClient wc = j.createWebClient().login(EXTENDED_READER_USERNAME)) {
73+
final XmlPage xmlPage = wc.goToXml(fs.getUrl() + "config.xml");
74+
assertThat(xmlPage.getWebResponse().getContentAsString(), containsString("<authToken>" + OLDTOKEN + "</authToken>"));
75+
}
76+
}
77+
78+
assertThat(j.jenkins.getAdministrativeMonitor("OldData").isActivated(), equalTo(true));
79+
80+
try (JenkinsRule.WebClient wc = j.createWebClient().login(ADMIN_USERNAME)) {
81+
{
82+
final HtmlPage oldDataPage = wc.goTo(OLD_DATA_PAGE_URL);
83+
final String oldDataContent = oldDataPage.getWebResponse().getContentAsString();
84+
assertThat(oldDataContent, containsString(JOB_NAME));
85+
assertThat(oldDataContent, not(containsString(JOB_WITHOUT_TOKEN_NAME)));
86+
87+
assertThat(oldDataContent, containsString(VERSION_NUMBER));
88+
assertThat(oldDataContent, not(containsString("No old data")));
89+
90+
final HtmlForm form = oldDataPage.getFormByName("oldDataUpgrade");
91+
form.submit(form.getButtonByName("Submit"));
92+
}
93+
{
94+
final HtmlPage oldDataPage = wc.goTo(OLD_DATA_PAGE_URL);
95+
final String oldDataContent = oldDataPage.getWebResponse().getContentAsString();
96+
assertThat(oldDataContent, not(containsString(JOB_NAME)));
97+
// TODO Add the following assertion back when we change the version string to something not appearing everywhere on the page:
98+
//assertThat(oldDataContent, not(containsString(VERSION_NUMBER)));
99+
assertThat(oldDataContent, containsString("No old data"));
100+
}
101+
}
102+
103+
assertThat(j.jenkins.getAdministrativeMonitor("OldData").isActivated(), equalTo(false));
104+
105+
{
106+
// New auth token
107+
final BuildAuthorizationToken authToken = fs.getAuthToken();
108+
assertThat(authToken, notNullValue());
109+
assertThat(authToken.getToken(), equalTo(OLDTOKEN));
110+
final Secret newEncryptedToken = authToken.getEncryptedToken();
111+
assertThat(newEncryptedToken.getPlainText(), equalTo(OLDTOKEN));
112+
113+
assertThat(fs.getConfigFile().asString(), not(containsString("<authToken>" + OLDTOKEN + "</authToken>")));
114+
assertThat(fs.getConfigFile().asString(), containsString("<authToken>" + newEncryptedToken.getEncryptedValue() + "</authToken>"));
115+
116+
// Redacted after save
117+
try (JenkinsRule.WebClient wc = j.createWebClient().login(EXTENDED_READER_USERNAME)) {
118+
final XmlPage xmlPage = wc.goToXml(fs.getUrl() + "config.xml");
119+
assertThat(xmlPage.getWebResponse().getContentAsString(), containsString("<authToken>********</authToken>"));
120+
}
121+
}
122+
}
123+
124+
@Test
125+
void testPostConfigXmlAndClearing() throws Exception {
126+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
127+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().toEveryone().grant(Jenkins.ADMINISTER).everywhere().to(ADMIN_USERNAME));
128+
129+
assertThat(j.jenkins.getAdministrativeMonitor("OldData").isActivated(), equalTo(false));
130+
131+
final FreeStyleProject freeStyleProject = j.createFreeStyleProject();
132+
133+
try (JenkinsRule.WebClient wc = j.createWebClient().login(ADMIN_USERNAME)) {
134+
{
135+
// POST config.xml with plain token
136+
final WebRequest webRequest = new WebRequest(new URL(j.getURL(), freeStyleProject.getUrl() + "config.xml?" +
137+
j.jenkins.getCrumbIssuer().getDescriptor().getCrumbRequestField() + "=" + j.jenkins.getCrumbIssuer().getCrumb((ServletRequest) null)),
138+
HttpMethod.POST);
139+
webRequest.setAdditionalHeader("Content-Type", "application/xml");
140+
webRequest.setRequestBody(IOUtils.resourceToString("/" + getClass().getName().replace(".", "/") + "/job-config-with-plain-token.xml", StandardCharsets.UTF_8));
141+
final Page apiPage = wc.getPage(webRequest);
142+
assertThat(apiPage.getWebResponse().getStatusCode(), equalTo(200));
143+
}
144+
145+
assertThat(j.jenkins.getAdministrativeMonitor("OldData").isActivated(), equalTo(true));
146+
147+
{
148+
// Testing that GET config.xml does not clear the in-memory flag via ConverterImpl
149+
final XmlPage xmlPage = wc.goToXml(freeStyleProject.getUrl() + "config.xml");
150+
assertThat(xmlPage.getWebResponse().getContentAsString(), containsString("plain_token"));
151+
152+
assertThat(freeStyleProject.getConfigFile().asString(), containsString("plain_token"));
153+
}
154+
155+
// Saving clears the admin monitor
156+
freeStyleProject.save();
157+
assertThat(j.jenkins.getAdministrativeMonitor("OldData").isActivated(), equalTo(false));
158+
159+
{
160+
final XmlPage xmlPage = wc.goToXml(freeStyleProject.getUrl() + "config.xml");
161+
assertThat(xmlPage.getWebResponse().getContentAsString(), not(containsString("plain_token")));
162+
assertThat(freeStyleProject.getConfigFile().asString(), not(containsString("plain_token")));
163+
}
164+
165+
}
166+
}
167+
}

‎test/src/test/resources/hudson/model/BuildAuthorizationTokenMigrationTest/basicMigration/jobs/job_without_token/config.xml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?xml version='1.1' encoding='UTF-8'?>
2+
<project>
3+
<actions/>
4+
<description></description>
5+
<keepDependencies>false</keepDependencies>
6+
<properties/>
7+
<scm class="hudson.scm.NullSCM"/>
8+
<canRoam>true</canRoam>
9+
<disabled>false</disabled>
10+
<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
11+
<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
12+
<triggers/>
13+
<concurrentBuild>false</concurrentBuild>
14+
<builders/>
15+
<publishers/>
16+
<buildWrappers/>
17+
</project>

‎test/src/test/resources/hudson/model/BuildAuthorizationTokenMigrationTest/basicMigration/jobs/my_freestyle_job/config.xml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version='1.1' encoding='UTF-8'?>
2+
<project>
3+
<actions/>
4+
<description></description>
5+
<keepDependencies>false</keepDependencies>
6+
<properties/>
7+
<scm class="hudson.scm.NullSCM"/>
8+
<canRoam>true</canRoam>
9+
<disabled>false</disabled>
10+
<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
11+
<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
12+
<authToken>oldtoken</authToken>
13+
<triggers/>
14+
<concurrentBuild>false</concurrentBuild>
15+
<builders/>
16+
<publishers/>
17+
<buildWrappers/>
18+
</project>

‎test/src/test/resources/hudson/model/BuildAuthorizationTokenMigrationTest/job-config-with-plain-token.xml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version='1.1' encoding='UTF-8'?>
2+
<project>
3+
<actions/>
4+
<description></description>
5+
<keepDependencies>false</keepDependencies>
6+
<properties/>
7+
<scm class="hudson.scm.NullSCM"/>
8+
<canRoam>true</canRoam>
9+
<disabled>false</disabled>
10+
<blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
11+
<blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
12+
<authToken>plain_token</authToken>
13+
<triggers/>
14+
<concurrentBuild>false</concurrentBuild>
15+
<builders/>
16+
<publishers/>
17+
<buildWrappers/>
18+
</project>

0 commit comments

Comments
 (0)