Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

ZIP File Encryption #157

Closed
Plecra opened this issue Jun 23, 2020 · 8 comments
Closed

ZIP File Encryption #157

Plecra opened this issue Jun 23, 2020 · 8 comments

Comments

@Plecra
Copy link
Member

Plecra commented Jun 23, 2020

When creating a ZIP archive, we have a few options for encrypting the contents:

  • traditional PKWARE encryption/"ZipCrypto"
    • Insecure
  • Strong Encryption
    • Patented with no clear licensing option
  • AE-x
    • fractured support from zip utilities (Windows Explorer can't read them)

For this project, AE-x is the only standard we could practically implement, however, even with the stronger algorithm, it doesn't obscure file metadata. Since this could be taken advantage of, users who need their data to be secure are recommended to encrypt the whole archive.

Given all of these issues, I don't think there is a clear choice for an encryption API in the crate.

So for now, this issue is tracking any use cases for these features. If there is a real need for AE-x/ZipCrypto, we can further explore the design.

@BenjaminRi
Copy link

BenjaminRi commented Jun 29, 2020

When it comes to crypto, I think we should aim for feature parity with 7-zip ( https://www.7-zip.org/ ). 7-zip does a lot of things right, the set of features it supports is among them (and because it's open-source, it's easy to get some inspiration if we get stuck). When I open the "Add to archive..." dialog, I get the following window:

7zip-dialog

They support ZipCrypto and AES256.

  • ZipCrypto is still surprisingly common. Despite the existence of various cryptographic attacks, it remains somewhat popular and widespread. We have ZipCrypto decryption now. Doing encryption isn't a big effort anymore. We need some cryptographically secure randomness, but I've looked into the rand crate (which we already depend on anyway) and it seems to provide such randomness. However, should we even provide ZipCrypto encryption, knowing it is insecure?

  • AES256 is often supported and strong. Depending on how up to date your Linux distro is, the regular unzip tool can do it. Yes, the metadata is not encrypted, but that's perfectly fine if you e.g. AES256 encrypt your Bitcoin wallet private key. To support AES256, we have to introduce some new dependency that handles the algorithm such that it is secure and has good performance. AES256 is often directly supported by the CPU, many provide specific AES instructions. We don't want to implement this ourselves. It'll probably come down to an optional feature flag, possibly linking a C library. Though Rust crypto has come a long way, so maybe we can depend straight on a Rust library (I'd prefer that). We may also need stuff like SHA1 and SHA256 and so on, I haven't looked into that. It would be convenient to depend on a library that provides that for us too. Then we need to write the code to make it work with Zip (headers, password validation, etc.). That'll probably be a bit of non-trivial work. We have to be particularly careful with encryption because even minor mistakes can render the archive insecure. There are countless subtle pitfalls when implementing stuff like that. But by now, the specification is mature and that will help.

I think if we support those two algorithms, we support most crypto needs for Zip archives.

@mvdnes
Copy link
Collaborator

mvdnes commented Jun 29, 2020

I would strongly discourage encrypting with ZipCrypto as it serves no good purpose due to its weaknesses. Decrypting is fine, to be compatible with existing archives.

@Plecra
Copy link
Member Author

Plecra commented Jun 30, 2020

The aes-gcm crate has been audited, and seems to be high quality. I'd be fine using that if we were to implement AE-X encryption.

I'm still not sure it's worth it though - I don't think it's appropriate for a tool to claim to "encrypt your data" while leaving important information in plaintext.

I think it would be better to encourage users to write to an encrypted io::Write (if they need security) and make sure our ZipWriter doesn't require io::Seek.

Edit: to be clear, we should absolutely be able to read AE-X archives.

@Plecra
Copy link
Member Author

Plecra commented Aug 23, 2020

I'd be receptive to a PR implementing a (non-default) insecure-encryption feature if it turns out that it has its use cases (there's bound to be an API out there that requires ZipCrypto). Due to the weaknesses of the spec, we also wouldn't demand a particularly defensive implementation.

@damccull
Copy link

Well I would love to be able to open zips with some or all of the files aes encrypted. Don't need to write it for my purpose but either seems impossible to do in rust.

@Plecra
Copy link
Member Author

Plecra commented May 29, 2021

#203 addresses decryption, and looks like a reasonable implementation so far. I'll need to fix the conflicts with the other PRs that have been merged in the meantime.

@mbr
Copy link
Contributor

mbr commented Aug 12, 2021

#203 has been cleaned up and rebased in the meantime, courtesy of @Lireer. Maybe it is time to give it another look? :)

@zamazan4ik
Copy link
Contributor

I will look into the #203

@zip-rs zip-rs locked and limited conversation to collaborators Feb 1, 2023
@Plecra Plecra converted this issue into discussion #345 Feb 1, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

7 participants