Skip to content

Add SymmetricAlgorithm.SetKey(ROSpan) #113146

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 6 commits into from
Mar 7, 2025
Merged

Add SymmetricAlgorithm.SetKey(ROSpan) #113146

merged 6 commits into from
Mar 7, 2025

Conversation

bartonjs
Copy link
Member

@bartonjs bartonjs commented Mar 5, 2025

This change adds SymmetricAlgorithm.SetKey to set the key of an instance based on a ReadOnlySpan.

The AesImplementation type takes advantage of it by moving the key to a FixedMemoryKeyBox. None of the other algorithms are being changed at this time, mostly because no one should be using them.

AesCng could also change to using FixedMemoryKeyBox, but it's a non-trivial change due to the externalization of state into the CngSymmetricAlgorithmCore struct; and the main intended purpose for the AesCng type is to open a persisted key (where the change wouldn't matter).

Fixes #111154.

@bartonjs bartonjs added this to the 10.0.0 milestone Mar 5, 2025
@bartonjs bartonjs self-assigned this Mar 5, 2025
@Copilot Copilot AI review requested due to automatic review settings March 5, 2025 00:55
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request introduces a new SetKey(ReadOnlySpan key) API for symmetric algorithms. It refactors key management by using a FixedMemoryKeyBox in implementations such as AES and AppleCCCryptor, and it updates tests across AES, RC2, DES, TripleDES, and one-shot paths to verify the new SetKey functionality.

Reviewed Changes

File Description
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs Adds tests checking behavior when setting the key via property assignment and SetKey.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RC2/RC2CipherTests.cs Introduces SetKey_Sanity tests to verify key exchange between instances.
src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DES/DESCipherTests.cs Updates tests to use SetKey for ensuring consistency of key and IV handling.
src/libraries/System/Security/Cryptography/src/System/Security/Cryptography/AesImplementation.cs Refactors key handling with FixedMemoryKeyBox support and overrides Key, KeySize and GenerateKey.
src/libraries/System/Security/Cryptography/src/System/Security/Cryptography/AppleCCCryptor.cs Upgrades key storage to use FixedMemoryKeyBox and disposes the key in Dispose.
Other files (AesImplementation.OpenSsl.cs, AesImplementation.Windows.cs, ref/System.Security.Cryptography.cs, etc.) Update API definitions and implementations to support SetKey overload with ReadOnlySpan.

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

@bartonjs bartonjs merged commit 3258163 into dotnet:main Mar 7, 2025
84 checks passed
@bartonjs bartonjs deleted the setkey branch March 7, 2025 02:46
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: add possibility to pass key as Span<byte> in SymmetricAlgorithm and KeyedHashAlgorithm
3 participants