Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn on CSRF protection. #90

Merged
merged 6 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ docker run --name=idp -p 8090:8080 -e SIMPLESAMLPHP_SP_ENTITY_ID=https://sp.pass
```
Note the volume mount which is set the user information appropriately for PASS.

# CSRF protection

Requests which have side effects (not a GET, HEAD, or OPTIONS and any request to /doi) are protected from CSRF through the use of a token. The client must provide a cookie XSRF-TOKEN and set a header X-XSRF-TOKEN to the same value. Clients can use any value they want. Browser clients will have the cookie value set by responses and so must first make a non-protected request.

# App service

The PASS application is available at `/app/` and `/` is redirected to `/app/`. Requests are resolved against the location given by the environment variable `PASS_CORE_APP_LOCATION`. If a request cannot be resolved, then `/app/index.html` will be returned. This allows the user interface to handle paths which may not resolve to files.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.eclipse.pass.main.security;

import java.io.IOException;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.web.csrf.CsrfToken;
import org.springframework.web.filter.OncePerRequestFilter;

/**
* Ensure CSRF token cookie is added. See
* https://docs.spring.io/spring-security/reference/servlet/exploits/csrf.html#csrf-integration-javascript-spa
*/
public class CsrfCookieFilter extends OncePerRequestFilter {
@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {
CsrfToken csrfToken = (CsrfToken) request.getAttribute("_csrf");
// Render the token value to a cookie by causing the deferred token to be loaded
csrfToken.getToken();

filterChain.doFilter(request, response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configurers.AnonymousConfigurer;
import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer;
import org.springframework.security.config.annotation.web.configurers.FormLoginConfigurer;
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
import org.springframework.security.saml2.provider.service.web.DefaultRelyingPartyRegistrationResolver;
Expand All @@ -37,10 +36,13 @@
import org.springframework.security.saml2.provider.service.web.authentication.Saml2WebSsoAuthenticationFilter;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.logout.CookieClearingLogoutHandler;
import org.springframework.security.web.csrf.CookieCsrfTokenRepository;
import org.springframework.security.web.csrf.CsrfFilter;
import org.springframework.security.web.header.writers.ContentSecurityPolicyHeaderWriter;
import org.springframework.security.web.header.writers.DelegatingRequestMatcherHeaderWriter;
import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.OrRequestMatcher;


/**
Expand Down Expand Up @@ -77,10 +79,20 @@ public class SecurityConfiguration {
@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// Disable unused functionality
http.csrf(CsrfConfigurer::disable);
http.formLogin(FormLoginConfigurer::disable);
http.anonymous(AnonymousConfigurer::disable);

// Enable CSRF protection using a cookie to send the token. See
// https://docs.spring.io/spring-security/reference/servlet/exploits/csrf.html#csrf-integration-javascript-spa
// Make sure the cookie value can be parsed when returned in a header
// Do not protect /logout so it can be triggered with GET
markpatton marked this conversation as resolved.
Show resolved Hide resolved
// Ensure that GET requests to the doi service are protected since they have side effects
http.csrf(csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
markpatton marked this conversation as resolved.
Show resolved Hide resolved
.ignoringRequestMatchers("/logout")
.requireCsrfProtectionMatcher(new OrRequestMatcher(CsrfFilter.DEFAULT_CSRF_MATCHER,
new AntPathRequestMatcher("/doi/**")))
.csrfTokenRequestHandler(new SpaCsrfTokenRequestHandler()));

// Set Content Security Policy header only for /app/
ContentSecurityPolicyHeaderWriter cspHeaderWriter = new ContentSecurityPolicyHeaderWriter();
cspHeaderWriter.setPolicyDirectives(contentSecurityPolicy);
Expand Down Expand Up @@ -111,6 +123,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
loginProcessingUrl(loginProcessingPath));

http.saml2Metadata(Customizer.withDefaults());

http.saml2Logout(Customizer.withDefaults());

// Delete specified cookies on logout.
Expand All @@ -124,12 +137,17 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
return c;
}).toArray(Cookie[]::new);

// Allow GET on /logout
CookieClearingLogoutHandler logoutHandler = new CookieClearingLogoutHandler(cookies);
http.logout(l -> l.logoutSuccessUrl(logoutSuccessUrl).addLogoutHandler(logoutHandler));
http.logout(l -> l.logoutSuccessUrl(logoutSuccessUrl).
logoutRequestMatcher(new AntPathRequestMatcher("/logout")).addLogoutHandler(logoutHandler));

// Map SAML user to PASS user
http.addFilterAfter(passAuthFilter, Saml2WebSsoAuthenticationFilter.class);

// Ensure that CSRF cookie is set
http.addFilterAfter(new CsrfCookieFilter(), Saml2WebSsoAuthenticationFilter.class);

return http.build();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package org.eclipse.pass.main.security;

import java.util.function.Supplier;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.web.csrf.CsrfToken;
import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler;
import org.springframework.security.web.csrf.CsrfTokenRequestHandler;
import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler;
import org.springframework.util.StringUtils;

/**
* Handle case of single page application which puts CSRF token cookie value into header. See
* https://docs.spring.io/spring-security/reference/servlet/exploits/csrf.html#csrf-integration-javascript-spa
*/
public class SpaCsrfTokenRequestHandler extends CsrfTokenRequestAttributeHandler {
private final CsrfTokenRequestHandler delegate = new XorCsrfTokenRequestAttributeHandler();

@Override
public void handle(HttpServletRequest request, HttpServletResponse response, Supplier<CsrfToken> csrfToken) {
/*
* Always use XorCsrfTokenRequestAttributeHandler to provide BREACH protection of
* the CsrfToken when it is rendered in the response body.
*/
this.delegate.handle(request, response, csrfToken);
}

@Override
public String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfToken) {
/*
* If the request contains a request header, use CsrfTokenRequestAttributeHandler
* to resolve the CsrfToken. This applies when a single-page application includes
* the header value automatically, which was obtained via a cookie containing the
* raw CsrfToken.
*/
String token = request.getHeader(csrfToken.getHeaderName());

if (StringUtils.hasText(token)) {
return super.resolveCsrfTokenValue(request, csrfToken);
}

/*
* In all other cases (e.g. if the request contains a request parameter), use
* XorCsrfTokenRequestAttributeHandler to resolve the CsrfToken. This applies
* when a server-side rendered form includes the _csrf request parameter as a
* hidden input.
*/
return this.delegate.resolveCsrfTokenValue(request, csrfToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import org.eclipse.pass.main.IntegrationTest;
import org.eclipse.pass.main.SimpleIntegrationTest;
import org.eclipse.pass.object.PassClient;
import org.eclipse.pass.object.PassClientResult;
import org.eclipse.pass.object.PassClientSelector;
import org.eclipse.pass.object.RSQL;
import org.eclipse.pass.object.model.Journal;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

public class DoiServiceTest extends IntegrationTest {
public class DoiServiceTest extends SimpleIntegrationTest {
private static String credentials = Credentials.basic(BACKEND_USER, BACKEND_PASSWORD);

@Autowired
protected RefreshableElide refreshableElide;
Expand All @@ -34,8 +36,12 @@ protected PassClient getNewClient() {
return PassClient.newInstance(refreshableElide);
}

private OkHttpClient httpClient = new OkHttpClient();
private String credentials = Credentials.basic(BACKEND_USER, BACKEND_PASSWORD);
private OkHttpClient httpClient;

@BeforeEach
protected void setupClient() throws IOException {
httpClient = newOkhttpClient();
}

/**
* throw in a "moo" doi, expect a 400 error
Expand All @@ -48,13 +54,15 @@ public void invalidDoiTest() throws Exception {

Request okHttpRequest = new Request.Builder()
.url(url).header("Authorization", credentials)
.header("X-XSRF-TOKEN", getCsrfToken(httpClient))
.build();
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(400, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Supplied DOI is not in valid DOI format.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -70,13 +78,15 @@ public void nullDoiTest() throws Exception {

Request okHttpRequest = new Request.Builder()
.url(url).header("Authorization", credentials)
.header("X-XSRF-TOKEN", getCsrfToken(httpClient))
.build();
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(400, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Supplied DOI is not in valid DOI format.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -93,13 +103,15 @@ public void noSuchDoiTest() throws Exception {

Request okHttpRequest = new Request.Builder()
.url(url).header("Authorization", credentials)
.header("X-XSRF-TOKEN", getCsrfToken(httpClient))
.build();
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(404, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"The resource for DOI 10.1212/abc.DEF could not be found on Crossref.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -119,13 +131,15 @@ public void bookDoiFailTest() throws Exception {

Request okHttpRequest = new Request.Builder()
.url(url).header("Authorization", credentials)
.header("X-XSRF-TOKEN", getCsrfToken(httpClient))
.build();
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(422, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Insufficient information to locate or specify a journal entry.\"}",
okHttpResponse.body().string());
body);
}
}

Expand Down Expand Up @@ -156,13 +170,15 @@ public void realJournalTest() {

Request okHttpRequest = new Request.Builder()
.url(url).header("Authorization", credentials)
.header("X-XSRF-TOKEN", getCsrfToken(httpClient))
.build();

Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(200, okHttpResponse.code());
assert okHttpResponse.body() != null;
JsonReader jsonReader1 = Json.createReader(new StringReader(okHttpResponse.body().string()));
String body = okHttpResponse.body().string();
assert body != null;
JsonReader jsonReader1 = Json.createReader(new StringReader(body));
JsonObject successReport = jsonReader1.readObject();
jsonReader1.close();
assertNotNull(successReport.getString("journal-id"));
Expand All @@ -180,8 +196,9 @@ public void realJournalTest() {
//verify that the returned object for the same request has the right id
call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assert okHttpResponse.body() != null;
JsonReader jsonReader2 = Json.createReader(new StringReader(okHttpResponse.body().string()));
String body = okHttpResponse.body().string();
assert body != null;
JsonReader jsonReader2 = Json.createReader(new StringReader(body));
JsonObject successReport = jsonReader2.readObject();
jsonReader2.close();
assertEquals(id, successReport.getString("journal-id"));
Expand Down
Loading
Loading