Skip to content

Key sizes can only be certain lengths #3992

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 3 commits into from
Apr 2, 2020
Merged

Key sizes can only be certain lengths #3992

merged 3 commits into from
Apr 2, 2020

Conversation

tkhadimullin
Copy link
Contributor

The wording could be interpreted as "any value between 128 and 256 bits is valid" leading to confision.
Specifying exact values will hopefully provide more clarity and eliminate confusion.
See https://github.com/microsoft/referencesource/blob/master/System.Core/System/Security/Cryptography/AesCryptoServiceProvider.cs#L323 for implementation details.

Summary

Describe your changes here.

Fixes #Issue_Number (if available)

The wording could be interpreted as "any value between 128 and 256 bits is valid" leading to confision.
Specifying exact values will hopefully provide more clarity and eliminate confusion.
See https://github.com/microsoft/referencesource/blob/master/System.Core/System/Security/Cryptography/AesCryptoServiceProvider.cs#L323 for implementation details.
Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

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

It's probably worth checking with AesCng and Aes (the base class) to make sure that they're all equally clear.

added same valid `KeySize` clarification to related `AesCng` class
@tkhadimullin
Copy link
Contributor Author

Documentation for Aes itself does not seem to make any representations regarding legal key sizes. The source code actually defines what sizes are valid: https://github.com/microsoft/referencesource/blob/master/mscorlib/system/security/cryptography/aes.cs#L21.
As far as I understand it, the logic is "between 128 and 256 with increments of 64 bits. Which still yields the same 3 key sizes. Arguably this mathematics is implementation details and likely does not belong in documentation?

AesCng actually exposes documentation regarding this property, so I updated it with the same wording.

@tkhadimullin tkhadimullin requested a review from bartonjs March 16, 2020 19:38
code review changes
@tkhadimullin
Copy link
Contributor Author

was there anything else I could do to move this PR along?

@bartonjs
Copy link
Member

bartonjs commented Apr 2, 2020

@dotnet/docs This is ready for final review

@tdykstra tdykstra merged commit dc05b0d into dotnet:master Apr 2, 2020
@tkhadimullin tkhadimullin deleted the patch-2 branch April 21, 2020 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants