fix(MAVLink/Signing): wall-clock timestamp refresh, re-sign at send, drop qtkeychain keystore#14430
Merged
HTRamsey merged 1 commit intoJun 6, 2026
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #14430 +/- ##
==========================================
+ Coverage 25.47% 28.04% +2.57%
==========================================
Files 769 766 -3
Lines 65912 66277 +365
Branches 30495 30585 +90
==========================================
+ Hits 16788 18589 +1801
+ Misses 37285 34915 -2370
- Partials 11839 12773 +934
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 202 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Contributor
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 2 passed, 45 failed, 7 skipped. Test Resultslinux-coverage: 89 passed, 0 skipped Code Coverage
Artifact Sizes
Updated: 2026-06-05 18:31:42 UTC • Triggered by: Android |
|
Hi, thanks for working on this! I've been testing this draft for a week, but the bug is still present. Tested on build ID 81416f0 timestamp 2026-05-22T16:29:33-04:00 (artifact of this PR's action) |
HTRamsey
force-pushed
the
fix/signing-timestamp-wallclock-refresh
branch
from
9b62660 to
582906c
Compare
|
Just tested the new commit. The bug seems fixed! |
HTRamsey
force-pushed
the
fix/signing-timestamp-wallclock-refresh
branch
2 times, most recently
from
d3b98f3 to
50d7f5a
Compare
HTRamsey
changed the title
…keychain mavlink#14375 — outgoing signing timestamp drift libmavlink only increments signing->timestamp by +1 per packet, and cached/retried sends (Vehicle::sendMessageMultiple) reuse the same already-encoded bytes, so the wire timestamp freezes and drifts behind wall clock. Peers that pin signing.timestamp to wall clock (ArduPilot) then reject the packet as OLD_TIMESTAMP. Re-sign at the single send chokepoint: SigningChannel::signOutgoing() stamps a current, monotonic timestamp and recomputes the signature just before mavlink_msg_to_send_buffer() in Vehicle::sendMessageOnLinkThreadSafe(). The secret key never leaves the signing layer. signMessage() and verifySignature() share one hash helper so the wire format cannot diverge. The 1 Hz refresh timer is kept for the re-encode paths (command retries). mavlink#14447 — keystore QLockFile test hang MAVLinkSigningKeys guarded its store with a cross-process QLockFile whose stale-lock heuristic fails on macOS after a SIGKILL'd run, blocking 5 s per write. RunGuard already enforces single-instance, so the lock is redundant; remove it. mavlink#14466 — qtkeychain libgcrypt link failure qtkeychain's Linux libsecret backend pulls in libgcrypt/libgpg-error, whose cross-distro version skew breaks AppImage and source builds. Remove qtkeychain and the QGCKeychain wrapper entirely; store signing key bytes directly in the existing QSettings store (file-permission protected) under a keys/ subgroup. Drops the libgcrypt early-init and the libgcrypt/libsecret apt dependencies. Fixes mavlink#14375 Fixes mavlink#14447 Fixes mavlink#14466
HTRamsey
force-pushed
the
fix/signing-timestamp-wallclock-refresh
branch
from
50d7f5a to
a8ace2c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens MAVLink signing so QGC reliably connects to an already-running, signing-enabled vehicle, and simplifies key storage. Fixes #14375 (GCS fails to connect when SITL is already running —
OLD_TIMESTAMPrejection).Root cause
SigningChannel::init()seeds_signing.timestampfrom wall clock once. After that, libmavlink'smavlink_sign_packetonly advances the value by+1per outbound packet — it does not track real time. Two paths then drift behind wall clock and get rejected by peers that pinsigning.timestampto wall clock (ArduPilot'sGCS_Signing::update_signing_timestamp):init()value (~3-minute offset in MAVLink signing: GCS fails to connect when SITL is already running #14375).Vehicle::sendMessageMultiplecaches the already-signed bytes and re-emits them across retries, so the frozen signature timestamp drifts behind real time.Fix
SigningChannel::refreshOutgoingTimestamp()bumps_signing.timestampup to current wall clock under the write lock (no-op when disabled or already ahead).SigningControllerdrives it from a 1 HzQTimer, well under the receiver's 6 sMAVLINK_SIGNING_TIMESTAMP_LIMIT. The timer is now gated on channel-signing state — it runs only while a channel is actually signing, not for every link's lifetime.SigningChannel::signOutgoing()re-signs a cached outgoing message in place with a current, monotonic timestamp immediately before it hits the wire, fixing the cached-resend drift. The secret key never leaves the signing layer.QGCKeychain(qtkeychain) and theQLockFilecross-process guard; keys persist viaQSettingswith an explicitsync(). Removes the qtkeychain dependency and its install plumbing.VehicleSigningControllersplits handler wiring from transmit (_wireConfirmHandlersonce per op,_sendAndStartRetransmitfor the wire), so the retransmit path can't double-deliver the one-shot confirm/fail connections.Test plan
SigningTest— new regressions:_testRefreshOutgoingTimestamp(3-minute stall catch-up, no-op when ahead/disabled),_testSignOutgoingRefreshesCachedTimestamp(cached-resend re-sign stays monotonic and still verifies),_testKeyStorePersistRoundTrip(QSettings persist/reload/remove).SigningControllerTest— new:_testWallClockTimerRefreshesAfterEnable/_testWallClockTimerStoppedWhenIdle(timer gating). Burst-alert / auto-detect / state-machine unchanged.MockLinkSigningTest— new:_testEnableDisableReEnableCycle(handlers re-wire per op; no spurious failure). Enable/disable FSM unchanged.SigningController::setTimeoutForTesting()seam so timeout-path tests no longer pay the production 5 s wait (suite runtime 35 s → 17 s).