diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a3f177315b16c99ece09428b31306ba88161287..f81160d115fbb75fba9c3807c4a35e93c2f6af6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,49 @@ # Changelog +## 2.4.1 +This release fixes a security vulnerability caused by the CORS implementation. +Origins allowed for CORS can now be set in the configuration via +`security.cors.origins`. (Reported by Rainer Rillke at Wikimedia) + +Additionally, authentication via disabled services is now entirely blocked to +fix a security vulnerability allowing guest access despite the setting +`security.guest.enabled=false`. (Reported by Rainer Rillke at Wikimedia) + +Additional changes: +* Libraries have been upgraded to fix potential bugs + +## 2.3.3 +This release fixes a security vulnerability caused by the CORS implementation. +Origins allowed for CORS can now be set in the configuration via +`security.cors.origins`. (Reported by Rainer Rillke at Wikimedia) + +Additional changes: +* Libraries have been upgraded to fix potential bugs + +## 2.2.2 +This release fixes a security vulnerability caused by the CORS implementation. +Origins allowed for CORS can now be set in the configuration via +`security.cors.origins`. (Reported by Rainer Rillke at Wikimedia) + +Additional changes: +* Libraries have been upgraded to fix potential bugs + +## 2.1.2 +This release fixes a security vulnerability caused by the CORS implementation. +Support for cross-origin requests has been removed. Use ARSnova version 2.2 or +newer for proper CORS. (Reported by Rainer Rillke at Wikimedia) + +Additional changes: +* Libraries have been upgraded to fix potential bugs + +## 2.0.4 +This release fixes a security vulnerability caused by the CORS implementation. +Support for cross-origin requests has been removed. Use ARSnova version 2.2 or +newer for proper CORS. (Reported by Rainer Rillke at Wikimedia) + +Additional changes: +* Libraries have been upgraded to fix potential bugs + ## 2.4 Major features: * Support for new use case and feature settings has been added. diff --git a/pom.xml b/pom.xml index 8ff30ad6302bd23a751c26e2cbc92f29b58c223c..9aeffcae55a972345cc7c1cf0b1dfb0dae1ea73c 100644 --- a/pom.xml +++ b/pom.xml @@ -80,7 +80,7 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-javadoc-plugin</artifactId> - <version>2.10.3</version> + <version>2.10.4</version> <configuration></configuration> </plugin> </plugins> @@ -177,7 +177,7 @@ <dependency> <groupId>cglib</groupId> <artifactId>cglib</artifactId> - <version>3.2.2</version> + <version>3.2.4</version> </dependency> <dependency> <groupId>org.slf4j</groupId> @@ -325,7 +325,7 @@ <plugin> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-maven-plugin</artifactId> - <version>9.2.16.v20160414</version> + <version>9.2.17.v20160517</version> <configuration> <scanIntervalSeconds>1</scanIntervalSeconds> <webApp> @@ -365,7 +365,7 @@ <plugin> <groupId>org.jacoco</groupId> <artifactId>jacoco-maven-plugin</artifactId> - <version>0.7.6.201602180812</version> + <version>0.7.7.201606060606</version> <executions> <execution> <id>default-prepare-agent</id> diff --git a/src/main/java/de/thm/arsnova/config/ExtraConfig.java b/src/main/java/de/thm/arsnova/config/ExtraConfig.java index ff713d4e95c408fe4c36882c9ca7b11fc363146b..e37d28ed49a21e562cda22c90a96f6776a0cf44a 100644 --- a/src/main/java/de/thm/arsnova/config/ExtraConfig.java +++ b/src/main/java/de/thm/arsnova/config/ExtraConfig.java @@ -18,6 +18,7 @@ package de.thm.arsnova.config; import de.thm.arsnova.ImageUtils; +import de.thm.arsnova.web.CorsFilter; import de.thm.arsnova.connector.client.ConnectorClient; import de.thm.arsnova.connector.client.ConnectorClientImpl; import de.thm.arsnova.socket.ARSnovaSocket; @@ -40,6 +41,9 @@ import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import java.util.Arrays; +import java.util.Collections; + /** * Loads property file and configures non-security related beans and components. */ @@ -61,6 +65,7 @@ public class ExtraConfig extends WebMvcConfigurerAdapter { @Value(value = "${security.ssl}") private boolean socketUseSll; @Value(value = "${security.keystore}") private String socketKeystore; @Value(value = "${security.storepass}") private String socketStorepass; + @Value(value = "${security.cors.origins:}") private String[] corsOrigins; private static int testPortOffset = 0; @@ -84,6 +89,11 @@ public class ExtraConfig extends WebMvcConfigurerAdapter { return propertiesFactoryBean; } + @Bean + public CorsFilter corsFilter() { + return new CorsFilter(Arrays.asList(corsOrigins)); + } + @Bean(name = "connectorClient") public ConnectorClient connectorClient() { if (!connectorEnable) { diff --git a/src/main/java/de/thm/arsnova/controller/ConfigurationController.java b/src/main/java/de/thm/arsnova/controller/ConfigurationController.java index 5981304dda8e489f5d715c3bc28e947c57774b46..0783751bbb9e83bfb69e12cbdf664b1af37e1659 100644 --- a/src/main/java/de/thm/arsnova/controller/ConfigurationController.java +++ b/src/main/java/de/thm/arsnova/controller/ConfigurationController.java @@ -36,9 +36,6 @@ import java.util.HashMap; @Controller @RequestMapping({"/configuration", "/arsnova-config"}) public class ConfigurationController extends AbstractController { - @Value("${security.guest.enabled}") - private String guestEnabled; - public static final Logger LOGGER = LoggerFactory .getLogger(ConfigurationController.class); diff --git a/src/main/java/de/thm/arsnova/controller/LoginController.java b/src/main/java/de/thm/arsnova/controller/LoginController.java index e42a2a3a4da34691d68a00177d47c263a8bc8473..3df278fccc2f00dd669c8c5528c6c9ab4aa499be 100644 --- a/src/main/java/de/thm/arsnova/controller/LoginController.java +++ b/src/main/java/de/thm/arsnova/controller/LoginController.java @@ -75,46 +75,46 @@ public class LoginController extends AbstractController { @Value("${api.path:}") private String apiPath; @Value("${customization.path}") private String customizationPath; - @Value("${security.guest.enabled}") private String guestEnabled; + @Value("${security.guest.enabled}") private boolean guestEnabled; @Value("${security.guest.allowed-roles:speaker,student}") private String[] guestRoles; @Value("${security.guest.order}") private int guestOrder; - @Value("${security.custom-login.enabled}") private String customLoginEnabled; + @Value("${security.custom-login.enabled}") private boolean customLoginEnabled; @Value("${security.custom-login.allowed-roles:speaker,student}") private String[] customLoginRoles; @Value("${security.custom-login.title:University}") private String customLoginTitle; @Value("${security.custom-login.login-dialog-path}") private String customLoginDialog; @Value("${security.custom-login.image:}") private String customLoginImage; @Value("${security.custom-login.order}") private int customLoginOrder; - @Value("${security.user-db.enabled}") private String dbAuthEnabled; + @Value("${security.user-db.enabled}") private boolean dbAuthEnabled; @Value("${security.user-db.allowed-roles:speaker,student}") private String[] dbAuthRoles; @Value("${security.user-db.title:ARSnova}") private String dbAuthTitle; @Value("${security.user-db.login-dialog-path}") private String dbAuthDialog; @Value("${security.user-db.image:}") private String dbAuthImage; @Value("${security.user-db.order}") private int dbAuthOrder; - @Value("${security.ldap.enabled}") private String ldapEnabled; + @Value("${security.ldap.enabled}") private boolean ldapEnabled; @Value("${security.ldap.allowed-roles:speaker,student}") private String[] ldapRoles; @Value("${security.ldap.title:LDAP}") private String ldapTitle; @Value("${security.ldap.login-dialog-path}") private String ldapDialog; @Value("${security.ldap.image:}") private String ldapImage; @Value("${security.ldap.order}") private int ldapOrder; - @Value("${security.cas.enabled}") private String casEnabled; + @Value("${security.cas.enabled}") private boolean casEnabled; @Value("${security.cas.allowed-roles:speaker,student}") private String[] casRoles; @Value("${security.cas.title:CAS}") private String casTitle; @Value("${security.cas.image:}") private String casImage; @Value("${security.cas.order}") private int casOrder; - @Value("${security.facebook.enabled}") private String facebookEnabled; + @Value("${security.facebook.enabled}") private boolean facebookEnabled; @Value("${security.facebook.enabled-roles:speaker,student}") private String[] facebookRoles; @Value("${security.facebook.order}") private int facebookOrder; - @Value("${security.google.enabled}") private String googleEnabled; + @Value("${security.google.enabled}") private boolean googleEnabled; @Value("${security.google.allowed-roles:speaker,student}") private String[] googleRoles; @Value("${security.google.order}") private int googleOrder; - @Value("${security.twitter.enabled}") private String twitterEnabled; + @Value("${security.twitter.enabled}") private boolean twitterEnabled; @Value("${security.twitter.allowed-roles:speaker,student}") private String[] twitterRoles; @Value("${security.twitter.order}") private int twitterOrder; @@ -162,7 +162,7 @@ public class LoginController extends AbstractController { userSessionService.setRole(role); - if ("arsnova".equals(type)) { + if (dbAuthEnabled && "arsnova".equals(type)) { Authentication authRequest = new UsernamePasswordAuthenticationToken(username, password); try { Authentication auth = daoProvider.authenticate(authRequest); @@ -180,7 +180,7 @@ public class LoginController extends AbstractController { userService.increaseFailedLoginCount(addr); response.setStatus(HttpStatus.UNAUTHORIZED.value()); - } else if ("ldap".equals(type)) { + } else if (ldapEnabled && "ldap".equals(type)) { if (!"".equals(username) && !"".equals(password)) { org.springframework.security.core.userdetails.User user = new org.springframework.security.core.userdetails.User( @@ -206,7 +206,7 @@ public class LoginController extends AbstractController { userService.increaseFailedLoginCount(addr); response.setStatus(HttpStatus.UNAUTHORIZED.value()); } - } else if ("guest".equals(type)) { + } else if (guestEnabled && "guest".equals(type)) { List<GrantedAuthority> authorities = new ArrayList<GrantedAuthority>(); authorities.add(new SimpleGrantedAuthority("ROLE_GUEST")); if (username == null || !username.startsWith("Guest") || username.length() != MAX_USERNAME_LENGTH) { @@ -221,6 +221,8 @@ public class LoginController extends AbstractController { SecurityContextHolder.getContext().setAuthentication(token); request.getSession(true).setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext()); + } else { + response.setStatus(HttpStatus.BAD_REQUEST.value()); } } @@ -262,19 +264,21 @@ public class LoginController extends AbstractController { request.getSession().setAttribute("ars-login-success-url", serverUrl + successUrl); request.getSession().setAttribute("ars-login-failure-url", serverUrl + failureUrl); - if ("cas".equals(type)) { + if (casEnabled && "cas".equals(type)) { casEntryPoint.commence(request, response, null); - } else if ("twitter".equals(type)) { + } else if (twitterEnabled && "twitter".equals(type)) { final String authUrl = twitterProvider.getAuthorizationUrl(new HttpUserSession(request)); result = new RedirectView(authUrl); - } else if ("facebook".equals(type)) { + } else if (facebookEnabled && "facebook".equals(type)) { facebookProvider.setFields("id,link"); facebookProvider.setScope(""); final String authUrl = facebookProvider.getAuthorizationUrl(new HttpUserSession(request)); result = new RedirectView(authUrl); - } else if ("google".equals(type)) { + } else if (googleEnabled && "google".equals(type)) { final String authUrl = googleProvider.getAuthorizationUrl(new HttpUserSession(request)); result = new RedirectView(authUrl); + } else { + response.setStatus(HttpStatus.BAD_REQUEST.value()); } return result; @@ -313,7 +317,7 @@ public class LoginController extends AbstractController { /* The first parameter is replaced by the backend, the second one by the frondend */ String dialogUrl = apiPath + "/auth/dialog?type={0}&successurl='{0}'"; - if ("true".equals(guestEnabled)) { + if (guestEnabled) { ServiceDescription sdesc = new ServiceDescription( "guest", "Guest", @@ -324,7 +328,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(customLoginEnabled) && !"".equals(customLoginDialog)) { + if (customLoginEnabled && !"".equals(customLoginDialog)) { ServiceDescription sdesc = new ServiceDescription( "custom", customLoginTitle, @@ -336,7 +340,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(dbAuthEnabled) && !"".equals(dbAuthDialog)) { + if (dbAuthEnabled && !"".equals(dbAuthDialog)) { ServiceDescription sdesc = new ServiceDescription( "arsnova", dbAuthTitle, @@ -348,7 +352,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(ldapEnabled) && !"".equals(ldapDialog)) { + if (ldapEnabled && !"".equals(ldapDialog)) { ServiceDescription sdesc = new ServiceDescription( "ldap", ldapTitle, @@ -360,7 +364,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(casEnabled)) { + if (casEnabled) { ServiceDescription sdesc = new ServiceDescription( "cas", casTitle, @@ -371,7 +375,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(facebookEnabled)) { + if (facebookEnabled) { ServiceDescription sdesc = new ServiceDescription( "facebook", "Facebook", @@ -382,7 +386,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(googleEnabled)) { + if (googleEnabled) { ServiceDescription sdesc = new ServiceDescription( "google", "Google", @@ -393,7 +397,7 @@ public class LoginController extends AbstractController { services.add(sdesc); } - if ("true".equals(twitterEnabled)) { + if (twitterEnabled) { ServiceDescription sdesc = new ServiceDescription( "twitter", "Twitter", diff --git a/src/main/java/de/thm/arsnova/web/CorsFilter.java b/src/main/java/de/thm/arsnova/web/CorsFilter.java index f4c434e02cc65f0a89da48cdb9acda407780cdd1..acd2f7dc1541bbee46c504e0a90d2a5d9065e3de 100644 --- a/src/main/java/de/thm/arsnova/web/CorsFilter.java +++ b/src/main/java/de/thm/arsnova/web/CorsFilter.java @@ -17,36 +17,52 @@ */ package de.thm.arsnova.web; -import org.springframework.stereotype.Component; -import org.springframework.web.filter.OncePerRequestFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.web.cors.CorsConfiguration; +import org.springframework.web.cors.UrlBasedCorsConfigurationSource; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; +import java.util.List; -/** - * Sets response headers to allow CORS requests. - */ -@Component -public class CorsFilter extends OncePerRequestFilter { +public class CorsFilter extends org.springframework.web.filter.CorsFilter { + protected final Logger logger = LoggerFactory.getLogger(CorsFilter.class); + + public CorsFilter(List<String> origins) { + super(configurationSource(origins)); + logger.info("CorsFilter initialized. Allowed origins: {}", origins); + } - @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) - throws ServletException, IOException { - response.addHeader("Access-Control-Allow-Credentials", "true"); - response.addHeader("Access-Control-Allow-Methods", "GET"); - response.addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, X-Requested-With"); + private static UrlBasedCorsConfigurationSource configurationSource(List<String> origins) { + CorsConfiguration config; + UrlBasedCorsConfigurationSource source; - if (request.getHeader("origin") != null) { - response.addHeader("Access-Control-Allow-Origin", sanitizeOriginUrl(request.getHeader("origin"))); - } + /* Grant full access from specified origins */ + config = new CorsConfiguration(); + config.setAllowedOrigins(origins); + config.addAllowedHeader("Accept"); + config.addAllowedHeader("Content-Type"); + config.addAllowedHeader("X-Requested-With"); + config.addAllowedMethod("GET"); + config.addAllowedMethod("POST"); + config.addAllowedMethod("PUT"); + config.addAllowedMethod("DELETE"); + config.setAllowCredentials(true); + source = new UrlBasedCorsConfigurationSource(); + source.registerCorsConfiguration("/**", config); - filterChain.doFilter(request, response); - } + /* Grant limited access from all origins */ + config = new CorsConfiguration(); + config.addAllowedOrigin("*"); + config.addAllowedHeader("Accept"); + config.addAllowedHeader("X-Requested-With"); + config.addAllowedMethod("GET"); + config.setAllowCredentials(true); + source = new UrlBasedCorsConfigurationSource(); + source.registerCorsConfiguration("/", config); + source.registerCorsConfiguration("/arsnova-config", config); + source.registerCorsConfiguration("/configuration/", config); + source.registerCorsConfiguration("/statistics", config); - private String sanitizeOriginUrl(String originUrl) { - return originUrl.replaceAll("[\n\r]+", " "); + return source; } } diff --git a/src/main/resources/arsnova.properties.example b/src/main/resources/arsnova.properties.example index f7efb747218faba8df18a1964abd541f243b72fe..7e7358990eefc45facb1a8aafbedf01cee2f92b6 100644 --- a/src/main/resources/arsnova.properties.example +++ b/src/main/resources/arsnova.properties.example @@ -57,6 +57,8 @@ security.authentication.login-try-limit=50 # Configuration parameters for authentication services: # enabled: enable or disable the service +# allowed-roles: enable/disable service for a specific role (valid roles: speaker, student) +# (security note: this is currently only handled by the frontend!) # title: the title which is displayed by frontends # login-dialog-path: URL of a login dialog page # image: an image which is used for frontend buttons @@ -167,6 +169,15 @@ security.google.key= security.google.secret= +################################################################################ +# Cross-Origin Resource Sharing +################################################################################ +# CORS grants full API access to client-side (browser) applications from other +# domains. Multiple entries are separated by commas. Untrusted and vulnerable +# applications running on these domains pose a security risk to ARSnova users. +#security.cors.origins=https:// + + ################################################################################ # ARSnova Connector (for LMS) ################################################################################ diff --git a/src/main/webapp/WEB-INF/web.xml b/src/main/webapp/WEB-INF/web.xml index 7daa0e4c21695641e954b8e1b98c8239c3c9e33a..95f404bd0d1c9ec724d76b00eb5aba0bff6f4818 100644 --- a/src/main/webapp/WEB-INF/web.xml +++ b/src/main/webapp/WEB-INF/web.xml @@ -63,7 +63,7 @@ <filter> <filter-name>corsFilter</filter-name> - <filter-class>de.thm.arsnova.web.CorsFilter</filter-class> + <filter-class>org.springframework.web.filter.DelegatingFilterProxy</filter-class> <async-supported>true</async-supported> </filter> <filter-mapping> diff --git a/src/test/resources/arsnova.properties.example b/src/test/resources/arsnova.properties.example index 3f118a73331349cb3c25362bf29e8e5db0dd5e71..7e7358990eefc45facb1a8aafbedf01cee2f92b6 100644 --- a/src/test/resources/arsnova.properties.example +++ b/src/test/resources/arsnova.properties.example @@ -57,6 +57,8 @@ security.authentication.login-try-limit=50 # Configuration parameters for authentication services: # enabled: enable or disable the service +# allowed-roles: enable/disable service for a specific role (valid roles: speaker, student) +# (security note: this is currently only handled by the frontend!) # title: the title which is displayed by frontends # login-dialog-path: URL of a login dialog page # image: an image which is used for frontend buttons @@ -64,13 +66,14 @@ security.authentication.login-try-limit=50 # Guest authentication # security.guest.enabled=true +security.guest.allowed-roles=speaker,student security.guest.order=0 -security.guest.lecturer.enabled=true # Setup combined login if you want to use a single, customized login page # which is used for multiple authentication services. # security.custom-login.enabled=false +security.custom-login.allowed-roles=speaker,student security.custom-login.title=University security.custom-login.login-dialog-path= security.custom-login.image= @@ -88,6 +91,7 @@ security.custom-login.order=0 # replaced by the value of activation-path. # security.user-db.enabled=true +security.user-db.allowed-roles=speaker,student security.user-db.title=ARSnova security.user-db.login-dialog-path=account.html security.user-db.activation-path=account.html @@ -111,6 +115,7 @@ security.user-db.reset-password-mail.body=You requested to reset your \ # server. {0} will be replaced with the user ID by ARSnova. # security.ldap.enabled=false +security.ldap.allowed-roles=speaker,student security.ldap.title=LDAP security.ldap.login-dialog-path=login-ldap.html security.ldap.image= @@ -128,6 +133,7 @@ security.ldap.user-dn-pattern=uid={0},ou=arsnova # CAS authentication # security.cas.enabled=false +security.cas.allowed-roles=speaker,student security.cas.title=CAS security.cas.image= security.cas.order=0 @@ -141,6 +147,7 @@ security.cas-server-url=https://example.com/cas # Facebook # security.facebook.enabled=false +security.facebook.allowed-roles=speaker,student security.facebook.order=0 security.facebook.key= security.facebook.secret= @@ -148,6 +155,7 @@ security.facebook.secret= # Twitter # security.twitter.enabled=false +security.twitter.allowed-roles=speaker,student security.twitter.order=0 security.twitter.key= security.twitter.secret= @@ -155,11 +163,21 @@ security.twitter.secret= # Google # security.google.enabled=false +security.google.allowed-roles=speaker,student security.google.order=0 security.google.key= security.google.secret= +################################################################################ +# Cross-Origin Resource Sharing +################################################################################ +# CORS grants full API access to client-side (browser) applications from other +# domains. Multiple entries are separated by commas. Untrusted and vulnerable +# applications running on these domains pose a security risk to ARSnova users. +#security.cors.origins=https:// + + ################################################################################ # ARSnova Connector (for LMS) ################################################################################ @@ -214,7 +232,7 @@ question.answer-option-limit=8 # options should be used cautiously since it could lead to display errors. # Answer options will still not be parsed in diagrams. This setting has no # effect if neither MathJax nor Markdown are enabled. -question.parse-answer-option-formatting=true +question.parse-answer-option-formatting=false # Demo session id to show above session login button # You can freely use the demo session of https://arsnova.eu/mobile which can be