-
Notifications
You must be signed in to change notification settings - Fork 2
fix: add security headers to Java backend to address ZAP findings #416
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
base: main
Are you sure you want to change the base?
Changes from all commits
5a78d54
241bdd8
279c2e4
7674c0e
c904d82
9502885
c26ff76
92b3e40
bed232d
8c0c42c
ab370aa
f234c85
2723e8e
663a9f1
255af1a
df15ca3
88f6940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,257 @@ | ||||||
| package ca.bc.gov.nrs.api.security; | ||||||
|
|
||||||
| import jakarta.ws.rs.container.ContainerRequestContext; | ||||||
| import jakarta.ws.rs.container.ContainerResponseContext; | ||||||
| import jakarta.ws.rs.container.ContainerResponseFilter; | ||||||
| import jakarta.ws.rs.core.MultivaluedMap; | ||||||
| import jakarta.ws.rs.ext.Provider; | ||||||
| import java.util.ArrayList; | ||||||
| import java.util.List; | ||||||
|
|
||||||
| /** | ||||||
| * Security headers filter to address ZAP penetration test findings. | ||||||
| * | ||||||
| * Addresses the following ZAP alerts: | ||||||
| * - Content Security Policy (CSP) Header Not Set [10038] | ||||||
| * - Missing Anti-clickjacking Header [10020] | ||||||
| * - Proxy Disclosure [40025] | ||||||
| * - Cookie with SameSite Attribute None [10054] | ||||||
| * - Permissions Policy Header Not Set [10063] | ||||||
| * - Strict-Transport-Security Header Not Set [10035] | ||||||
| * - X-Content-Type-Options Header Missing [10021] | ||||||
| * - Re-examine Cache-control Directives [10015] | ||||||
| * - Non-Storable Content [10049] | ||||||
| * - Storable and Cacheable Content [10049] | ||||||
| */ | ||||||
| @Provider | ||||||
| public class SecurityHeadersFilter implements ContainerResponseFilter { | ||||||
|
|
||||||
| // Headers to remove to prevent proxy/server disclosure | ||||||
| private static final String[] HEADERS_TO_REMOVE = { | ||||||
| "Server", | ||||||
| "X-Powered-By", | ||||||
| "Via", | ||||||
| "X-AspNet-Version", | ||||||
| "X-AspNetMvc-Version" | ||||||
| }; | ||||||
|
|
||||||
| @Override | ||||||
| public void filter( | ||||||
| ContainerRequestContext requestContext, ContainerResponseContext responseContext) { | ||||||
| MultivaluedMap<String, Object> headers = responseContext.getHeaders(); | ||||||
|
|
||||||
| // Security headers to address ZAP alerts | ||||||
|
|
||||||
| // X-Content-Type-Options: Prevents MIME type sniffing | ||||||
| // Addresses: X-Content-Type-Options Header Missing [10021] | ||||||
| headers.add("X-Content-Type-Options", "nosniff"); | ||||||
|
|
||||||
| // X-Frame-Options: Prevents clickjacking attacks | ||||||
| // Addresses: Missing Anti-clickjacking Header [10020] | ||||||
| headers.add("X-Frame-Options", "DENY"); | ||||||
|
|
||||||
| // Strict-Transport-Security: Enforces HTTPS | ||||||
| // Addresses: Strict-Transport-Security Header Not Set [10035] | ||||||
| // Only set HSTS when the request is served over HTTPS | ||||||
| // Check both direct HTTPS and proxy-forwarded HTTPS (for reverse proxy scenarios) | ||||||
| // Check multiple common proxy headers to ensure HSTS is applied correctly | ||||||
| String scheme = requestContext.getUriInfo().getRequestUri().getScheme(); | ||||||
| String xForwardedProto = requestContext.getHeaderString("X-Forwarded-Proto"); | ||||||
| String xForwardedScheme = requestContext.getHeaderString("X-Forwarded-Scheme"); | ||||||
| String xForwardedSsl = requestContext.getHeaderString("X-Forwarded-SSL"); | ||||||
| String frontEndHttps = requestContext.getHeaderString("Front-End-Https"); | ||||||
| boolean isHttps = "https".equals(scheme) | ||||||
| || "https".equalsIgnoreCase(xForwardedProto) | ||||||
| || "https".equalsIgnoreCase(xForwardedScheme) | ||||||
| || "on".equalsIgnoreCase(xForwardedSsl) | ||||||
| || "on".equalsIgnoreCase(frontEndHttps) | ||||||
| || "true".equalsIgnoreCase(frontEndHttps); | ||||||
| if (isHttps) { | ||||||
| headers.add( | ||||||
| "Strict-Transport-Security", "max-age=31536000; includeSubDomains; preload"); | ||||||
| } | ||||||
|
|
||||||
| // Content-Security-Policy: Restrictive CSP for API endpoints | ||||||
| // Addresses: Content Security Policy (CSP) Header Not Set [10038] | ||||||
| // Note: This is a restrictive policy suitable for APIs. For web applications with | ||||||
| // inline scripts/styles or external resources, customize this policy accordingly. | ||||||
| headers.add("Content-Security-Policy", "default-src 'self'"); | ||||||
|
|
||||||
| // Permissions-Policy: Controls browser features | ||||||
| // Addresses: Permissions Policy Header Not Set [10063] | ||||||
| headers.add( | ||||||
| "Permissions-Policy", | ||||||
| "geolocation=(), microphone=(), camera=(), payment=(), usb=(), magnetometer=()," | ||||||
| + " gyroscope=(), speaker-selection=()"); | ||||||
|
||||||
| + " gyroscope=(), speaker-selection=()"); | |
| + "gyroscope=(), speaker-selection=()"); |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation on line 120 is incomplete. It states "If SameSite is missing or set to None, replaces with Strict" but the implementation also replaces "Lax" with "Strict". The documentation should be updated to accurately reflect the actual behavior: "If SameSite is missing or set to None or Lax, replaces with Strict".
| * If SameSite is missing or set to None, replaces with Strict. | |
| * If SameSite is missing or set to None or Lax, replaces with Strict. |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cookie attribute detection logic has a bug when checking for HttpOnly, Secure, and Path attributes. The code uses case-sensitive indexOf to find these attributes (e.g., cookie.indexOf("; HttpOnly")), which will fail to detect these attributes if they have different casing (e.g., "; httponly", "; HTTPONLY"). Cookie attributes are case-insensitive according to RFC 6265, so this could result in incorrect insertion position for the SameSite attribute. Consider using case-insensitive matching or converting to lowercase for comparison.
Uh oh!
There was an error while loading. Please reload this page.