Skip to content

Commit 361a287

Browse files
cescoffiergsmet
cescoffier
authored and
gsmet
committed
Strip matrix parameters from request paths during HTTP security policy matching
Matrix parameters (semicolon-delimited values in URL path segments, e.g. /api;v=1/resource) could bypass HTTP security policy path matching. This commit strips matrix parameters before matching across all security-relevant paths: HTTP policy matcher, Keycloak policy enforcer, OIDC tenant resolver, CSRF filter, and Undertow servlet policy. Additionally, the build now fails if an HTTP security policy path contains a literal semicolon character.
1 parent 2fd4c6d commit 361a287

23 files changed

Lines changed: 1117 additions & 38 deletions

File tree

‎docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,38 @@ include::_attributes.adoc[]
1313
Quarkus incorporates a pluggable web security layer.
1414
When security is active, the system performs a permission check on all HTTP requests to determine if they should proceed.
1515

16-
[NOTE]
17-
====
18-
If you use Jakarta RESTful Web Services, consider using `quarkus.security.jaxrs.deny-unannotated-endpoints` or `quarkus.security.jaxrs.default-roles-allowed` to set default security requirements instead of HTTP path-level matching because annotations can override these properties on an individual endpoint.
19-
====
20-
2116
Authorization is based on user roles that the security provider provides.
2217
To customize these roles, a `SecurityIdentityAugmentor` can be created, see
2318
xref:security-customization.adoc#security-identity-customization[Security Identity Customization].
2419

2520
[[authorization-using-configuration]]
2621
== Authorization using configuration
2722

23+
[NOTE]
24+
====
25+
If you work with Jakarta RESTful Web Services (JAX-RS) and need to set default security requirements, consider using <<standard-security-annotations>> and `quarkus.security.jaxrs.deny-unannotated-endpoints` or `quarkus.security.jaxrs.default-roles-allowed` properties instead of the HTTP security policy path-level matching because the security annotations can override these properties on an individual JAX-RS resource or method level.
26+
====
27+
2828
Permissions are defined in the Quarkus configuration by permission sets, each specifying a policy for access control.
2929

3030
[NOTE]
3131
====
3232
When a security policy's `paths` property contains the most specific path that matches the current request path, it takes precedence over other security policies with matching paths and is said to win.
3333
====
3434

35+
[NOTE]
36+
====
37+
Configured HTTP security policy must not contain a semicolon ';' character in its `paths` property.
38+
Use <<custom-http-security-policy>> when a security policy decision depends on a presence of certain matrix parameters in the request path.
39+
====
40+
41+
[IMPORTANT]
42+
====
43+
Be careful with creating complex, possibly overlapping HTTP security policy path expressions.
44+
Make sure your HTTP policy configuration is thoroughly tested.
45+
If you work with Jakarta RESTful Web Services (JAX-RS) and need to create complex security policies, consider using <<standard-security-annotations>> instead.
46+
====
47+
3548
.{project-name} policies summary
3649
[options="header"]
3750
|===

‎extensions/keycloak-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/KeycloakPolicyEnforcerAuthorizer.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import io.quarkus.security.spi.runtime.BlockingSecurityExecutor;
3030
import io.quarkus.vertx.http.runtime.CurrentVertxRequest;
3131
import io.quarkus.vertx.http.runtime.security.HttpSecurityPolicy;
32+
import io.quarkus.vertx.http.runtime.security.HttpSecurityUtils;
3233
import io.smallrye.mutiny.Uni;
3334
import io.vertx.ext.web.RoutingContext;
3435

@@ -61,7 +62,8 @@ public Uni<CheckResult> apply(PolicyEnforcer policyEnforcer) {
6162
return blockingExecutor.executeBlocking(new Supplier<PathConfig>() {
6263
@Override
6364
public PathConfig get() {
64-
return policyEnforcer.getPathMatcher().matches(routingContext.normalizedPath());
65+
return policyEnforcer.getPathMatcher().matches(HttpSecurityUtils
66+
.pathWithoutMatrixParams(routingContext.normalizedPath()));
6567
}
6668
}).flatMap(new Function<PathConfig, Uni<? extends CheckResult>>() {
6769
@Override

‎extensions/keycloak-authorization/runtime/src/main/java/io/quarkus/keycloak/pep/runtime/VertxHttpFacade.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import io.netty.handler.codec.http.HttpHeaderNames;
1212
import io.quarkus.vertx.http.runtime.VertxInputStream;
13+
import io.quarkus.vertx.http.runtime.security.HttpSecurityUtils;
1314
import io.vertx.core.http.Cookie;
1415
import io.vertx.core.http.HttpServerRequest;
1516
import io.vertx.core.http.HttpServerResponse;
@@ -119,7 +120,7 @@ public String getURI() {
119120

120121
@Override
121122
public String getRelativePath() {
122-
return routingContext.normalizedPath();
123+
return HttpSecurityUtils.pathWithoutMatrixParams(routingContext.normalizedPath());
123124
}
124125

125126
@Override

‎extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/StaticTenantResolver.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import io.quarkus.oidc.OidcTenantConfig;
2121
import io.quarkus.oidc.TenantResolver;
2222
import io.quarkus.oidc.common.runtime.OidcCommonUtils;
23+
import io.quarkus.vertx.http.runtime.security.HttpSecurityUtils;
2324
import io.quarkus.vertx.http.runtime.security.ImmutablePathMatcher;
2425
import io.smallrye.mutiny.Uni;
2526
import io.vertx.core.json.JsonArray;
@@ -125,12 +126,13 @@ private DefaultStaticTenantResolver(TenantConfigBean tenantConfigBean) {
125126

126127
@Override
127128
public String resolve(RoutingContext context) {
128-
String[] pathSegments = context.request().path().split(PATH_SEPARATOR);
129-
for (String segment : pathSegments) {
130-
if (tenantConfigBean.getStaticTenant(segment) != null) {
129+
String[] pathSegments = HttpSecurityUtils.pathWithoutMatrixParams(context.normalizedPath())
130+
.split(PATH_SEPARATOR);
131+
for (String canonicalSegment : pathSegments) {
132+
if (tenantConfigBean.getStaticTenant(canonicalSegment) != null) {
131133
LOG.debugf(
132-
"Tenant id '%s' is selected on the '%s' request path", segment, context.normalizedPath());
133-
return segment;
134+
"Tenant id '%s' is selected on the '%s' request path", canonicalSegment, context.normalizedPath());
135+
return canonicalSegment;
134136
}
135137
}
136138
return null;
@@ -157,10 +159,11 @@ private static PathMatchingTenantResolver of(Map<String, TenantConfigContext> st
157159

158160
@Override
159161
public String resolve(RoutingContext context) {
160-
String tenantId = staticTenantPaths.match(context.normalizedPath()).getValue();
162+
String canonicalPath = HttpSecurityUtils.pathWithoutMatrixParams(context.normalizedPath());
163+
String tenantId = staticTenantPaths.match(canonicalPath).getValue();
161164
if (tenantId != null) {
162165
LOG.debugf(
163-
"Tenant id '%s' is selected on the '%s' request path", tenantId, context.normalizedPath());
166+
"Tenant id '%s' is selected on the '%s' request path", tenantId, canonicalPath);
164167
return tenantId;
165168
}
166169
return null;

‎extensions/resteasy-classic/resteasy/deployment/src/test/java/io/quarkus/resteasy/test/security/JakartaRestAuthenticationWithMatrixTest.java‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package io.quarkus.resteasy.test.security;
2+
3+
import static org.apache.commons.codec.binary.Base64.encodeBase64URLSafeString;
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
6+
import java.net.URL;
7+
import java.time.Duration;
8+
9+
import jakarta.inject.Inject;
10+
import jakarta.ws.rs.GET;
11+
import jakarta.ws.rs.Path;
12+
13+
import org.junit.jupiter.api.BeforeAll;
14+
import org.junit.jupiter.api.Test;
15+
import org.junit.jupiter.api.extension.RegisterExtension;
16+
import org.junit.jupiter.params.ParameterizedTest;
17+
import org.junit.jupiter.params.provider.ValueSource;
18+
19+
import io.quarkus.security.Authenticated;
20+
import io.quarkus.security.test.utils.TestIdentityController;
21+
import io.quarkus.security.test.utils.TestIdentityProvider;
22+
import io.quarkus.test.QuarkusExtensionTest;
23+
import io.quarkus.test.common.http.TestHTTPResource;
24+
import io.smallrye.mutiny.Uni;
25+
import io.vertx.core.http.HttpMethod;
26+
import io.vertx.mutiny.core.Vertx;
27+
import io.vertx.mutiny.core.http.HttpClientRequest;
28+
29+
public class JakartaRestAuthenticationWithMatrixTest {
30+
private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(20);
31+
32+
@RegisterExtension
33+
static QuarkusExtensionTest runner = new QuarkusExtensionTest()
34+
.withApplicationRoot((jar) -> jar
35+
.addClasses(TestIdentityProvider.class, TestIdentityController.class, ApiResource.class,
36+
ApiWithMatrixResource.class, ApiWithEncodedSemicolonResource.class));
37+
38+
@BeforeAll
39+
public static void setup() {
40+
TestIdentityController.resetRoles()
41+
.add("admin", "admin", "admin")
42+
.add("test", "test", "test");
43+
}
44+
45+
@TestHTTPResource
46+
URL url;
47+
48+
@Inject
49+
Vertx vertx;
50+
51+
@ParameterizedTest
52+
@ValueSource(strings = {
53+
"/api", "/api/service",
54+
"/api;", "/api;/service;",
55+
"/api;a", "/api;/service;a",
56+
"/api-with-percent-encoded%3Ba"
57+
})
58+
public void testNotAuthenticated(String path) {
59+
assureAuthenticationFailure(path);
60+
}
61+
62+
@ParameterizedTest
63+
@ValueSource(strings = {
64+
"/api", "/api/service",
65+
"/api;", "/api;/service;",
66+
"/api;a", "/api;/service;a",
67+
"/api-with-percent-encoded%3Ba"
68+
})
69+
public void testAuthenticated(String path) {
70+
assureAuthenticationSuccess(path);
71+
}
72+
73+
@Test
74+
public void testMatrixInPathAnnotation() {
75+
assurePath("/api-with-matrix;a", 404);
76+
}
77+
78+
@Test
79+
public void testMatrixInsteadOfPercentEncodedNotFound() {
80+
assureNotFound("/api-with-percent-encoded;a");
81+
}
82+
83+
@Path("/api")
84+
@Authenticated
85+
public static class ApiResource {
86+
87+
@GET
88+
public String api() {
89+
return "api";
90+
}
91+
92+
@GET
93+
@Path("/service")
94+
public String service() {
95+
return "service";
96+
}
97+
}
98+
99+
@Path("/api-with-matrix;a")
100+
public static class ApiWithMatrixResource {
101+
102+
@GET
103+
public String apiWithMatrix() {
104+
throw new RuntimeException();
105+
}
106+
}
107+
108+
@Path("/api-with-percent-encoded%3Ba")
109+
@Authenticated
110+
public static class ApiWithEncodedSemicolonResource {
111+
112+
@GET
113+
public String apiWithEncodedSemicolon() {
114+
return "hello";
115+
}
116+
}
117+
118+
private void assureAuthenticationFailure(String path) {
119+
assurePath(path, 401);
120+
}
121+
122+
private void assureAuthenticationSuccess(String path) {
123+
assurePath(path, 200);
124+
}
125+
126+
private void assureNotFound(String path) {
127+
assurePath(path, 404);
128+
}
129+
130+
private void assurePath(String path, int expectedStatusCode) {
131+
var httpClient = vertx.createHttpClient();
132+
try {
133+
httpClient
134+
.request(HttpMethod.GET, url.getPort(), url.getHost(), path)
135+
.map(r -> {
136+
if (expectedStatusCode == 200) {
137+
r.putHeader("Authorization",
138+
"Basic " + encodeBase64URLSafeString("admin:admin".getBytes()));
139+
}
140+
return r;
141+
})
142+
.flatMap(HttpClientRequest::send)
143+
.invoke(r -> assertEquals(expectedStatusCode, r.statusCode(), path))
144+
.flatMap(r -> {
145+
return Uni.createFrom().nullItem();
146+
})
147+
.await()
148+
.atMost(REQUEST_TIMEOUT);
149+
} finally {
150+
httpClient
151+
.close()
152+
.await()
153+
.atMost(REQUEST_TIMEOUT);
154+
}
155+
}
156+
}

0 commit comments

Comments
 (0)