Skip to content

Conversation

@vcsjones
Copy link
Member

@vcsjones vcsjones commented Feb 1, 2022

The Reset method accepted an IV, but for OpenSSL and Android, the IV was discarded. Instead, the IV that was originally used to create the cipher context was used.

This wasn't a problem because we've always considered the Key and IV a tuple that belong together. The Lite ciphers are meant to decouple the key from the IV, so the cipher instance can be re-used with different IVs.

In order to facilitate re-usable lite cipher instances with different IVs, all platforms need to support resetting with a different IV than when it was originally constructed.

This also simplifies the reset on OpenSSL since the managed side always supplies the IV, and removes a workaround from OpenSSL 3.

@ghost ghost assigned vcsjones Feb 1, 2022
@ghost ghost added the area-System.Security label Feb 1, 2022
@ghost
Copy link

ghost commented Feb 1, 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

The Reset method accepted an IV, but for OpenSSL and Android, the IV was discarded. Instead, the IV that was originally used to create the cipher context was used.

This wasn't a problem because we've always considered the Key and IV a tuple that belong together. The Lite ciphers are meant to decouple the key from the IV, so the cipher instance can be re-used with different IVs.

In order to facilitate re-usable lite cipher instances with different IVs, all platforms need to support resetting with a different IV than when it was originally constructed.

This also simplifies the reset on OpenSSL since the managed side always supplies the IV, and removes a workaround from OpenSSL 3.

Author: vcsjones
Assignees: vcsjones
Labels:

area-System.Security

Milestone: -

@vcsjones
Copy link
Member Author

vcsjones commented Feb 1, 2022

I ran the Android tests locally and everything passed, I'm uncertain if CI is covering Android right now, but I suspect not.

@vcsjones
Copy link
Member Author

vcsjones commented Feb 3, 2022

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vcsjones
Copy link
Member Author

vcsjones commented Feb 3, 2022

Guessing it didn't run because I merged / pushed when GitHub was unavailable yesterday.

@bartonjs
Copy link
Member

bartonjs commented Feb 3, 2022

runtime (Libraries Test Run checked coreclr OSX x64 Debug) failure is that the test matrix still contain{s/ed} an entry for macOS 10.14, but we've shut down that machine configuration.

Since everything else passed, and this change doesn't affect OSX anyways, merging.

@bartonjs bartonjs merged commit 50e6f07 into dotnet:main Feb 3, 2022
@vcsjones vcsjones deleted the ossl-iv-reset branch February 3, 2022 19:17
@ghost ghost locked as resolved and limited conversation to collaborators Mar 6, 2022
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.

2 participants