proposal: archive/zip: support for encrypted archives

[raichu]

ZIP file format specification includes encrypted archives. Current implementation doesn't support writing/reading such archives.

[alexmullins]

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: The encryption/decryption method used is WinZip AES Encryption Specification (http://www.winzip.com/aes_info.htm). There are 2 other encryption methods specified in the original Zip Spec (https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT, Sections 6 and 7) which aren't implemented. The method described in Section 6, also known as ZipCrypto, is generally considered 'weak' and isn't advisable to use. The method described in Section 7 seems to be exclusively used with PKWARE products as I haven't seen any other opensource implementations support it.

Public API additions:

• FileHeader.DeferAuth
• func (*FileHeader) IsEncrypted
• func (*FileHeader) SetPassword
• func (*Writer) Encrypt

Current roadblocks:

• The method to turn a password into a master-key is PBKDF2 and I'm getting that functionality from the 3rd party package golang.org/x/crypto/pbkdf2.
• Duplicating the cipher.NewCTR implementation from Go's Standard Library. (See here: https://github.com/alexmullins/zip/blob/master/crypto.go#L109). It was clarified for me that Go uses a right-aligned counter which is the standard way of doing it. WinZip AES uses a left-aligned counter. The difference between the two is:

Go CTR: 00000000000000000000000000000001 00000000000000000000000000000002

WinZip CTR: 01000000000000000000000000000000 02000000000000000000000000000000

Any feedback is welcome. Thanks.

[yeka]

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 😄

[ghost]

Any updates here? officially supporting protected files would be good enough :)

[florianpinel]

@yeka Could you add a license to your git repo?

[jeffwidman]

Sounds like there are two popular file formats as noted in https://github.com/golang/go/issues/52458:

There are two common encryption formats that we should support:

1. ZipCrypto: This is the original encryption scheme. It has widespread support, so should be added for compatibility purposes. However, it is not secure, so should come with a big disclaimer that new files should only be created for compatibility purposes and the encryption should not be trusted.
2. WinZip’s AES: This is what WinZip and 7Zip use by default to create encrypted zip files.

https://github.com/golang/go/issues/52458 also explains why an external lib isn't ideal:

Adding support for both of these formats into the standard library is relatively straightforward. Unfortunately, as it stands today, it’s impossible to add support for encryption as a wrapper around the standard library archive/zip. A custom compressor/decompressor handler approach doesn’t work because encryption needs at minimum access to the FileHeader. In the case of WinZip AE-2 it even needs to disable the CRC. So libraries have to completely fork archive/zip.

[ianlancetaylor]

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 .

[ahilananantha]

A first take at the API changes:

type EncryptionMethod int

const (
    NoEncryption EncryptionMethod = iota
    ZipCrypto
    WinZipAES128
    WinZipAES192
    WinZipAES256
)

type FileHeader struct {
    /* ... */

    Password func() []byte

    Encryption EncryptionMethod

    WinZipAEVersion int

    /* ... */
}

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.

[ianlancetaylor]

CC @dsnet

[ianlancetaylor]

I don't know how zip encryption works. If it requires a password, how should that password be supplied when reading a zip archive?

[ahilananantha]

@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.

    zReader, err := zip.NewReader(data, size)
    // ...
    for _, file := range zReader.File {
        // We can see that it's encrypted
        if file.Encryption != zip.NoEncryption {
            // and set appropriate per file password
            file.Password = func() []byte {
                return []byte("password")
            }
        }
        rc, err := file.Open()
        // ...
    }

[ahilananantha]

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.

[alexmullins]

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.

[jeffwidman]

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 https://github.com/golang/go/issues/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.

[ianlancetaylor]

If necessary we can vendor it into the standard library. See $GOROOT/src/vendor/golang.org/x/crypto for other vendored crypto packages.

[iangudger]

@rsc any chance a decision could be made on this proposal?

[ior308]

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.

[rsc]

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

[jeffwidman]

@rsc does ☝️ mean the core go team is interested in the feature?

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.

[zephyrtronium]

@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.

[miquella]

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 archive/zip sounded very unappealing, but so did maintaining a fork of it. Instead, I opted for a middle-ground that could still be troublesome: wrapping the "raw" file interfaces. Although it hasn't been without trouble, it has largely worked well so far.

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:

• https://github.com/LuaDist/unzip
• https://github.com/alexmullins/zip
• https://github.com/yeka/zip
• https://github.com/kdungs/zip

[rsc]

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:

type invBlock struct {
    cipher.Block
}

func reverse(b []byte) {
    for i, j := 0, len(b)-1; i < j; i, j = i+1, j-1 {
        b[i], b[j] = b[j], b[i]
    }
}

func (b *invBlock) Encrypt(dst, src []byte) {
    reverse(src)
    b.Block.Encrypt(dst, src)
}

func (b *invBlock) Decrypt(dst, src []byte) {
    b.Block.Decrypt(dst, src)
    reverse(src)
}

func NewInvCTR(block cipher.Block, iv []byte) cipher.Stream {
    return cipher.NewCTR(&invBlock{block}, iv)
}

[rsc]

I am not sure what the path forward here looks like. It appears that any API would have to take into account:

1. Each file entry can have a separate encryption password.
2. Each file entry can have a separate encryption certificate as well.
3. The overall zip file can have a separate encryption password for the central directory (used to find out about the other files).
4. The overall zip file can have a separate encryption certificate as well.
5. There are a large variety of defined encryption algorithms, and we don't want to make archive/zip depend on them all.

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.

[starius]

@rsc CTR mode of encryption never calls block.Decrypt, so invBlock.Decrypt can panic.

[rsc]

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.

[jeffwidman]

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.

[rsc]

Based on the discussion above, this proposal seems like a likely decline. — rsc for the proposal review group

[rsc]

No change in consensus, so declined. — rsc for the proposal review group