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

Fixed issue by using lower 16 bytes as IV for AES256 #260

Merged
merged 4 commits into from Sep 16, 2018

Conversation

3 participants
@FastJack2
Contributor

FastJack2 commented Aug 16, 2018

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@piksel piksel added this to To do in Encryption Overhaul Aug 18, 2018

@piksel piksel moved this from To do to In progress in Encryption Overhaul Aug 18, 2018

@piksel piksel self-assigned this Aug 24, 2018

@piksel

This comment has been minimized.

Show comment
Hide comment
@piksel

piksel Aug 24, 2018

Collaborator

Unfortunately this does not fix the issue. It turns out that the real bug here is the use of key2 as the AES IV. It's only supposed to be used for creating the MAC (which it's also used for atm). Instead, no IV ( all zero bytes ) should be used for AES initialization.
I still don't know why using key2 as IV works for 128bit AES though...
The IV is not actually used in ECB mode, but resizing key2 to fit in the IV bytes causes the MAC to be wrong.

Collaborator

piksel commented Aug 24, 2018

Unfortunately this does not fix the issue. It turns out that the real bug here is the use of key2 as the AES IV. It's only supposed to be used for creating the MAC (which it's also used for atm). Instead, no IV ( all zero bytes ) should be used for AES initialization.
I still don't know why using key2 as IV works for 128bit AES though...
The IV is not actually used in ECB mode, but resizing key2 to fit in the IV bytes causes the MAC to be wrong.

@piksel

This comment has been minimized.

Show comment
Hide comment
@piksel

piksel Aug 24, 2018

Collaborator

Unmodified test results:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/1.0-git88302c5_91/job/vn600e3dgelh77wo/tests
(System.ArgumentNullException : Buffer cannot be null.)

IV truncation test results:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/job/9hjla7ffnvwn99pe/tests
(Archive verification failed)

Collaborator

piksel commented Aug 24, 2018

Unmodified test results:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/1.0-git88302c5_91/job/vn600e3dgelh77wo/tests
(System.ArgumentNullException : Buffer cannot be null.)

IV truncation test results:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/job/9hjla7ffnvwn99pe/tests
(Archive verification failed)

@piksel

This comment has been minimized.

Show comment
Hide comment
@piksel

piksel Aug 24, 2018

Collaborator

AES256 encrypted zip is now successfully verified by 7-zip.

Btw. @FastJack2, why did you replace Aes.Create() with new AesManaged()?

Collaborator

piksel commented Aug 24, 2018

AES256 encrypted zip is now successfully verified by 7-zip.

Btw. @FastJack2, why did you replace Aes.Create() with new AesManaged()?

@stealthrabbi

This comment has been minimized.

Show comment
Hide comment
@stealthrabbi

stealthrabbi Sep 13, 2018

Is it possible to merge this in and produce a new release?

stealthrabbi commented Sep 13, 2018

Is it possible to merge this in and produce a new release?

@piksel piksel merged commit 18b6e47 into icsharpcode:master Sep 16, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@piksel

This comment has been minimized.

Show comment
Hide comment
@piksel

piksel Sep 16, 2018

Collaborator

@stealthrabbi Until v1.1 is released you can get the pre-release nuget packages here:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/job/7a2vcixlb41a953f/artifacts

Collaborator

piksel commented Sep 16, 2018

@stealthrabbi Until v1.1 is released you can get the pre-release nuget packages here:
https://ci.appveyor.com/project/icsharpcode/sharpziplib/build/job/7a2vcixlb41a953f/artifacts

@piksel piksel moved this from In progress to Done in Encryption Overhaul Sep 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment