Skip to content

Commit c7d6f05

Browse files
authored
fix(webrtc): decode errors during opening phase are not recoverable (#622)
Explanation of why being so strict over decoding errors within the opening phases can be found [here](#589 (comment)). Closes #589
1 parent 161a17e commit c7d6f05

2 files changed

Lines changed: 45 additions & 8 deletions

File tree

src/transport/webrtc/connection.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
// DEALINGS IN THE SOFTWARE.
2020

2121
use crate::{
22-
error::{Error, ParseError, SubstreamError},
22+
error::{Error, SubstreamError},
2323
multistream_select::{
2424
webrtc_listener_negotiate, HandshakeResult, ListenerSelectResult, NegotiationError,
2525
WebRtcDialerState,
@@ -581,7 +581,21 @@ impl WebRtcConnection {
581581
);
582582

583583
// Decode errors are not recoverable.
584-
let payload = WebRtcMessage::decode(&data)?.payload.ok_or(Error::InvalidData)?;
584+
let WebRtcMessage {
585+
payload: Some(payload),
586+
flag: None,
587+
} = WebRtcMessage::decode(&data)
588+
.map_err(|err| SubstreamError::NegotiationError(err.into()))?
589+
else {
590+
tracing::debug!(
591+
target: LOG_TARGET,
592+
peer = ?self.peer,
593+
?channel_id,
594+
"non-payload frame during inbound opening, closing channel"
595+
);
596+
return Err(Error::ConnectionClosed);
597+
};
598+
585599
let protocols = self.protocol_set.protocols_with_keep_alives();
586600
let protocol_names = protocols.keys().cloned().collect();
587601
let (response, negotiated) =
@@ -666,11 +680,21 @@ impl WebRtcConnection {
666680
"handle opening outbound substream",
667681
);
668682

669-
let rtc_message = WebRtcMessage::decode(&data)
670-
.map_err(|err| SubstreamError::NegotiationError(err.into()))?;
671-
let message = rtc_message.payload.ok_or(SubstreamError::NegotiationError(
672-
ParseError::InvalidData.into(),
673-
))?;
683+
// Decode errors are not recoverable.
684+
let WebRtcMessage {
685+
payload: Some(message),
686+
flag: None,
687+
} = WebRtcMessage::decode(&data)
688+
.map_err(|err| SubstreamError::NegotiationError(err.into()))?
689+
else {
690+
tracing::debug!(
691+
target: LOG_TARGET,
692+
peer = ?self.peer,
693+
?channel_id,
694+
"non-payload frame during outbound opening, closing channel"
695+
);
696+
return Err(SubstreamError::ConnectionClosed);
697+
};
674698

675699
let protocol = match dialer_state.register_response(message)? {
676700
HandshakeResult::Succeeded(protocol) => protocol,

src/transport/webrtc/opening.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,20 @@ impl OpeningWebRtcConnection {
322322
return Err(Error::InvalidState);
323323
};
324324

325-
let message = WebRtcMessage::decode(&body)?.payload.ok_or(Error::InvalidData)?;
325+
// Decode errors are not recoverable.
326+
let WebRtcMessage {
327+
payload: Some(message),
328+
flag: None,
329+
} = WebRtcMessage::decode(&body)?
330+
else {
331+
tracing::debug!(
332+
target: LOG_TARGET,
333+
connection_id = ?self.connection_id,
334+
peer = ?self.peer_address,
335+
"non-payload frame during noise handshake, closing channel"
336+
);
337+
return Err(Error::ConnectionClosed);
338+
};
326339
let remote_peer_id = context.get_remote_peer_id(&message)?;
327340

328341
tracing::trace!(

0 commit comments

Comments
 (0)