diff --git a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java index 9087755f93..acc58fed15 100644 --- a/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java +++ b/scm-webapp/src/main/java/sonia/scm/ScmServletModule.java @@ -162,6 +162,7 @@ import sonia.scm.net.ahc.ContentTransformer; import sonia.scm.net.ahc.DefaultAdvancedHttpClient; import sonia.scm.net.ahc.JsonContentTransformer; import sonia.scm.net.ahc.XmlContentTransformer; +import sonia.scm.security.XsrfProtectionFilter; import sonia.scm.web.UserAgentParser; /** @@ -365,6 +366,9 @@ public class ScmServletModule extends ServletModule filter(PATTERN_ALL).through(LoggingFilter.class); } + // protect api agains xsrf attacks + filter(PATTERN_RESTAPI).through(XsrfProtectionFilter.class); + /* * filter(PATTERN_PAGE, * PATTERN_STATIC_RESOURCES).through(StaticResourceFilter.class); diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java index b7f02187cb..36d296e1c5 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AuthenticationResource.java @@ -79,9 +79,11 @@ import sonia.scm.util.HttpUtil; import java.util.Collection; import java.util.Collections; import java.util.List; +import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import javax.ws.rs.DefaultValue; import javax.ws.rs.FormParam; @@ -96,6 +98,7 @@ import javax.ws.rs.core.Response; import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlRootElement; +import sonia.scm.security.XsrfCookies; /** * @@ -261,6 +264,9 @@ public class AuthenticationResource public Response logout(@Context HttpServletRequest request, @Context HttpServletResponse response) { + // remove xsrf token + XsrfCookies.remove(request, response); + Subject subject = SecurityUtils.getSubject(); subject.logout(); diff --git a/scm-webapp/src/main/java/sonia/scm/security/XsrfCookies.java b/scm-webapp/src/main/java/sonia/scm/security/XsrfCookies.java new file mode 100644 index 0000000000..de3c26995a --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/XsrfCookies.java @@ -0,0 +1,87 @@ +/** + * Copyright (c) 2014, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ +package sonia.scm.security; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * Util methods to handle XsrfCookies. + * + * @author Sebastian Sdorra + * @version 1.47 + */ +public final class XsrfCookies +{ + + private XsrfCookies() + { + } + + /** + * Creates a new xsrf protection cookie and add it to the response. + * + * @param request http servlet request + * @param response http servlet response + * @param token xsrf token + */ + public static void create(HttpServletRequest request, HttpServletResponse response, String token){ + applyCookie(request, response, new Cookie(XsrfProtectionFilter.KEY, token)); + + } + + /** + * Removes the current xsrf protection cookie from response. + * + * @param request http servlet request + * @param response http servlet response + */ + public static void remove(HttpServletRequest request, HttpServletResponse response) + { + Cookie[] cookies = request.getCookies(); + if ( cookies != null ){ + for ( Cookie c : cookies ){ + if ( XsrfProtectionFilter.KEY.equals(c.getName()) ){ + c.setMaxAge(0); + c.setValue(null); + applyCookie(request, response, c); + } + } + } + } + + private static void applyCookie(HttpServletRequest request, HttpServletResponse response, Cookie cookie){ + cookie.setPath(request.getContextPath()); + response.addCookie(cookie); + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/XsrfProtectionFilter.java b/scm-webapp/src/main/java/sonia/scm/security/XsrfProtectionFilter.java new file mode 100644 index 0000000000..7e7391190c --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/security/XsrfProtectionFilter.java @@ -0,0 +1,112 @@ +/** + * Copyright (c) 2014, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ +package sonia.scm.security; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import java.io.IOException; +import java.util.UUID; +import javax.inject.Singleton; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.util.HttpUtil; +import sonia.scm.web.filter.HttpFilter; + +/** + * Xsrf protection http filter. The filter will issue an cookie with an xsrf protection token on the first ajax request + * of the scm web interface and marks the http session as xsrf protected. On every other request within a protected + * session, the web interface has to send the token from the cookie as http header on every request. If the filter + * receives an request to a protected session, without proper xsrf header the filter will abort the request and send an + * http error code back to the client. If the filter receives an request to a non protected session, from a non web + * interface client the filter will call the chain. + * + * TODO for scm-manager 2 we have to store the csrf token as part of the jwt token instead of session. + * + * @see https://bitbucket.org/sdorra/scm-manager/issues/793/json-hijacking-vulnerability-cwe-116-cwe + * @author Sebastian Sdorra + * @version 1.47 + */ +@Singleton +public final class XsrfProtectionFilter extends HttpFilter +{ + + /** + * the logger for XsrfProtectionFilter + */ + private static final Logger logger = LoggerFactory.getLogger(XsrfProtectionFilter.class); + + /** + * Key used for session, header and cookie. + */ + static final String KEY = "X-XSRF-Token"; + + @Override + protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws + IOException, ServletException + { + HttpSession session = request.getSession(true); + String storedToken = (String) session.getAttribute(KEY); + if ( ! Strings.isNullOrEmpty(storedToken) ){ + String headerToken = request.getHeader(KEY); + if ( storedToken.equals(headerToken) ){ + logger.trace("received valid xsrf protected request"); + chain.doFilter(request, response); + } else { + // is forbidden the correct status code? + logger.warn("received request to a xsrf protected session without proper xsrf token"); + response.sendError(HttpServletResponse.SC_FORBIDDEN); + } + } else if (HttpUtil.isWUIRequest(request)) { + logger.debug("received wui request, mark session as xsrf protected and issue a new token"); + String token = createToken(); + session.setAttribute(KEY, token); + XsrfCookies.create(request, response, token); + chain.doFilter(request, response); + } else { + // handle non webinterface clients, which does not need xsrf protection + logger.trace("received request to a non xsrf protected session"); + chain.doFilter(request, response); + } + } + + private String createToken() + { + // TODO create interface and use a better method + return UUID.randomUUID().toString(); + } + +} diff --git a/scm-webapp/src/main/webapp/resources/js/sonia.global.js b/scm-webapp/src/main/webapp/resources/js/sonia.global.js index 32dec370a9..4513a05e6e 100644 --- a/scm-webapp/src/main/webapp/resources/js/sonia.global.js +++ b/scm-webapp/src/main/webapp/resources/js/sonia.global.js @@ -37,6 +37,19 @@ Ext.Ajax.defaultHeaders = { 'X-SCM-Client': 'WUI' }; +// XSRF protection +Ext.Ajax.on('beforerequest', function(conn, options){ + var token = Ext.util.Cookies.get('X-XSRF-Token'); + console.log(token); + console.log(options); + if (token){ + if (!options.headers){ + options.headers = {}; + } + options.headers['X-XSRF-Token'] = token; + } +}); + var state = null; var admin = false; diff --git a/scm-webapp/src/test/java/sonia/scm/security/XsrfCookiesTest.java b/scm-webapp/src/test/java/sonia/scm/security/XsrfCookiesTest.java new file mode 100644 index 0000000000..64dd27bc3b --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/XsrfCookiesTest.java @@ -0,0 +1,108 @@ +/** + * Copyright (c) 2014, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + +package sonia.scm.security; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.junit.Test; +import static org.junit.Assert.*; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import static org.mockito.Mockito.*; +import org.mockito.runners.MockitoJUnitRunner; + +/** + * Tests for the {@link XsrfCookies} util class. + * + * @author Sebastian Sdorra + */ +@RunWith(MockitoJUnitRunner.class) +public class XsrfCookiesTest { + + @Mock + private HttpServletRequest request; + + @Mock + private HttpServletResponse response; + + /** + * Prepare mocks for testing. + */ + @Before + public void prepareMocks(){ + when(request.getContextPath()).thenReturn("/scm"); + } + + /** + * Tests create method. + */ + @Test + public void testCreate() + { + XsrfCookies.create(request, response, "mytoken"); + + // capture cookie + ArgumentCaptor captor = ArgumentCaptor.forClass(Cookie.class); + verify(response).addCookie(captor.capture()); + + // check for cookie + Cookie cookie = captor.getValue(); + assertEquals(XsrfProtectionFilter.KEY, cookie.getName()); + assertEquals("/scm", cookie.getPath()); + assertEquals("mytoken", cookie.getValue()); + } + + /** + * Tests remove method. + */ + @Test + public void testRemove(){ + Cookie cookie = new Cookie(XsrfProtectionFilter.KEY, "mytoken"); + cookie.setMaxAge(15); + when(request.getCookies()).thenReturn(new Cookie[]{cookie}); + XsrfCookies.remove(request, response); + + // capture cookie + ArgumentCaptor captor = ArgumentCaptor.forClass(Cookie.class); + verify(response).addCookie(captor.capture()); + + // check the captured cookie + Cookie c = captor.getValue(); + assertEquals("cookie max age should be set to 0", 0, c.getMaxAge()); + assertEquals("cookie path should be equals", cookie.getPath(), c.getPath()); + assertNull("cookie value shuld be null", c.getValue()); + } + +} \ No newline at end of file diff --git a/scm-webapp/src/test/java/sonia/scm/security/XsrfProtectionFilterTest.java b/scm-webapp/src/test/java/sonia/scm/security/XsrfProtectionFilterTest.java new file mode 100644 index 0000000000..2cb187c24d --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/security/XsrfProtectionFilterTest.java @@ -0,0 +1,181 @@ +/** + * Copyright (c) 2014, Sebastian Sdorra + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. Neither the name of SCM-Manager; nor the names of its + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + +package sonia.scm.security; + +import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import org.junit.Test; +import static org.junit.Assert.*; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import static org.mockito.Mockito.*; +import org.mockito.runners.MockitoJUnitRunner; +import sonia.scm.util.HttpUtil; + +/** + * Unit tests for {@link XsrfProtectionFilter}. + * + * @author Sebastian Sdorra + */ +@RunWith(MockitoJUnitRunner.class) +public class XsrfProtectionFilterTest { + + @Mock + private HttpServletRequest request; + + @Mock + private HttpServletResponse response; + + @Mock + private HttpSession session; + + @Mock + private FilterChain chain; + + private final XsrfProtectionFilter filter = new XsrfProtectionFilter(); + + /** + * Prepare mocks for testing. + */ + @Before + public void setUp(){ + when(request.getSession(true)).thenReturn(session); + when(request.getContextPath()).thenReturn("/scm"); + } + + /** + * Test filter method for non web interface clients. + * + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoFilterFromNonWuiClient() throws IOException, ServletException + { + filter.doFilter(request, response, chain); + verify(chain).doFilter(request, response); + } + + /** + * Test filter method for first web interface request. + * + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoFilterIssuesTokenOnFirstWuiRequest() throws IOException, ServletException + { + when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); + + // call the filter + filter.doFilter(request, response, chain); + + // capture cookie + ArgumentCaptor captor = ArgumentCaptor.forClass(Cookie.class); + verify(response).addCookie(captor.capture()); + + // check for cookie + Cookie cookie = captor.getValue(); + assertEquals(XsrfProtectionFilter.KEY, cookie.getName()); + assertEquals("/scm", cookie.getPath()); + assertNotNull(cookie.getValue()); + + // ensure filter chain is called + verify(chain).doFilter(request, response); + } + + /** + * Test filter method on protected session with an invalid xsrf token. + * + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoFilterWithInvalidToken() throws IOException, ServletException { + when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); + when(request.getHeader(XsrfProtectionFilter.KEY)).thenReturn("invalidtoken"); + when(session.getAttribute(XsrfProtectionFilter.KEY)).thenReturn("mytoken"); + + // call the filter + filter.doFilter(request, response, chain); + + // ensure response send forbidden and the chain was never called + verify(response).sendError(HttpServletResponse.SC_FORBIDDEN); + verify(chain, never()).doFilter(request, response); + } + + /** + * Test filter method on protected session without xsrf token. + * + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoFilterOnProtectedSessionWithoutToken() throws IOException, ServletException { + when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); + when(session.getAttribute(XsrfProtectionFilter.KEY)).thenReturn("mytoken"); + + // call the filter + filter.doFilter(request, response, chain); + + // ensure response send forbidden and the chain was never called + verify(response).sendError(HttpServletResponse.SC_FORBIDDEN); + verify(chain, never()).doFilter(request, response); + } + + /** + * Test filter method on protected session with valid xsrf token. + * + * @throws IOException + * @throws ServletException + */ + @Test + public void testDoFilterOnProtectedSessionWithValidToken() throws IOException, ServletException { + when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); + when(request.getHeader(XsrfProtectionFilter.KEY)).thenReturn("mytoken"); + when(session.getAttribute(XsrfProtectionFilter.KEY)).thenReturn("mytoken"); + + // call the filter + filter.doFilter(request, response, chain); + + // ensure chain was called + verify(chain).doFilter(request, response); + } + +} \ No newline at end of file