Commit Graph
4 Commits
Author SHA1 Message Date
Julian Oes 107d8bb038 fix(mavlink): forward received frames unchanged (#28891)
* fix(mavlink): forward received frames unchanged

Forwarded messages were queued as a truncated copy of mavlink_message_t
and re-serialized on the way out. The signature bytes were never
copied, so forwarded signed messages went out with a garbage signature.

Messages which are not in our dialect were not forwarded at all: the
parser can't check their CRC without CRC_EXTRA, reports them as bad CRC,
and mavlink_parse_char() drops them.

Frames are now queued as they arrived, checksum and signature included,
and written out as is. Following the MAVLink routing guide, frames that
can't be processed locally are still forwarded but not handled: unknown
MAVLink 2 messages, and messages whose signature can't be verified. The
signature isn't checked or removed, that's up to the receiver, which
may well have a different key than PX4. SETUP_SIGNING is still never
forwarded.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): only apply SETUP_SIGNING addressed to us

The target of SETUP_SIGNING was ignored, so a key meant for another
system or component, e.g. a companion computer, was applied to PX4 and
all its links instead.

SETUP_SIGNING is now only applied when it is broadcast or addressed to
us. Otherwise it is dropped, as it must never be forwarded, and a
warning tells the user that it didn't reach its target.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* refactor(mavlink): forward messages from one place

Messages which were handled locally were forwarded from the end of
Mavlink::handle_message(), while frames which can only be forwarded
took a separate path in the receive loop. Forwarding is now decided in
one place in the receive loop for both, and Mavlink::handle_message()
only deals with SETUP_SIGNING.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* refactor(mavlink): check forwarding enabled only once

Whether to forward is decided by forward_if_enabled(), so
forward_only_frame() only needs to decide whether a frame is valid but
can't be processed locally. As a result, unknown and incorrectly signed
frames no longer count as parse errors, independent of whether
forwarding is enabled.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* feat(mavlink): count unknown and incorrectly signed messages

Messages which are not in our dialect or whose signature can't be
verified are not processed and no longer count as parse errors. Count
them per link in telemetry_status instead, so they show up in the log
and in mavlink status, e.g. to find a sender using the wrong key.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): count lost messages correctly across sequence wrap-around

The sequence number wraps from 255 to 0, so the gap across the wrap is
seq + 256 - expected, not seq + 255 - expected. Every wrap-around with
loss undercounted by one message.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): report rx message loss as a fraction of all messages

rx_message_lost_rate was lost / received, which exceeds 1 with heavy
loss and is 0/0 before anything was received, and mavlink status then
printed that ratio as a percentage without scaling it. It is now
lost / (received + lost), 0 if nothing was received yet, and printed
as a percentage.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): don't forward unauthenticated frames when signing is active

With a signing key loaded, frames which fail PX4's signature check were
forwarded to the other links. PX4 keeps unauthenticated traffic from
e.g. the radio away from components which don't sign themselves, as the
signing docs describe, so drop them again, and count them as bad
signatures.

Unknown messages are reported as bad CRC by the parser, which doesn't
report their signature result, so apply the same rules to them: with
signing active they are only forwarded if signed with PX4's key or
allowed unsigned.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): don't count forwarded unknown messages as lost

Unknown messages didn't advance the sender's sequence, so the next
message from it counted them as lost. Track their sequence as well, but
only for components we have already seen: the header of an unknown
message isn't CRC checked, and garbage IDs would fill the component
table, which is also used for routing.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): forward signed frames PX4 can't verify

With signing active, a signed frame which fails PX4's check might be
signed with the receiver's key, so forward it for the receiver to
check, as the MAVLink routing guide says, but don't process it.
Unsigned frames are still dropped, apart from the unsigned allowlist,
so an attacker still can't send unsigned messages through PX4.

Suggested by @dakejahl in review.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* fix(mavlink): drop signed frames PX4 can't verify again

This reverts commit 80639d05d8. Setting the signed flag and appending a
random signature is trivial, so forwarding signed frames that fail
verification would let an attacker reach components behind PX4 that
don't check signatures themselves, just like unsigned frames. In
practice all systems share one key anyway, so with signing active only
forward what verifies with PX4's key, or is on the unsigned allowlist.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

* docs(mavlink): signing assumes one shared key

Make explicit that all systems and components share one symmetric key,
that components communicating through PX4 need to use PX4's key, and
that incorrectly signed messages are not forwarded, so they don't reach
components behind PX4 which don't check signatures themselves.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Julian Oes <julian@oes.ch>

---------

Signed-off-by: Julian Oes <julian@oes.ch>
2026-09-30 10:55:25 +13:00
05de941399 Add Traffic Avoidance System (ADSB/FLARM) Arming Check (#26390)
* Add traffic avoidance system checks

* Update msg/px4_msgs_old/msg/VehicleStatusV1.msg

Co-authored-by: Hamish Willee <hamishwillee@gmail.com>

* docs: add arming check for traffic avoidance systems in ADS-B/FLARM documentation

* fix formating

* trafficAvoidanceCheck: switch case for configuration options

---------

Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Matthias Grob <maetugr@gmail.com>
2026-02-18 17:37:03 +01:00
Matthias Grob 0b370ab5d3 Remove obstacle avoidance MAVLink Heartbeat check 2025-02-18 14:33:16 +01:00
Daniel Agar cea185268e msg ROS2 compatibility, microdds_client improvements (timesync, reduced code size, added topics, etc), fastrtps purge
- update all msgs to be directly compatible with ROS2
 - microdds_client improvements
   - timesync
   - reduced code size
   - add to most default builds if we can afford it
   - lots of other little changes
 - purge fastrtps (I tried to save this multiple times, but kept hitting roadblocks)
2022-10-19 19:36:47 -04:00