Conversation
…Socket connection The Grizzly client wraps its wss:// connections in an SSLEngine, which validates the certificate chain but never checks the certificate against the host, so anyone holding a certificate the client trusts could sit in the middle of the connection. Have JSSE do "HTTPS" endpoint identification during the handshake, against the host of the remote URI also when the socket goes through a proxy. ConnectionManagerConfig gets a hostnameVerification switch that defaults to the system property the legacy client honours, org.identityconnectors.framework.remote.hostnameVerification. A failed handshake is logged at WARN and surfaces as the cause of the "before WebSocket is established" failure instead of a bare connection close. The test certificates expired in January 2025 and are regenerated; the server certificate now names localhost, 127.0.0.1 and ::1 as the client verifies them, and serverKeystore-cn-only.jks keeps one without subjectAltName for the negative test.
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
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.
Companion to #122, which fixes the same class of problem in the legacy connector server client (CodeQL alert #24). This one covers the Grizzly WebSocket client in
connector-framework-server, which CodeQL does not model because theSSLEngineis created inside Grizzly.Problem
ConnectionManagerwrapswss://connections in a GrizzlySSLFilterwhoseSSLEnginevalidates the certificate chain but never checks the certificate against the host. Anyone holding a certificate the client trusts could sit in the middle of the connection to the connector server; with the usual setup (server certificate imported into the JVM truststore next to the public CAs) any publicly trusted certificate was enough.Change
ConnectionManager.SwitchingSSLFilter:createClientSSLEnginecreates the engine for the host of the remote URI (not for the proxy the socket may be connected to; IPv6 brackets stripped) and enables JSSE "HTTPS" endpoint identification viaSSLParameters, so the certificate is verified against that host during the handshake, before any application data is sent. Applies to custom trust managers too, plainX509TrustManagerincluded.notifyHandshakeFailedlogsTLS handshake with connector server host:port failed: <cause>at WARN and records the cause on the connection.ClientRemoteConnectorInfoManager: the close listener reports that cause —TLS handshake failed before WebSocket is established: …with theSSLHandshakeExceptionchained — instead of the bareConnection is closed before WebSocket is established.ConnectionManagerConfig:hostnameVerificationgetter/setter, defaulting to the same system property the legacy client honours in Verify the connector server certificate against the host over the legacy SSL connection #122,org.identityconnectors.framework.remote.hostnameVerification. Only the literalfalsedisables the check.Tests
ClientHostnameVerificationTest(connector-server-grizzly): a server with a CN-only certificate is rejected when connecting towss://127.0.0.1(cause chain carriesSSLHandshakeException: No subject alternative names present), a certificate with matching subjectAltName is accepted and the test connector is looked up over it, and the mismatching certificate is accepted whenhostnameVerification=false.The module's test certificates had expired in January 2025 (they only kept working because a self-signed end-entity certificate that is itself a trust anchor skips validity checks). They are regenerated with 30 years of validity;
serverKeystore.jksnow nameslocalhost,127.0.0.1and::1, which the client verifies,serverKeystore-cn-only.jkskeeps a certificate without subjectAltName for the negative test, andREADME.txthas thekeytoolcommands.AsyncRemoteSecureConnectorInfoManagerTestruns unchanged against the new certificate with verification on.Local runs: connector-framework-server 29 tests, connector-server-grizzly 37 tests, connector-server-jetty 34 tests, all green.
Compatibility
Deployments whose connector server certificate does not name the host used in the
wss://URI fail the handshake now, with the reason in the log and in the exception. The fix is a certificate that names the host; the config switch / system property is the temporary escape hatch. Worth a release-notes entry together with #122.Noticed, not changed
WebSocketConnectionGroup.shutdown()throwsNullPointerExceptionfromremote.getPromise().cancel(true)when a client is closed while a request is registered but not yet sent (RemoteRequest.promiseis null until the message is sent). Hit while writing the test; separate issue.connector-framework-internaland can be shared once Verify the connector server certificate against the host over the legacy SSL connection #122 is merged.