diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 00d453f794..8731504eb8 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -131,6 +131,7 @@ import org.apache.tools.zip.ZipFile; import org.jenkinsci.remoting.RoleChecker; import org.jenkinsci.remoting.RoleSensitive; import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.Stapler; diff --git a/core/src/main/java/hudson/scheduler/CronTab.java b/core/src/main/java/hudson/scheduler/CronTab.java index 14c67b9034..120b8bc2a1 100644 --- a/core/src/main/java/hudson/scheduler/CronTab.java +++ b/core/src/main/java/hudson/scheduler/CronTab.java @@ -204,7 +204,7 @@ public final class CronTab { } void setTo(Calendar c, int i) { - c.set(field,i-offset); + c.set(field,Math.min(i-offset, c.getActualMaximum(field))); } void clear(Calendar c) { diff --git a/core/src/main/java/hudson/security/BasicAuthenticationFilter.java b/core/src/main/java/hudson/security/BasicAuthenticationFilter.java index 1ec0be214f..ae186b8dd3 100644 --- a/core/src/main/java/hudson/security/BasicAuthenticationFilter.java +++ b/core/src/main/java/hudson/security/BasicAuthenticationFilter.java @@ -29,6 +29,7 @@ import hudson.util.Scrambler; import jenkins.security.ApiTokenProperty; import jenkins.security.SecurityListener; import org.acegisecurity.Authentication; +import jenkins.security.BasicApiTokenHelper; import org.acegisecurity.context.SecurityContextHolder; import org.acegisecurity.userdetails.UserDetails; @@ -135,12 +136,9 @@ public class BasicAuthenticationFilter implements Filter { 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){ UserDetails userDetails = u.getUserDetailsForImpersonation(); Authentication auth = u.impersonate(userDetails); diff --git a/core/src/main/java/hudson/security/SecurityRealm.java b/core/src/main/java/hudson/security/SecurityRealm.java index d2d637911a..5a8325b242 100644 --- a/core/src/main/java/hudson/security/SecurityRealm.java +++ b/core/src/main/java/hudson/security/SecurityRealm.java @@ -304,6 +304,9 @@ public abstract class SecurityRealm extends AbstractDescribableImpl0 ? req.getContextPath() : "/"); rsp.addCookie(cookie); diff --git a/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java b/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java index 9b81ae5b19..ff0f374ca4 100644 --- a/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java +++ b/core/src/main/java/hudson/security/TokenBasedRememberMeServices2.java @@ -123,19 +123,39 @@ public class TokenBasedRememberMeServices2 extends TokenBasedRememberMeServices @Override public Authentication autoLogin(HttpServletRequest request, HttpServletResponse response) { - try { - return super.autoLogin(request, response); - } catch (Exception e) { - cancelCookie(request, response, "Failed to handle remember-me cookie: "+Functions.printThrowable(e)); + if(Jenkins.getInstance().isDisableRememberMe()){ + cancelCookie(request, response, null); return null; + }else { + try { + return super.autoLogin(request, response); + } catch (Exception e) { + cancelCookie(request, response, "Failed to handle remember-me cookie: " + Functions.printThrowable(e)); + return null; + } } } - @Override - protected Cookie makeValidCookie(String tokenValueBase64, HttpServletRequest request, long maxAge) { - Cookie cookie = super.makeValidCookie(tokenValueBase64, request, maxAge); + @Override + protected Cookie makeValidCookie(String tokenValueBase64, HttpServletRequest request, long maxAge) { + Cookie cookie = super.makeValidCookie(tokenValueBase64, request, maxAge); + secureCookie(cookie, request); + return cookie; + } + + @Override + protected Cookie makeCancelCookie(HttpServletRequest request) { + Cookie cookie = super.makeCancelCookie(request); + secureCookie(cookie, request); + return cookie; + } + + /** + * Force always the http-only flag and depending on the request, put also the secure flag. + */ + private void secureCookie(Cookie cookie, HttpServletRequest request){ // if we can mark the cookie HTTP only, do so to protect this cookie even in case of XSS vulnerability. - if (SET_HTTP_ONLY!=null) { + if (SET_HTTP_ONLY!=null) { try { SET_HTTP_ONLY.invoke(cookie,true); } catch (IllegalAccessException e) { @@ -148,12 +168,10 @@ public class TokenBasedRememberMeServices2 extends TokenBasedRememberMeServices // if the user is running Jenkins over HTTPS, we also want to prevent the cookie from leaking in HTTP. // whether the login is done over HTTPS or not would be a good enough approximation of whether Jenkins runs in // HTTPS or not, so use that. - if (request.isSecure()) - cookie.setSecure(true); - return cookie; - } + cookie.setSecure(request.isSecure()); + } - /** + /** * Used to compute the token signature securely. */ private static final HMACConfidentialKey MAC = new HMACConfidentialKey(TokenBasedRememberMeServices.class,"mac"); diff --git a/core/src/main/java/jenkins/security/ApiTokenProperty.java b/core/src/main/java/jenkins/security/ApiTokenProperty.java index 3d828cc5e1..3ffab72616 100644 --- a/core/src/main/java/jenkins/security/ApiTokenProperty.java +++ b/core/src/main/java/jenkins/security/ApiTokenProperty.java @@ -567,5 +567,6 @@ public class ApiTokenProperty extends UserProperty { * 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) */ @Deprecated - 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); } diff --git a/core/src/main/java/jenkins/security/BasicApiTokenHelper.java b/core/src/main/java/jenkins/security/BasicApiTokenHelper.java new file mode 100644 index 0000000000..bafece0e57 --- /dev/null +++ b/core/src/main/java/jenkins/security/BasicApiTokenHelper.java @@ -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 clazz; + try { + clazz = (Class) 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; + } +} diff --git a/core/src/test/java/hudson/scheduler/CronTabTest.java b/core/src/test/java/hudson/scheduler/CronTabTest.java index e2acaf53ba..cae0fdc7ed 100644 --- a/core/src/test/java/hudson/scheduler/CronTabTest.java +++ b/core/src/test/java/hudson/scheduler/CronTabTest.java @@ -314,4 +314,11 @@ public class CronTabTest { assertEquals("[35, 56]", times.toString()); } + @Issue("SECURITY-790") + @Test(timeout = 1000L) public void testLongMonths() throws Exception { + Calendar cal = Calendar.getInstance(); + cal.set(Calendar.MONTH, Calendar.JULY); + new CronTab("0 0 31 7 *").floor(cal); // would infinite loop + } + } diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test_SEC996.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test_SEC996.java new file mode 100644 index 0000000000..b2fa21ce6c --- /dev/null +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test_SEC996.java @@ -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 hudson.security; + +import com.gargoylesoftware.htmlunit.util.Cookie; +import com.gargoylesoftware.htmlunit.xml.XmlPage; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.text.IsEmptyString.isEmptyString; +import static org.hamcrest.xml.HasXPath.hasXPath; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; + +public class TokenBasedRememberMeServices2Test_SEC996 { + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + @Issue("SECURITY-996") + public void rememberMeToken_shouldNotBeRead_ifOptionIsDisabled() throws Exception { + j.jenkins.setDisableRememberMe(false); + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + + Cookie rememberMeCookie = null; + { + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login("alice", "alice", true); + + + // we should see a remember me cookie + rememberMeCookie = getRememberMeCookie(wc); + assertNotNull(rememberMeCookie); + assertThat(rememberMeCookie.getValue(), not(isEmptyString())); + } + + j.jenkins.setDisableRememberMe(true); + { + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.getCookieManager().addCookie(rememberMeCookie); + + // the application should not use the cookie to connect + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", not(is("alice")))); + } + + j.jenkins.setDisableRememberMe(false); + { + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.getCookieManager().addCookie(rememberMeCookie); + + // if we reactivate the remember me feature, it's ok + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", is("alice"))); + } + } + + private Cookie getRememberMeCookie(JenkinsRule.WebClient wc) { + return wc.getCookieManager().getCookie(TokenBasedRememberMeServices2.ACEGI_SECURITY_HASHED_REMEMBER_ME_COOKIE_KEY); + } +} diff --git a/test/src/test/java/jenkins/security/BasicHeaderApiTokenAuthenticatorTest_SEC986.java b/test/src/test/java/jenkins/security/BasicHeaderApiTokenAuthenticatorTest_SEC986.java new file mode 100644 index 0000000000..f72c407b70 --- /dev/null +++ b/test/src/test/java/jenkins/security/BasicHeaderApiTokenAuthenticatorTest_SEC986.java @@ -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 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 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 boolean allows(Class type, ExtensionComponent 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)); + } +}