feat: allow custom salt by Its-Just-Nans · Pull Request #680 · zip-rs/zip2

Its-Just-Nans · 2026-02-18T02:17:16Z

gemini-code-assist · 2026-02-18T02:17:40Z

Summary of Changes

Hello @Its-Just-Nans, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces the capability to specify a custom salt when performing AES encryption within ZIP archives. This feature addresses the need for reproducible encryption, which can be valuable for testing and specific scenarios where a fixed salt is desired. While generally not recommended for security-sensitive applications due to reduced randomness, it provides flexibility for advanced use cases. The changes involve adding a new CustomSalt type, updating the AES encryption writer to accept this salt, and exposing a new FileOptions method for its usage.

Highlights

Custom Salt for AES Encryption: Introduced a new CustomSalt struct, allowing users to specify a custom salt for AES encryption, which can be useful for testing or achieving reproducible encryption results.
Enhanced AES Writer: Modified the AesWriter to accept an optional CustomSalt, enabling the use of either a randomly generated salt or a user-defined one during the encryption process.
New FileOptions Method: Added FileOptions::with_aes_encryption_and_salt to provide a public API for configuring AES encryption with a custom salt.
Internal Data Structure Updates: Updated internal data structures such as FileOptions, EncryptWith, and ZipFileData to properly store and propagate the optional CustomSalt information throughout the encryption workflow.
Reproducibility Test Case: Included a new test case (aes_custom_salt_for_reproducible_zip) to verify that using the same custom salt consistently produces identical encrypted ZIP file contents.

Changelog

src/aes.rs
- Defined CustomSalt struct with methods for creation and access.
- Refactored AesWriter::new into new_with_options to support custom salts.
- Modified AesWriter to use either a provided custom salt or generate a random one.
src/lib.rs
- Exported the new CustomSalt type.
src/read.rs
- Adjusted the aes_mode tuple when preserving metadata to account for the new Option<CustomSalt> field, setting it to None during read operations.
src/types.rs
- Imported CustomSalt.
- Implemented Display for AesMode.
- Added MAX_SALT_SIZE constant to AesMode.
- Extended the aes_mode tuple in FileOptions and ZipFileData to include an Option<CustomSalt>.
- Modified ZipFileData::new to correctly handle the updated aes_mode tuple.
src/write.rs
- Added FileOptions::with_aes_encryption_and_salt to allow specifying a custom salt.
- Updated FileOptions::with_aes_encryption to explicitly pass None for salt.
- Modified ZipWriter::start_file to pass the salt from EncryptWith::Aes to the aes_mode tuple.
- Updated the ZipWriter to use AesWriter::new_with_options when encrypting.
- Adjusted existing test cases to explicitly set salt: None in EncryptWith::Aes.
src/zipcrypto.rs
- Added an Option<crate::aes::CustomSalt> field to the EncryptWith::Aes enum variant.
tests/aes_encryption.rs
- Added a new test aes_custom_salt_for_reproducible_zip to verify that using the same custom salt produces identical encrypted ZIP files.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

amazon-q-developer

This PR adds support for custom salts in AES encryption, which is useful for testing and reproducible encryption. The implementation correctly:

Validates custom salt length against AES mode requirements
Preserves the existing random salt generation as default behavior
Properly gates the feature behind aes-crypto feature flag
Includes appropriate warnings in documentation about not reusing custom salts

I've identified one minor cleanup opportunity with a redundant .as_ref() call. Otherwise, the implementation looks sound and ready for merge once that's addressed.

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

src/aes.rs

src/types.rs

Its-Just-Nans · 2026-02-18T02:19:26Z

src/read.rs

        if let Some(aes) = self.get_metadata().aes_mode {
            // Preserve AES metadata in options for downstream writers.
            // This is metadata-only and does not trigger encryption.
+            let aes = (aes.0, aes.1, aes.2, None);


Here we don't retreive the custom salt when we want the "options for this file"

gemini-code-assist

Code Review

The pull request introduces the ability to provide a custom salt for AES encryption, which is a valuable feature for scenarios requiring reproducible encryption results. The implementation correctly integrates this into the FileOptions builder and the internal AesWriter. However, there are some concerns regarding the security of the API (potential mode mismatch leading to weak salts), the use of non-idiomatic error types (String) in public methods, and the increasing complexity of internal metadata tuples which impacts maintainability, as highlighted by the recommendation to encapsulate related options into structs for better API design.

src/write.rs

src/aes.rs

src/types.rs

tests/aes_encryption.rs

src/write.rs

src/aes.rs

src/zipcrypto.rs

Its-Just-Nans · 2026-02-18T02:50:56Z

Things to do/check

check the comment about option() function feat: allow custom salt #680 (comment)
comment about the potential "security" issue feat: allow custom salt #680 (comment) -> I think it's true, and we can
- change the custom salt to have the mode inside
- ignore since if someone want to use the custom salt it's at his own risk
fix the as_ref() feat: allow custom salt #680 (comment)
Potentially, create a new type like gemnini suggested is good feat: allow custom salt #680 (comment) but optionnal, I can do it if needed

tests/aes_encryption.rs

im7mortal · 2026-02-18T08:19:18Z

@Pr0methean @Its-Just-Nans Please read this comment

src/aes.rs

Its-Just-Nans · 2026-02-21T05:04:30Z

Even if the CI pass, I think we should maybe wait for #223 (comment)

src/aes.rs

src/write.rs

Its-Just-Nans · 2026-02-21T18:50:39Z

#680 (comment)

Yes I thought about that but if we do it like that the function will return an Error.
And that will break the chaining of FileOptions::new():

FileOptions::new().opt_1().opt_2() - works
FileOptions::new().opt_1().with_aes_encryption_and_salt()?.opt_2() - notice the need of ?

And this way feel more difficult to handle the error as a lib user. If the lib user want to handle the error easily I think it's better to this way.

That why, with the current method there is two steps:

first create a valid salt (and handle error if needed)
then use the option

Its-Just-Nans · 2026-02-21T19:14:43Z

Note that I think it would be better to merge #686 first

Than edit this pull request to have the password as &[u8] with with_aes_encryption_and_salt()

Signed-off-by: Chris Hennick <4961925+Pr0methean@users.noreply.github.com>

Signed-off-by: n4n5 <its.just.n4n5@gmail.com>

Its-Just-Nans · 2026-03-02T03:19:13Z

I think there was a github runner issue

Its-Just-Nans requested a review from Pr0methean February 18, 2026 02:17

Its-Just-Nans self-assigned this Feb 18, 2026

amazon-q-developer bot reviewed Feb 18, 2026

View reviewed changes

src/aes.rs Outdated Show resolved Hide resolved