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

Add support for reading ZIP files utilising AES encryption #179

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

muz
Copy link

@muz muz commented Aug 6, 2014

Working AES-128, -192, -256 decryption

Caveats:

  • Doesn't currently check the authentication code
  • Won't read zip files which have a data description block without
    the signature. (See notes in code)
  • Has no tests yet

jphastings and others added 16 commits September 29, 2013 22:25
Caveats:
* Doesn't currently check the authentication code
* Won't read zip files which have a data description block without
  the signature. (See notes in code)
* Has no tests yet...
Working AES-128, -192, -256 decryption
Fixed bug with checking for password= function
…ypt-perf

* 'master' of https://github.com/rubyzip/rubyzip:
  Explicitly add the released 2.1.0 Ruby version Remove branch restriction
  Fix Rubinius by adding newly required gems, updating label in .travis.yml
  Update README.md
  Update README.md
  Make File.open_buffer support Tempfiles
  Version bump
  Update Changelog with Ruby 1.9 requirement
  Update README to reflect 1.9 requirement
  Fix rubyzip#106 Set options about restoring ownerships, permissions and times. restore permissions enabled by default.
  fix jRuby Building rubyzip#104
  Fix rubyzip#28 and rubyzip#103
  disable jRuby for a while
  Fix rubyzip#102 recover file permissions if zip file was exist
  Add missing Zip::Entry arguments to Zip::File#get_output_stream. Fixes rubyzip#100
  fix string encoding of zip64 header ids for ruby 2.0
  Add read/write support for zip64 extensions

Conflicts:
	lib/zip/extra_field.rb
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.08%) when pulling f780b39 on muz:aes_encryption into 7869cba on rubyzip:master.

@simonoff
Copy link
Member

@muz would be great to have tests in any case. Because it's much important thing.

@muz
Copy link
Author

muz commented Aug 27, 2014

@simonoff 100% agree. As it stands though, we know this PR hasn't regressed the code at the very least as the existing tests continue to pass.

That said, I've taken some time to look into this, and this would involve adding in tests against AES encrypted ZIP files. This in itself isn't a problem, what is a problem is the provisioning of them.

As I understand it, and correct me if I'm wrong, currently the tests create ZIP files prior to the tests executing by calling zip. This commandline tool isn't able to create AES encrypted ZIPs though (only old style encrypted ZIPs, with PKZIP ciphers). So certainly there would be some effort involved to get the tests to provision the correct type of encrypted ZIP test files first.

Have we any strong preferences for which route to take with this; as this would incur a dependency change for being able to run the tests too - unless we just bundle some AES encrypted ZIPs as part of the codebase (which I'm somewhat reluctant to do)

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.08%) when pulling 207a3a9 on muz:aes_encryption into 7869cba on rubyzip:master.

@hainesr
Copy link
Member

hainesr commented Aug 31, 2014

I'm generally happy to bundle things like zip files for the tests myself. They don't need to be too big for testing and it ensures that we really are testing against fixtures that are externally generated. It also keeps the tests themselves simpler, which is no bad thing. I say go for it and bundle.

@johnnyshields
Copy link
Contributor

@muz @jphastings my team is working on traditional encryption (work-in-progress) here: http://github.com/johnnyshields/rubyzip. We've come up with a structure that I believe is a bit more extensible--should be easy to plug AES into it. Please take a look.

@jphastings
Copy link

Will do @johnnyshields, where would you recommend we start looking - in the master branch?

@johnnyshields
Copy link
Contributor

@jphastings here's a diff johnnyshields/rubyzip@rubyzip:master...master (Github is so cool!)

We've added the structures for both encryption and decryption. I think one of the biggest things to determine is what the public interface should be, in particular OutputStream#open and #write_buffer methods and how we can support both aes and traditional encryption on them.

The code itself is a work in progress, doesn't seem to be 100% working yet but the structure is there. Copying @matsu911 to the thread.

@johnnyshields
Copy link
Contributor

FYI I've raised the PR for Traditional Encryption

@simonoff
Copy link
Member

@muz can you rebase with current master?

@cielavenir
Copy link

I have skimmed the code and noticed that AES encryption is compression method 99. So actually AES has to be implemented as a decompressor, which makes current "transparent decryption" difficult.

@bensomers
Copy link

No action on this PR for several years now - what are thoughts like on the prospects for this PR/this feature more generally?

@jphastings
Copy link

Hey @bensomers; I used to work with @muz while the company we completed this work for still existed. I can put some work into getting this up to par if @cielavenir's concerns aren't problematic to the maintainers.

@bensomers
Copy link

I'd be a fan of that, though it's really the maintainers we'd want to hear from before you spend the effort.

I actually need to be writing out AES files, not just reading them, so this would only be a start point for my needs - but it's pretty silly for me to submit a PR for writing before the capability to read is in.

@tramuntanal
Copy link

tramuntanal commented Jan 3, 2020

Hello everybody, is there a plan to add AES support (read/write) to rubyzip? It may be interesting for me to work on this with a small guidance.

Maybe #194 is a good start point? (@johnnyshields )

cc/ @simonoff @hainesr

@johnnyshields
Copy link
Contributor

Hi, I won't be working on this. With some googling you can probably find a ruby implementation or a python/javascript implementation that could be ported.

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.

None yet

9 participants