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

Increase test coverage for AesCipher #1232

Merged
merged 2 commits into from
Nov 5, 2023

Conversation

Rob-Hague
Copy link
Collaborator

The tests were generated by a script which is also added for posterity. The script works by running openssl enc [...] (via WSL) to generate the expected encrypted values, and also verifies those values against the .NET BCL implementation as an extra validation before generating the tests.

The new tests uncovered a bug in OfbCipherMode, or rather, that it was an exact duplicate of CfbCipherMode. The second commit fixes the new failing tests (by following the nice pictures in https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Output_feedback_(OFB)).

I would like to help with #865, this should add confidence there.

The tests were generated by a script which is also added for posterity.
The script works by running "openssl enc [...]" (via WSL) to generate the
expected encrypted values, and also verifies those values against the .NET
BCL implementation as an extra validation (it uncovered a difference in
CFB mode between the two relating to the feedback size).
It was an exact copy of CfbCipherMode
@Rob-Hague Rob-Hague deleted the aesciphertests branch November 5, 2023 09:00
@zybexXL
Copy link
Contributor

zybexXL commented Nov 5, 2023

@Rob-Hague
Thanks for this :) I've tested my code on #865 with this and all tests pass under all supported frameworks.
That PR failed one unrelated test, seems like a spurious fail.

Copy link
Collaborator

@WojciechNagorski WojciechNagorski left a comment

Choose a reason for hiding this comment

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

I've tested this PR.
LGTM

@zybexXL
Copy link
Contributor

zybexXL commented Nov 5, 2023

image
Looks like Github doesn't handle this DST change very well :)

@WojciechNagorski WojciechNagorski added this to the 2023.0.1 milestone Nov 16, 2023
@WojciechNagorski
Copy link
Collaborator

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1

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.

3 participants