Skip to content

mavgen_javascript: fix signing replay timestamp updates - #1329

Open
Johnny-Kao wants to merge 1 commit into
ArduPilot:masterfrom
Johnny-Kao:fix/js-signing-replay-state
Open

Johnny-Kao wants to merge 1 commit into
ArduPilot:masterfrom
Johnny-Kao:fix/js-signing-replay-state

Conversation

@Johnny-Kao

Copy link
Copy Markdown

Hi, this is my first contribution to pymavlink.

I noticed that the JavaScript_NextGen generator has a similar signing issue to the one fixed in #1301.

There are two related problems:

  • When a new stream receives a message with an invalid signature, its timestamp is saved before the signature is checked. This can cause a later valid message to be rejected.
  • When an existing stream receives a valid message, its timestamp is not updated. This means an older signed message may be accepted again.

I moved the timestamp update to after successful signature verification, following the approach used in #1301.

The change is small and does not modify the public API or MAVLink message format.

I also added regression tests using the project's existing pytest and JavaScript test helpers. The original generator fails these tests, while the updated version passes. The existing JavaScript/C integration tests and 2,983 Mocha tests passed in staging as well.

Staging checks: https://github.com/Johnny-Kao/mavlink-js-signing-research/actions/runs/42226080374

Thanks for taking a look!

@Johnny-Kao
Johnny-Kao marked this pull request as ready for review November 6, 2026 13:54
Move per-stream timestamp updates after successful signature verification,
so invalid signatures cannot poison replay state and accepted packets
advance the replay barrier.

Signed-off-by: Johnny Kao <19386515+Johnny-Kao@users.noreply.github.com>
@Johnny-Kao
Johnny-Kao force-pushed the fix/js-signing-replay-state branch from 74bb942 to eba49a8 Compare November 6, 2026 14:06
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