Skip to content

security: SafeguardJava 8.3.0 — coordinated security review fixes#162

Open
petrsnd wants to merge 7 commits into
OneIdentity:mainfrom
petrsnd:security/review-20260522-java
Open

security: SafeguardJava 8.3.0 — coordinated security review fixes#162
petrsnd wants to merge 7 commits into
OneIdentity:mainfrom
petrsnd:security/review-20260522-java

Conversation

@petrsnd
Copy link
Copy Markdown
Member

@petrsnd petrsnd commented May 26, 2026

Summary

Coordinated SafeguardJava security review fixes for the 8.3.0 snapshot release.

Findings cited from E:\source\OpenSource\security-findings-safeguardjava.md

  • F-SafeguardJava-003: generic SSLContext.getInstance("TLS") could allow TLS 1.0/1.1 negotiation on misconfigured JVMs.
  • F-SafeguardJava-005: dependency update hygiene for jackson-databind, gson, and httpclient5.
  • F-SafeguardJava-001/002/004: ignoreSsl=true certificate/hostname-verification risk documented with clearer guidance rather than changing the explicit opt-in behavior.
  • F-SafeguardJava-006: low-confidence redirect/token hardening item tracked separately as repo hygiene/supply-chain follow-up.

Release notes

  • TLS pinned to TLSv1.2 floor — eliminates fallback to TLS 1.0/1.1 on misconfigured JVMs; TLS 1.3 can still be negotiated by the runtime.
  • REST response body size cap (FP-004) — prevents OOM from oversized responses.
  • Dependency bumps: jackson-databind, gson, httpclient5 to current stable releases.
  • Utils.getResponse signature: now throws checked SafeguardForJavaException; in-tree migration is complete. Downstream consumers calling this internal helper directly may need to update try/catch. Surfaced here for release notes, but treated as non-breaking per D-002 scope decision.
  • Reconnect helper deferred to Phase 2 (D-014) — handleDisconnect delegates to DefaultDisconnectHandler; reference impl in SafeguardDotNet/Event/ReconnectBackoff.cs.
  • Multi-stage agent-orchestrated review (S1–S6), 2 cycles, 0 blockers / 0 majors at close.
  • C-SafeguardJava-001 confirmed-fixed: Javadoc citation added.

Verification

  • mvn -B verify passes: 10 tests, 0 failures/errors/skips; SpotBugs 0 findings.
  • Version bumped from 8.2.0-SNAPSHOT to 8.3.0-SNAPSHOT.

petrsnd added 7 commits May 23, 2026 01:00
@petrsnd
security: pin TLS >= 1.2 in RestClient and SafeguardEventListener (FP…
5d8af2d 
…-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).
@petrsnd
docs: add ignoreSsl security guidance to README and Javadoc (FP-Safeg…
3ca808f 
…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).
@petrsnd
build: bump jackson-databind, gson, httpclient5 to stable releases (F…
5d30d00 
…P-SafeguardJava-003)

Stable transitive-dep refresh per F-SafeguardJava-005:

  jackson-databind  2.18.3 -> 2.21.3
  gson              2.11.0 -> 2.14.0
  httpclient5       5.4.3  -> 5.6.1

All three are stable (non-preview, non-alpha) releases and are
drop-in compatible with the SDK's current usage. mvn verify (build +
4 unit tests + spotbugs) green.

Deferred per D-014 / owner no-preview-tech rule:
- okhttp 4.12.0 (latest is 5.x alpha)
- signalr 8.0.12 (latest is 11.0.0-preview)
- slf4j-api 2.0.17 (latest is 2.1.0-alpha1)

These three will be revisited once each upstream reaches GA.

Resolves F-SafeguardJava-005 (M, stable subset).
@petrsnd
security: cap REST response body reads (FP-SafeguardJava-004)
2fdf154 
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.
@petrsnd
docs: document reconnect delegation design (FP-SafeguardJava-005)
0dd6502 
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.
@petrsnd
docs: cite SafeguardDotNet ReconnectBackoff as Phase-2 reference (C-S…
2c34349 
…afeguardJava-001, D-014)
@petrsnd
chore(release): bump version to 8.3.0-SNAPSHOT for security review
451a40c
@petrsnd petrsnd requested a review from a team as a code owner May 26, 2026 21:46
@petrsnd
Copy link
Copy Markdown
Member Author

petrsnd commented May 26, 2026

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

@petrsnd
Copy link
Copy Markdown
Member Author

petrsnd commented May 27, 2026

Full live appliance sweep re-run (mutation allowed) completed against 192.168.117.15.

Results:

  • SafeguardDotNet (security/review-20260522-dotnet): 15 suites, 71 passed / 0 failed / 2 skipped. SpsIntegration excluded because no SPS appliance was in the lease. Cleanup audit: no SgDnTest objects remained.
  • PySafeguard (security/review-20260522-py): after installing the optional SignalR extra required by event tests, full pytest passed: 453 passed / 0 failed / 0 skipped. Cleanup audit found one leaked PySg_ event-test user; it was deleted and re-audit showed 0 remaining.
  • safeguard.js (security/review-20260522-js): integration suite passed: 11 files, 55 passed / 0 failed / 0 skipped. Cleanup audit: no SgJs_ objects remained.
  • safeguard-bash (security/review-20260522-bash): full suite executed with SAFEGUARD_ALLOW_LOCALHOST=1 after the stock runner PKCE preflight failed against the private appliance address. Result: 14 suites, 323 passed / 10 failed / 0 skipped. Failures are confined to A2A and A2A Access Request Broker retrieval/broker negative-path checks. Cleanup audit: no SgBashTest objects remained.
  • SafeguardJava (security/review-20260522-java): PowerShell integration runner passed: 9 suites, 59 passed / 0 failed / 0 skipped; SpsIntegration excluded because no SPS appliance was in the lease. FP-004 cap regression unit test also passed: 6 passed / 0 failed / 0 skipped. Cleanup audit: no SgJTest objects remained.

Lease released in SECURITY-REVIEW.md. Follow-up needed: investigate safeguard-bash A2A failures and the PySafeguard event-test cleanup leak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@petrsnd