Skip to content

Consolidate RSA tests into S.S.C.#129924

Open
PranavSenthilnathan wants to merge 12 commits into
dotnet:mainfrom
PranavSenthilnathan:consolidate-rsa-tests
Open

Consolidate RSA tests into S.S.C.#129924
PranavSenthilnathan wants to merge 12 commits into
dotnet:mainfrom
PranavSenthilnathan:consolidate-rsa-tests

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Member

Moves RSA tests from CNG, CSP and OpenSSL test projects into the S.S.C. test project. This uses the same approach as DSA (#129320) and ECDSA (#129552):

  • Create RSA factories
  • Change static RSA test classes to instance RSA test classes with virtualized factory accessors.
  • Add a test registration class for each factory as a leaf of the instance-ified classes.
  • XUnit conditional attributes can only reference static members, so instead move to throwing SkipTestException where needed.

Contributes to #66338

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

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

This PR consolidates RSA-related tests from the CNG/CSP/OpenSSL test projects into System.Security.Cryptography.Tests, aligning RSA with the prior DSA/EC consolidation approach by introducing provider/factory abstractions and per-provider test registrations.

Changes:

  • Refactors shared RSA test implementations under src/libraries/Common/tests/.../RSA/ from static to abstract instance-based tests with a provider (RSAProvider) accessor.
  • Moves provider-specific RSA tests/providers into src/libraries/System.Security.Cryptography/tests/, adds per-provider “registration” leaf classes, and updates project files to compile them under the right platform conditions.
  • Replaces some xUnit static-conditional attributes with SkipTestException-based skipping where provider capabilities differ.

Reviewed changes

Copilot reviewed 36 out of 38 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj Adds/remaps RSA-related sources into the consolidated crypto test project and removes older RSAFactory-linked sources.
src/libraries/System.Security.Cryptography/tests/RSATestRegistration.OpenSsl.cs Adds OpenSSL leaf registration classes for the instance-based RSA test bases.
src/libraries/System.Security.Cryptography/tests/RSATestRegistration.Default.cs Adds default-provider leaf registration classes for the instance-based RSA test bases.
src/libraries/System.Security.Cryptography/tests/RSATestRegistration.Csp.cs Adds CSP leaf registration classes for the instance-based RSA test bases.
src/libraries/System.Security.Cryptography/tests/RSATestRegistration.Cng.cs Adds CNG leaf registration classes for the instance-based RSA test bases.
src/libraries/System.Security.Cryptography/tests/RSASignatureFormatterTests.NoKey.cs Splits “no key” RSA signature formatter coverage into a standalone static test class.
src/libraries/System.Security.Cryptography/tests/RsaOpenSslTests.cs Moves RSAOpenSsl behavioral tests into the consolidated test project (Unix/OpenSSL-conditioned).
src/libraries/System.Security.Cryptography/tests/RSAOpenSslProvider.cs Introduces an OpenSSL-backed RSAProvider implementation for RSA test instantiation.
src/libraries/System.Security.Cryptography/tests/RSAKeyExchangeFormatterTests.Default.cs Moves RSAKeyExchange formatter argument tests into a default-only static test class.
src/libraries/System.Security.Cryptography/tests/RSAImportExportCspBlobTests.cs Moves/imports CSP blob import/export tests into the consolidated test project.
src/libraries/System.Security.Cryptography/tests/RSACryptoServiceProviderTests.cs Updates CSP RSA tests to use SkipTestException capability checks instead of static conditional-member gating.
src/libraries/System.Security.Cryptography/tests/RSACryptoServiceProviderProvider.cs Introduces an RSAProvider implementation for RSACryptoServiceProvider-backed runs.
src/libraries/System.Security.Cryptography/tests/RSACryptoServiceProviderBackCompat.cs Updates back-compat tests to use the new provider capability properties.
src/libraries/System.Security.Cryptography/tests/RsaCngTests.cs Moves CNG RSA tests into the consolidated test project and updates provider checks.
src/libraries/System.Security.Cryptography/tests/RSACngProvider.cs Introduces a CNG-backed RSAProvider implementation.
src/libraries/System.Security.Cryptography/tests/DefaultRSATests.cs Renames/moves default RSA base-virtual behavior tests into a DefaultRSATests static class.
src/libraries/System.Security.Cryptography/tests/DefaultRSAProvider.cs Replaces the prior IRSAProvider/RSAFactory pattern with a concrete RSAProvider singleton.
src/libraries/System.Security.Cryptography/tests/AsymmetricSignatureFormatterTests.cs Broadens helper accessibility to enable reuse from non-derived static test types.
src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj Removes RSA sources now hosted by the consolidated crypto tests.
src/libraries/System.Security.Cryptography.OpenSsl/tests/RSAOpenSslProvider.cs Removes the old OpenSSL RSA provider/factory wiring.
src/libraries/System.Security.Cryptography.Csp/tests/System.Security.Cryptography.Csp.Tests.csproj Removes RSA sources now hosted by the consolidated crypto tests.
src/libraries/System.Security.Cryptography.Csp/tests/RSACryptoServiceProviderProvider.cs Removes the old CSP RSA provider/factory wiring.
src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj Removes RSA sources now hosted by the consolidated crypto tests.
src/libraries/System.Security.Cryptography.Cng/tests/RSACngProvider.cs Removes the old CNG RSA provider/factory wiring.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.netcoreapp.cs Converts netcoreapp-specific Sign/Verify test variants to abstract bases for provider-driven execution.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.cs Refactors core Sign/Verify tests to instance-based execution with provider capability checks.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAXml.cs Converts RSA XML tests to instance-based execution with provider capability checks.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSASignatureFormatterTests.cs Converts signature formatter tests to instance-based execution with provider capability checks.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAProvider.cs Adds the new provider abstraction used by instance-based RSA test suites.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAKeyPemTests.cs Converts PEM tests to instance-based execution with a provider accessor.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAKeyFileTests.cs Converts key file tests to instance-based execution with provider capability checks.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAKeyExchangeFormatterTests.cs Converts key exchange formatter tests to instance-based execution with a provider accessor.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAFactoryTests.cs Converts factory tests to instance-based execution with a provider accessor.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/RSAFactory.cs Removes the old IRSAProvider + static RSAFactory indirection.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/KeyGeneration.cs Converts key generation tests to instance-based execution with a provider accessor.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/ImportExport.cs Converts import/export tests to instance-based execution with provider capability checks.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.netcoreapp.cs Converts netcoreapp-specific Encrypt/Decrypt test variants to abstract bases.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/EncryptDecrypt.cs Refactors core Encrypt/Decrypt tests to instance-based execution with provider capability checks.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 22:56

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Security.Cryptography/tests/RsaCngTests.cs:97

  • The assert message still refers to the old static RSAFactory property, but the assertion now checks RSACngProvider.Instance. Updating the message avoids confusion when this assert fails.

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated 2 comments.

Comment thread src/libraries/System.Security.Cryptography/tests/RSACngProvider.cs Outdated
Comment thread src/libraries/System.Security.Cryptography/tests/DefaultRSATests.cs Outdated

@bartonjs bartonjs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't "like" it. But since it's the same thing I came up with, it'll do. (Clarification: The registration here feels giant, and like it's easy to be missing something... but I don't have a better answer, so :shipit:)

LGTM aside from the one misleading rename.

Copilot AI review requested due to automatic review settings July 2, 2026 23:42

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated 4 comments.

Comment thread src/libraries/System.Security.Cryptography/tests/RSAOpenSslProvider.cs Outdated
Comment thread src/libraries/System.Security.Cryptography/tests/DefaultRSAProvider.cs Outdated
@PranavSenthilnathan

Copy link
Copy Markdown
Member Author

@dotnet-policy-service rerun

Copilot AI review requested due to automatic review settings July 9, 2026 22:36

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated 2 comments.

@PranavSenthilnathan

Copy link
Copy Markdown
Member Author

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 10, 2026 01:27

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated 2 comments.

Comment thread src/libraries/System.Security.Cryptography/tests/RSACngProvider.cs
Copilot AI review requested due to automatic review settings July 10, 2026 01:34

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Security.Cryptography/tests/RSASignatureFormatterTests.NoKey.cs:9

  • RSASignatureFormatterTests_NoKey used to inherit [SkipOnPlatform(TestPlatforms.Browser, ...)] behavior via AsymmetricSignatureFormatterTests, but it is now a standalone test class with no Browser skip. Since AsymmetricSignatureFormatterTests is explicitly marked as unsupported on Browser, these tests will now run on Browser and are likely to fail during browser test runs. Add the same SkipOnPlatform attribute to this class.

Copilot AI review requested due to automatic review settings July 14, 2026 01:19

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

Copilot reviewed 36 out of 38 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 14, 2026 16:49
@PranavSenthilnathan PranavSenthilnathan enabled auto-merge (squash) July 14, 2026 16:50

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

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants