From 8a9874d350ceca8210f8b8605f428d63e35c55c8 Mon Sep 17 00:00:00 2001
From: jmatsuok <jmatsuok@redhat.com>
Date: Tue, 17 Dec 2024 21:11:04 -0500
Subject: [PATCH 01/10] Enforce TLS by default, add new configuration property
 to control this

---
 .../java/io/cryostat/agent/ConfigModule.java  |  9 ++++
 .../java/io/cryostat/agent/MainModule.java    | 51 ++++++++++++++-----
 .../META-INF/microprofile-config.properties   |  2 +
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java
index c09fa4c3..31976be3 100644
--- a/src/main/java/io/cryostat/agent/ConfigModule.java
+++ b/src/main/java/io/cryostat/agent/ConfigModule.java
@@ -232,6 +232,8 @@ public abstract class ConfigModule {
             "(?<host>[A-Za-z0-9-.]+)(?:\\[(?<script>.+)\\])?";
     private static final Pattern HOST_SCRIPT_PATTERN = Pattern.compile(HOST_SCRIPT_PATTERN_STRING);
 
+    public static final String CRYOSTAT_AGENT_TLS_ENABLED = "cryostat.agent.tls.enabled";
+
     @Provides
     @Singleton
     public static Config provideConfig() {
@@ -959,6 +961,13 @@ public static long provideCryostatSmartTriggerEvaluationPeriodMs(Config config)
         return config.getValue(CRYOSTAT_AGENT_SMART_TRIGGER_EVALUATION_PERIOD_MS, long.class);
     }
 
+    @Provides
+    @Singleton
+    @Named(CRYOSTAT_AGENT_TLS_ENABLED)
+    public static boolean provideCryostatAgentTlsEnabled(Config config) {
+        return config.getValue(CRYOSTAT_AGENT_TLS_ENABLED, boolean.class);
+    }
+
     public enum URIRange {
         LOOPBACK(u -> check(u, u2 -> true, InetAddress::isLoopbackAddress)),
         LINK_LOCAL(
diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index 803bce87..6ca2e656 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -236,10 +236,20 @@ public static SSLContext provideClientSslContext(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PASS_CHARSET)
                     String clientAuthKeyPassFileCharset,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_MANAGER_TYPE)
-                    String clientAuthKeyManagerType) {
+                    String clientAuthKeyManagerType,
+            @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) URI baseUri,
+            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
         try {
             KeyManager[] keyManagers = null;
-            if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent()) {
+            if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent() && tlsEnabled) {
+                if (!baseUri.getScheme().contains("https")) {
+                    throw new IllegalArgumentException(
+                            String.format(
+                                    "If TLS is enabled via the (%s) property, the base URI (%s)"
+                                            + " must be an https connection.",
+                                    ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED,
+                                    ConfigModule.CRYOSTAT_AGENT_BASEURI));
+                }
                 KeyStore ks = KeyStore.getInstance(clientAuthKeystoreType);
                 Optional<CharBuffer> keystorePass =
                         readPass(
@@ -302,13 +312,17 @@ public static SSLContext provideClientSslContext(
                     clearBuffer(keystorePass);
                     clearBuffer(keyPass);
                 }
-            } else if (clientAuthCertPath.isPresent() || clientAuthKeyPath.isPresent()) {
+            } else if (clientAuthCertPath.isPresent()
+                    || clientAuthKeyPath.isPresent()
+                    || tlsEnabled) {
                 throw new IllegalArgumentException(
                         String.format(
                                 "To use TLS client authentication, both the certificate (%s) and"
-                                        + " private key (%s) properties must be set.",
+                                    + " private key (%s) properties must be set. The (%s) property"
+                                    + " must be true as well.",
                                 ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_CERT_PATH,
-                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PATH));
+                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PATH,
+                                ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED));
             }
 
             X509TrustManager trustManager = null;
@@ -610,18 +624,28 @@ public static HttpClient provideHttpClient(
                     boolean verifyHostname,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_CONNECT_TIMEOUT_MS) int connectTimeout,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS) int responseTimeout,
-            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT) int retryCount) {
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT) int retryCount,
+            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
         SSLConnectionSocketFactory sslSocketFactory =
                 new SSLConnectionSocketFactory(
                         sslContext,
                         verifyHostname
                                 ? SSLConnectionSocketFactory.getDefaultHostnameVerifier()
                                 : NoopHostnameVerifier.INSTANCE);
-        Registry<ConnectionSocketFactory> socketFactoryRegistry =
-                RegistryBuilder.<ConnectionSocketFactory>create()
-                        .register("https", sslSocketFactory)
-                        .register("http", new PlainConnectionSocketFactory())
-                        .build();
+
+        Registry<ConnectionSocketFactory> socketFactoryRegistry;
+        if (tlsEnabled) {
+            socketFactoryRegistry =
+                    RegistryBuilder.<ConnectionSocketFactory>create()
+                            .register("https", sslSocketFactory)
+                            .build();
+        } else {
+            socketFactoryRegistry =
+                    RegistryBuilder.<ConnectionSocketFactory>create()
+                            .register("https", sslSocketFactory)
+                            .register("http", new PlainConnectionSocketFactory())
+                            .build();
+        }
         HttpClientConnectionManager connMan =
                 new BasicHttpClientConnectionManager(socketFactoryRegistry);
         return HttpClients.custom()
@@ -674,10 +698,11 @@ public static HttpServer provideHttpServer(
             ScheduledExecutorService executor,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBSERVER_HOST) String host,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBSERVER_PORT) int port,
