Correct handling of `UnknownRequiredFeature` deserialization 2022-01-correct-req-feature-handling
authorMatt Corallo <git@bluematt.me>
Wed, 26 Jan 2022 02:04:20 +0000 (02:04 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 26 Jan 2022 02:12:35 +0000 (02:12 +0000)
Quite some time ago, `UnknownRequiredFeature` was only used when a
gossip message has a missing required feature. These days, its also
used for any required TLV which we do not understand in any
message. However, the handling of it was never updated in
`PeerManager`, leaving it printing a warning about gossip and
ignoring the message entirely.

Instead, we send a warning message and disconnect.

Closes #1236, as caught by @jkczyz.

lightning/src/ln/peer_handler.rs

index 86cd344ce352dea1544f27d894a94813a6ca3930..b910d4bf7798135ec08d8e84fc550fb907e318fb 100644 (file)
@@ -902,7 +902,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                        Ok(x) => x,
                                                                                        Err(e) => {
                                                                                                match e {
-                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, _) => {
+                                                                                                       // Note that to avoid recursion we never call
+                                                                                                       // `do_attempt_write_data` from here, causing
+                                                                                                       // the messages enqueued here to not actually
+                                                                                                       // be sent before the peer is disconnected.
+                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, Some(ty)) if is_gossip_msg(ty) => {
                                                                                                                log_gossip!(self.logger, "Got a channel/node announcement with an unknown required feature flag, you may want to update!");
                                                                                                                continue;
                                                                                                        }
@@ -916,6 +920,11 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                                                self.enqueue_message(peer, &msgs::WarningMessage { channel_id: [0; 32], data: "Unreadable/bogus gossip message".to_owned() });
                                                                                                                continue;
                                                                                                        }
+                                                                                                       (msgs::DecodeError::UnknownRequiredFeature, ty) => {
+                                                                                                               log_gossip!(self.logger, "Received a message with an unknown required feature flag or TLV, you may want to update!");
+                                                                                                               self.enqueue_message(peer, &msgs::WarningMessage { channel_id: [0; 32], data: format!("Received an unknown required feature/TLV in message type {:?}", ty) });
+                                                                                                               return Err(PeerHandleError { no_connection_possible: false });
+                                                                                                       }
                                                                                                        (msgs::DecodeError::UnknownVersion, _) => return Err(PeerHandleError { no_connection_possible: false }),
                                                                                                        (msgs::DecodeError::InvalidValue, _) => {
                                                                                                                log_debug!(self.logger, "Got an invalid value while deserializing message");