Skip to content

fix: make SSL socket hostname verification visible to CodeQL - #482

Open
tony19 wants to merge 1 commit into
mainfrom
ccr-b0a20a80-ftqdbf
Open

tony19 wants to merge 1 commit into
mainfrom
ccr-b0a20a80-ftqdbf

Conversation

@tony19

@tony19 tony19 commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Addresses code scanning alert #5 (java/unsafe-cert-trust, Critical) at AbstractSocketAppender.java:264.

Why the alert was still open

Since #479, client SSL sockets verify the server's hostname by default. But SSLParameters.setEndpointIdentificationAlgorithm("HTTPS") and setSSLParameters() were called on the delegate field inside SSLConfigurableSocket. CodeQL only counts that as a sanitizer when the setSSLParameters() qualifier flows locally to the socket on the path from (SSLSocket) factory.createSocket(...) to socket.getOutputStream(). So it couldn't see the verification.

Change

  • ConfigurableSSLSocketFactory now applies hostname verification to the SSLSocket it returns, through a private configure() that returns SSLConfigurableSocket.applyHostnameVerification(socket, verify). That helper sets the endpoint identification algorithm and returns the socket, so the returned socket passes through the sanitizer.
  • SSLParametersConfiguration.configureExceptHostnameVerification() (package-private) still makes the decision and logs it: client sockets verify by default, <hostnameVerification>false</hostnameVerification> opts out, and below API 24 a warning is logged. The public configure(SSLConfigurable) behaves exactly as before.
  • SSLConfigurableSocket.setHostnameVerification(boolean) delegates to the same helper, so the logic lives in one place.

No behavior change: each socket gets the same parameters as before, set once.

Tests

Added socketFactoryVerifiesHostnameByDefault and socketFactoryHostnameVerificationCanBeDisabled to SSLHostnameVerificationTest, which cover the factory path end to end. I couldn't run Gradle locally: the session's network policy blocks dl.google.com, so the Android Gradle plugin doesn't resolve. ./scripts/check-architecture-docs.sh passes. CI needs to confirm the build and tests.

Deferred work considered: none of the 6 open issues touch core.net.ssl or the socket appenders. No upstream port applies, because upstream logback hits the same CodeQL pattern.

馃 Generated with Claude Code

https://claude.ai/code/session_019BddTdtdSnfEZadKuaxXYz


Generated by Claude Code

CodeQL's java/unsafe-cert-trust (alert #5) flagged the socket written to
in AbstractSocketAppender. Client sockets already verify the server's
hostname by default (#479), but the endpoint identification algorithm was
set inside SSLConfigurableSocket, on a field, so the query could not see
it on the path from ConfigurableSSLSocketFactory.createSocket() to
getOutputStream().

ConfigurableSSLSocketFactory now applies hostname verification to the
SSLSocket it returns, through SSLConfigurableSocket.applyHostnameVerification(),
which returns the socket. SSLParametersConfiguration still makes the
decision (and logs it) in configureExceptHostnameVerification(); its
public configure() behaves as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019BddTdtdSnfEZadKuaxXYz
@tony19 tony19 added the run-ci-on-drafts label Sep 29, 2026 — with Claude
@tony19
tony19 marked this pull request as ready for review September 29, 2026 07:47
@tony19 tony19 removed the run-ci-on-drafts label Sep 29, 2026 — with Claude

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants