-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
proposal: archive/zip: support for encrypted archives #12081
Comments
I've got a working implementation of archive/zip that can read/write password protected archives. The code is at https://github.com/alexmullins/zip. Details: Public API additions:
Current roadblocks:
Any feedback is welcome. |
I was working on a project that requires me to send a password protected zip file to a system that can only read Zip Standard Encryption. Not finding any go source for this, I manage to create a working code based on @alexmullins code. Special thanks to you Sir 👍 While it's not advisable to use Zip Standard Encryption for security reason, for those who have to work with it, you can check my code at https://github.com/yeka/zip I hope the awesome Go Team can integrate encryption into their standard zip/archive 😄 |
Any updates here? officially supporting protected files would be good enough :) |
@yeka Could you add a license to your git repo? |
Sounds like there are two popular file formats as noted in #52458:
#52458 also explains why an external lib isn't ideal:
|
What the proposal process needs is a description of the new API that would be required to implement this. Anybody want to try to write that down? I haven't looked at the work mentioned above by @alexmullins . |
A first take at the API changes:
The three additional fields to FileHeader change the behavior when writing a zip. When FileHeader.Encryption is not equal to NoEncryption and FileHeader.Password is non-nil, the specified encryption method is used with the password bytes returned by by calling FileHeader.Password.. While FileHeader.Encryption is one of the Winzip methods, the AE file format "version" must be decided upon. WinZip chooses between AE-1 and AE-2 using logic described here: https://www.winzip.com/en/support/aes-encryption/#winzip11 If FileHeader.WinZipAEVersion is 0 in a FileHeader passed to Writer.CreateHeader(), an automatic decision is taken. A value of 1 or 2 would be taken as choosing AE-1 and AE-2 respectively. Any other value is erroneous. When reading a zip file, the FileHeader.Encryption and FileHeader.WinZipAEVersion fields will be populated. FileHeader.WinZipAEVersion will be 0 in the case of no encryption or ZipCrypto. Prior to calling File.Open() on a file marked as encrypted, the user must specify a function for FileHeader.Password. If nil, the open will fail with an invalid password error. If non-nil and the function returns a password that fails to decrypt the file, the same invalid password error will be returned. Updated : had neglected the use of the password on the read path. |
CC @dsnet |
I don't know how zip encryption works. If it requires a password, how should that password be supplied when reading a zip archive? |
@ianlancetaylor good point, I missed that part. FileHeader.Password would also be called in the read path. It would have to be set prior to calling File.Open. This is modeled on @alexmullins 's fork.
|
I didn't mention this, since it's something the user would not have to do. Writer.CreateHeader() would be expected to update FileHeader.Flags to indicate encryption. In the same that FileHeader.Flags gets updated when FileHeader.NonUTF8 gets set by the user. |
Hi, wanted to chime in. One roadblock that was hit when I initially was looking to integrate this into the std zip package is that there's a dependency on golang.org/x/crypto/pbkdf2. Not sure if there's a way around this problem. |
Not sure if you remember, but you raised that as an issue over in #13979. Two members of the go org chimed in to say that given that it's ~40 LOC, vendoring or copy/pasting is probably fine. |
If necessary we can vendor it into the standard library. See $GOROOT/src/vendor/golang.org/x/crypto for other vendored crypto packages. |
@rsc any chance a decision could be made on this proposal? |
Frankly, I am puzzled that a standard golang library has such a big gap on the zip protocol that its possible use on a large scale vanished. |
This proposal has been added to the active column of the proposals project |
@rsc does ☝️ mean the core I'm on paternity leave so have a little extra time in my life right now and happy to take a stab at clarifying the API design + implementing it (probably leveraging @alexmullins initial work ☝️), but first want to ensure the feature is of interest to the maintainers. |
@jeffwidman A proposal being in active status means that the proposal team will review the proposal, clarify details, and wait for consensus on whether and how it should be implemented. If it's accepted, an implementation can be merged into the standard library. You can find a full description of the proposal process here: https://github.com/golang/proposal#readme. If you intend to contribute an implementation based on someone else's work, make sure you understand the terms of the CLA. |
As it turns out, I've been working on a new module for reading and writing zip files (https://github.com/miquella/zipenc). I would happily donate any portion of it, if it would be helpful. I was hopeful that I could use an existing implementation, but had various troubles getting the existing implementations to work (e.g. missing password validation, poor key initialization, compilation issues, etc.). Reluctantly, I started a new implementation. Reimplementing substantial portions of The primary trouble is accessing the registered (de)compressors, which is why the module's current state only supports uncompressed files (Store method). I have tests and things coming, but wanted to get the basic version (which I believe works) posted. Only uncompressed zipcrypto is supported for now, since that's what I needed first. Note: Although I've tried to implement things according to the APPNOTE spec, it has been absolutely invaluable to have other implementations to reference. So thank you to all of those who came before! Here are a few that I've referenced while working on this project: |
Above there is a comment about needing a different CTR mode where the counter is written in little-endian order instead of big-endian order. This can be done without forking the crypto/cipher.NewCTR code with something along these lines:
|
I am not sure what the path forward here looks like. It appears that any API would have to take into account:
zip.FileHeader is meant to be pure data about the header; it seems wrong to drop in a func field. I wonder what the minimal set of changes would be to allow code to "bring your own encryption" instead, a bit like how we allow registering compressors and decompressors. |
@rsc CTR mode of encryption never calls block.Decrypt, so invBlock.Decrypt can |
I think we still don't have a path forward here. I am leaning toward declining this for now. It is not that important and we don't know what to do. |
Agree. I've looked at this a little from a design standpoint and didn't see an ergonomic API so I shelved it intending to circle back later in case I was missing something but no clean solution is jumping out at me. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
ZIP file format specification includes encrypted archives.
Current implementation doesn't support writing/reading such archives.
The text was updated successfully, but these errors were encountered: