Always Encrypted | Align reads of CekMdVersion and EkValueCount with TDS specification#4240
Open
edwardneal wants to merge 4 commits into
Open
Always Encrypted | Align reads of CekMdVersion and EkValueCount with TDS specification#4240edwardneal wants to merge 4 commits into
edwardneal wants to merge 4 commits into
Conversation
Member
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Test code uses reflection to call SqlTceCipherInfoEntry.Add
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns Always Encrypted metadata parsing/serialization with the MS-TDS specification by treating CekMdVersion as an 8-byte ULONGLONG (ulong) and EkValueCount/CEK table size as USHORT (ushort). This improves correctness for large key tables and removes unnecessary allocations by avoiding intermediate byte-array representations.
Changes:
- Update CEK metadata version handling from
byte[8]/byte[]toulongacross the Always Encrypted metadata flow. - Read CEK table size as
ushort(instead ofshort) when processingCOLMETADATAcipher info tables. - Adjust functional Always Encrypted tests/utilities to match the updated internal signatures and representations.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/AlwaysEncryptedTests/Utility.cs | Updates test reflection helper signature to pass CEK metadata version as ulong. |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/AlwaysEncryptedTests/ExceptionsAlgorithmErrors.cs | Updates test calls to the helper to provide CEK metadata version as a numeric ulong value. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs | Reads CekMdVersion as 8-byte integer and reads CEK table size as ushort; updates bulk-copy serialization to write ulong. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs | Converts binary(8) metadata version from sp_describe_parameter_encryption into a ulong using BinaryPrimitives. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ColumnEncryptionKeyInfo.cs | Stores/serializes key metadata version as ulong and writes it as little-endian bytes. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs | Propagates ulong CEK metadata version through cipher info structures and serialized parameter wire format. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SqlClient contains a the structure of a table of
CIPHER_INFOentries. The table is represented as aSqlTceCipherInfoTableand each entry is represented as aSqlTceCipherInfoEntryinstance. This corresponds to anEK_INFOstructure in the TDS specification. A table ofEK_INFOentries appears in theCOLMETADATAstructure.In the TDS specification, the
EK_INFOstructure'sCekMdVersionfield is defined as aULONGLONGand theCOLMETADATAstructure'sEkValueCountfield is defined as aUSHORT.This PR aligns SqlClient with these type definitions. The
cekMdVersionfield onSqlTceCipherInfoTableis currently defined as an eight byte array, and is redefined as aulong. This eliminates one allocation, so it technically improves performance (marginally.) ThetableSizeis currently defined a short, and is redefined as aushort. This is a correctness improvement and a bugfix in an edge case where the same column is encrypted with an unreasonably large number of encryption keys.One slight edge case lies in SqlCommand.Encryption.cs, where we read the CekMdVersion from a byte array as a little-endian ulong. This byte array is the result of reading the
column_encryption_key_metadata_versioncolumn from the first result set ofsp_describe_parameter_encryption, and this column is declared asbinary(8). I've usedBinaryPrimitives.ReadUInt64LittleEndianto bridge this gap between the stored procedure's result set and the TDS specification.Issues
None.
Testing
Automated Always Encrypted tests continue to pass.