Skip to content
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

[Question] [Android] Crash due to crypto asserts #77258

Closed
ShortDevelopment opened this issue Oct 20, 2022 · 10 comments
Closed

[Question] [Android] Crash due to crypto asserts #77258

ShortDevelopment opened this issue Oct 20, 2022 · 10 comments

Comments

@ShortDevelopment
Copy link

ShortDevelopment commented Oct 20, 2022

I'm currently experiencing this exact issue (#61783) during calls involving hmac such as...

  • ECDiffieHellman.DeriveKeyFromHmac
  • HKDF.Expand

This issue has already been fixed in #61827.

My question is: How can I now use the fixed runtime version?
I'm targeting net6.0-android31.0 (.net 6.0.402 sdk).

Sorry for opening an issue for that...

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 20, 2022
@ghost
Copy link

ghost commented Oct 20, 2022

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

I'm currently experiencing this exact issue (#61783) during calls involving hmac such as...

  • ECDiffieHellman.DeriveKeyFromHmac
  • HKDF.Expand

This issue has already been fixed in #61827.

My question is: How can I now use the fixed runtime version?
I'm targeting net6.0-android31.0 (.net 6.0.402 sdk).

Sorry for opening an issue for that...

Author: ShortDevelopment
Assignees: -
Labels:

os-android, untriaged

Milestone: -

@marek-safar marek-safar added the question Answer questions and provide assistance, not an issue with source code or documentation. label Oct 20, 2022
@ghost
Copy link

ghost commented Oct 20, 2022

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

Issue Details

I'm currently experiencing this exact issue (#61783) during calls involving hmac such as...

  • ECDiffieHellman.DeriveKeyFromHmac
  • HKDF.Expand

This issue has already been fixed in #61827.

My question is: How can I now use the fixed runtime version?
I'm targeting net6.0-android31.0 (.net 6.0.402 sdk).

Sorry for opening an issue for that...

Author: ShortDevelopment
Assignees: -
Labels:

question, area-System.Security, os-android, untriaged

Milestone: -

@vcsjones
Copy link
Member

vcsjones commented Oct 20, 2022

#61827 didn't make the cut for .NET 6, so it's only present in .NET 7. Can you confirm if the issue is resolved in the RC2 for .NET 7?

I'm currently experiencing this exact issue (#61783) during calls involving hmac such as...

HKDF.Expand

Looks like that can happen when an empty info is passed to HKDF (an entirely reasonable thing to do)

which will trip the abort.

ECDiffieHellman.DeriveKeyFromHmac

For ECDH the assert will trip if the secretPrepend or secretAppend are empty (an entirely reasonable thing to do)

So that explains where/why those APIs are hitting the empty HMAC update.

@bartonjs @steveisok do we want to fix this for .NET 6? Since those higher-level primitives can hit the abort when passing empty data to HMAC, there isn't much consumers can do about that. If we want to backport something, I can prepare a backport PR.

@bartonjs
Copy link
Member

Yeah, process crashes are bad. We should service them.

Are all of these already fixed in 7 and we just need to backport to 6?

@vcsjones
Copy link
Member

Are all of these already fixed in 7 and we just need to backport to 6?

To my knowledge, yes. I will start a backport PR.

@vcsjones vcsjones self-assigned this Oct 20, 2022
@vcsjones vcsjones removed the question Answer questions and provide assistance, not an issue with source code or documentation. label Oct 20, 2022
@steveisok
Copy link
Member

I think backporting to 6 is fine and low risk.

@ShortDevelopment
Copy link
Author

Thanks for your quick response!

Can you confirm if the issue is resolved in the RC2 for .NET 7?

I tried to test that, but I couldn't get my android project to compile with the net7.0 preview in the limited amount of time I had.

Looks like that can happen when an empty info is passed to HKDF

That’s exactly my scenario!

I can prepare a backport PR

If you could successfully backport the fix, that would be great!

@vcsjones
Copy link
Member

If you could successfully backport the fix, that would be great!

A backport PR has been opened for this at #77283. That doesn't definitely mean it's going to be fixed for .NET 6, yet. It still needs approval for servicing.

@vcsjones
Copy link
Member

vcsjones commented Nov 7, 2022

The backport PR has been approved and merged, and should be in .NET 6.0.12 when it's released. I don't think there is anything else this issue is tracking, so closing this out.

@vcsjones vcsjones closed this as completed Nov 7, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants