Skip to content

Use a 12-byte nonce as an input to Chacha20-Poly1305 #43

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 3, 2025
Merged

Use a 12-byte nonce as an input to Chacha20-Poly1305 #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/util/key_obfuscator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ impl KeyObfuscator {
fn generate_synthetic_nonce(&self, initial_nonce_material: &[u8]) -> [u8; NONCE_LENGTH] {
let hmac = Self::hkdf(&self.hashing_key, initial_nonce_material);
let mut nonce = [0u8; NONCE_LENGTH];
// TODO: While the RFC specifies a 12-byte nonce, we use an 8-byte nonce for
// backwards compatibility with the rust-lightning implementation of
// Chacha20Poly1305. We now use the rust-bitcoin implementation, which allows
// for 12-byte nonces, so we should figure out an upgrade path for this.
nonce[4..].copy_from_slice(&hmac[..8]);
nonce
}
Expand Down
2 changes: 1 addition & 1 deletion src/util/storable_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<T: EntropySource> StorableBuilder<T> {
&self, input: Vec<u8>, version: i64, data_encryption_key: &[u8; 32], aad: &[u8],
) -> Storable {
let mut nonce = [0u8; NONCE_LENGTH];
self.entropy_source.fill_bytes(&mut nonce[4..]);
Copy link
Contributor

@tnull tnull Oct 31, 2025

Choose a reason for hiding this comment

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

It seems we have another instance of this in KeyObfuscator's generate_synthetic_nonce. But there I doubt we can easily change it without breaking backwards compatibility. Maybe we should at least leave a comment there for future context why we only copy 8 bytes?

Copy link
Contributor

@tnull tnull Oct 31, 2025

Choose a reason for hiding this comment

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

Maybe more generally, should we add an upgrade test here? I.e., serialize a v0.3.1 Storable and make sure we can still deserialize it with our current code and aad = []?

Copy link
Contributor

@tnull tnull Oct 31, 2025

Choose a reason for hiding this comment

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

Maybe more generally, should we add an upgrade test here? I.e., serialize a v0.3.1 Storable and make sure we can still deserialize it with our current code and aad = []?

Now had Claude do it and opened #45.

Copy link
Contributor Author

@tankyleo tankyleo Nov 3, 2025

Choose a reason for hiding this comment

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

It seems we have another instance of this in KeyObfuscator's generate_synthetic_nonce. But there I doubt we can easily change it without breaking backwards compatibility. Maybe we should at least leave a comment there for future context why we only copy 8 bytes?

Convinced myself this is the case, and added a TODO, let me know how that sounds. Thinking about how we would upgrade this thing without putting some hacky version byte somewhere.

Copy link
Contributor Author

@tankyleo tankyleo Nov 3, 2025

Choose a reason for hiding this comment

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

Thank you for the upgrade test ! will review today.

self.entropy_source.fill_bytes(&mut nonce);

let mut data_blob = PlaintextBlob { value: input, version }.encode_to_vec();

Expand Down
Loading