Skip to content
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

fix(tls): use TLS by default in the Agent Client #747

Merged
merged 9 commits into from
Dec 20, 2024
1 change: 1 addition & 0 deletions compose/cryostat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ services:
CRYOSTAT_DISCOVERY_JDP_ENABLED: ${CRYOSTAT_DISCOVERY_JDP_ENABLED:-true}
CRYOSTAT_DISCOVERY_PODMAN_ENABLED: ${CRYOSTAT_DISCOVERY_PODMAN_ENABLED:-true}
CRYOSTAT_DISCOVERY_DOCKER_ENABLED: ${CRYOSTAT_DISCOVERY_DOCKER_ENABLED:-true}
CRYOSTAT_AGENT_TLS_REQUIRED: ${ENFORCE_AGENT_TLS:-true}
JAVA_OPTS_APPEND: >-
-XX:+FlightRecorder
-XX:StartFlightRecording=filename=/tmp/,name=onstart,settings=default,disk=true,maxage=5m
Expand Down
2 changes: 2 additions & 0 deletions smoketest.bash
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ DEPLOY_GRAFANA=${DEPLOY_GRAFANA:-true}
DRY_RUN=${DRY_RUN:-false}
USE_TLS=${USE_TLS:-true}
SAMPLE_APPS_USE_TLS=${SAMPLE_APPS_USE_TLS:-true}
ENFORCE_AGENT_TLS=${ENFORCE_AGENT_TLS:-true}

display_usage() {
echo "Usage:"
Expand Down Expand Up @@ -91,6 +92,7 @@ while getopts "hs:prGtAOVXc:bnk" opt; do
;;
A)
SAMPLE_APPS_USE_TLS=false
ENFORCE_AGENT_TLS=false
;;
O)
PULL_IMAGES=false
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/io/cryostat/ConfigProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,6 @@ public class ConfigProperties {
public static final String SSL_TRUSTSTORE_DIR = "ssl.truststore.dir";

public static final String URI_RANGE = "cryostat.target.uri-range";

public static final String AGENT_TLS_REQUIRED = "cryostat.agent.tls.required";
}
29 changes: 29 additions & 0 deletions src/main/java/io/cryostat/discovery/Discovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Set;
import java.util.UUID;

import io.cryostat.ConfigProperties;
import io.cryostat.discovery.DiscoveryPlugin.PluginCallback;
import io.cryostat.targets.TargetConnectionManager;
import io.cryostat.util.URIUtil;
Expand Down Expand Up @@ -96,6 +97,9 @@ public class Discovery {
@ConfigProperty(name = "cryostat.discovery.plugins.ping-period")
Duration discoveryPingPeriod;

@ConfigProperty(name = ConfigProperties.AGENT_TLS_REQUIRED)
boolean agentTlsRequired;

@Inject Logger logger;
@Inject ObjectMapper mapper;
@Inject EventBus bus;
Expand Down Expand Up @@ -210,6 +214,14 @@ public PluginRegistration register(@Context RoutingContext ctx, JsonObject body)
+ " current URI range settings",
remoteURI));
}

if (agentTlsRequired && !callbackUri.getScheme().equals("https")) {
throw new BadRequestException(
String.format(
"TLS for agent connections is required by (%s)",
ConfigProperties.AGENT_TLS_REQUIRED));
}

URI location;
DiscoveryPlugin plugin;
if (StringUtils.isNotBlank(pluginId) && StringUtils.isNotBlank(priorToken)) {
Expand Down Expand Up @@ -327,6 +339,23 @@ public void publish(
+ " current URI range settings",
b.target.connectUrl));
}
if (!uriUtil.isJmxUrl(b.target.connectUrl)) {
if (agentTlsRequired && !b.target.connectUrl.getScheme().equals("https")) {
throw new BadRequestException(
String.format(
"TLS for agent connections is required by (%s)",
ConfigProperties.AGENT_TLS_REQUIRED));
}
if (!b.target.connectUrl.getScheme().equals("https")
&& !b.target.connectUrl.getScheme().equals("http")) {
throw new BadRequestException(
String.format(
"Target connect URL is neither JMX nor HTTP(S): (%s)",
b.target.connectUrl.toString()));
}
}
// Continue since we've verified the connect URL is either JMX or HTTPS with
// TLS verification enabled, or HTTP with TLS verification disabled.
b.target.discoveryNode = b;
b.target.discoveryNode.parent = plugin.realm;
b.parent = plugin.realm;
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/io/cryostat/targets/AgentClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ public class AgentClient {

public static final String NULL_CREDENTIALS = "No credentials found for agent";

@ConfigProperty(name = ConfigProperties.AGENT_TLS_REQUIRED)
private boolean tlsEnabled;

private final Target target;
private final WebClient webClient;
private final Duration httpTimeout;
Expand Down Expand Up @@ -443,6 +446,14 @@ private <T> Uni<HttpResponse<T>> invoke(
HttpMethod mtd, String path, Buffer payload, BodyCodec<T> codec) {
logger.debugv("{0} {1} {2}", mtd, getUri(), path);

if (tlsEnabled && !getUri().getScheme().equals("https")) {
throw new IllegalArgumentException(
String.format(
"Agent is configured with TLS enabled (%s) but the agent URI is not an"
+ " https connection.",
ConfigProperties.AGENT_TLS_REQUIRED));
}

Credential credential =
DiscoveryPlugin.<DiscoveryPlugin>find("callback", getUri())
.singleResult()
Expand Down
17 changes: 15 additions & 2 deletions src/main/java/io/cryostat/targets/AgentConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.cryostat.targets;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.util.Set;

Expand All @@ -28,6 +29,7 @@
import org.openjdk.jmc.rjmx.common.IConnectionHandle;
import org.openjdk.jmc.rjmx.common.ServiceNotAvailableException;

import io.cryostat.ConfigProperties;
import io.cryostat.core.net.CryostatFlightRecorderService;
import io.cryostat.core.net.JFRConnection;
import io.cryostat.core.templates.RemoteTemplateService;
Expand All @@ -40,6 +42,7 @@

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.jboss.logging.Logger;

class AgentConnection implements JFRConnection {
Expand All @@ -48,6 +51,9 @@ class AgentConnection implements JFRConnection {
private final TemplateService customTemplateService;
private final Logger logger = Logger.getLogger(getClass());

@ConfigProperty(name = ConfigProperties.AGENT_TLS_REQUIRED)
private static boolean TLS_REQUIRED;

AgentConnection(AgentClient client, TemplateService customTemplateService) {
this.client = client;
this.customTemplateService = customTemplateService;
Expand Down Expand Up @@ -156,8 +162,15 @@ public static class Factory {
@Inject S3TemplateService customTemplateService;
@Inject Logger logger;

public AgentConnection createConnection(Target target) {
return new AgentConnection(clientFactory.create(target), customTemplateService);
public AgentConnection createConnection(Target target) throws MalformedURLException {
if (TLS_REQUIRED && target.connectUrl.getScheme().equals("https")) {
return new AgentConnection(clientFactory.create(target), customTemplateService);
} else {
throw new MalformedURLException(
String.format(
"Agent connections are required to be TLS by (%s)",
ConfigProperties.AGENT_TLS_REQUIRED));
}
}
}
}
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/util/URIUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public boolean validateUri(URI uri) throws MalformedURLException {
return URIRange.fromString(uriRange).validate(uri.getHost());
}

boolean isJmxUrl(URI uri) {
public boolean isJmxUrl(URI uri) {
try {
new JMXServiceURL(uri.toString());
return true;
Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ cryostat.http.proxy.path=/

cryostat.target.uri-range=PUBLIC

cryostat.agent.tls.required=true

conf-dir=/opt/cryostat.d
templates-dir=${conf-dir}/templates.d
preset-templates-dir=${conf-dir}/presets.d
Expand Down
Loading