Skip to content

Commit

Permalink
[SECURITY-896]
Browse files Browse the repository at this point in the history
  • Loading branch information
Wadeck authored and daniel-beck committed Jul 31, 2018
1 parent ef9583a commit 832144e
Show file tree
Hide file tree
Showing 5 changed files with 308 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import hudson.model.User;
import jenkins.model.Jenkins;
import hudson.util.Scrambler;
import jenkins.security.ApiTokenProperty;
import jenkins.security.BasicApiTokenHelper;
import org.acegisecurity.context.SecurityContextHolder;

import javax.servlet.Filter;
Expand Down Expand Up @@ -132,12 +132,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
return;
}

{// attempt to authenticate as API token
// create is true as the user may not have been saved and the default api token may be in use.
// validation of the user will be performed against the underlying realm in impersonate.
User u = User.getById(username, true);
ApiTokenProperty t = u.getProperty(ApiTokenProperty.class);
if (t!=null && t.matchesPassword(password)) {
{
User u = BasicApiTokenHelper.isConnectingUsingApiToken(username, password);
if(u != null){
SecurityContextHolder.getContext().setAuthentication(u.impersonate());
try {
chain.doFilter(request,response);
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/jenkins/security/ApiTokenProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,6 @@ public HttpResponse doChangeToken(@AncestorInPath User u, StaplerResponse rsp) t
/**
* We don't want an API key that's too long, so cut the length to 16 (which produces 32-letter MAC code in hexdump)
*/
private static final HMACConfidentialKey API_KEY_SEED = new HMACConfidentialKey(ApiTokenProperty.class,"seed",16);
@Restricted(NoExternalUse.class)
public static final HMACConfidentialKey API_KEY_SEED = new HMACConfidentialKey(ApiTokenProperty.class,"seed",16);
}
88 changes: 88 additions & 0 deletions core/src/main/java/jenkins/security/BasicApiTokenHelper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* The MIT License
*
* Copyright (c) 2018, 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 jenkins.security;

import hudson.Util;
import hudson.model.User;
import jenkins.model.GlobalConfiguration;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import javax.annotation.CheckForNull;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;

@Restricted(NoExternalUse.class)
public class BasicApiTokenHelper {
public static @CheckForNull User isConnectingUsingApiToken(String username, String tokenValue){
User user = User.getById(username, false);
if(user == null){
if(isLegacyTokenGeneratedOnUserCreation()){
String generatedTokenOnCreation = Util.getDigestOf(ApiTokenProperty.API_KEY_SEED.mac(username));
boolean areTokenEqual = MessageDigest.isEqual(
generatedTokenOnCreation.getBytes(StandardCharsets.US_ASCII),
tokenValue.getBytes(StandardCharsets.US_ASCII)
);
if(areTokenEqual){
// directly return the user freshly created
// and no need to check its token as the generated token
// will be the same as the one we checked just above
return User.getById(username, true);
}
}
}else{
ApiTokenProperty t = user.getProperty(ApiTokenProperty.class);
if (t!=null && t.matchesPassword(tokenValue)) {
return user;
}
}

return null;
}

//TODO could be simplified when the patch is merged in 2.129+
@SuppressWarnings("unchecked")
static boolean isLegacyTokenGeneratedOnUserCreation(){
Class<GlobalConfiguration> clazz;
try {
clazz = (Class<GlobalConfiguration>) BasicApiTokenHelper.class.getClassLoader().loadClass("jenkins.security.apitoken.ApiTokenPropertyConfiguration");
} catch (ClassNotFoundException e) {
// case the new api token system is not in place
return true;
}

Object apiTokenConfiguration = GlobalConfiguration.all().get(clazz);
boolean tokenGenerationOnCreationEnabled;
try {
Method isTokenGenerationOnCreationEnabled = clazz.getDeclaredMethod("isTokenGenerationOnCreationEnabled");
tokenGenerationOnCreationEnabled = (boolean) isTokenGenerationOnCreationEnabled.invoke(apiTokenConfiguration);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
return false;
}

return tokenGenerationOnCreationEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,20 @@
public class BasicHeaderApiTokenAuthenticator extends BasicHeaderAuthenticator {
@Override
public Authentication authenticate(HttpServletRequest req, HttpServletResponse rsp, String username, String password) throws ServletException {
// attempt to authenticate as API token
User u = User.getById(username, true);
ApiTokenProperty t = u.getProperty(ApiTokenProperty.class);
if (t!=null && t.matchesPassword(password)) {
try {
return u.impersonate();
} catch (UsernameNotFoundException x) {
// The token was valid, but the impersonation failed. This token is clearly not his real password,
// so there's no point in continuing the request processing. Report this error and abort.
LOGGER.log(WARNING, "API token matched for user "+username+" but the impersonation failed",x);
throw new ServletException(x);
} catch (DataAccessException x) {
throw new ServletException(x);
User u = BasicApiTokenHelper.isConnectingUsingApiToken(username, password);
if(u != null) {
ApiTokenProperty t = u.getProperty(ApiTokenProperty.class);
if (t != null && t.matchesPassword(password)) {
try {
return u.impersonate();
} catch (UsernameNotFoundException x) {
// The token was valid, but the impersonation failed. This token is clearly not his real password,
// so there's no point in continuing the request processing. Report this error and abort.
LOGGER.log(WARNING, "API token matched for user " + username + " but the impersonation failed", x);
throw new ServletException(x);
} catch (DataAccessException x) {
throw new ServletException(x);
}
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/*
* The MIT License
*
* Copyright (c) 2018, 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 jenkins.security;

import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
import com.gargoylesoftware.htmlunit.xml.XmlPage;
import hudson.ExtensionComponent;
import hudson.model.User;
import jenkins.ExtensionFilter;
import jenkins.model.Jenkins;
import org.junit.Assume;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.RestartableJenkinsRule;
import org.jvnet.hudson.test.TestExtension;

import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.concurrent.atomic.AtomicReference;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.xml.HasXPath.hasXPath;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;

public class BasicHeaderApiTokenAuthenticatorTest_SEC986 {
@Rule
public RestartableJenkinsRule rr = new RestartableJenkinsRule();

@Test
public void legacyToken_regularCase() {
AtomicReference<String> token = new AtomicReference<>();
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
assumeLegacyTokenCreation();

configureSecurity();

{
JenkinsRule.WebClient wc = rr.j.createWebClient();
// default SecurityListener will save the user when adding the LastGrantedAuthoritiesProperty
// and so the user is persisted
wc.login("user1");
HtmlPage page = wc.goTo("user/user1/configure");
String tokenValue = ((HtmlTextInput) page.getDocumentElement().querySelector("#apiToken")).getText();
token.set(tokenValue);
}
}
});
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
User user = User.getById("user1", false);
assertNotNull(user);

JenkinsRule.WebClient wc = rr.j.createWebClient();
wc.getOptions().setThrowExceptionOnFailingStatusCode(false);

{ // for invalid token, no effect
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user1", "invalid-token"));
assertThat(wc.getPage(request).getWebResponse().getStatusCode(), equalTo(401));
}
{ // for invalid user, no effect
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user-not-valid", token.get()));
assertThat(wc.getPage(request).getWebResponse().getStatusCode(), equalTo(401));
}

assertNull(User.getById("user-not-valid", false));

{ // valid user with valid token, ok
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user1", token.get()));
XmlPage xmlPage = wc.getPage(request);
assertThat(xmlPage, hasXPath("//name", is("user1")));
}
}
});
}

/*
* The user is not saved after login without the default SecurityListener#fireAuthenticated
*/
@Test
public void legacyToken_withoutLastGrantedAuthorities() {
AtomicReference<String> token = new AtomicReference<>();
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
assumeLegacyTokenCreation();

configureSecurity();

{
JenkinsRule.WebClient wc = rr.j.createWebClient();
wc.login("user1");
HtmlPage page = wc.goTo("user/user1/configure");
String tokenValue = ((HtmlTextInput) page.getDocumentElement().querySelector("#apiToken")).getText();
token.set(tokenValue);
}
}
});
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
User user = User.getById("user1", false);
assertNull(user);

JenkinsRule.WebClient wc = rr.j.createWebClient();
wc.getOptions().setThrowExceptionOnFailingStatusCode(false);

{ // for invalid token, no effect
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user1", "invalid-token"));
assertThat(wc.getPage(request).getWebResponse().getStatusCode(), equalTo(401));
}
{ // for invalid user, no effect
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user-not-valid", token.get()));
assertThat(wc.getPage(request).getWebResponse().getStatusCode(), equalTo(401));
}

assertNull(User.getById("user1", false));
assertNull(User.getById("user-not-valid", false));

{ // valid user with valid token, ok
WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml"));
request.setAdditionalHeader("Authorization", base64("user1", token.get()));
XmlPage xmlPage = wc.getPage(request);
assertThat(xmlPage, hasXPath("//name", is("user1")));
}
}
});
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
User user = User.getById("user1", false);
assertNull(user);
}
});
}

@TestExtension("legacyToken_withoutLastGrantedAuthorities")
public static class RemoveDefaultSecurityListener extends ExtensionFilter {
@Override
public <T> boolean allows(Class<T> type, ExtensionComponent<T> component) {
return !SecurityListener.class.isAssignableFrom(type);
}
}

private void configureSecurity() throws Exception {
rr.j.jenkins.setSecurityRealm(rr.j.createDummySecurityRealm());
rr.j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
.grant(Jenkins.ADMINISTER).everywhere().toEveryone());
rr.j.jenkins.save();
}

@SuppressWarnings("unchecked")
private void assumeLegacyTokenCreation() {
Assume.assumeTrue(
"New API token system present AND legacy token is not generated on user creation",
BasicApiTokenHelper.isLegacyTokenGeneratedOnUserCreation()
);
}

private String base64(String login, String password) {
return "Basic " + Base64.getEncoder().encodeToString((login + ":" + password).getBytes(StandardCharsets.UTF_8));
}
}

0 comments on commit 832144e

Please sign in to comment.