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>

docs/en/mavlink/message_signing.md

@@ -11,9 +11,9 @@ This mechanism does not _encrypt_ the message payload.
 When signing is enabled, PX4 appends a 13-byte [signature](https://mavlink.io/en/guide/message_signing.html#signature) to every outgoing MAVLink 2 message.
 
 Incoming messages are checked against the shared secret key, and unsigned or incorrectly signed messages are rejected (with [exceptions for safety-critical messages](#unsigned-message-allowlist)).
-Rejected messages are also not forwarded to other links.
-This includes messages that PX4 does not know (for example custom messages), which are only forwarded if they are signed with PX4's key.
-Components connected through PX4 therefore need to use the same key.
+Unsigned messages are not forwarded to other links either.
+Signed messages that PX4 can't verify, for example because they are signed with the key of another component, are not processed but still forwarded, so that the receiving component can check them with its own key.
+Components connected through PX4 need to check signatures themselves if they should only accept authenticated messages.
 
 The signing implementation is built into the MAVLink module and is always available, with no special build flags required.
 The key is stored in an SD card:
@@ -138,6 +138,8 @@ When signing is active, **all links require signed messages**.
 This means:
 
 - An attacker cannot send unsigned commands on any link.
+- Signed messages that fail verification are not processed by PX4, but they are forwarded to other links (see [Overview](#overview)).
+  Components connected through PX4 must check signatures themselves.
 - Changing or disabling the key requires sending a `SETUP_SIGNING` message **signed with the current key**.
 - Signing can be disabled via MAVLink by sending a signed `SETUP_SIGNING` with an all-zero key.
 

src/modules/mavlink/mavlink_receiver.cpp

@@ -3978,7 +3978,8 @@ MavlinkReceiver::run()
 					// that actually send take lock_send() locally.
 					_mavlink.lock_send();
 					const uint8_t framing = mavlink_frame_char(_mavlink.get_channel(), buf[i], &msg, &_status);
-					const FrameCheck frame_check = check_frame(framing, msg);
+					bool bad_signature = false;
+					const FrameCheck frame_check = check_frame(framing, msg, bad_signature);
 
 					if (frame_check == FrameCheck::Invalid) {
 						reset_parser_after_rejected_frame(buf[i]);
@@ -4017,13 +4018,17 @@ MavlinkReceiver::run()
 						}
 
 					} else if (frame_check == FrameCheck::ForwardOnly) {
-						_unknown_message_counter++;
+						if (mavlink_get_msg_entry(msg.msgid) == nullptr) {
+							_unknown_message_counter++;
+						}
 
-						// The header of an unknown message isn't CRC checked, so only
-						// track the sequence of components we have already seen.
+						// These frames are not verified, the header of an unknown message
+						// isn't even CRC checked, so only track the sequence of components
+						// we have already seen.
 						update_rx_stats(msg, false);
+					}
 
-					} else if (frame_check == FrameCheck::BadSignature) {
+					if (bad_signature) {
 						_bad_signature_counter++;
 					}
 
@@ -4104,16 +4109,24 @@ MavlinkReceiver::run()
 	}
 }
 
-MavlinkReceiver::FrameCheck MavlinkReceiver::check_frame(uint8_t framing, const mavlink_message_t &message)
+MavlinkReceiver::FrameCheck MavlinkReceiver::check_frame(uint8_t framing, const mavlink_message_t &message,
+		bool &bad_signature)
 {
+	bad_signature = false;
+	const bool is_signed = message.incompat_flags & MAVLINK_IFLAG_SIGNED;
+
 	switch (framing) {
 	case MAVLINK_FRAMING_OK:
 		return FrameCheck::Ok;
 
 	case MAVLINK_FRAMING_BAD_SIGNATURE:
-		// With signing enabled, PX4 keeps unauthenticated traffic away from the
-		// other links, so we don't forward it either.
-		return FrameCheck::BadSignature;
+		bad_signature = true;
+
+		// A signed frame we can't verify might be signed with the receiver's key,
+		// so it's forwarded for the receiver to check, see
+		// https://mavlink.io/en/guide/routing.html
+		// Unsigned frames are not forwarded while signing is active.
+		return is_signed ? FrameCheck::ForwardOnly : FrameCheck::Unsigned;
 
 	case MAVLINK_FRAMING_BAD_CRC:
 		break;
@@ -4136,12 +4149,18 @@ MavlinkReceiver::FrameCheck MavlinkReceiver::check_frame(uint8_t framing, const
 		return FrameCheck::ForwardOnly;
 	}
 
-	if (message.incompat_flags & MAVLINK_IFLAG_SIGNED) {
+	if (is_signed) {
 		// The parser has checked the signature of this frame already.
-		return (signing->last_status == MAVLINK_SIGNING_STATUS_OK) ? FrameCheck::ForwardOnly : FrameCheck::BadSignature;
+		bad_signature = (signing->last_status != MAVLINK_SIGNING_STATUS_OK);
+		return FrameCheck::ForwardOnly;
 	}
 
-	return _mavlink.accept_unsigned(message.msgid) ? FrameCheck::ForwardOnly : FrameCheck::BadSignature;
+	if (_mavlink.accept_unsigned(message.msgid)) {
+		return FrameCheck::ForwardOnly;
+	}
+
+	bad_signature = true;
+	return FrameCheck::Unsigned;
 }
 
 void MavlinkReceiver::reset_parser_after_rejected_frame(uint8_t c)

src/modules/mavlink/mavlink_receiver.h

@@ -288,14 +288,17 @@ private:
 	void update_rx_stats(const mavlink_message_t &message, bool add_component);
 
 	enum class FrameCheck {
-		Incomplete,   ///< no complete frame yet
-		Ok,           ///< valid, handle and forward
-		ForwardOnly,  ///< not in our dialect, can't be handled but can be forwarded
-		BadSignature, ///< signature missing or not valid with our key, drop
-		Invalid,      ///< bad CRC, drop
+		Incomplete,  ///< no complete frame yet
+		Ok,          ///< valid, handle and forward
+		ForwardOnly, ///< unknown or signed with another key, can't be handled but can be forwarded
+		Unsigned,    ///< unsigned while signing is active, drop
+		Invalid,     ///< bad CRC, drop
 	};
 
-	FrameCheck check_frame(uint8_t framing, const mavlink_message_t &message);
+	/**
+	 * @param bad_signature set if the signature is missing or not valid with our key
+	 */
+	FrameCheck check_frame(uint8_t framing, const mavlink_message_t &message, bool &bad_signature);
 
 	/**
 	 * Reset the parser after a rejected frame, same as mavlink_parse_char() does.