Skip to content

Fix trustStoreType JVM property consultation in SSL connections (#2691) - #2724

Merged
Ananya Garg (Ananya2) merged 1 commit into
mainfrom
user/anagarg/issue#2691
Aug 6, 2025
Merged

Fix trustStoreType JVM property consultation in SSL connections (#2691)#2724
Ananya Garg (Ananya2) merged 1 commit into
mainfrom
user/anagarg/issue#2691

Conversation

@Ananya2

Copy link
Copy Markdown
Contributor

Problem

The JDBC driver was not properly consulting the JVM system property javax.net.ssl.trustStoreType when the trustStoreType connection property was not explicitly set. This affected all SSL connection scenarios where users expected -Djavax.net.ssl.trustStoreType=Windows-ROOT (or other values) to be honored.

Root Cause

The trust manager selection logic in IOBuffer.enableSSL() was missing the standard fallback hierarchy for trustStoreType resolution when entering the KeyStore loading path.

Solution

Added comprehensive JVM system property consultation logic that follows the standard Java property resolution hierarchy:

  1. Check connection property trustStoreType
  2. If empty, check JVM system property javax.net.ssl.trustStoreType
  3. If still empty, use default value "JKS"

Changes

  • IOBuffer.java: Added System.getProperty("javax.net.ssl.trustStoreType") fallback logic in lines 1678-1683
  • Applies to all SSL connection modes: encrypt=true, encrypt=strict, login-only encryption, etc.
  • Maintains backward compatibility and existing explicit property behavior

Testing

  • Verified that all encryption modes now properly respect JVM system properties
  • Confirmed that explicit connection property values still take precedence
  • Validated that default behavior (JKS) is preserved when neither property is set
  • Tested with various trustStoreType values including Windows-ROOT, PKCS12, etc.

Impact

  • Enables Windows certificate store integration across all SSL connection types
  • Restores expected Java standard property consultation behavior
  • Improves compatibility with existing deployment scripts and system configurations
  • No breaking changes to existing functionality

Fixes #2691

@codecov

codecov Bot commented Jul 30, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 51.68%. Comparing base (c8c028f) to head (69b6a6c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2724      +/-   ##
============================================
+ Coverage     51.66%   51.68%   +0.02%     
+ Complexity     4112     4109       -3     
============================================
  Files           149      149              
  Lines         34240    34242       +2     
  Branches       5718     5719       +1     
============================================
+ Hits          17690    17699       +9     
+ Misses        14110    14103       -7     
  Partials       2440     2440              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ananya2
Ananya Garg (Ananya2) merged commit e49fe6f into main Aug 6, 2025
19 checks passed
Dongjoon Hyun (dongjoon-hyun) added a commit to apache/spark that referenced this pull request Nov 3, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` test dependency to `13.2.1.jre11`.

### Why are the changes needed?

To use the latest features (like JDK 23 official support) and bug fixed versions during testing.
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.1
  - microsoft/mssql-jdbc#2801
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.0
  - microsoft/mssql-jdbc#2724
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.2
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.1
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.0
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.9.0
  - microsoft/mssql-jdbc#2515
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #52856 from dongjoon-hyun/SPARK-54155.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Dongjoon Hyun (dongjoon-hyun) added a commit to apache/spark that referenced this pull request Nov 3, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` test dependency to `13.2.1.jre11`.

### Why are the changes needed?

To use the latest features (like JDK 23 official support) and bug fixed versions during testing.
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.1
  - microsoft/mssql-jdbc#2801
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.0
  - microsoft/mssql-jdbc#2724
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.2
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.1
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.0
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.9.0
  - microsoft/mssql-jdbc#2515
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #52856 from dongjoon-hyun/SPARK-54155.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 34c3dde)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
huangxiaoping (huangxiaopingRD) pushed a commit to huangxiaopingRD/spark that referenced this pull request Nov 25, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` test dependency to `13.2.1.jre11`.

### Why are the changes needed?

To use the latest features (like JDK 23 official support) and bug fixed versions during testing.
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.1
  - microsoft/mssql-jdbc#2801
- https://github.com/microsoft/mssql-jdbc/releases/tag/v13.2.0
  - microsoft/mssql-jdbc#2724
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.2
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.1
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.10.0
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.9.0
  - microsoft/mssql-jdbc#2515
- https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#52856 from dongjoon-hyun/SPARK-54155.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
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.

[QUESTION] Why is the trustStoreType=Windows-ROOT connection string parameter required?

4 participants