-            @Named(HTTP_SERVER_SSL_CTX) Optional<SSLContext> sslContext) {
+            @Named(HTTP_SERVER_SSL_CTX) Optional<SSLContext> sslContext,
+            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
         try {
             HttpServer http;
-            if (sslContext.isEmpty()) {
+            if (sslContext.isEmpty() || !tlsEnabled) {
                 http = HttpServer.create(new InetSocketAddress(host, port), 0);
             } else {
                 http = HttpsServer.create(new InetSocketAddress(host, port), 0);
diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties
index a2a3748b..b57a71d5 100644
--- a/src/main/resources/META-INF/microprofile-config.properties
+++ b/src/main/resources/META-INF/microprofile-config.properties
@@ -82,3 +82,5 @@ cryostat.agent.harvester.max-size-b=0
 
 cryostat.agent.smart-trigger.definitions=
 cryostat.agent.smart-trigger.evaluation.period-ms=1000
+
+cryostat.agent.tls.enabled=true

From 351604125b43eff9187e302d1314474b2e9d1dae Mon Sep 17 00:00:00 2001
From: jmatsuok <jmatsuok@redhat.com>
Date: Tue, 17 Dec 2024 21:36:05 -0500
Subject: [PATCH 02/10] Document new option in README

---
 README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/README.md b/README.md
index edf5e797..8928a8e4 100644
--- a/README.md
+++ b/README.md
@@ -255,6 +255,7 @@ and how it advertises itself to a Cryostat server instance. Properties that requ
 - [ ] `cryostat.agent.callback.port` [`int`]: An override for the port portion of the `cryostat.agent.callback` URL.
 - [ ] `rht.insights.java.opt-out` [`boolean`]: for the Red Hat build of Cryostat, set this to true to disable data collection for Red Hat Insights. Defaults to `false`. Red Hat Insights data collection is always disabled for community builds of Cryostat.
 - [ ] `rht.insights.java.debug` [`boolean`]: for the Red Hat build of Cryostat, set this to true to enable debug logging for the Red Hat Insights Java Agent. Defaults to `false`. Red Hat Insights data collection is always disabled for community builds of Cryostat.
+- [ ] `cryostat.agent.tls.enabled` [`boolean`]: Specify wether the agent should use TLS by default, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
 
 These properties can be set by JVM system properties or by environment variables. For example, the property
 `cryostat.agent.baseuri` can be set using `-Dcryostat.agent.baseuri=https://mycryostat.example.com:1234/` or

From a500c9eae5b8a9f24da2473c34212007ac5a3451 Mon Sep 17 00:00:00 2001
From: jmatsuok <jmatsuok@redhat.com>
Date: Wed, 18 Dec 2024 11:07:41 -0500
Subject: [PATCH 03/10] Remove enforcement on internal webserver,g fix .equals

---
 src/main/java/io/cryostat/agent/MainModule.java | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index 6ca2e656..fd38e6d8 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -242,7 +242,7 @@ public static SSLContext provideClientSslContext(
         try {
             KeyManager[] keyManagers = null;
             if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent() && tlsEnabled) {
-                if (!baseUri.getScheme().contains("https")) {
+                if (!baseUri.getScheme().equals("https")) {
                     throw new IllegalArgumentException(
                             String.format(
                                     "If TLS is enabled via the (%s) property, the base URI (%s)"
@@ -698,11 +698,10 @@ public static HttpServer provideHttpServer(
             ScheduledExecutorService executor,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBSERVER_HOST) String host,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBSERVER_PORT) int port,
-            @Named(HTTP_SERVER_SSL_CTX) Optional<SSLContext> sslContext,
-            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
+            @Named(HTTP_SERVER_SSL_CTX) Optional<SSLContext> sslContext) {
         try {
             HttpServer http;
-            if (sslContext.isEmpty() || !tlsEnabled) {
+            if (sslContext.isEmpty()) {
                 http = HttpServer.create(new InetSocketAddress(host, port), 0);
             } else {
                 http = HttpsServer.create(new InetSocketAddress(host, port), 0);

From 84dbcc45486e1da9b5b567ecefb9b6b6067c20b2 Mon Sep 17 00:00:00 2001
From: jmatsuok <jmatsuok@redhat.com>
Date: Wed, 18 Dec 2024 11:13:48 -0500
Subject: [PATCH 04/10] Rename config property

---
 README.md                                                | 2 +-
 src/main/java/io/cryostat/agent/ConfigModule.java        | 9 +++++----
 src/main/java/io/cryostat/agent/MainModule.java          | 8 ++++----
 .../resources/META-INF/microprofile-config.properties    | 2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/README.md b/README.md
index 8928a8e4..cdfe6792 100644
--- a/README.md
+++ b/README.md
@@ -188,6 +188,7 @@ and how it advertises itself to a Cryostat server instance. Properties that requ
 - [ ] `cryostat.agent.webclient.tls.version` [`String`]: the version of TLS used for the Agent's client SSL context. Default `TLSv1.2`.
 - [ ] `cryostat.agent.webclient.tls.trust-all` [`boolean`]: control whether the agent trusts all certificates presented by the Cryostat server. Default `false`. This should only be overridden for development and testing purposes, never in production.
 - [ ] `cryostat.agent.webclient.tls.verify-hostname` [`boolean`]: control whether the agent verifies hostnames on certificates presented by the Cryostat server. Default `true`. This should only be overridden for development and testing purposes, never in production.
+- [ ] `cryostat.agent.webclient.tls.required` [`boolean`]: Specify wether the agent should use TLS by default, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
 - [ ] `cryostat.agent.webclient.tls.trustore.cert` [`list`]: the list of truststoreConfig objects with alias, path, and type properties for certificates to be stored in the agent's truststore. For example, 'cryostat.agent.webclient.tls.truststore.cert[0].type' would be the type of the first certificate in this list. A truststoreConfig object must contain all three properties to be a valid certificate entry.
 - [ ] `cryostat.agent.webclient.tls.truststore.type` [`String`]: the type of truststore used for the agent's client truststore. Default `JKS`.
 - [ ] `cryostat.agent.webclient.tls.truststore.path` [`String`]: the filepath to the agent's webclient truststore. This takes precedence over `cryostat.agent.webclient.tls.truststore.cert` and must be configured with the truststore's pass with `cryostat.agent.webclient.tls.truststore.pass.file` or `cryostat.agent.webclient.tls.truststore.pass`.
@@ -255,7 +256,6 @@ and how it advertises itself to a Cryostat server instance. Properties that requ
 - [ ] `cryostat.agent.callback.port` [`int`]: An override for the port portion of the `cryostat.agent.callback` URL.
 - [ ] `rht.insights.java.opt-out` [`boolean`]: for the Red Hat build of Cryostat, set this to true to disable data collection for Red Hat Insights. Defaults to `false`. Red Hat Insights data collection is always disabled for community builds of Cryostat.
 - [ ] `rht.insights.java.debug` [`boolean`]: for the Red Hat build of Cryostat, set this to true to enable debug logging for the Red Hat Insights Java Agent. Defaults to `false`. Red Hat Insights data collection is always disabled for community builds of Cryostat.
-- [ ] `cryostat.agent.tls.enabled` [`boolean`]: Specify wether the agent should use TLS by default, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
 
 These properties can be set by JVM system properties or by environment variables. For example, the property
 `cryostat.agent.baseuri` can be set using `-Dcryostat.agent.baseuri=https://mycryostat.example.com:1234/` or
diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java
index 31976be3..0ae940e3 100644
--- a/src/main/java/io/cryostat/agent/ConfigModule.java
+++ b/src/main/java/io/cryostat/agent/ConfigModule.java
@@ -137,6 +137,9 @@ public abstract class ConfigModule {
     public static final String CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT =
             "cryostat.agent.webclient.response.retry-count";
 
+    public static final String CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED =
+            "cryostat.agent.webclient.tls.required";
+
     public static final String CRYOSTAT_AGENT_WEBSERVER_HOST = "cryostat.agent.webserver.host";
     public static final String CRYOSTAT_AGENT_WEBSERVER_PORT = "cryostat.agent.webserver.port";
     public static final String CRYOSTAT_AGENT_WEBSERVER_TLS_VERSION =
@@ -232,8 +235,6 @@ public abstract class ConfigModule {
             "(?<host>[A-Za-z0-9-.]+)(?:\\[(?<script>.+)\\])?";
     private static final Pattern HOST_SCRIPT_PATTERN = Pattern.compile(HOST_SCRIPT_PATTERN_STRING);
 
-    public static final String CRYOSTAT_AGENT_TLS_ENABLED = "cryostat.agent.tls.enabled";
-
     @Provides
     @Singleton
     public static Config provideConfig() {
@@ -963,9 +964,9 @@ public static long provideCryostatSmartTriggerEvaluationPeriodMs(Config config)
 
     @Provides
     @Singleton
-    @Named(CRYOSTAT_AGENT_TLS_ENABLED)
+    @Named(CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED)
     public static boolean provideCryostatAgentTlsEnabled(Config config) {
-        return config.getValue(CRYOSTAT_AGENT_TLS_ENABLED, boolean.class);
+        return config.getValue(CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED, boolean.class);
     }
 
     public enum URIRange {
diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index fd38e6d8..af73670e 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -238,7 +238,7 @@ public static SSLContext provideClientSslContext(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_MANAGER_TYPE)
                     String clientAuthKeyManagerType,
             @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) URI baseUri,
-            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsEnabled) {
         try {
             KeyManager[] keyManagers = null;
             if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent() && tlsEnabled) {
@@ -247,7 +247,7 @@ public static SSLContext provideClientSslContext(
                             String.format(
                                     "If TLS is enabled via the (%s) property, the base URI (%s)"
                                             + " must be an https connection.",
-                                    ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED,
+                                    ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED,
                                     ConfigModule.CRYOSTAT_AGENT_BASEURI));
                 }
                 KeyStore ks = KeyStore.getInstance(clientAuthKeystoreType);
@@ -322,7 +322,7 @@ public static SSLContext provideClientSslContext(
                                     + " must be true as well.",
                                 ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_CERT_PATH,
                                 ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PATH,
-                                ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED));
+                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED));
             }
 
             X509TrustManager trustManager = null;
@@ -625,7 +625,7 @@ public static HttpClient provideHttpClient(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_CONNECT_TIMEOUT_MS) int connectTimeout,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS) int responseTimeout,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT) int retryCount,
-            @Named(ConfigModule.CRYOSTAT_AGENT_TLS_ENABLED) boolean tlsEnabled) {
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsEnabled) {
         SSLConnectionSocketFactory sslSocketFactory =
                 new SSLConnectionSocketFactory(
                         sslContext,
diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties
index b57a71d5..7fa3a563 100644
--- a/src/main/resources/META-INF/microprofile-config.properties
+++ b/src/main/resources/META-INF/microprofile-config.properties
@@ -16,6 +16,7 @@ cryostat.agent.webclient.response.retry-count=3
 cryostat.agent.webclient.tls.version=TLSv1.2
 cryostat.agent.webclient.tls.trust-all=false
 cryostat.agent.webclient.tls.verify-hostname=true
+cryostat.agent.webclient.tls.required=true
 cryostat.agent.webclient.tls.truststore.type=JKS
 cryostat.agent.webclient.tls.truststore.pass-charset=utf-8
 
@@ -83,4 +84,3 @@ cryostat.agent.harvester.max-size-b=0
 cryostat.agent.smart-trigger.definitions=
 cryostat.agent.smart-trigger.evaluation.period-ms=1000
 
-cryostat.agent.tls.enabled=true

From 2b2638674b664da90baae5fe502d90f27877f427 Mon Sep 17 00:00:00 2001
From: jmatsuok <jmatsuok@redhat.com>
Date: Fri, 20 Dec 2024 16:28:38 -0500
Subject: [PATCH 05/10] Separate out TLS requirement check, rename variable

---
 src/main/java/io/cryostat/agent/MainModule.java | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index af73670e..4aecc9a8 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -238,10 +238,10 @@ public static SSLContext provideClientSslContext(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_MANAGER_TYPE)
                     String clientAuthKeyManagerType,
             @Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) URI baseUri,
-            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsEnabled) {
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsRequired) {
         try {
             KeyManager[] keyManagers = null;
-            if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent() && tlsEnabled) {
+            if (tlsRequired) {
                 if (!baseUri.getScheme().equals("https")) {
                     throw new IllegalArgumentException(
                             String.format(
@@ -250,6 +250,8 @@ public static SSLContext provideClientSslContext(
                                     ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED,
                                     ConfigModule.CRYOSTAT_AGENT_BASEURI));
                 }
+            }
+            if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent()) {
                 KeyStore ks = KeyStore.getInstance(clientAuthKeystoreType);
                 Optional<CharBuffer> keystorePass =
                         readPass(
@@ -312,17 +314,13 @@ public static SSLContext provideClientSslContext(
                     clearBuffer(keystorePass);
                     clearBuffer(keyPass);
                 }
-            } else if (clientAuthCertPath.isPresent()
-                    || clientAuthKeyPath.isPresent()
-                    || tlsEnabled) {
+            } else if (clientAuthCertPath.isPresent() || clientAuthKeyPath.isPresent()) {
                 throw new IllegalArgumentException(
                         String.format(
                                 "To use TLS client authentication, both the certificate (%s) and"
-                                    + " private key (%s) properties must be set. The (%s) property"
-                                    + " must be true as well.",
+                                        + " private key (%s) properties must be set.",
                                 ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_CERT_PATH,
-                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PATH,
-                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED));
+                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_CLIENT_AUTH_KEY_PATH));
             }
 
             X509TrustManager trustManager = null;

From e68fc5ca5938914f31bf329f4f23fec56e96962f Mon Sep 17 00:00:00 2001
From: Andrew Azores <aazores@redhat.com>
Date: Fri, 20 Dec 2024 16:42:22 -0500
Subject: [PATCH 06/10] combine conditions

---
 src/main/java/io/cryostat/agent/MainModule.java | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index 4aecc9a8..2d45487f 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -241,15 +241,13 @@ public static SSLContext provideClientSslContext(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsRequired) {
         try {
             KeyManager[] keyManagers = null;
-            if (tlsRequired) {
-                if (!baseUri.getScheme().equals("https")) {
-                    throw new IllegalArgumentException(
-                            String.format(
-                                    "If TLS is enabled via the (%s) property, the base URI (%s)"
-                                            + " must be an https connection.",
-                                    ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED,
-                                    ConfigModule.CRYOSTAT_AGENT_BASEURI));
-                }
+            if (tlsRequired && !baseUri.getScheme().equals("https")) {
+                throw new IllegalArgumentException(
+                        String.format(
+                                "If TLS is enabled via the (%s) property, the base URI (%s)"
+                                        + " must be an https connection.",
+                                ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED,
+                                ConfigModule.CRYOSTAT_AGENT_BASEURI));
             }
             if (clientAuthCertPath.isPresent() && clientAuthKeyPath.isPresent()) {
                 KeyStore ks = KeyStore.getInstance(clientAuthKeystoreType);

From 257ba6fc1a90ffef3714b16aed767e8b67be99f6 Mon Sep 17 00:00:00 2001
From: Andrew Azores <aazores@redhat.com>
Date: Fri, 20 Dec 2024 16:43:20 -0500
Subject: [PATCH 07/10] rename

---
 src/main/java/io/cryostat/agent/MainModule.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index 2d45487f..482886cc 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -621,7 +621,7 @@ public static HttpClient provideHttpClient(
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_CONNECT_TIMEOUT_MS) int connectTimeout,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS) int responseTimeout,
             @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_RETRY_COUNT) int retryCount,
-            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsEnabled) {
+            @Named(ConfigModule.CRYOSTAT_AGENT_WEBCLIENT_TLS_REQUIRED) boolean tlsRequired) {
         SSLConnectionSocketFactory sslSocketFactory =
                 new SSLConnectionSocketFactory(
                         sslContext,
@@ -630,7 +630,7 @@ public static HttpClient provideHttpClient(
                                 : NoopHostnameVerifier.INSTANCE);
 
         Registry<ConnectionSocketFactory> socketFactoryRegistry;
-        if (tlsEnabled) {
+        if (tlsRequired) {
             socketFactoryRegistry =
                     RegistryBuilder.<ConnectionSocketFactory>create()
                             .register("https", sslSocketFactory)

From cb561cfc750267e66c7549000eff69d2bf825e43 Mon Sep 17 00:00:00 2001
From: Andrew Azores <aazores@redhat.com>
Date: Fri, 20 Dec 2024 16:45:26 -0500
Subject: [PATCH 08/10] refactor

---
 .../java/io/cryostat/agent/MainModule.java    | 20 ++++++-------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java
index 482886cc..6bd4ea17 100644
--- a/src/main/java/io/cryostat/agent/MainModule.java
+++ b/src/main/java/io/cryostat/agent/MainModule.java
@@ -86,7 +86,6 @@
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.protocol.HttpClientContext;
-import org.apache.http.config.Registry;
 import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.HttpClientConnectionManager;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
@@ -629,21 +628,14 @@ public static HttpClient provideHttpClient(
                                 ? SSLConnectionSocketFactory.getDefaultHostnameVerifier()
                                 : NoopHostnameVerifier.INSTANCE);
 
-        Registry<ConnectionSocketFactory> socketFactoryRegistry;
-        if (tlsRequired) {
-            socketFactoryRegistry =
-                    RegistryBuilder.<ConnectionSocketFactory>create()
-                            .register("https", sslSocketFactory)
-                            .build();
-        } else {
-            socketFactoryRegistry =
-                    RegistryBuilder.<ConnectionSocketFactory>create()
-                            .register("https", sslSocketFactory)
-                            .register("http", new PlainConnectionSocketFactory())
-                            .build();
+        RegistryBuilder<ConnectionSocketFactory> socketFactoryRegistryBuilder =
+                RegistryBuilder.<ConnectionSocketFactory>create()
+                        .register("https", sslSocketFactory);
+        if (!tlsRequired) {
+            socketFactoryRegistryBuilder.register("http", new PlainConnectionSocketFactory());
         }
         HttpClientConnectionManager connMan =
-                new BasicHttpClientConnectionManager(socketFactoryRegistry);
+                new BasicHttpClientConnectionManager(socketFactoryRegistryBuilder.build());
         return HttpClients.custom()
                 .setSSLContext(sslContext)
                 .setSSLSocketFactory(sslSocketFactory)

From 4bb94c68318b0846311bb590367c69861f69814b Mon Sep 17 00:00:00 2001
From: Andrew Azores <aazores@redhat.com>
Date: Fri, 20 Dec 2024 16:46:08 -0500
Subject: [PATCH 09/10] rephrase

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index cdfe6792..5583c3ec 100644
--- a/README.md
+++ b/README.md
@@ -188,7 +188,7 @@ and how it advertises itself to a Cryostat server instance. Properties that requ
 - [ ] `cryostat.agent.webclient.tls.version` [`String`]: the version of TLS used for the Agent's client SSL context. Default `TLSv1.2`.
 - [ ] `cryostat.agent.webclient.tls.trust-all` [`boolean`]: control whether the agent trusts all certificates presented by the Cryostat server. Default `false`. This should only be overridden for development and testing purposes, never in production.
 - [ ] `cryostat.agent.webclient.tls.verify-hostname` [`boolean`]: control whether the agent verifies hostnames on certificates presented by the Cryostat server. Default `true`. This should only be overridden for development and testing purposes, never in production.
-- [ ] `cryostat.agent.webclient.tls.required` [`boolean`]: Specify wether the agent should use TLS by default, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
+- [ ] `cryostat.agent.webclient.tls.required` [`boolean`]: Specify wether the agent should require TLS on Cryostat server connections, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
 - [ ] `cryostat.agent.webclient.tls.trustore.cert` [`list`]: the list of truststoreConfig objects with alias, path, and type properties for certificates to be stored in the agent's truststore. For example, 'cryostat.agent.webclient.tls.truststore.cert[0].type' would be the type of the first certificate in this list. A truststoreConfig object must contain all three properties to be a valid certificate entry.
 - [ ] `cryostat.agent.webclient.tls.truststore.type` [`String`]: the type of truststore used for the agent's client truststore. Default `JKS`.
 - [ ] `cryostat.agent.webclient.tls.truststore.path` [`String`]: the filepath to the agent's webclient truststore. This takes precedence over `cryostat.agent.webclient.tls.truststore.cert` and must be configured with the truststore's pass with `cryostat.agent.webclient.tls.truststore.pass.file` or `cryostat.agent.webclient.tls.truststore.pass`.

From 705cf38ed7887e8d0d04e07cdd2f6da6d241e029 Mon Sep 17 00:00:00 2001
From: Andrew Azores <aazores@redhat.com>
Date: Fri, 20 Dec 2024 16:46:23 -0500
Subject: [PATCH 10/10] typo

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 5583c3ec..0ca3f729 100644
--- a/README.md
+++ b/README.md
@@ -188,7 +188,7 @@ and how it advertises itself to a Cryostat server instance. Properties that requ
 - [ ] `cryostat.agent.webclient.tls.version` [`String`]: the version of TLS used for the Agent's client SSL context. Default `TLSv1.2`.
 - [ ] `cryostat.agent.webclient.tls.trust-all` [`boolean`]: control whether the agent trusts all certificates presented by the Cryostat server. Default `false`. This should only be overridden for development and testing purposes, never in production.
 - [ ] `cryostat.agent.webclient.tls.verify-hostname` [`boolean`]: control whether the agent verifies hostnames on certificates presented by the Cryostat server. Default `true`. This should only be overridden for development and testing purposes, never in production.
-- [ ] `cryostat.agent.webclient.tls.required` [`boolean`]: Specify wether the agent should require TLS on Cryostat server connections, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
+- [ ] `cryostat.agent.webclient.tls.required` [`boolean`]: Specify whether the agent should require TLS on Cryostat server connections, expecting the base URI to be an https connection with a certificate it trusts. Defaults to `true`. Should only be disabled for testing/prototyping purposes.
 - [ ] `cryostat.agent.webclient.tls.trustore.cert` [`list`]: the list of truststoreConfig objects with alias, path, and type properties for certificates to be stored in the agent's truststore. For example, 'cryostat.agent.webclient.tls.truststore.cert[0].type' would be the type of the first certificate in this list. A truststoreConfig object must contain all three properties to be a valid certificate entry.
 - [ ] `cryostat.agent.webclient.tls.truststore.type` [`String`]: the type of truststore used for the agent's client truststore. Default `JKS`.
 - [ ] `cryostat.agent.webclient.tls.truststore.path` [`String`]: the filepath to the agent's webclient truststore. This takes precedence over `cryostat.agent.webclient.tls.truststore.cert` and must be configured with the truststore's pass with `cryostat.agent.webclient.tls.truststore.pass.file` or `cryostat.agent.webclient.tls.truststore.pass`.