Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

Update crypto deps and add a new Cipher associated type #15

Merged
merged 10 commits into from
Oct 23, 2023
Merged

Conversation

Nashtare
Copy link
Member

@Nashtare Nashtare commented Oct 17, 2023

Description

This PR generalizes the use of AES to any AEAD, to be used during share encryption, and updates the mode used in testing to be AES-GCM.

Additions and Changes

The outstanding changes are:

  • a new associated type to CipherSuite, namely Cipher, to be used during Dkg / Resharing to encrypt/decrypt secret shares. It must implement the Aead (for (de/en)crypting) and KeyInit (for Key initialization from the output of the HKDF) traits (+ Clone because that's convenient).
  • a type change for the nonce field of EncryptedSecretShare: it now is a Nonce<C::Cipher> which conveniently wraps around a GenericArray to give some guarantees on the expected underlying length, and remove the current hardcoded nonce length.
  • the default Cipher for testing is now Aes128Gcm. The rationale is that
    • following a bump of the aes crate from v0.7 to v0.8, there's been a big refactor removing among others the ctr mode
    • gcm is much better than ctr

Note that I didn't go for the aes-gcm-siv crate as:

  • the available structure implementing the Ciphersuite trait is, as mentioned, only for testing purposes
  • we don't really care about nonce reuse at this level, as even if Aes128Gcm was used as internal cipher in production, we'd need over 4 billion encryption over the same initial DH key to have problems...

Note that because of the new C::Cipher type, and the use of Aes128Gcm in testing, I had to manually implement several of the previously derived traits for EncryptedSecretShare. The nonce field for (de)serialization requires collecting, though shouldn't be a bottleneck hence I left it as a Vec.

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@Nashtare Nashtare added the enhancement New feature or request label Oct 17, 2023
@Nashtare Nashtare self-assigned this Oct 17, 2023
@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (a9c3b0c) 78.39% compared to head (e9b3dd2) 78.04%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
- Coverage   78.39%   78.04%   -0.36%     
==========================================
  Files          14       14              
  Lines         884      929      +45     
==========================================
+ Hits          693      725      +32     
- Misses        191      204      +13     
Files Coverage Δ
src/ciphersuite.rs 100.00% <ø> (ø)
src/dkg/complaint.rs 98.14% <ø> (ø)
src/dkg/key_generation.rs 79.43% <ø> (-3.58%) ⬇️
src/lib.rs 100.00% <ø> (ø)
src/error.rs 0.00% <0.00%> (ø)
src/dkg/secret_share.rs 87.28% <81.25%> (+2.87%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nashtare Nashtare requested review from 4l0n50 and removed request for hratoanina October 18, 2023 13:55
@Nashtare Nashtare changed the title Update crypto deps and make Cipher an associated type Update crypto deps and add a new Cipher associated type Oct 18, 2023
@Nashtare Nashtare requested review from hratoanina and removed request for LindaGuiga October 20, 2023 22:54
Copy link
Member

@4l0n50 4l0n50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@Nashtare Nashtare merged commit bab2625 into main Oct 23, 2023
9 checks passed
@Nashtare Nashtare deleted the crypto-deps branch October 23, 2023 20:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants