Skip to content

Commit b65ef0e

Browse files
fix(security): replace insecure TrustManager with default JVM trust validation
HIGH-severity TLS validation bypass (CWE-295, code-scanning alert #8). The previous HttpClientFactory honored AxonFlowConfig.insecureSkipVerify(true) as a single-flag opt-in to a permissive X509TrustManager whose checkClientTrusted / checkServerTrusted methods returned void and whose getAcceptedIssuers returned an empty array. With the flag set, the SDK trusted ANY server certificate over HTTPS, including attacker-presented certs in MITM scenarios. Fix: double-gate the insecure path so it activates only when BOTH - AxonFlowConfig#insecureSkipVerify(true) is set on the builder, AND - AXONFLOW_INSECURE_TLS is set to "true" (case-insensitive) or "1" in the runtime environment are present. Otherwise the JVM's default TrustManager (validating against the system + JDK trust store) is used. When the builder flag is set but the env var is not, the SDK logs a warning at client construction time and keeps verification enabled. When the insecure path actually activates, a loud *** SECURITY WARNING *** is logged. Rationale for keeping a development carve-out: AxonFlow demos and self-hosted local stacks ship with self-signed agent certs in some configurations; teams need a way to opt in for local dev without a DIY trust-store override. The double-gate makes accidental production bypass much harder: a stray builder flag in app code is no longer sufficient. Tests: HttpClientFactoryTest gains two assertions. The first verifies that with insecureSkipVerify(true) but no env var, the resulting OkHttpClient retains OkHttp's default OkHostnameVerifier (not the permissive (h, s) -> true verifier). The second verifies the env-var gate helper returns false when AXONFLOW_INSECURE_TLS is unset, which is the default in CI and dev shells. Default behavior in production: standard TLS validation against the JDK + system trust store. No regressions in the existing 1224-test suite (mvn test). Resolves code-scanning alert #8.
1 parent fa27583 commit b65ef0e

4 files changed

Lines changed: 84 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
- **BREAKING (test API):** Package-private `TelemetryReporter.isEnabled` and `TelemetryReporter.sendPing` overloads no longer accept a `String doNotTrack` parameter. The remaining `String axonflowTelemetry` parameter is the sole opt-out signal accepted by the testability surface.
1717

18+
### Security
19+
20+
- **TLS verification bypass closed (CWE-295).** `HttpClientFactory` previously honored `insecureSkipVerify(true)` on `AxonFlowConfig` as a single-flag opt-in to a permissive `X509TrustManager` that accepted ANY server certificate, including attacker-presented certificates in MITM scenarios. The insecure path is now double-gated: it activates only if both `insecureSkipVerify(true)` is set on the builder AND the `AXONFLOW_INSECURE_TLS` environment variable is set to `true` (or `1`). When the builder flag is set without the env var, the SDK logs a warning and keeps the JVM's default `TrustManager` in place. A loud `*** SECURITY WARNING ***` is logged whenever the insecure path actually activates. Default behavior — and behavior in production environments without the env var — uses standard JDK + system trust-store validation. Resolves code-scanning alert #8.
21+
1822
### Fixed
1923

2024
- The one-line `DO_NOT_TRACK=1 is deprecated...` `logger.warn` is no longer emitted. Removing the warning eliminates log noise that previously appeared on every telemetry decision when `DO_NOT_TRACK=1` was set.

src/main/java/com/getaxonflow/sdk/AxonFlowConfig.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,22 @@ public Builder debug(boolean debug) {
463463
}
464464

465465
/**
466-
* Skips SSL certificate verification.
466+
* Requests that TLS certificate verification be skipped for outbound HTTPS calls.
467467
*
468-
* <p><strong>Warning:</strong> Only use this in development/testing.
468+
* <p><strong>Security:</strong> Setting this flag is <em>not by itself</em> sufficient to
469+
* disable TLS verification. To actually disable verification, the environment variable {@code
470+
* AXONFLOW_INSECURE_TLS} must <strong>also</strong> be set to {@code "true"} or {@code "1"} in
471+
* the runtime environment. This double-gate is intentional defense-in-depth: a stray builder
472+
* call in application code cannot silently bypass certificate validation in production.
469473
*
470-
* @param insecureSkipVerify true to skip verification
474+
* <p>If the builder flag is set but the environment variable is not, the SDK will log a
475+
* warning at client construction time and keep TLS verification enabled.
476+
*
477+
* <p><strong>Use cases:</strong> local development against self-signed certificates only.
478+
* <strong>Never</strong> enable in production.
479+
*
480+
* @param insecureSkipVerify true to request that verification be skipped (also requires
481+
* {@code AXONFLOW_INSECURE_TLS=true} environment variable)
471482
* @return this builder
472483
*/
473484
public Builder insecureSkipVerify(boolean insecureSkipVerify) {

src/main/java/com/getaxonflow/sdk/util/HttpClientFactory.java

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,27 @@ public final class HttpClientFactory {
3131

3232
private static final Logger logger = LoggerFactory.getLogger(HttpClientFactory.class);
3333

34+
/**
35+
* Environment variable that must be explicitly set to {@code "true"} (or {@code "1"}) to allow
36+
* disabling TLS certificate verification, in addition to {@code insecureSkipVerify(true)} on the
37+
* config builder. Both the builder flag AND this environment variable are required to activate
38+
* the insecure path. This is intentional defense-in-depth: a stray builder flag in application
39+
* code is not sufficient to silently disable TLS validation in production environments.
40+
*/
41+
static final String INSECURE_TLS_ENV_VAR = "AXONFLOW_INSECURE_TLS";
42+
3443
private HttpClientFactory() {
3544
// Utility class
3645
}
3746

3847
/**
3948
* Creates an OkHttpClient configured according to the SDK configuration.
4049
*
50+
* <p>By default, the client uses the JVM's default {@code TrustManager}, which validates server
51+
* certificates against the system + JDK trust store. Certificate validation is only disabled if
52+
* {@link AxonFlowConfig#isInsecureSkipVerify()} is {@code true} AND the environment variable
53+
* {@value #INSECURE_TLS_ENV_VAR} is set to {@code "true"} or {@code "1"}.
54+
*
4155
* @param config the SDK configuration
4256
* @return a configured OkHttpClient
4357
*/
@@ -50,7 +64,17 @@ public static OkHttpClient create(AxonFlowConfig config) {
5064
.callTimeout(config.getTimeout().toMillis() * 2, TimeUnit.MILLISECONDS);
5165

5266
if (config.isInsecureSkipVerify()) {
53-
configureInsecureSsl(builder);
67+
if (isInsecureTlsEnvVarEnabled()) {
68+
configureInsecureSsl(builder);
69+
} else {
70+
logger.warn(
71+
"insecureSkipVerify(true) was set on AxonFlowConfig but environment variable {} is "
72+
+ "not set to 'true' or '1'. TLS certificate verification REMAINS ENABLED. To "
73+
+ "disable verification (development/self-signed certs only), export {}=true. "
74+
+ "This double-gate is intentional to prevent accidental TLS bypass in production.",
75+
INSECURE_TLS_ENV_VAR,
76+
INSECURE_TLS_ENV_VAR);
77+
}
5478
}
5579

5680
if (config.isDebug()) {
@@ -111,11 +135,26 @@ public X509Certificate[] getAcceptedIssuers() {
111135
(hostname, session) -> true); // lgtm[java/insecure-hostname-verifier]
112136

113137
logger.warn(
114-
"SSL certificate verification is DISABLED (insecureSkipVerify=true). "
115-
+ "Do NOT use this in production. This is intended only for development environments "
116-
+ "with self-signed certificates.");
138+
"*** SECURITY WARNING *** TLS certificate verification is DISABLED for AxonFlow SDK "
139+
+ "HTTP client. Both insecureSkipVerify(true) and {}=true were set. All HTTPS calls "
140+
+ "will accept ANY server certificate, including attacker-presented certificates in "
141+
+ "MITM scenarios. This MUST NOT be used in production. Intended only for local "
142+
+ "development against self-signed certificates.",
143+
INSECURE_TLS_ENV_VAR);
117144
} catch (Exception e) {
118145
logger.error("Failed to configure insecure SSL", e);
119146
}
120147
}
148+
149+
/**
150+
* Returns {@code true} if the {@value #INSECURE_TLS_ENV_VAR} environment variable is set to
151+
* {@code "true"} (case-insensitive) or {@code "1"}. Package-private to allow test verification.
152+
*/
153+
static boolean isInsecureTlsEnvVarEnabled() {
154+
String value = System.getenv(INSECURE_TLS_ENV_VAR);
155+
if (value == null) {
156+
return false;
157+
}
158+
return "true".equalsIgnoreCase(value) || "1".equals(value);
159+
}
121160
}

src/test/java/com/getaxonflow/sdk/util/HttpClientFactoryTest.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,34 @@ void shouldCreateClientWithDebugMode() {
7070
}
7171

7272
@Test
73-
@DisplayName("should create client with insecure skip verify")
74-
void shouldCreateClientWithInsecureSkipVerify() {
73+
@DisplayName(
74+
"should NOT disable TLS verification when insecureSkipVerify=true but env var is unset")
75+
void shouldKeepTlsVerificationEnabledWithoutEnvVar() {
76+
// Builder flag alone is not enough — AXONFLOW_INSECURE_TLS env var must also be set.
77+
// We don't set the env var in this test, so the insecure path must remain inactive.
7578
AxonFlowConfig config =
7679
AxonFlowConfig.builder().agentUrl("http://localhost:8080").insecureSkipVerify(true).build();
7780

7881
OkHttpClient client = HttpClientFactory.create(config);
7982

8083
assertThat(client).isNotNull();
81-
// Should have a custom hostname verifier that accepts all hosts
82-
assertThat(client.hostnameVerifier()).isNotNull();
84+
// The default OkHttp hostname verifier (OkHostnameVerifier) should be in place — NOT the
85+
// permissive (hostname, session) -> true verifier. We assert the default class name to ensure
86+
// we did not silently install the insecure verifier.
87+
assertThat(client.hostnameVerifier().getClass().getName())
88+
.as("default hostname verifier should be in place when env var is unset")
89+
.contains("OkHostnameVerifier");
90+
}
91+
92+
@Test
93+
@DisplayName("env-var gate helper should be false when AXONFLOW_INSECURE_TLS is unset")
94+
void envVarHelperShouldBeFalseWhenUnset() {
95+
// We cannot reliably set env vars in-process across JVMs, but we can assert the helper's
96+
// behaviour against the real environment, which is unset in CI and dev shells by default.
97+
assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
98+
.as(
99+
"AXONFLOW_INSECURE_TLS must default to false; if this fails, the test environment "
100+
+ "has the env var set and the insecure path could activate")
101+
.isFalse();
83102
}
84103
}

0 commit comments

Comments
 (0)