Skip to content

ML-KEM: Change public API surface from API review #114710

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 1 commit into from
Apr 16, 2025

Conversation

vcsjones
Copy link
Member

The reacts to the API changes made in #114453 (so far)

Diff:

- public void Decapsulate(ReadOnlySpan<byte> ciphertext, Span<byte> sharedSecret, out int sharedSecretBytesWritten);
- public byte[] Encapsulate(System.Span<byte> sharedSecret);
- Encapsulate(Span<byte> ciphertext, Span<byte> sharedSecret, out int ciphertextBytesWritten, out int sharedSecretBytesWritten);
- protected void ThrowIfDisposed();

- public byte[] Encapsulate(out byte[] sharedSecret);
+ public void Encapsulate(out byte[] ciphertext, out byte[] sharedSecret);

Contributes to #113508

@vcsjones vcsjones added this to the 10.0.0 milestone Apr 15, 2025
@vcsjones vcsjones requested a review from bartonjs April 15, 2025 19:57
@vcsjones vcsjones self-assigned this Apr 15, 2025
@Copilot Copilot AI review requested due to automatic review settings April 15, 2025 19:57
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.

Pull Request Overview

This PR updates the public API for the ML-KEM implementation to reflect recent API review recommendations.

  • Removed several overloads for Decapsulate and Encapsulate.
  • Introduced a new signature for Encapsulate that returns both ciphertext and shared secret via out parameters.
  • Updated tests across libraries to use the new API surface.

Reviewed Changes

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

File Description
src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs Updated public API signatures for ML-KEM.
src/libraries/Common/tests/System/Security/Cryptography/MLKemContractTests.cs Removed tests for removed overloads; added tests for the new API.
src/libraries/Common/tests/System/Security/Cryptography/MLKemBaseTests.cs Updated tests to align with the revised API, removing references to obsolete overloads.
src/libraries/Common/src/System/Security/Cryptography/MLKem.cs Refactored implementation of Encapsulate and ThrowIfDisposed to support the new API.

Copy link
Contributor

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

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.

2 participants