diff --git a/compose/auth_proxy.yml b/compose/auth_proxy.yml index 1d3dfcfb1..42a494995 100644 --- a/compose/auth_proxy.yml +++ b/compose/auth_proxy.yml @@ -7,6 +7,7 @@ services: CRYOSTAT_HTTP_PROXY_PORT: "${CRYOSTAT_HTTP_PORT}" QUARKUS_HTTP_PROXY_PROXY_ADDRESS_FORWARDING: "true" QUARKUS_HTTP_PROXY_ALLOW_X_FORWARDED: "true" + QUARKUS_HTTP_PROXY_TRUSTED_PROXIES: "auth" QUARKUS_HTTP_PROXY_ENABLE_FORWARDED_HOST: "true" QUARKUS_HTTP_PROXY_ENABLE_FORWARDED_PREFIX: "true" auth: diff --git a/schema-generator/dependency-reduced-pom.xml b/schema-generator/dependency-reduced-pom.xml index d1aa77ed6..2c2443f89 100644 --- a/schema-generator/dependency-reduced-pom.xml +++ b/schema-generator/dependency-reduced-pom.xml @@ -15,6 +15,10 @@ 21 + + maven-surefire-plugin + 3.5.6 + maven-jar-plugin 3.5.1 @@ -101,12 +105,35 @@ + + + org.junit.jupiter + junit-jupiter + 5.13.4 + test + + + junit-jupiter-api + org.junit.jupiter + + + junit-jupiter-params + org.junit.jupiter + + + junit-jupiter-engine + org.junit.jupiter + + + + 21 3.7.0 21 21 UTF-8 + 5.13.4 1.33.0 5.0.0 diff --git a/schema-generator/pom.xml b/schema-generator/pom.xml index e2855ef9f..f578adbdc 100644 --- a/schema-generator/pom.xml +++ b/schema-generator/pom.xml @@ -17,6 +17,7 @@ 21 21 UTF-8 + 5.13.4 3.7.0 @@ -38,6 +39,13 @@ snakeyaml 2.6 + + + org.junit.jupiter + junit-jupiter + ${junit.jupiter.version} + test + @@ -51,6 +59,12 @@ + + org.apache.maven.plugins + maven-surefire-plugin + 3.5.6 + + org.apache.maven.plugins @@ -146,4 +160,4 @@ - \ No newline at end of file + diff --git a/schema-generator/src/main/java/io/cryostat/schema/PayloadTypeAnalyzer.java b/schema-generator/src/main/java/io/cryostat/schema/PayloadTypeAnalyzer.java index 08ef7edce..669b86a35 100644 --- a/schema-generator/src/main/java/io/cryostat/schema/PayloadTypeAnalyzer.java +++ b/schema-generator/src/main/java/io/cryostat/schema/PayloadTypeAnalyzer.java @@ -28,8 +28,10 @@ import com.github.javaparser.StaticJavaParser; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; import com.github.javaparser.ast.body.RecordDeclaration; +import com.github.javaparser.ast.body.TypeDeclaration; import com.github.javaparser.ast.expr.Expression; import com.github.javaparser.ast.expr.MethodCallExpr; import com.github.javaparser.ast.expr.NameExpr; @@ -322,7 +324,7 @@ private Map analyzeRecord(RecordDeclaration record) { param -> { String fieldName = param.getNameAsString(); String fieldType = param.getTypeAsString(); - properties.put(fieldName, analyzeFieldType(fieldType)); + properties.put(fieldName, analyzeFieldType(fieldType, param)); }); if (!properties.isEmpty()) { @@ -381,7 +383,8 @@ private Map analyzeClass(ClassOrInterfaceDeclaration classDecl) String fieldName = var.getNameAsString(); String fieldType = var.getTypeAsString(); properties.put( - fieldName, analyzeFieldType(fieldType)); + fieldName, + analyzeFieldType(fieldType, var)); }); } }); @@ -393,7 +396,8 @@ private Map analyzeClass(ClassOrInterfaceDeclaration classDecl) return schema; } - private Map analyzeFieldType(String fieldType) { + private Map analyzeFieldType( + String fieldType, com.github.javaparser.ast.Node context) { // Remove generic type parameters for analysis String baseType = fieldType.replaceAll("<.*>", ""); @@ -422,6 +426,12 @@ private Map analyzeFieldType(String fieldType) { default: // For complex types, try to analyze them inline (avoid circular references) if (!analyzedTypes.contains(baseType)) { + Optional contextualRecord = + findContextualRecordDeclaration(context, baseType); + if (contextualRecord.isPresent()) { + analyzedTypes.add(baseType); + return analyzeRecord(contextualRecord.get()); + } return analyzeType(baseType); } // If already analyzed (circular reference), just describe it @@ -429,6 +439,53 @@ private Map analyzeFieldType(String fieldType) { } } + Optional findContextualRecordDeclaration(Node context, String typeName) { + Node ancestor = context; + while ((ancestor = ancestor.getParentNode().orElse(null)) != null) { + if (!(ancestor instanceof TypeDeclaration enclosingType)) { + continue; + } + + if (enclosingType instanceof RecordDeclaration enclosingRecord + && matchesTypeName( + enclosingRecord.getNameAsString(), + enclosingRecord.getFullyQualifiedName().orElse(""), + typeName)) { + return Optional.of(enclosingRecord); + } + + Optional memberRecord = + enclosingType.getMembers().stream() + .filter(RecordDeclaration.class::isInstance) + .map(RecordDeclaration.class::cast) + .filter( + record -> + matchesTypeName( + record.getNameAsString(), + record.getFullyQualifiedName().orElse(""), + typeName)) + .findFirst(); + if (memberRecord.isPresent()) { + return memberRecord; + } + } + + return context.findCompilationUnit() + .flatMap( + cu -> + cu.getTypes().stream() + .filter(RecordDeclaration.class::isInstance) + .map(RecordDeclaration.class::cast) + .filter( + record -> + matchesTypeName( + record.getNameAsString(), + record.getFullyQualifiedName() + .orElse(""), + typeName)) + .findFirst()); + } + private Map createMapSchema() { Map schema = new LinkedHashMap<>(); schema.put("type", "object"); diff --git a/schema-generator/src/test/java/io/cryostat/schema/PayloadTypeAnalyzerTest.java b/schema-generator/src/test/java/io/cryostat/schema/PayloadTypeAnalyzerTest.java new file mode 100644 index 000000000..6c65f240c --- /dev/null +++ b/schema-generator/src/test/java/io/cryostat/schema/PayloadTypeAnalyzerTest.java @@ -0,0 +1,74 @@ +/* + * Copyright The Cryostat Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.cryostat.schema; + +import static org.junit.jupiter.api.Assertions.assertSame; + +import java.nio.file.Path; + +import com.github.javaparser.JavaParser; +import com.github.javaparser.ParserConfiguration; +import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; +import com.github.javaparser.ast.body.RecordDeclaration; +import org.junit.jupiter.api.Test; + +class PayloadTypeAnalyzerTest { + + @Test + void shouldResolveNestedRecordFromLexicalScope() { + var compilationUnit = + new JavaParser( + new ParserConfiguration() + .setLanguageLevel( + ParserConfiguration.LanguageLevel.JAVA_21)) + .parse( + """ + package example; + + class First { + record Payload(String first) {} + record Event(Payload payload) {} + } + + class Second { + record Payload(String second) {} + record Event(Payload payload) {} + } + """) + .getResult() + .orElseThrow(); + ClassOrInterfaceDeclaration second = compilationUnit.getClassByName("Second").orElseThrow(); + RecordDeclaration expected = findNestedRecord(second, "Payload"); + RecordDeclaration event = findNestedRecord(second, "Event"); + + var analyzer = new PayloadTypeAnalyzer(Path.of(".")); + + assertSame( + expected, + analyzer.findContextualRecordDeclaration(event.getParameter(0), "Payload") + .orElseThrow()); + } + + private static RecordDeclaration findNestedRecord( + ClassOrInterfaceDeclaration enclosingType, String name) { + return enclosingType.getMembers().stream() + .filter(RecordDeclaration.class::isInstance) + .map(RecordDeclaration.class::cast) + .filter(record -> record.getNameAsString().equals(name)) + .findFirst() + .orElseThrow(); + } +} diff --git a/src/main/java/io/cryostat/discovery/Discovery.java b/src/main/java/io/cryostat/discovery/Discovery.java index 7b908b399..1b7687d26 100644 --- a/src/main/java/io/cryostat/discovery/Discovery.java +++ b/src/main/java/io/cryostat/discovery/Discovery.java @@ -875,7 +875,22 @@ private CallbackValidation validateCallback( ConfigProperties.AGENT_TLS_REQUIRED)); } - return new CallbackValidation(callbackUri, unauthCallback, remoteAddress); + try { + for (InetAddress callbackAddress : InetAddress.getAllByName(callbackUri.getHost())) { + if (remoteAddress.equals(callbackAddress)) { + return new CallbackValidation(callbackUri, unauthCallback, remoteAddress); + } + } + } catch (UnknownHostException e) { + throw new BadRequestException( + String.format("%s host could not be resolved: %s", parameterName, callbackUri), + e); + } + + throw new BadRequestException( + String.format( + "%s host does not resolve to the client address %s: %s", + parameterName, remoteAddress.getHostAddress(), callbackUri)); } private DiscoveryPlugin findOrCreatePlugin( @@ -1448,11 +1463,6 @@ private InetAddress getRemoteAddress(RoutingContext ctx) { if (ctx.request() != null && ctx.request().remoteAddress() != null) { addr = jwtValidator.tryResolveAddress(addr, ctx.request().remoteAddress().host()); } - if (ctx.request() != null && ctx.request().headers() != null) { - addr = - jwtValidator.tryResolveAddress( - addr, ctx.request().headers().get(X_FORWARDED_FOR)); - } return addr; } diff --git a/src/main/java/io/cryostat/discovery/DiscoveryJwtValidator.java b/src/main/java/io/cryostat/discovery/DiscoveryJwtValidator.java index 3425e6d81..145558452 100644 --- a/src/main/java/io/cryostat/discovery/DiscoveryJwtValidator.java +++ b/src/main/java/io/cryostat/discovery/DiscoveryJwtValidator.java @@ -31,7 +31,6 @@ import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.proc.BadJWTException; import io.quarkus.security.UnauthorizedException; -import io.vertx.core.MultiMap; import io.vertx.core.http.HttpServerRequest; import io.vertx.ext.web.RoutingContext; import jakarta.enterprise.context.ApplicationScoped; @@ -74,8 +73,9 @@ public JWT validateJwt( if (req.remoteAddress() != null) { addr = tryResolveAddress(addr, req.remoteAddress().host()); } - MultiMap headers = req.headers(); - addr = tryResolveAddress(addr, headers.get(Discovery.X_FORWARDED_FOR)); + if (addr == null) { + throw new UnauthorizedException("Could not determine request address"); + } URI hostUri = new URI( diff --git a/src/test/java/io/cryostat/discovery/DiscoveryPluginTest.java b/src/test/java/io/cryostat/discovery/DiscoveryPluginTest.java index 30a5aa9e4..d9a9003e1 100644 --- a/src/test/java/io/cryostat/discovery/DiscoveryPluginTest.java +++ b/src/test/java/io/cryostat/discovery/DiscoveryPluginTest.java @@ -16,6 +16,9 @@ package io.cryostat.discovery; import static io.restassured.RestAssured.given; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import java.net.URI; import java.util.HashMap; @@ -28,6 +31,7 @@ import io.quarkus.narayana.jta.QuarkusTransaction; import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.mockito.InjectSpy; import io.restassured.http.ContentType; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; @@ -41,6 +45,8 @@ public class DiscoveryPluginTest extends AbstractTransactionalTestBase { private static final String DISCOVERY_HEADER = "Cryostat-Discovery-Authentication"; + @InjectSpy PluginCallbackFactory callbackFactory; + @ParameterizedTest @NullAndEmptySource @ValueSource(strings = {"invalid uri", "no.protocol.example.com"}) @@ -62,6 +68,63 @@ void rejectsInvalidCallback(String callback) { .statusCode(400); } + @Test + void rejectsAgentCallbackMatchingUntrustedForwardedAddress() { + given().log() + .all() + .when() + .header(Discovery.X_FORWARDED_FOR, "192.0.2.1") + .body( + Map.of( + "realm", + "mismatched_callback_test_realm", + "callback", + "http://192.0.2.1", + "credential", + Map.of( + "matchExpression", "true", + "username", "user", + "password", "pass"), + "nodes", + List.of(), + "fillStrategy", + "NONE", + "context", + Map.of())) + .contentType(ContentType.JSON) + .post("/api/v4.3/discovery/agents") + .then() + .log() + .all() + .and() + .assertThat() + .statusCode(400); + + verify(callbackFactory, never()).create(any(URI.class), any(Credential.class)); + } + + @Test + void rejectsPluginCallbackMatchingUntrustedForwardedAddress() { + given().log() + .all() + .when() + .header(Discovery.X_FORWARDED_FOR, "192.0.2.1") + .body( + Map.of( + "realm", + "mismatched_plugin_callback_test_realm", + "callback", + "http://192.0.2.1")) + .contentType(ContentType.JSON) + .post("/api/v4/discovery") + .then() + .log() + .all() + .and() + .assertThat() + .statusCode(400); + } + @ParameterizedTest @NullAndEmptySource void rejectsInvalidRealmName(String realm) { diff --git a/src/test/java/io/cryostat/resources/AgentApplicationResource.java b/src/test/java/io/cryostat/resources/AgentApplicationResource.java index 954439b36..e757d9825 100644 --- a/src/test/java/io/cryostat/resources/AgentApplicationResource.java +++ b/src/test/java/io/cryostat/resources/AgentApplicationResource.java @@ -18,20 +18,16 @@ import java.io.IOException; import java.net.ServerSocket; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Optional; -import io.quarkus.test.common.DevServicesContext; import io.quarkus.test.common.QuarkusTestResourceLifecycleManager; import org.apache.commons.lang3.StringUtils; import org.testcontainers.containers.GenericContainer; -import org.testcontainers.containers.Network; -import org.testcontainers.containers.wait.strategy.HostPortWaitStrategy; +import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.utility.DockerImageName; -public class AgentApplicationResource - implements QuarkusTestResourceLifecycleManager, DevServicesContext.ContextAware { +public class AgentApplicationResource implements QuarkusTestResourceLifecycleManager { private static final String DEFAULT_IMAGE = "quay.io/redhat-java-monitoring/quarkus-cryostat-agent:latest"; @@ -58,7 +54,7 @@ protected Map getEnvMap() { "CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED", "false", "CRYOSTAT_AGENT_WEBSERVER_HOST", - "0.0.0.0", + "127.0.0.1", "CRYOSTAT_AGENT_WEBSERVER_PORT", Integer.toString(PORT), "CRYOSTAT_AGENT_BASEURI_RANGE", @@ -67,7 +63,6 @@ protected Map getEnvMap() { "true")); } - private Optional containerNetworkId; private GenericContainer container; @SuppressWarnings("resource") @@ -75,46 +70,30 @@ protected Map getEnvMap() { public Map start() { int cryostatPort = findFreePort(); int hostAgentPort = findFreePort(); - - Optional network = - containerNetworkId.map( - id -> - new Network() { - @Override - public String getId() { - return id; - } - - @Override - public void close() {} - }); + int agentAppPort = findFreePort(); + Map env = getEnvMap(); + env.put("QUARKUS_HTTP_PORT", Integer.toString(agentAppPort)); + env.put("CRYOSTAT_AGENT_WEBSERVER_PORT", Integer.toString(hostAgentPort)); + env.put("CRYOSTAT_AGENT_BASEURI", String.format("http://127.0.0.1:%d/", cryostatPort)); + env.put("CRYOSTAT_AGENT_CALLBACK", String.format("http://127.0.0.1:%d/", hostAgentPort)); String img = Optional.ofNullable(System.getenv("QUARKUS_TEST_IMAGE")) .filter(StringUtils::isNotBlank) .orElse(DEFAULT_IMAGE); + // Keep registration and callback traffic on the same loopback address. The discovery JWT + // and callback validation both bind the Agent to the address observed by Cryostat. this.container = new GenericContainer<>(DockerImageName.parse(img)) - .withExposedPorts(PORT) - .withEnv(getEnvMap()) - .withNetworkAliases(ALIAS) - .withExtraHost("host.docker.internal", "host-gateway") - .waitingFor(new HostPortWaitStrategy().forPorts(PORT)) + .withEnv(env) + .withNetworkMode("host") + .waitingFor(Wait.forLogMessage(".*Listening on:.*", 1)) .withStartupAttempts(3) .withCreateContainerCmdModifier( cmd -> cmd.getHostConfig() .withCpuShares(512) .withMemory(256L * 1024L * 1024L)); - network.ifPresent(container::withNetwork); - - container.setPortBindings(List.of(String.format("%d:%d", hostAgentPort, PORT))); - container.addEnv( - "CRYOSTAT_AGENT_BASEURI", - String.format("http://host.docker.internal:%d/", cryostatPort)); - container.addEnv( - "CRYOSTAT_AGENT_CALLBACK", String.format("http://localhost:%d/", hostAgentPort)); - container.start(); return Map.of( @@ -134,11 +113,6 @@ public void stop() { } } - @Override - public void setIntegrationTestContext(DevServicesContext context) { - containerNetworkId = context.containerNetworkId(); - } - private static int findFreePort() { try (ServerSocket ss = new ServerSocket(0)) { return ss.getLocalPort();