Skip to content

Commit

Permalink
[SECURITY-1695][SECURITY-1697]
Browse files Browse the repository at this point in the history
  • Loading branch information
Wadeck committed Jan 14, 2020
1 parent 5054bc6 commit 77f36f3
Show file tree
Hide file tree
Showing 3 changed files with 302 additions and 31 deletions.
26 changes: 23 additions & 3 deletions core/src/main/java/hudson/security/WhoAmI.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,26 @@
import hudson.model.UnprotectedRootAction;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import jenkins.util.MemoryReductionUtil;
import jenkins.model.Jenkins;

import org.acegisecurity.Authentication;
import org.acegisecurity.GrantedAuthority;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

import javax.annotation.Nonnull;

/**
* Expose the data needed for /whoAmI, so it can be exposed by Api.
*
Expand All @@ -26,6 +35,11 @@
@Extension @Symbol("whoAmI")
@ExportedBean
public class WhoAmI implements UnprotectedRootAction {
private static final Set<String> dangerousHeaders = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
"cookie",
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers#Authentication
"authorization", "www-authenticate", "proxy-authenticate", "proxy-authorization"
)));

public Api getApi() {
return new Api(this);
Expand All @@ -46,17 +60,17 @@ public boolean isAnonymous() {
return Functions.isAnonymous();
}

@Exported
// @Exported removed due to leak of sessionId with some SecurityRealm
public String getDetails() {
return auth().getDetails() != null ? auth().getDetails().toString() : null;
}

@Exported
// @Exported removed due to leak of sessionId with some SecurityRealm
public String getToString() {
return auth().toString();
}

private Authentication auth() {
private @Nonnull Authentication auth() {
return Jenkins.getAuthentication();
}

Expand All @@ -72,6 +86,12 @@ public String[] getAuthorities() {
return (String[]) authorities.toArray(new String[authorities.size()]);
}

// Used by Jelly
@Restricted(NoExternalUse.class)
public boolean isHeaderDangerous(@Nonnull String name) {
return dangerousHeaders.contains(name.toLowerCase(Locale.ENGLISH));
}

@Override
public String getIconFileName() {
return null;
Expand Down
50 changes: 22 additions & 28 deletions core/src/main/resources/hudson/security/WhoAmI/index.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -61,41 +61,35 @@ THE SOFTWARE.
</ul>
</td>
</tr>
<tr>
<td>Details:</td>
<td>${auth.details}</td>
</tr>
<tr>
<td>toString:</td>
<td>${auth}</td>
</tr>
</table>

<h2>Request Headers</h2>
<table>
<j:forEach var="n" items="${request.getHeaderNames()}">
<j:if test="${n.equalsIgnoreCase('Cookie')}">
<tr>
<td rowspan="1">${n}</td>
<td>
<i>(redacted for security reasons)</i>
</td>
</tr>
</j:if>
<j:if test="${!n.equalsIgnoreCase('Cookie')}">
<j:set var="values" value="${h.getRequestHeaders(n)}"/>
<tr>
<td rowspan="${values.size()}">${n}</td>
<td>
${values[0]}
</td>
</tr>
<j:forEach var="v" items="${values.subList(1,values.size())}">
<j:choose>
<j:when test="${it.isHeaderDangerous(n)}" >
<tr>
<td>${v}</td>
<td rowspan="1">${n}</td>
<td>
<i>(redacted for security reasons)</i>
</td>
</tr>
</j:forEach>
</j:if>
</j:when>
<j:otherwise>
<j:set var="values" value="${h.getRequestHeaders(n)}"/>
<tr>
<td rowspan="${values.size()}">${n}</td>
<td>
${values[0]}
</td>
</tr>
<j:forEach var="v" items="${values.subList(1,values.size())}">
<tr>
<td>${v}</td>
</tr>
</j:forEach>
</j:otherwise>
</j:choose>
</j:forEach>
</table>
</l:main-panel>
Expand Down
257 changes: 257 additions & 0 deletions test/src/test/java/hudson/security/WhoAmITest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
/*
* The MIT License
*
* Copyright (c) 2020, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.security;

import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.model.User;
import jenkins.model.Jenkins;
import jenkins.security.ApiTokenProperty;
import jenkins.security.apitoken.ApiTokenStore;
import org.acegisecurity.AuthenticationException;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.userdetails.UserDetails;
import org.acegisecurity.userdetails.UsernameNotFoundException;
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.kohsuke.stapler.Stapler;
import org.springframework.dao.DataAccessException;

import javax.servlet.http.HttpSession;

import java.nio.charset.StandardCharsets;
import java.util.Base64;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;

public class WhoAmITest {

@Rule
public final JenkinsRule j = new JenkinsRule();

@Test
@Issue("SECURITY-1695")
public void whoAmI_regular_doesNotProvideSensitiveInformation() throws Exception {
j.jenkins.setSecurityRealm(new SecurityRealmImpl());

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

JenkinsRule.WebClient wc = j.createWebClient();
wc.login("user");

HtmlPage whoAmIPage = wc.goTo("whoAmI");
String content = whoAmIPage.getWebResponse().getContentAsString();

String sessionId = wc.executeOnServer(() -> {
HttpSession session = Stapler.getCurrentRequest().getSession(false);
return session != null ? session.getId() : null;
});

assertThat(sessionId, not(nullValue()));

// dangerous stuff in Regular Login mode:
/*
* <td>Details:</td>
* <td>org.acegisecurity.ui.WebAuthenticationDetails@12afc: RemoteIpAddress: 127.0.0.1; SessionId: node0gbmv9ly0f3h517eppoupykq6n0</td>
*
* <td>toString:</td>
* <td>org.acegisecurity.providers.UsernamePasswordAuthenticationToken@d35a1467: Username: [toString()=S3cr3t];
* Password: [PROTECTED]; Authenticated: true; Details:
* org.acegisecurity.ui.WebAuthenticationDetails@12afc: RemoteIpAddress: 127.0.0.1; SessionId:
* node0gbmv9ly0f3h517eppoupykq6n0; Granted Authorities:
* </td>
*/
assertThat(content, not(anyOf(
containsString("S3cr3t"),
containsString("SessionId"),
containsString(sessionId)
)));
}

