implement Clone for LessSafeKey

[oconnor663]

OpeningKey and SealingKey intentionally avoid implementing Copy and
Clone, because they're attached to a fixed nonce sequence that should be
unique. LessSafeKey isn't attached to a nonce sequence, though, and
making it Clone lets callers avoid repeating key setup work.

Coming from #1158 (comment).

[codecov]

Codecov Report

Merging #1175 (2f29364) into main (b94d61e) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1175      +/-   ##
==========================================
+ Coverage   93.13%   93.17%   +0.04%     
==========================================
  Files         116      116              
  Lines       18081    18127      +46     
  Branches      195      195              
==========================================
+ Hits        16839    16890      +51     
+ Misses       1208     1202       -6     
- Partials       34       35       +1     

Impacted Files	Coverage Δ
src/aead.rs	65.90% <100.00%> (+14.74%)	⬆️
src/aead/aes.rs	82.96% <100.00%> (+0.15%)	⬆️
src/aead/aes_gcm.rs	98.99% <100.00%> (+<0.01%)	⬆️
src/aead/chacha.rs	100.00% <100.00%> (ø)
src/aead/gcm.rs	79.48% <100.00%> (+0.10%)	⬆️
src/aead/less_safe_key.rs	96.55% <100.00%> (+0.02%)	⬆️
tests/aead_tests.rs	99.53% <100.00%> (+0.04%)	⬆️
crypto/curve25519/curve25519.c	99.70% <0.00%> (-0.08%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b94d61e...2f29364. Read the comment docs.

[briansmith]

Please review PR #1185 and see if rebasing this on top of that PR will simplify this. AFAICT it will.

[oconnor663]

Yes it looks like the manual impl isn't needed on that branch. Everything is just #[derive(Clone)]: oconnor663@3275a8d

[briansmith]

Yes it looks like the manual impl isn't needed on that branch. Everything is just #[derive(Clone)]: oconnor663@3275a8d

Thanks! I have now merged that refactoring. If you rebase this on top of main, and add a test that encrypting some arbitrary data with the original key results in the same output as encrypting the same arbitrary data with a clone of the key (with the same nonce), then I'll prioritize merging this and ensure it is included in the next release.

[briansmith]

@oconnor663 Are you still interested in this? I'm preparing a new release that could include this if you rebase it onto main.

[oconnor663]

Done! Thanks for the reminder.

[oconnor663]

Addressed the comments above. Let me know what you think about the allow(unused_imports) workaround in particular. Also if you want me to combine the three separate tests into one test with a loop.

[briansmith]

@oconnor663 If you can rebase this and address the comment above, then I think we're good to go and I'll be able to include this in the next release.

[oconnor663]

I've rebased the branch and removed the extra commit that was just fixing a warning. Not sure about the codecov message. If I put a panic!() at the end of that function, cargo test fails as expected, so those lines are certainly getting run. I haven't yet figured out how to run codecov locally, but if you think that's the best next step let me know.

It looks like some repo settings have changed, and the CI run for the rebased branch won't start until I get some new permissions. Let me know if you need anything from me for that.

[briansmith]

It looks like some repo settings have changed, and the CI run for the rebased branch won't start until I get some new permissions. Let me know if you need anything from me for that.

I started the CI run. Recently GitHub started requiring manual approval for the first PR from a new contributor to a repo, to prevent cryptocurrency mining and other abuse of GitHub Actions.

[oconnor663]

Oof. As they say about nice things, and why we cannot have them...

[briansmith]

Thanks! Merged.

[oconnor663]

Great! Thanks for all the feedback.