Skip to content

[6.1.7 Cherry-pick] Address CodeQL finding: PKCS#1 padding (SM03799) - #4526

Open
github-actions[bot] wants to merge 2 commits into
release/6.1from
dev/automation/pr-4517-to-6.1.7
Open

[6.1.7 Cherry-pick] Address CodeQL finding: PKCS#1 padding (SM03799)#4526
github-actions[bot] wants to merge 2 commits into
release/6.1from
dev/automation/pr-4517-to-6.1.7

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Cherry-pick of #4517 (c642196) into release/6.1. Conflicts have been resolved manually.

Conflict resolution

The upstream commit touched four files, none of which exist on release/6.1:

Upstream file Status on release/6.1
.../AlwaysEncrypted/EncryptedColumnEncryptionKeyParameters.cs Does not exist (post-6.1 AE refactor)
.../AlwaysEncrypted/ColumnMasterKeyMetadata.cs Does not exist (post-6.1 AE refactor)
tools/PackageValidator/src/AssemblyInspector.cs Does not exist
tools/PackageValidator/src/PortablePdb.cs Does not exist

SM03799 (PKCS#1 signature padding) — ported

On main, the RSA PKCS#1 v1.5 sign/verify logic for all AE key store providers is centralized in EncryptedColumnEncryptionKeyParameters and ColumnMasterKeyMetadata. On release/6.1 that logic still lives in the individual providers, so the // CodeQL [SM03799] suppressions were applied to the equivalent pre-refactor call sites:

  • SqlColumnEncryptionCertificateStoreProvider.Windows.csRSAPKCS1SignatureFormatter / RSAPKCS1SignatureDeformatter
  • SqlColumnEncryptionCngProvider.Windows.csSignData / VerifyData with RSASignaturePadding.Pkcs1
  • SqlColumnEncryptionCspProvider.Windows.csRSACryptoServiceProvider.SignData / VerifyData

Always Encrypted requires RSA PKCS#1 v1.5 signature padding for both the encrypted CEK blob and the column master key metadata. These formats are fixed by the Always Encrypted specification and are produced/consumed by SQL Server tooling and other drivers, so switching to PSS would break interoperability.

SM02196 (weak hash) — not applicable

That portion of #4517 targeted tools/PackageValidator, which does not exist on release/6.1. No SHA-1 hashing occurs in the 6.1 driver sources outside the already-suppressed RSA_OAEP(SHA1) sites (SM03796). Nothing to port.

Checklist

  • Changes are comment-only (CodeQL suppression annotations); no behavioral change
  • No public API changes
  • No new tests required
  • No breaking changes introduced

To resolve, run:  git cherry-pick c642196
@github-actions github-actions Bot added this to the 6.1.7 milestone Aug 10, 2026
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Aug 10, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

… providers

Manual resolution of the cherry-pick of #4517 into release/6.1.

The upstream change annotated the sign/verify call sites in the refactored
EncryptedColumnEncryptionKeyParameters and ColumnMasterKeyMetadata types,
which do not exist on release/6.1. Applied the equivalent [SM03799]
suppressions to the corresponding pre-refactor call sites in the certificate
store, CNG, and CSP column encryption key store providers.

Always Encrypted requires RSA PKCS#1 v1.5 signature padding for the encrypted
CEK blob and column master key metadata. These formats are fixed by the Always
Encrypted specification and are produced/consumed by SQL Server tooling and
other drivers, so switching to PSS would break interoperability.

The SM02196 weak-hash portion of #4517 targeted tools/PackageValidator, which
does not exist on release/6.1, so it is not applicable here.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c71c86f5-f9e1-4ed5-a01b-a9f8113a171d
@cheenamalhotra
cheenamalhotra marked this pull request as ready for review August 10, 2026 22:18
Copilot AI lite review requested due to automatic review settings August 10, 2026 22:18
@cheenamalhotra
cheenamalhotra requested a review from a team as a code owner August 10, 2026 22:18
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@cheenamalhotra cheenamalhotra changed the title [6.1.7 Cherry-pick - CONFLICTS] Address CodeQL findings: PKCS#1 padding (SM03799) and weak hash (SM02196) [6.1.7 Cherry-pick] Address CodeQL finding: PKCS#1 padding (SM03799) Aug 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports the upstream CodeQL suppression for SM03799 (RSA PKCS#1 v1.5 signature padding) into the release/6.1 branch by annotating the pre-refactor Always Encrypted provider call sites where PKCS#1 padding is required for interoperability.

Changes:

  • Add // CodeQL [SM03799] suppression annotations at Always Encrypted RSA sign/verify call sites in the CSP provider.
  • Add // CodeQL [SM03799] suppression annotations at Always Encrypted RSA sign/verify call sites in the CNG provider.
  • Add // CodeQL [SM03799] suppression annotations at Always Encrypted RSAPKCS1 formatter/deformatter usage in the certificate store provider.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCspProvider.Windows.cs Adds SM03799 suppression comments on PKCS#1 v1.5 SignData/VerifyData usage.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCngProvider.Windows.cs Adds SM03799 suppression comments on PKCS#1 v1.5 SignData/VerifyData usage.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCertificateStoreProvider.Windows.cs Adds SM03799 suppression comments around RSAPKCS1SignatureFormatter/Deformatter usage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

4 participants