Use a 12-byte nonce as an input to Chacha20-Poly1305#43
Use a 12-byte nonce as an input to Chacha20-Poly1305#43tnull merged 1 commit intolightningdevkit:mainfrom
Conversation
tankyleo
commented
Oct 30, 2025
tankyleo
commented
|
👋 Thanks for assigning @tnull as a reviewer! |
| &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..]); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 = []?
There was a problem hiding this comment.
Maybe more generally, should we add an upgrade test here? I.e., serialize a v0.3.1
Storableand make sure we can still deserialize it with our current code andaad = []?
Now had Claude do it and opened #45.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thank you for the upgrade test ! will review today.
|
CI failure will be fixed by #44 |
tankyleo
force-pushed
the
25-10-use-12-byte-nonce
branch
from
e196e3c to
13ab628
Compare
Previously, we were using the Chacha20-Poly1305 implementation at `rust-lightning/lightning/src/crypto/chacha20poly1305rfc.rs`. That implementation required us to use an 8-byte nonce. Since we made the switch to the `rust-bitcoin/chacha20_poly1305` implementation, we can now use a full 12-byte nonce as specified in the RFC.
tankyleo
force-pushed
the
25-10-use-12-byte-nonce
branch
from
13ab628 to
4128ce8
Compare
|
ACK'ed the upgrade test, I let you merge the upgrade test when you are ready. |
|
Cherry picked the two upgrade tests on top of this commit, both still pass, along with the full test suite. |
3 participants
