Skip to content

Commit

Permalink
fix(jmx): restore error codes for auth and TLS connection failures (#350
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@andrewazores
andrewazores authored Apr 15, 2024
1 parent d090ed0 commit 794db33
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 55 deletions.
19 changes: 0 additions & 19 deletions src/main/java/io/cryostat/ExceptionMappers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
import java.util.concurrent.CompletionException;
import java.util.concurrent.ExecutionException;

import org.openjdk.jmc.rjmx.ConnectionException;

import io.cryostat.targets.TargetConnectionManager;
import io.cryostat.util.EntityExistsException;

import com.nimbusds.jwt.proc.BadJWTException;
Expand Down Expand Up @@ -89,22 +86,6 @@ public RestResponse<Void> mapIllegalArgumentException(IllegalArgumentException e
return RestResponse.status(HttpResponseStatus.BAD_REQUEST.code());
}

@ServerExceptionMapper
public RestResponse<Void> mapJmxConnectionException(ConnectionException ex) {
logger.warn(ex);
return RestResponse.status(HttpResponseStatus.BAD_GATEWAY.code());
}

@ServerExceptionMapper
public RestResponse<Void> mapFlightRecorderException(
org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException ex) {
logger.warn(ex);
if (TargetConnectionManager.isJmxAuthFailure(ex)) {
return RestResponse.status(HttpResponseStatus.FORBIDDEN.code());
}
return RestResponse.status(HttpResponseStatus.BAD_GATEWAY.code());
}

@ServerExceptionMapper
public RestResponse<Void> mapMutinyTimeoutException(TimeoutException ex) {
logger.warn(ex);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/recordings/RecordingHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@ public RecordingNotFoundException(Pair<String, String> key) {
}
}

static class SnapshotCreationException extends Exception {
public static class SnapshotCreationException extends Exception {
public SnapshotCreationException(String message) {
super(message);
}
Expand Down
87 changes: 53 additions & 34 deletions src/main/java/io/cryostat/targets/TargetConnectionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.concurrent.Semaphore;

import javax.management.remote.JMXServiceURL;
import javax.naming.ServiceUnavailableException;
import javax.security.sasl.SaslException;

import org.openjdk.jmc.rjmx.ConnectionException;
Expand All @@ -42,6 +41,7 @@
import io.cryostat.core.net.JFRConnectionToolkit;
import io.cryostat.credentials.Credential;
import io.cryostat.expressions.MatchExpressionEvaluator;
import io.cryostat.recordings.RecordingHelper.SnapshotCreationException;
import io.cryostat.targets.Target.EventKind;
import io.cryostat.targets.Target.TargetDiscovery;

Expand All @@ -54,6 +54,7 @@
import io.quarkus.vertx.ConsumeEvent;
import io.smallrye.mutiny.Uni;
import io.smallrye.mutiny.unchecked.Unchecked;
import io.vertx.ext.web.handler.HttpException;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.transaction.Transactional;
Expand Down Expand Up @@ -196,13 +197,24 @@ public <T> Uni<T> executeConnectedTaskUni(Target target, ConnectedTask<T> task)
}
}))
.onFailure(RuntimeException.class)
.transform(this::unwrapRuntimeException)
.transform(t -> unwrapNestedException(RuntimeException.class, t))
.onFailure()
.invoke(logger::warn)
.onFailure(t -> isTargetConnectionFailure(t) || isUnknownTargetFailure(t))
.onFailure(this::isJmxAuthFailure)
.transform(t -> new HttpException(427, t))
.onFailure(this::isJmxSslFailure)
.transform(t -> new HttpException(502, t))
.onFailure(this::isServiceTypeFailure)
.transform(t -> new HttpException(504, t))
.onFailure(
t ->
!(t instanceof HttpException)
&& !(t instanceof SnapshotCreationException))
.retry()
.withBackOff(failedBackoff)
.expireIn(failedTimeout.plusMillis(System.currentTimeMillis()).toMillis());
.expireIn(failedTimeout.plusMillis(System.currentTimeMillis()).toMillis())
.onFailure(this::isTargetConnectionFailure)
.transform(t -> new HttpException(504, t));
}

public <T> T executeConnectedTask(Target target, ConnectedTask<T> task) {
Expand All @@ -220,13 +232,24 @@ public <T> Uni<T> executeDirect(
}
}))
.onFailure(RuntimeException.class)
.transform(this::unwrapRuntimeException)
.transform(t -> unwrapNestedException(RuntimeException.class, t))
.onFailure()
.invoke(logger::warn)
.onFailure(t -> isTargetConnectionFailure(t) || isUnknownTargetFailure(t))
.onFailure(this::isJmxAuthFailure)
.transform(t -> new HttpException(427, t))
.onFailure(this::isJmxSslFailure)
.transform(t -> new HttpException(502, t))
.onFailure(this::isServiceTypeFailure)
.transform(t -> new HttpException(504, t))
.onFailure(
t ->
!(t instanceof HttpException)
&& !(t instanceof SnapshotCreationException))
.retry()
.withBackOff(failedBackoff)
.expireIn(failedTimeout.plusMillis(System.currentTimeMillis()).toMillis());
.expireIn(failedTimeout.plusMillis(System.currentTimeMillis()).toMillis())
.onFailure(this::isTargetConnectionFailure)
.transform(t -> new HttpException(504, t));
}

