diff --git a/api/src/org/labkey/api/security/AuthFilter.java b/api/src/org/labkey/api/security/AuthFilter.java index 9a903ea03a5..2dfe32c0eb8 100644 --- a/api/src/org/labkey/api/security/AuthFilter.java +++ b/api/src/org/labkey/api/security/AuthFilter.java @@ -53,6 +53,13 @@ public class AuthFilter implements Filter { private static final Object FIRST_REQUEST_LOCK = new Object(); + + public static final String STRICT_TRANSPORT_SECURITY_HEADER_NAME = "Strict-Transport-Security"; + public static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options"; + public static final String X_CONTENT_TYPE_OPTIONS_HEADER_NAME = "X-Content-Type-Options"; + public static final String REFERRER_POLICY_HEADER_NAME = "Referrer-Policy"; + public static final String SERVER_HEADER_NAME = "Server"; + private static boolean _firstRequestHandled = false; private static volatile boolean _sslChecked = false; private static SecurityPointcutService _securityPointcut = null; @@ -81,9 +88,9 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha if (ModuleLoader.getInstance().isStartupComplete()) { if (!"ALLOW".equals(AppProps.getInstance().getXFrameOption())) - resp.setHeader("X-Frame-Options", AppProps.getInstance().getXFrameOption()); - resp.setHeader("X-Content-Type-Options", "nosniff"); - resp.setHeader("Referrer-Policy", "origin-when-cross-origin" ); + resp.setHeader(X_FRAME_OPTIONS_HEADER_NAME, AppProps.getInstance().getXFrameOption()); + resp.setHeader(X_CONTENT_TYPE_OPTIONS_HEADER_NAME, "nosniff"); + resp.setHeader(REFERRER_POLICY_HEADER_NAME, "origin-when-cross-origin" ); if (AppProps.getInstance().isIncludeServerHttpHeader()) { @@ -91,7 +98,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha { _serverHeader = "LabKey/" + AppProps.getInstance().getReleaseVersion(); } - resp.setHeader("Server", _serverHeader); + resp.setHeader(SERVER_HEADER_NAME, _serverHeader); } } @@ -168,7 +175,7 @@ else if (!AppProps.getInstance().isDevMode()) { // Issue 51904: Strict-Transport-Security header when HTTPS is required // Avoid setting when in dev mode to make it easier to toggle HTTPS on and off again for local deployments - resp.setHeader("Strict-Transport-Security", "max-age=31536000;includeSubdomains"); + resp.setHeader(STRICT_TRANSPORT_SECURITY_HEADER_NAME, "max-age=31536000;includeSubdomains"); } } diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index b45f6ab80a9..c9e4640717c 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -35,6 +35,7 @@ import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.module.ModuleLoader; import org.labkey.api.search.SearchService; +import org.labkey.api.security.AuthFilter; import org.labkey.api.security.LoginUrls; import org.labkey.api.security.User; import org.labkey.api.security.UserManager; @@ -54,6 +55,7 @@ import org.labkey.api.view.template.PageConfig; import org.labkey.api.webdav.DavException; import org.labkey.api.writer.PrintWriters; +import org.labkey.filters.ContentSecurityPolicyFilter; import org.springframework.dao.DataAccessResourceFailureException; import jakarta.servlet.ServletException; @@ -594,7 +596,7 @@ else if (!local && AppProps.getInstance().isDevMode() && MothershipReport.isShow else if (!local) { String hash = hashStackTrace(stackTrace); - ExceptionTally tally = EXCEPTION_TALLIES.computeIfAbsent(hash, k -> new ExceptionTally()); + ExceptionTally tally = EXCEPTION_TALLIES.computeIfAbsent(hash, _ -> new ExceptionTally()); // Once we've reported an exception 5 times within this server session, don't report it more than every 5 minutes if (tally._count.incrementAndGet() > 5 && System.currentTimeMillis() - tally._lastReported < 1000 * 60 * 5) { @@ -822,18 +824,29 @@ static ActionURL handleException(@NotNull HttpServletRequest request, @NotNull H { try { - response.reset(); - // mostly to make security scanners happy - if (ModuleLoader.getInstance().isStartupComplete()) + // Capture security-related headers to re-apply after reset() + Map securityHeaders = new HashMap<>(); + for (String headerName : Arrays.asList( + ContentSecurityPolicyFilter.ContentSecurityPolicyType.Enforce.getHeaderName(), + ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName(), + AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME, + AuthFilter.X_FRAME_OPTIONS_HEADER_NAME, + AuthFilter.X_CONTENT_TYPE_OPTIONS_HEADER_NAME, + AuthFilter.REFERRER_POLICY_HEADER_NAME, + AuthFilter.SERVER_HEADER_NAME)) { - if (!"ALLOW".equals(AppProps.getInstance().getXFrameOption())) - response.setHeader("X-Frame-Options", AppProps.getInstance().getXFrameOption()); - response.setHeader("X-Content-Type-Options", "nosniff"); + String value = response.getHeader(headerName); + if (value != null) + securityHeaders.put(headerName, value); } + + response.reset(); + + securityHeaders.forEach(response::setHeader); } catch (IllegalStateException x) { - // This is fine, just can't clear the existing response as its + // This is fine, just can't clear the existing response as it's // been at least partially written back to the client } } @@ -1246,7 +1259,7 @@ public static boolean decorateException(Throwable t, Enum key, String value, t = unwrapException(t); synchronized (_exceptionDecorations) { - HashMap, String> m = _exceptionDecorations.computeIfAbsent(t, k -> new HashMap<>()); + HashMap, String> m = _exceptionDecorations.computeIfAbsent(t, _ -> new HashMap<>()); if (overwrite || !m.containsKey(key)) { LOG.debug("add decoration to " + t.getClass() + "@" + System.identityHashCode(t) + " " + key + "=" + value); @@ -1305,15 +1318,13 @@ public static String getExtendedMessage(Throwable t) @Nullable public static Throwable getCause(Throwable t) { - Throwable cause; - if (t instanceof RuntimeSQLException) - cause = ((RuntimeSQLException)t).getSQLException(); - else if (t instanceof ServletException) - cause = ((ServletException)t).getRootCause(); - else if (t instanceof BatchUpdateException) - cause = ((BatchUpdateException)t).getNextException(); - else - cause = t.getCause(); + Throwable cause = switch (t) + { + case RuntimeSQLException runtimeSQLException -> runtimeSQLException.getSQLException(); + case ServletException servletException -> servletException.getRootCause(); + case BatchUpdateException throwables -> throwables.getNextException(); + case null, default -> t.getCause(); + }; return cause==t ? null : cause; } @@ -1330,7 +1341,7 @@ public static class TestCase extends Assert ExceptionResponse handleIt(final User user, Exception ex) { final MockServletResponse res = new MockServletResponse(); - InvocationHandler h = (o, method, objects) -> { + InvocationHandler h = (_, method, objects) -> { // still calls in 'headers' for validation res.addHeader(method.getDeclaringClass().getSimpleName() + "." + method.getName(), objects.length==0 ? "" : objects.length==1 ? String.valueOf(objects[0]) : Arrays.toString(objects)); return null; @@ -1485,9 +1496,35 @@ public void testServerError() } @Test - public void testUnwrap() + public void testSecurityHeaderRetention() { + final User user = UserManager.getGuestUser(); + final MockServletResponse res = new MockServletResponse(); + res.setHeader(ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName(), "default-src 'self'"); + res.setHeader(AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME, "max-age=31536000"); + final String otherHeader = "Some-Other-Header"; + res.setHeader(otherHeader, "should-be-gone"); + + HttpServletRequestWrapper req = new HttpServletRequestWrapper(TestContext.get().getRequest()) + { + @Override + public Principal getUserPrincipal() + { + return user; + } + + @Override + public String getMethod() + { + return "GET"; + } + }; + + ExceptionUtil.handleException(req, res, new RuntimeException("Test Exception"), null, false); + assertEquals("default-src 'self'", res.getHeader(ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName())); + assertEquals("max-age=31536000", res.getHeader(AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME)); + assertNull(res.getHeader(otherHeader)); } } @@ -1701,9 +1738,9 @@ public boolean isCommitted() public void reset() { resetBuffer(); + headers.clear(); status = 0; message = null; - // headers.clear(); } @Override