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

Added Clone to Cocoon and MiniCocoon #30

Closed
wants to merge 2 commits into from
Closed

Added Clone to Cocoon and MiniCocoon #30

wants to merge 2 commits into from

Conversation

Deaths-Door
Copy link
Contributor

#29

@fadeevab
Copy link
Owner

@Deaths-Door grcov (for the coverage) glitches once again giving us ~6% of coverage. This time it has detected 3636 branches in the src/lib.rs comparing to ~300 in the previous runs which gives us such a low percentage of coverage. I have to try something else instead of grcov.

@fadeevab
Copy link
Owner

fadeevab commented Sep 1, 2024

@Deaths-Door I pulled your PR locally and tried to unit-test it, and I found that Cocoon is not actually cloneable, although the MiniCocoon is cloneable. It seems like the reason is that the Cocoon structure doesn't copy the password inside, instead it holds the reference. It's done for security reasons, cocoon doesn't own unnecessary copies of secret data trying to zeroize all private keys.

Add a unit test with cocoon.clone() for MiniCocoon and remove the derive(Clone) for the Cocoon.

In your application, you can manage the user password independently (encrypt, erase, clone, and such).

If you need the Cocoon::clone() specifically, we might postpone this PR.

@Deaths-Door
Copy link
Contributor Author

Deaths-Door commented Sep 1, 2024

Add a unit test with cocoon.clone() for MiniCocoon and remove the derive(Clone) for the Cocoon

Ok I will do it. Strangely I didnt get an error for Coocon ( or I just missed it 😅)

If you need the Cocoon::clone() specifically, we might postpone this PR.

I have no use for the Cocoon::clone() at the moment.
But hypothetically , I am thinking that it maybe useful for sharing the instance between threads without reconstructing the Cocoon (please correct me if i am wrong, I have not used Cocoon yet)

@fadeevab
Copy link
Owner

fadeevab commented Sep 1, 2024

@Deaths-Door The Cocoon structure is pretty hollow and it does not actually represent the container structure, it just HELPS to dump and parse the container. If you needed to use Cocoon with the same password in 2 different threads, you would need to clone the password into these threads and create the Cocoon twice. Also, it's gonna challenge you to think twice whether to share or not the password with 2 different threads ;)

Overall, because of the Cocoon's limitations, the MiniCocoon was introduced.

@fadeevab
Copy link
Owner

fadeevab commented Sep 1, 2024

@Deaths-Door Please, rebase to the latest main branch commit, I fixed the code coverage.

@Deaths-Door Deaths-Door closed this by deleting the head repository Sep 1, 2024
@Deaths-Door
Copy link
Contributor Author

Deaths-Door commented Sep 1, 2024

Sorry I seemed to have closed this issue , I will make a new PR for this

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.

2 participants