security: pin TLS 1.2 floor in RestClient and EventListener#162
Merged
Conversation
Member
Author
|
Partial live verification against 192.168.117.15 admin smoke 2026-05-26. Manual Java SDK read-only probe passed (connect, GET Me, GET Users?limit=200, GET Status). Oversized response cap could not be safely triggered with available read-only appliance endpoints; cap remains unit-verified only. Full log: .security-review-impl-logs/live-sweep/java-live.log |
Member
Author
|
Full live appliance sweep re-run (mutation allowed) completed against 192.168.117.15. Results:
Lease released in |
…-SafeguardJava-001) Replaces the generic SSLContext.getInstance(TLS) call with an explicit TLSv1.2 protocol pin in both the REST transport (RestClient.getSSLContext) and the SignalR/WebSocket transport (SafeguardEventListener.ConfigureHttpClientBuilder). The TLS alias can resolve to TLS 1.0/1.1 on JVMs whose jdk.tls.disabledAlgorithms has been narrowed; pinning the version at the SDK layer makes the minimum handshake version a property of the SDK itself, independent of caller JVM configuration. TLS 1.3, where supported by both peers, is still negotiated normally by the underlying SSLContext. Both classes now expose a package-private TLS_PROTOCOL constant so the pinned version has a single auditable source of truth. Tests: - RestClientSSLContextTest: verifies TLS_PROTOCOL constant + actual SSLContext.getProtocol() value via reflection on getSSLContext. - SafeguardEventListenerSSLContextTest: mirror test for the event listener transport. Test infrastructure: pom.xml now declares junit 4.13.2 + okhttp3 mockwebserver 4.12.0 (test scope) and maven-surefire-plugin 3.2.5; previously there was no src/test/java suite at all. Resolves F-SafeguardJava-003 (W2).
…uardJava-002) Per cross-cutting decision D-001, the ignoreSsl feature is kept as a legitimate convenience for development against self-signed appliances, and the SDK does not emit a runtime warning. This commit closes the documentation gap: - README.md: new section 'TLS Certificate Verification and the ignoreSsl Flag'. Documents what ignoreSsl actually toggles (X.509 chain validation + a NoopHostnameVerifier, *not* the TLS version), a three-row table of supported configurations, and the recommended production paths (JVM truststore import, or a HostnameVerifier callback with ignoreSsl=false). - RestClient.java: Javadoc on the (String, String, char[], boolean, HostnameVerifier) constructor describing the security implications of ignoreSsl and pointing to the TLS_PROTOCOL constant. - SafeguardEventListener.java: equivalent Javadoc on the (String, char[], boolean, HostnameVerifier) constructor. No code or public API changes. Resolves F-SafeguardJava-001, F-SafeguardJava-002, F-SafeguardJava-004 (W3).
Add BoundedResponseReader that enforces a 10 MB default cap on in-memory HTTP response bodies. Pre-read check rejects oversized Content-Length headers; streaming counter trips on chunked overflow. Wired into Utils.getResponse and PkceAuthenticator.rstsFormPost. Explicit streaming download paths (StreamResponse, StreamingRequest) are unaffected — callers there manage their own sinks.
Audit outcome: Java event listener delegates reconnect to caller via SafeguardEventListenerDisconnectedException; no native tight-loop reconnect exists to harden. Distinct-but-valid design. A jittered exponential backoff helper is deferred to Phase 2.
…afeguardJava-001, D-014)
…sion - Remove BoundedResponseReader and ResponseTooLargeException (not needed; Safeguard appliances are closed hardened systems) - Revert Utils.getResponse and PkceAuthenticator to use EntityUtils - Remove mockwebserver test dependency and response size cap test - Remove handleDisconnect Phase-2 reconnect comment - Strip internal finding IDs from test Javadoc - Version 8.2.1-SNAPSHOT (patch bump, not minor)
petrsnd
force-pushed
the
security/review-20260522-java
branch
from
451a40c to
d8835a5
Compare
petrsnd
changed the title
SignalR 8.0.27 requires Java 9+ class format. Use Class.forName with UnsupportedClassVersionError catch to skip the test on Java 8 runtimes rather than failing the entire build.
The SignalR client dependency requires Java 9+ at runtime. All other SDK features remain compatible with Java 8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pin
SSLContext.getInstance("TLS")toSSLContext.getInstance("TLSv1.2")in both REST and SignalR transports. The generic"TLS"alias can resolve to TLS 1.0/1.1 on misconfigured JVMs; pinning the version at the SDK layer guarantees a TLS 1.2+ handshake regardless of JVM configuration. TLS 1.3 is still negotiated when supported by both peers.Changes
RestClientandSafeguardEventListener— eliminates fallback to TLS 1.0/1.1ignoreSslflag behavior and production guidanceRestClientandSafeguardEventListenerconstructors explainingignoreSslsemantics8.2.1-SNAPSHOTVerification
mvn verifypasses (4 unit tests, 0 failures; SpotBugs clean)