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

Clarify the nonce values/sizes #5613

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

esev
Copy link
Contributor

@esev esev commented Dec 19, 2024

This PR is not intended to have any functional changes, it is only meant to document/clarify how the nonce value is computed for #5543.

I've looked as far back as v2.0.0.18ab874 and as best I can tell the argument used for packetId has always been populated with the uint32_t MeshPacket id field. When encoded into the nonce in little-endian format as a uint64_t only the first 32-bits contain the id value. The remaining most significant 32-bits that follow have always been set to 0. The extraNonce then overwrites those most significant 32-bits if it is provided.

I found this partial overwriting of the 64-bit packetId field with the extraNonce and the mismatch between the MeshPacket.id field type to be confusing when reading the code. The comment in the header file about how the values are concatenated also misled me to believe the extraNonce came after the sending node number.

As long as extraNonce is described as always being 0 when using the AES-CTR (Channel/classic) algorithm, all prior implementations where it was assumed that the packetId was 64-bit will be unchanged and be fully backward compatible; the most significant bits will remain 0 as they were in v2.0.0.18ab874. For the newer AES-CCM (PKC/PKI) algorithm, I don't think the full 64-bit packetId has ever been populated in the nonce without being half overwritten by extraNonce, so I think there shouldn't be any issues with backward compatibility. This allows for a unified description of how the nonce is computed that works for both algorithms.

I'll start this as a draft. @jp-bennett & @caveman99 I welcome any feedback on this. I'm only intending to make things clearer, and if this PR doesn't do that then it isn't needed.

@esev
Copy link
Contributor Author

esev commented Dec 19, 2024

One thing I'm still not clear on: Is extraNonce intended to be interpreted as an integer value by clients, or is it intended to be treated as 4 raw/opaque bytes. If it is intended to be read as an integer (counter), can I clarify in the comments that this is also written to the nonce in little-endian format?

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.

1 participant