Seed in the skeleton of ML-DSA based on current prototyping #112891
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Copilot reviewed 6 out of 11 changed files in this pull request and generated no comments.
Files not reviewed (5)
- src/libraries/System.Security.Cryptography/src/Resources/Strings.resx: Language not supported
- src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj: Language not supported
- src/libraries/Common/src/System/Security/Cryptography/Oids.cs: Evaluated as low risk
- src/libraries/Common/src/System/Experimentals.cs: Evaluated as low risk
- src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs: Evaluated as low risk
src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs
Show resolved
Hide resolved
src/libraries/Common/src/System/Security/Cryptography/MLDsaAlgorithm.cs
Outdated
Show resolved
Hide resolved
|
|
||
| namespace System.Security.Cryptography | ||
| { | ||
| #if DESIGNTIMEINTERFACES |
There was a problem hiding this comment.
What's the purpose of this ifdef? This file appears to only be consumed into the S.S.Cryptography.csproj, which sets this.
There was a problem hiding this comment.
It’ll also be consumed in Microsoft.Bcl.Cryptography as the project gets further along
src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs
Outdated
Show resolved
Hide resolved
| private static void ThrowAlgorithmUnknown(AsnWriter encodedId) | ||
| { | ||
| #if NET9_0_OR_GREATER | ||
| throw encodedId.Encode(static encoded => |
There was a problem hiding this comment.
What's the benefit of this .NET 9 path as compared to the other one?
There was a problem hiding this comment.
At runtime, it prevents the creation of a 24-ish byte array that just gets passed in to ToHexString. (Since it's on an exception path that's not high value...)
At compile time, it makes us feel good about having created the callback-based overload.
Both because it's currently an internal type (and failing the build), but also because it should be sealed as public API.
|
/ba-g Uncalled code, and the test failures are only from "'findstr' is not recognized as an internal or external command," |
This seeds a lot of untested boilerplate code as non-public types in System.Security.Cryptography.
When we start work on ML-DSA, we can take this, make MLDsa and MLDsaAlgorithm public, start to write tests, and wire up an implementation. We can also take this and clone+customize it for the other PQC algorithms.
It does not define all span-vs-array overloads for all spanified members, just one candidate per method group.
As the PQC experiment needs to make it further before we can commit to the shape, it is both starting without API Review, and pre-emptively applying the Experimental attribute.