Skip to content

Commit

Permalink
[SECURITY-996]
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 e504691 commit ef9583a
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 13 deletions.
3 changes: 3 additions & 0 deletions core/src/main/java/hudson/security/SecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ public void doLogout(StaplerRequest req, StaplerResponse rsp) throws IOException

// reset remember-me cookie
Cookie cookie = new Cookie(ACEGI_SECURITY_HASHED_REMEMBER_ME_COOKIE_KEY,"");
cookie.setMaxAge(0);
cookie.setSecure(req.isSecure());
cookie.setHttpOnly(true);
cookie.setPath(req.getContextPath().length()>0 ? req.getContextPath() : "/");
rsp.addCookie(cookie);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,39 @@ public void loginSuccess(HttpServletRequest request, HttpServletResponse respons

@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) {
Expand All @@ -148,12 +168,10 @@ protected Cookie makeValidCookie(String tokenValueBase64, HttpServletRequest req
// 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");
Expand Down
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 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);
}
}

0 comments on commit ef9583a

Please sign in to comment.