Skip to content

Support ChaChaPoly1305 on Android if available #52814

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 5 commits into from
May 19, 2021

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented May 15, 2021

This implements ChaChaPoly1305 on Android, if it is available.

Contributes to #52482.

/cc @jkoritzinsky @steveisok

@ghost ghost added the area-System.Security label May 15, 2021
@ghost
Copy link

ghost commented May 15, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

This implements ChaChaPoly1305 on Android, if it is available.

/cc @jkoritzinsky @steveisok

Author: vcsjones
Assignees: -
Labels:

area-System.Security

Milestone: -

vcsjones added 3 commits May 15, 2021 15:07
The CIPHER_HAS_TAG was used to determine if the GCMParameterSpec
configuration is needed for variable length tags.

While ChaCha20Poly1305 has authentication tags, it does not permit a tag
length other than 16 bytes, so there is nothing to configure.

This renames the CIPHER_HAS_TAG to be more specific that the cipher
supports more than one tag length, and removes it from ChaCha20Poly1305.

This simplifies the IV initialization a bit.
Copy link
Member

@GrabYourPitchforks GrabYourPitchforks left a comment

Choose a reason for hiding this comment

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

I'm not an Android interop expert, but the control flow LGTM.

@vcsjones
Copy link
Member Author

Browser / wasm failure is most likely unrelated.

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.

Looks reasonable to me (modulo one nod and smile). I'm happy so long as @jkoritzinsky is happy with the JNI code.

@bartonjs bartonjs merged commit 5d2dc94 into dotnet:main May 19, 2021
@vcsjones vcsjones deleted the 52482-cha-cha-android branch May 19, 2021 16:55
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
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.

6 participants