Critical security vunerabilities in this java client

[oliverhenlich]

Summary

• Certificate validation is disabled by a custom X509TrustManager that trusts all certificates.
• Hostname verification is disabled by a custom HostnameVerifier that allows any hostname.
• These disable core TLS protections, enabling trivial MitM, token theft, and tampering.
• Additional concerns: allowing http:// base paths and setting a global JVM Authenticator for proxy credentials.

Impact (why this is critical)

• Man‑in‑the‑Middle: Any on‑path attacker can intercept and alter traffic.
• Credential/PII exposure: OAuth tokens, API keys, and document data can be exfiltrated.
• Response tampering: Server responses can be modified without detection.
• Cross‑JVM leakage: Global Authenticator may leak proxy credentials to other HTTP clients in the same JVM.

Evidence (where this happens)

• SecureTrustManager (inner class) implements X509TrustManager but does no checks (checkServerTrusted/checkClientTrusted empty; always trusts).
• InsecureHostnameVerifier (inner class) returns true for all hosts.
• buildHttpClient(...) installs both the above into the SSLContext and connection factory.
• invokeAPI(...) does not enforce https:// for basePath.
• buildHttpClient(...) sets a global proxy Authenticator via Authenticator.setDefault(...) when proxy creds are present.

Root cause / probable intent

Likely added to simplify local testing against self‑signed endpoints. However, shipping these defaults to end users leaves production clients insecure by default.

Recommendations (secure by default)

1. Remove trust‑all and insecure hostname verification by default
   • Use the platform default X509TrustManager and default HostnameVerifier.
   • If truly needed for local testing, gate with an explicit, off‑by‑default flag such as system property docusign.sdk.allowInsecure=true and add prominent warnings.
2. Enforce HTTPS
   • Reject any basePath that does not start with https:// (fail fast with clear error).
3. Avoid global proxy authenticator
   • Do not call Authenticator.setDefault(...). Prefer per‑connection authentication or document a pluggable proxy authenticator scoped to this client only.
4. Document security posture
   • Clearly document that production usage requires default TLS verification and HTTPS.

Acceptance criteria

• Removing or disabling SecureTrustManager and InsecureHostnameVerifier results in certificate/hostname validation being performed by default.
• Attempts to set basePath to http://... throw a clear exception.
• No calls to Authenticator.setDefault(...) are made by default; proxy creds are scoped to this client/connector only.
• Unit/integration tests cover: (a) valid TLS handshake with proper certificates, (b) rejection of invalid certificates/hostnames, (c) rejection of http:// base paths.

References

• OWASP Transport Layer Protection Cheat Sheet
• RFC 2818 (HTTP over TLS) — hostname verification
• Oracle Java Secure Socket documentation