Skip to content

Addressed and suppressed CodeQL warnings with explanatory comments in the JDBC codebase. #2677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 17, 2025

Conversation

Ananya2
Copy link
Contributor

@Ananya2 Ananya2 commented Jun 11, 2025

Description
CodeQL static analysis raised warnings for certain cryptographic algorithms and usages in the JDBC driver codebase. These warnings were triggered in files supporting NTLM authentication, Always Encrypted, legacy private key handling, and secure in-memory string encryption. However, these usages are intentional and required for compatibility with SQL Server features, industry standards, and appropriate security contexts.

Resolution details:
This PR adds CodeQL suppression comments to the affected lines of code. These suppressions are justified and documented to ensure clarity and maintain compliance with external standards or backward compatibility. No functional code changes were made. The updates are as follows:

  1. NTLMAuthentication.java (lines 605, 825)
    Suppression added for use of HmacMD5 algorithm, which is required for NTLM support.
    // CodeQL [SM05136] HmacMD5 is required for NTLM support
  2. KeyStoreProviderCommon.java (line 129)
    Suppression added for use of RSA_OAEP with SHA1, which is mandated by SQL Server for Always Encrypted.
    // CodeQL [SM03796] Required for an external standard: Always Encrypted only supports encrypting column encryption keys with RSA_OAEP(SHA1) (https://learn.microsoft.com/en-us/sql/t-sql/statements/create-column-encryption-key-transact-sql?view=sql-server-ver16)
  3. SQLServerCertificateUtils.java (lines 425, 485)
    Suppressions added to maintain backward compatibility with older private key formats.
    // CodeQL [SM05136] Required for backwards compatibility reading of old private keys
  4. SQLServerColumnEncryptionJavaKeyStoreProvider.java (line 371)
    Suppression added for RSA_OAEP(SHA1) usage required by Always Encrypted.
    // CodeQL [SM03796] Required for an external standard: Always Encrypted only supports encrypting column encryption keys with RSA_OAEP(SHA1) (https://learn.microsoft.com/en-us/sql/t-sql/statements/create-column-encryption-key-transact-sql?view=sql-server-ver16)
  5. SecureStringUtil.java (lines 88, 89)
    Suppressions added for the use of AES/GCM/NoPadding, which is a modern and secure cipher.
    // This cipher is used appropriately in a short-lived, in-memory scenario, with each nonce only used once for encryption.

Testing
No functional changes were made; therefore, no new tests were added. Existing test coverage remains valid, and this change is limited to documentation-only suppressions to pass CodeQL analysis while preserving required functionality.

Copy link

codecov bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 51.80%. Comparing base (c2a6ba4) to head (a7c01a9).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...m/microsoft/sqlserver/jdbc/NTLMAuthentication.java 0.00% 2 Missing ⚠️
...soft/sqlserver/jdbc/SQLServerCertificateUtils.java 0.00% 2 Missing ⚠️
...crosoft/sqlserver/jdbc/KeyStoreProviderCommon.java 0.00% 1 Missing ⚠️
...SQLServerColumnEncryptionJavaKeyStoreProvider.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2677      +/-   ##
============================================
- Coverage     51.82%   51.80%   -0.03%     
+ Complexity     4021     4019       -2     
============================================
  Files           147      147              
  Lines         33800    33800              
  Branches       5650     5650              
============================================
- Hits          17518    17509       -9     
- Misses        13811    13819       +8     
- Partials       2471     2472       +1     

☔ 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.

David-Engel
David-Engel previously approved these changes Jun 11, 2025
@Ananya2 Ananya2 merged commit 9a5477c into main Jun 17, 2025
17 of 19 checks passed
@machavan machavan added this to the 13.1.0 milestone Jun 17, 2025
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.

3 participants