Skip to content

Commit 07d2ff1

Browse files
Remove IP address from DefaultCrumbIssuer (#25918)
Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com> Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
1 parent 587983b commit 07d2ff1

23 files changed

+204
-369
lines changed

core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@
1010
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1111
import hudson.Extension;
1212
import hudson.Util;
13+
import hudson.model.AdministrativeMonitor;
1314
import hudson.model.ModelObject;
1415
import hudson.model.PersistentDescriptor;
16+
import jakarta.servlet.ServletException;
1517
import jakarta.servlet.ServletRequest;
1618
import jakarta.servlet.http.HttpServletRequest;
19+
import java.io.IOException;
1720
import java.nio.charset.StandardCharsets;
1821
import java.security.MessageDigest;
1922
import java.security.NoSuchAlgorithmException;
@@ -27,29 +30,56 @@
2730
import org.kohsuke.accmod.Restricted;
2831
import org.kohsuke.accmod.restrictions.NoExternalUse;
2932
import org.kohsuke.stapler.DataBoundConstructor;
33+
import org.kohsuke.stapler.QueryParameter;
3034
import org.kohsuke.stapler.StaplerRequest2;
35+
import org.kohsuke.stapler.StaplerResponse2;
36+
import org.kohsuke.stapler.verb.POST;
3137
import org.springframework.security.core.Authentication;
3238

3339
/**
34-
* A crumb issuing algorithm based on the request principal and the remote address.
40+
* A crumb issuing algorithm based on the request principal and the session ID.
3541
*
3642
* @author dty
3743
*/
3844
public class DefaultCrumbIssuer extends CrumbIssuer {
3945

4046
private transient MessageDigest md;
41-
private boolean excludeClientIPFromCrumb;
47+
private transient boolean excludeClientIPFromCrumb;
4248

4349
@Restricted(NoExternalUse.class)
4450
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console")
4551
public static /* non-final: Groovy Console */ boolean EXCLUDE_SESSION_ID = SystemProperties.getBoolean(DefaultCrumbIssuer.class.getName() + ".EXCLUDE_SESSION_ID");
4652

4753
@DataBoundConstructor
54+
public DefaultCrumbIssuer() {
55+
initializeMessageDigest();
56+
logSessionIdWarningIfNeeded();
57+
}
58+
59+
/**
60+
* @param excludeClientIPFromCrumb unused
61+
* @deprecated Use {@link #DefaultCrumbIssuer()} instead.
62+
*/
63+
@Deprecated
4864
public DefaultCrumbIssuer(boolean excludeClientIPFromCrumb) {
65+
this();
4966
this.excludeClientIPFromCrumb = excludeClientIPFromCrumb;
50-
initializeMessageDigest();
67+
logSessionIdWarningIfNeeded();
5168
}
5269

70+
private void logSessionIdWarningIfNeeded() {
71+
if (EXCLUDE_SESSION_ID) {
72+
LOGGER.warning("Jenkins no longer uses the client IP address as part of CSRF protection. " +
73+
"This controller is configured to also not use the session ID (hudson.security.csrf.DefaultCrumbIssuer.EXCLUDE_SESSION_ID), which is even less safe now. " +
74+
"This option will be removed in future releases.");
75+
}
76+
}
77+
78+
/**
79+
* @return the previously set value
80+
* @deprecated This setting is no longer effective.
81+
*/
82+
@Deprecated
5383
public boolean isExcludeClientIPFromCrumb() {
5484
return this.excludeClientIPFromCrumb;
5585
}
@@ -77,10 +107,6 @@ protected synchronized String issueCrumb(ServletRequest request, String salt) {
77107
StringBuilder buffer = new StringBuilder();
78108
Authentication a = Jenkins.getAuthentication2();
79109
buffer.append(a.getName());
80-
buffer.append(';');
81-
if (!isExcludeClientIPFromCrumb()) {
82-
buffer.append(getClientIP(req));
83-
}
84110
if (!EXCLUDE_SESSION_ID) {
85111
buffer.append(';');
86112
buffer.append(req.getSession().getId());
@@ -106,20 +132,6 @@ public boolean validateCrumb(ServletRequest request, String salt, String crumb)
106132
return false;
107133
}
108134

109-
private static final String X_FORWARDED_FOR = "X-Forwarded-For";
110-
111-
private String getClientIP(HttpServletRequest req) {
112-
String defaultAddress = req.getRemoteAddr();
113-
String forwarded = req.getHeader(X_FORWARDED_FOR);
114-
if (forwarded != null) {
115-
String[] hopList = forwarded.split(",");
116-
if (hopList.length >= 1) {
117-
return hopList[0];
118-
}
119-
}
120-
return defaultAddress;
121-
}
122-
123135
@Extension @Symbol("standard")
124136
public static final class DescriptorImpl extends CrumbIssuerDescriptor<DefaultCrumbIssuer> implements ModelObject, PersistentDescriptor {
125137

@@ -146,5 +158,41 @@ public DefaultCrumbIssuer newInstance(StaplerRequest2 req, JSONObject formData)
146158
}
147159
}
148160

161+
@Extension
162+
@Restricted(NoExternalUse.class)
163+
public static class ExcludeSessionIdAdministrativeMonitor extends AdministrativeMonitor {
164+
165+
@Override
166+
public boolean isActivated() {
167+
final CrumbIssuer crumbIssuer = Jenkins.get().getCrumbIssuer();
168+
if (crumbIssuer instanceof DefaultCrumbIssuer) {
169+
return EXCLUDE_SESSION_ID;
170+
}
171+
return false;
172+
}
173+
174+
@Override
175+
public boolean isSecurity() {
176+
return true;
177+
}
178+
179+
@Override
180+
public String getDisplayName() {
181+
return "Warn when session ID is excluded from Default Crumb Issuer"; // TODO i18n
182+
}
183+
184+
@POST
185+
public void doAct(StaplerRequest2 req, StaplerResponse2 rsp, @QueryParameter String learn, @QueryParameter String dismiss) throws IOException, ServletException {
186+
if (learn != null) {
187+
rsp.sendRedirect("https://www.jenkins.io/redirect/csrf-protection/");
188+
return;
189+
}
190+
if (dismiss != null) {
191+
disable(true);
192+
}
193+
rsp.forwardToPreviousPage(req);
194+
}
195+
}
196+
149197
private static final Logger LOGGER = Logger.getLogger(DefaultCrumbIssuer.class.getName());
150198
}

core/src/main/java/hudson/security/csrf/GlobalCrumbIssuerConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static CrumbIssuer createDefaultCrumbIssuer() {
7373
if (DISABLE_CSRF_PROTECTION) {
7474
return null;
7575
}
76-
return new DefaultCrumbIssuer(SystemProperties.getBoolean(Jenkins.class.getName() + ".crumbIssuerProxyCompatibility", false));
76+
return new DefaultCrumbIssuer();
7777
}
7878

7979
@Restricted(NoExternalUse.class)
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!--
2+
The MIT License
3+
4+
Copyright (c) 2025, CloudBees, Inc.
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
The above copyright notice and this permission notice shall be included in
14+
all copies or substantial portions of the Software.
15+
16+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
THE SOFTWARE.
23+
-->
24+
25+
<?jelly escape-by-default='true'?>
26+
<j:jelly xmlns:j="jelly:core">
27+
${%blurb}
28+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
blurb = This administrative monitor alerts when the Default Crumb Issuer is configured to exclude session IDs from CSRF protection. \
2+
Jenkins no longer includes the IP address in the CSRF crumb generation, so removing the session ID from the CSRF protection weakens security considerably.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!--
2+
The MIT License
3+
4+
Copyright (c) 2025, CloudBees, Inc.
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
The above copyright notice and this permission notice shall be included in
14+
all copies or substantial portions of the Software.
15+
16+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
THE SOFTWARE.
23+
-->
24+
25+
<?jelly escape-by-default='true'?>
26+
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
27+
<div class="jenkins-alert jenkins-alert-warning">
28+
<form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
29+
<f:submit name="learn" value="${%Learn more}"/>
30+
<f:submit name="dismiss" value="${%Dismiss}"/>
31+
</form>
32+
${%blurb}
33+
</div>
34+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
blurb = Jenkins no longer includes the IP address in the CSRF crumb generation and verification. \
2+
However, the flag <code>hudson.security.csrf.DefaultCrumbIssuer.EXCLUDE_SESSION_ID</code> is set in this controller. \
3+
As a result, there is currently no part of the CSRF crumb that is unique to a user session. \
4+
It is strongly recommended to unset this flag, to ensure that CSRF protection remains effective.
Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
11
<?jelly escape-by-default='true'?>
22
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
3-
<f:entry field="excludeClientIPFromCrumb">
4-
<f:checkbox checked="${instance.isExcludeClientIPFromCrumb()}" title="${%Enable proxy compatibility}" />
5-
</f:entry>
63
</j:jelly>

core/src/main/resources/hudson/security/csrf/DefaultCrumbIssuer/config_bg.properties

Lines changed: 0 additions & 24 deletions
This file was deleted.

core/src/main/resources/hudson/security/csrf/DefaultCrumbIssuer/config_de.properties

Lines changed: 0 additions & 23 deletions
This file was deleted.

core/src/main/resources/hudson/security/csrf/DefaultCrumbIssuer/config_es.properties

Lines changed: 0 additions & 23 deletions
This file was deleted.

0 commit comments

Comments
 (0)