mirror of
https://github.com/meshtastic/firmware.git
synced 2026-10-09 14:41:19 -04:00
95906609db
* Bind request_id and reply_id into the XEdDSA signing buffer
A signed reply can be re-pointed at a different message today. The client sets
reply_id on an outgoing text to make a tapback, firmware signs the resulting
broadcast, but reply_id lives in the Data envelope rather than the payload the
signature covers - and channel crypto is AES-CTR with no MAC, so anyone holding
the PSK can rewrite it in flight and the signature still verifies. request_id
has the same shape and is bound with it.
Packets carrying neither field keep the existing [from|id|portnum|payload]
layout, byte-identical to what v2.8.0 alphas are signing today, so the bulk of
signed traffic - broadcasts - stays verifiable in both directions across the
upgrade. Only packets that actually carry one of the two fields use the extended
layout. Both sides pick the layout from the packet's own decoded fields, so
nothing is transmitted to select it.
Open question for review, deliberately not decided here: a format/version byte
in the buffer would be cleaner than a conditional layout, because the safety
argument for the conditional form has to be re-derived whenever a portnum is
added. It costs nothing on the wire since the buffer is never transmitted, but
it changes every signature and so breaks verification against the alphas that
are already signing. If we want it, better done once and before 2.8.0 leaves
alpha.
Tests: request_id/reply_id flips alongside the existing from/id/portnum negative
cases; a hand-built alpha-format signature that must still verify, and must not
be reinterpretable as the extended layout or vice versa; and receive-path cases
for a retargeted tapback, a retargeted response, and an ordinary signed
broadcast that must be unaffected.
* Sign the whole Data envelope, in one unambiguous layout
Replaces the conditional two-layout signing buffer with a single fixed one:
version(1) | from(4) | id(4) | to(4) | portnum(4) | request_id(4)
| reply_id(4) | emoji(4) | bitfield(4) | flags(1) | payload(N)
The conditional scheme was ambiguous. Base was header || arbitrary payload, so
any byte string the extended layout emitted was also a legal base payload: an
attacker could move eight payload bytes into request_id/reply_id and truncate a
signed message while its signature still verified. No marker placed only in the
extended layout fixes that, because base can always reproduce it. A fixed-length
header does - the payload boundary is total - XEDDSA_SIGNED_HEADER_LEN and never
depends on content.
Binding reply_id alone was also not enough, because the fields around it are just
as malleable:
emoji a reaction is a text packet with the emoji in the payload,
reply_id naming the parent, and this flag telling the client to
render it as a reaction. Flipping it turns a signed reply into a
signed reaction, so it has to travel with reply_id.
bitfield bit 0 is OK_TO_MQTT, the sender's consent to upload to a public
broker, and the exploitable direction is the one that leaks. The
whole uint32 is signed so bits 2..31 are covered in advance, and
presence is signed separately so stripping it is not the same as
sending it zero.
want_response bit 1 of bitfield mirrors it and Router merges the two with |=,
so signing either alone protects neither.
to without it a signed broadcast can be re-addressed as a direct
message and still verify, delivering a public statement as an
apparent private one. Relays rewrite hop_limit, next_hop and
relay_node, never `to`.
Left out: dest and source (one write in the tree, no readers), channel (the wire
carries a hash where the decoded packet carries an index), and the hop fields,
which relays rewrite by design. Signing the encoded Data wholesale is not an
option either - a relay that holds the channel key decodes and re-encodes it, so
byte fidelity is lost and unknown fields are stripped. The ack_proof excision
trick does not transfer for the same reason: Routing survives because it rides
inside the opaque payload, which Data itself does not.
sign/verify now take the Data rather than a field list, so adding to the covered
set cannot silently miss a call site. Integers are explicitly little-endian, as
in ackProofCompute. The buffer is sized from the schema's own maximum payload
rather than from what the fits-on-air gate currently admits, because an overflow
makes buildSigningBuffer return 0 and signing fail with no error; MAX_BLOCKSIZE
is left alone so the AES-CTR guard and scratch buffers sharing it are unaffected.
This changes every signature, so it is not compatible with the v2.8.0 alphas. It
is deliberately being done while 2.8.0 is still prerelease: a verify failure
against an authoritative key is an unconditional drop, so the same change after
2.8.0 goes stable would split signed traffic across versions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>