/**
Expand Down Expand Up @@ -374,17 +397,21 @@ public interface ConnectedTask<T> {
T execute(JFRConnection connection) throws Exception;
}

public Throwable unwrapRuntimeException(Throwable t) {
public Throwable unwrapNestedException(Class<?> klazz, Throwable t) {
final int maxDepth = 10;
int depth = 0;
Throwable cause = t;
while (cause instanceof RuntimeException && depth++ < maxDepth) {
while (klazz.isInstance(t) && depth++ < maxDepth) {
var c = cause.getCause();
if (c == null) {
break;
}
cause = cause.getCause();
}
return cause;
}

public static boolean isTargetConnectionFailure(Throwable t) {
private boolean isTargetConnectionFailure(Throwable t) {
if (!(t instanceof Exception)) {
return false;
}
Expand All @@ -393,16 +420,26 @@ public static boolean isTargetConnectionFailure(Throwable t) {
|| ExceptionUtils.indexOfType(e, FlightRecorderException.class) >= 0;
}

public static boolean isJmxAuthFailure(Throwable t) {
/**
* Check if the exception happened because the connection required authentication, and we had no
* credentials to present.
*/
private boolean isJmxAuthFailure(Throwable t) {
if (!(t instanceof Exception)) {
return false;
}
Exception e = (Exception) t;
return ExceptionUtils.indexOfType(e, SecurityException.class) >= 0
return ExceptionUtils.indexOfType(e, javax.security.auth.login.FailedLoginException.class)
>= 0
|| ExceptionUtils.indexOfType(e, SecurityException.class) >= 0
|| ExceptionUtils.indexOfType(e, SaslException.class) >= 0;
}

public static boolean isJmxSslFailure(Throwable t) {
/**
* Check if the exception happened because the connection presented an SSL/TLS cert which we
* don't trust.
*/
private boolean isJmxSslFailure(Throwable t) {
if (!(t instanceof Exception)) {
return false;
}
Expand All @@ -412,7 +449,7 @@ public static boolean isJmxSslFailure(Throwable t) {
}

/** Check if the exception happened because the port connected to a non-JMX service. */
public static boolean isServiceTypeFailure(Throwable t) {
private boolean isServiceTypeFailure(Throwable t) {
if (!(t instanceof Exception)) {
return false;
}
Expand All @@ -421,23 +458,9 @@ public static boolean isServiceTypeFailure(Throwable t) {
&& ExceptionUtils.indexOfType(e, SocketTimeoutException.class) >= 0;
}

public static boolean isUnknownTargetFailure(Throwable t) {
if (!(t instanceof Exception)) {
return false;
}
Exception e = (Exception) t;
return ExceptionUtils.indexOfType(e, java.net.UnknownHostException.class) >= 0
|| ExceptionUtils.indexOfType(e, java.rmi.UnknownHostException.class) >= 0
|| ExceptionUtils.indexOfType(e, ServiceUnavailableException.class) >= 0;
}

@Name("io.cryostat.net.TargetConnectionManager.TargetConnectionOpened")
@Name("io.cryostat.targets.TargetConnectionManager.TargetConnectionOpened")
@Label("Target Connection Opened")
@Category("Cryostat")
// @SuppressFBWarnings(
// value = "URF_UNREAD_FIELD",
// justification = "The event fields are recorded with JFR instead of accessed
// directly")
public static class TargetConnectionOpened extends Event {
String serviceUri;
boolean exceptionThrown;
Expand All @@ -452,13 +475,9 @@ void setExceptionThrown(boolean exceptionThrown) {
}
}

@Name("io.cryostat.net.TargetConnectionManager.TargetConnectionClosed")
@Name("io.cryostat.targets.TargetConnectionManager.TargetConnectionClosed")
@Label("Target Connection Closed")
@Category("Cryostat")
// @SuppressFBWarnings(
// value = "URF_UNREAD_FIELD",
// justification = "The event fields are recorded with JFR instead of accessed
// directly")
public static class TargetConnectionClosed extends Event {
URI serviceUri;
boolean exceptionThrown;
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/itest/TargetEventsGetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void testGetTargetEventsV2WithQueryReturnsRequestedEvents() throws Except
LinkedHashMap<String, Object> expectedResults = new LinkedHashMap<String, Object>();
expectedResults.put("name", "Target Connection Opened");
expectedResults.put(
"typeId", "io.cryostat.net.TargetConnectionManager.TargetConnectionOpened");
"typeId", "io.cryostat.targets.TargetConnectionManager.TargetConnectionOpened");
expectedResults.put("description", "");
expectedResults.put("category", List.of("Cryostat"));
expectedResults.put(
Expand Down

0 comments on commit 794db33

Please sign in to comment.