@Test
@Issue("SECURITY-1695")
public void whoAmI_regularApi_doesNotProvideSensitiveInformation() throws Exception {
j.jenkins.setSecurityRealm(new SecurityRealmImpl());

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

JenkinsRule.WebClient wc = j.createWebClient();
wc.login("user");

Page whoAmIPage = wc.goTo("whoAmI/api/json", "application/json");
String content = whoAmIPage.getWebResponse().getContentAsString();

String sessionId = wc.executeOnServer(() -> {
HttpSession session = Stapler.getCurrentRequest().getSession(false);
return session != null ? session.getId() : null;
});

assertThat(sessionId, not(nullValue()));

// dangerous stuff in Regular Login mode with the api/json call:
/*
* {
* "_class": "hudson.security.WhoAmI",
* "anonymous": false,
* "authenticated": true,
* "authorities": [],
* "details": "org.acegisecurity.ui.WebAuthenticationDetails@fffc7f0c: RemoteIpAddress: 127.0.0.1; SessionId: node0g4xbfaaq1qb91pwyv0ctilrfu0",
* "name": "user",
* "toString": "org.acegisecurity.providers.UsernamePasswordAuthenticationToken@66074b8a: Username: [toString()=S3cr3t]; Password: [PROTECTED]; Authenticated: true; Details: org.acegisecurity.ui.WebAuthenticationDetails@fffc7f0c: RemoteIpAddress: 127.0.0.1; SessionId: node0g4xbfaaq1qb91pwyv0ctilrfu0; Granted Authorities: "
* }
*/
assertThat(content, not(anyOf(
containsString("S3cr3t"),
containsString("SessionId"),
containsString(sessionId)
)));
}

@Test
@Issue("SECURITY-1697")
public void whoAmI_basic_doesNotProvideSensitiveInformation() throws Exception {
j.jenkins.setSecurityRealm(new SecurityRealmImpl());

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

JenkinsRule.WebClient wc = j.createWebClient().withBasicCredentials("user", "user");

HtmlPage whoAmIPage = wc.goTo("whoAmI");
String content = whoAmIPage.getWebResponse().getContentAsString();

// dangerous stuff in Basic mode:
/*
* <td>toString:</td>
* <td>org.acegisecurity.providers.UsernamePasswordAuthenticationToken@e8fd00a7: Username: [toString()=S3cr3t];
*
* <td rowspan="1">Authorization</td>
* <td>Basic dXNlcjp1c2Vy</td>
*/
assertThat(content, not(anyOf(
containsString("S3cr3t"),
containsString("SessionId"),
// base64 of user:user
containsString(Base64.getEncoder().encodeToString("user:user".getBytes(StandardCharsets.UTF_8)))
)));
}

@Test
@Issue("SECURITY-1697")
public void whoAmI_apiToken_doesNotProvideSensitiveInformation() throws Exception {
j.jenkins.setSecurityRealm(new SecurityRealmImpl());

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

User user = User.getById("user", true);
ApiTokenProperty prop = user.getProperty(ApiTokenProperty.class);
ApiTokenStore.TokenUuidAndPlainValue token = prop.getTokenStore().generateNewToken("test");

JenkinsRule.WebClient wc = j.createWebClient().withBasicCredentials("user", token.plainValue);
String base64ApiToken = new String(Base64.getEncoder().encode(("user:" + token.plainValue).getBytes(StandardCharsets.UTF_8)), StandardCharsets.UTF_8);

HtmlPage whoAmIPage = wc.goTo("whoAmI");
String content = whoAmIPage.getWebResponse().getContentAsString();

// dangerous stuff in API Token mode:
/*
* <td rowspan="1">Authorization</td>
* <td>Basic dXNlcjoxMTRiNGRmMWNhZTVkNDQ2MjgxZTJkZWEzMDY1NTEyZDBi</td>
*/
assertThat(content, not(anyOf(
containsString("S3cr3t"),
containsString("SessionId"),
containsString(base64ApiToken)
)));
}

private class SecurityRealmImpl extends AbstractPasswordBasedSecurityRealm {

@Override
protected UserDetails authenticate(String username, String password) throws AuthenticationException {
return createUserDetails(username);
}

@Override public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
return createUserDetails(username);
}

@Override public GroupDetails loadGroupByGroupname(String groupname) throws UsernameNotFoundException, DataAccessException {
return null;
}

private UserDetails createUserDetails(String username) {
return new UserDetails() {

@Override public String getUsername() {
return username;
}

@Override
public String toString() {
return "[toString()=S3cr3t]";
}

@Override public GrantedAuthority[] getAuthorities() {
return new GrantedAuthority[0];
}

@Override public String getPassword() {
return null;
}

@Override public boolean isAccountNonExpired() {
return true;
}

@Override public boolean isAccountNonLocked() {
return true;
}

@Override public boolean isCredentialsNonExpired() {
return true;
}

@Override public boolean isEnabled() {
return true;
}
};
}
}
}

0 comments on commit 77f36f3

Please sign in to comment.