Skip to content

Filter legacy TLS protocols without pinning TLS versions - #301

Open
jpantonow wants to merge 2 commits into
etesync:masterfrom
jpantonow:fix/build-sslcontextspec-d28032440293-7704eb55e9
Open

Filter legacy TLS protocols without pinning TLS versions#301
jpantonow wants to merge 2 commits into
etesync:masterfrom
jpantonow:fix/build-sslcontextspec-d28032440293-7704eb55e9

Conversation

@jpantonow

Copy link
Copy Markdown

What does this implement/fix?

Property Value
Method build
Class com.etesync.syncadapter.HttpClient$Builder
File app/src/main/java/com/etesync/syncadapter/HttpClient.kt
Specification SSLContextSpec
Analysis tool ape

Summary

Observed dynamic-analysis failure

The dynamic-analysis run by ape reported (SSLContextSpec) while exercising com.etesync.syncadapter.HttpClient$Builder.build:

expecting one of {TLSv1.2, TLSv1.3} but found TLS.

Root cause

A source-level literal whitelist mismatch is confirmed: HttpClient.Builder.build creates an SSLContext with the generic algorithm literal "TLS", rather than either listed literal "TLSv1.2" or "TLSv1.3". This confirms the bounded detector predicate mismatch, but does not establish that legacy TLS is enabled or negotiated at runtime.

Correction strategy

Retain SSLContext("TLS") and begin with each provider-created socket's enabled-protocol set. Remove only explicit legacy SSL and TLSv1/TLSv1.0/TLSv1.1 names, retain recognized and unknown non-legacy future protocol names, never enable provider-disabled protocols, and fail closed if no acceptable protocol remains.

Coordinated changes

  • app/src/main/java/com/etesync/syncadapter/HttpClient.kt: Provides a top-level internal, non-reflective test seam and an internal delegating SSLSocketFactory. Each createSocket overload configures only the newly created socket, filtering its current provider-enabled protocol list in order. The helper removes all SSL-prefixed names and exactly TLSv1, TLSv1.0, and TLSv1.1, retains all other provider-enabled names, and throws SSLException before any enabledProtocols assignment when no acceptable protocol remains.; Retains the generic TLS SSLContext and its existing key-manager/trust-manager initialization, but installs the filtering wrapper as the socket factory. The same selected trustManager remains the second sslSocketFactory argument, and the existing hostnameVerifier wiring is unchanged.
  • app/src/androidTest/java/com/etesync/syncadapter/HttpClientProtocolFilterTest.kt: Exercises the top-level internal filterEnabledTlsProtocols seam supplied by the coordinated HttpClient.kt edit. The first test verifies removal of SSL-prefixed names and TLSv1/TLSv1.0/TLSv1.1, preservation of TLSv1.2, TLSv1.3, and an unknown future TLSv1.4 name in provider order, and confirms that no absent provider protocol is introduced. The second test verifies that an all-legacy input fails with SSLException rather than yielding an empty or broadened enabled-protocol configuration.

Verification included

  • In app/src/androidTest/java/com/etesync/syncadapter/HttpClientProtocolFilterTest.kt, verify removal of representative SSL-prefixed names plus TLSv1, TLSv1.0, and TLSv1.1.
  • In app/src/androidTest/java/com/etesync/syncadapter/HttpClientProtocolFilterTest.kt, verify TLSv1.2, TLSv1.3, and an unknown future non-legacy TLS name remain in original order and no absent provider protocol is introduced.
  • In app/src/androidTest/java/com/etesync/syncadapter/HttpClientProtocolFilterTest.kt, verify an input containing only filtered legacy protocols fails closed with SSLException.
  • Verify SSL-prefixed names and TLSv1/TLSv1.0/TLSv1.1 are removed from a representative provider-enabled list.
  • Verify TLSv1.2, TLSv1.3, and an unknown future TLS protocol remain in original order and that no provider-disabled protocol is introduced.
  • Verify an all-legacy provider-enabled list throws SSLException to fail closed.

Residual review requirements

The patch remains review-only until these assumptions are confirmed:

  • The Android instrumentation test compilation is configured as a Kotlin friend of the main module, permitting access to this file's internal top-level filtering seam.
  • The project's Android/Kotlin toolchain accepts nullable parameter overrides for the unannotated Java SSLSocketFactory overloads; this is the standard platform-type override form but should be compiled on the supported toolchain.
  • Supported TLS endpoints negotiate at least one protocol that remains provider-enabled after the per-socket legacy filtering; endpoint/runtime validation remains required.
  • The analyzer may continue to report the retained SSLContext.getInstance("TLS") literal even though effective socket configuration rejects legacy protocols.
  • The coordinated production edit exposes filterEnabledTlsProtocols as the specified top-level internal function in the same package, allowing this Android test source to access it through Kotlin module friend-path configuration.
  • JUnit 4 APIs are available to the androidTest source set, as is customary for Android instrumentation tests.
  • The production filter treats every SSL-prefixed name as legacy and preserves non-legacy future TLS names such as TLSv1.4 exactly as required by the supplied contract.

How was this tested?

  • Automated test coverage added in 2 test file(s)
  • Project compiles successfully (:app:assembleDebugAndroidTest)
  • Confirmed by a project maintainer

Reviewer notes

Do not merge until these assumptions and blockers are resolved:

  • The Android instrumentation test compilation is configured as a Kotlin friend of the main module, permitting access to this file's internal top-level filtering seam.
  • The project's Android/Kotlin toolchain accepts nullable parameter overrides for the unannotated Java SSLSocketFactory overloads; this is the standard platform-type override form but should be compiled on the supported toolchain.
  • Supported TLS endpoints negotiate at least one protocol that remains provider-enabled after the per-socket legacy filtering; endpoint/runtime validation remains required.
  • The analyzer may continue to report the retained SSLContext.getInstance("TLS") literal even though effective socket configuration rejects legacy protocols.
  • The coordinated production edit exposes filterEnabledTlsProtocols as the specified top-level internal function in the same package, allowing this Android test source to access it through Kotlin module friend-path configuration.
  • JUnit 4 APIs are available to the androidTest source set, as is customary for Android instrumentation tests.
  • The production filter treats every SSL-prefixed name as legacy and preserves non-legacy future TLS names such as TLSv1.4 exactly as required by the supplied contract.

Additional context

I’m an undergraduate Computer Engineering student at the University of Brasília (UnB), and this contribution is part of a research project involving software security analysis.

I’m happy to adjust the implementation to better match the project’s architecture, coding conventions, or maintainers’ recommendations.

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.

1 participant