Skip to content

Commit

Permalink
fix(tls): use TLS by default in the Agent Client (#747)
Browse files Browse the repository at this point in the history
Co-authored-by: Andrew Azores <[email protected]>
  • Loading branch information
Josh-Matsuoka and andrewazores authored Dec 20, 2024
1 parent cef5e78 commit 443c9c0
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 3 deletions.
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

0 comments on commit 443c9c0

Please sign in to comment.