Open fmoessbauer opened 1 week ago
This is due to the differences between ed25519 and x25519, to the best of my understanding. I don't fully understand the difference myself, but this is in various implementations, e.g. https://github.com/paulmillr/noble-curves/blob/187654df5e6811a21d06e0819414e7e87293246a/src/ed25519.ts#L58 within the noble
library being used here, and https://rweather.github.io/arduinolibs/Curve25519_8cpp_source.html#l00245 within the library used by firmware.
Ah, found it in the RFC: https://www.rfc-editor.org/rfc/rfc7748#section-5
Scalars are assumed to be randomly generated bytes. For X25519, in order to decode 32 random bytes as an integer scalar, set the three least significant bits of the first byte and the most significant bit of the last to zero, set the second most significant bit of the last byte to 1 and, finally, decode as little-endian. This means that the resulting integer is of the form 2^254 plus eight times a value between 0 and 2^251 - 1 (inclusive). Likewise, for X448, set the two least significant bits of the first byte to 0, and the most significant bit of the last byte to 1. This means that the resulting integer is of the form 2^447 plus four times a value between 0 and 2^445 - 1 (inclusive).
Ah, found it in the RFC: https://www.rfc-editor.org/rfc/rfc7748#section-5
Thanks! Can someone of the original authors (@jp-bennett / @KomelT) please add this reference as a comment next to the code? This is not obvious and these (seemingly random) changes will always ring the bells of security auditors. I don't want to make that comment, as I'm still not 100% sure what exactly is returned by the library function.
Hey, thanks for tagging me. Yes, this is specified in the RFCs, to create valid keys. You're absolutely right that it should be documented. We have a security policy for the firmware itself. I'll see if we can expand it to the whole org.
Hardware
Not Applicable
Connection Type
HTTP
Local or Hosted
http://meshtastic.local
Firmware Version
2.5.3
Description
In d0bd02980de89146341133e34cc152eb0ca2b107, the generation of the Curve25519 based keypair was reworked, setting multiple bits of the private key to fixed values. This change was done without any further explanation in the commit message or in code. While this is suspicious by itself, ~I also don't understand why these particular bits are pinned~. A possible explanation is given in https://neilmadden.blog/2020/05/28/whats-the-curve25519-clamping-all-about/. Anyways, this code change must be commented.
Problematic code:
Diff: https://github.com/meshtastic/web/commit/d0bd02980de89146341133e34cc152eb0ca2b107#diff-01dc5badd89a6aa375c3b6218d6c7abb9e72f62f0d6fa9cbddada0cfd0837338L4
PS: Please add a security policy to this project so that users can report findings in a confidential way. Otherwise no responsible disclosure is possible.
Relevant console output
No response