Skip to content

Commit 162c63d

Browse files
committed
fix(p2p): improve error types for connection signaling
(#2193) Add AnswerNotProvided variant to P2pConnectionErrorResponse to distinguish empty relay answers from generic internal errors in the discovery signaling path. Add OfferDecryptErrorKind enum to distinguish actual decryption failures from identity key mismatches in the exchange signaling path. Key mismatches now log a security warning while still responding with SignalDecryptionFailed to avoid leaking information to potential attackers.
1 parent 57b560c commit 162c63d

File tree

8 files changed

+38
-8
lines changed

8 files changed

+38
-8
lines changed

crates/p2p/src/channels/p2p_channels_effectful_effects.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use crate::webrtc::{Offer, P2pConnectionResponse};
66
use super::{
77
signaling::{
88
discovery::{P2pChannelsSignalingDiscoveryAction, SignalingDiscoveryChannelMsg},
9-
exchange::{P2pChannelsSignalingExchangeAction, SignalingExchangeChannelMsg},
9+
exchange::{
10+
OfferDecryptErrorKind, P2pChannelsSignalingExchangeAction, SignalingExchangeChannelMsg,
11+
},
1012
},
1113
ChannelMsg, MsgId, P2pChannelsEffectfulAction, P2pChannelsService,
1214
};
@@ -85,13 +87,20 @@ impl P2pChannelsEffectfulAction {
8587
Err(_) => {
8688
store.dispatch(P2pChannelsSignalingExchangeAction::OfferDecryptError {
8789
peer_id,
90+
error: OfferDecryptErrorKind::DecryptionFailed,
8891
});
8992
}
9093
Ok(offer) if offer.identity_pub_key != pub_key => {
91-
// TODO(binier): propagate specific error.
92-
// This is invalid behavior either from relayer or offerer.
94+
// The offer's embedded identity key doesn't match the
95+
// expected public key. This indicates a compromised relay,
96+
// offer tampering, or wrong key pair usage.
97+
//
98+
// We deliberately respond with SignalDecryptionFailed
99+
// rather than a specific mismatch error to avoid leaking
100+
// information to a potential attacker.
93101
store.dispatch(P2pChannelsSignalingExchangeAction::OfferDecryptError {
94102
peer_id,
103+
error: OfferDecryptErrorKind::IdentityKeyMismatch,
95104
});
96105
}
97106
Ok(offer) => {

crates/p2p/src/channels/signaling/discovery/p2p_channels_signaling_discovery_reducer.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,9 @@ impl P2pChannelsSignalingDiscoveryState {
375375

376376
let dispatcher = state_context.into_dispatcher();
377377
match answer {
378-
// TODO(binier): custom error
379378
None => dispatcher.push(P2pConnectionOutgoingAction::AnswerRecvError {
380379
peer_id: target_public_key.peer_id(),
381-
error: P2pConnectionErrorResponse::InternalError,
380+
error: P2pConnectionErrorResponse::AnswerNotProvided,
382381
}),
383382
Some(answer) => dispatcher.push(
384383
P2pChannelsEffectfulAction::SignalingDiscoveryAnswerDecrypt {

crates/p2p/src/channels/signaling/exchange/p2p_channels_signaling_exchange_actions.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ use crate::{
1111

1212
use super::{P2pChannelsSignalingExchangeState, SignalingExchangeState};
1313

14+
#[derive(Debug, Clone, Serialize, Deserialize)]
15+
pub enum OfferDecryptErrorKind {
16+
DecryptionFailed,
17+
IdentityKeyMismatch,
18+
}
19+
1420
#[derive(Debug, Clone, Serialize, Deserialize, ActionEvent)]
1521
#[action_event(fields(display(peer_id)))]
1622
pub enum P2pChannelsSignalingExchangeAction {
@@ -36,6 +42,7 @@ pub enum P2pChannelsSignalingExchangeAction {
3642
},
3743
OfferDecryptError {
3844
peer_id: PeerId,
45+
error: OfferDecryptErrorKind,
3946
},
4047
OfferDecryptSuccess {
4148
peer_id: PeerId,
@@ -126,7 +133,7 @@ impl redux::EnablingCondition<P2pState> for P2pChannelsSignalingExchangeAction {
126133
}
127134
_ => false,
128135
}),
129-
P2pChannelsSignalingExchangeAction::OfferDecryptError { peer_id } => state
136+
P2pChannelsSignalingExchangeAction::OfferDecryptError { peer_id, .. } => state
130137
.get_ready_peer(peer_id)
131138
.is_some_and(|p| match &p.channels.signaling.exchange {
132139
P2pChannelsSignalingExchangeState::Ready { local, .. } => {

crates/p2p/src/channels/signaling/exchange/p2p_channels_signaling_exchange_reducer.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,14 @@ impl P2pChannelsSignalingExchangeState {
117117
});
118118
Ok(())
119119
}
120-
P2pChannelsSignalingExchangeAction::OfferDecryptError { .. } => {
120+
P2pChannelsSignalingExchangeAction::OfferDecryptError { error, .. } => {
121+
if matches!(error, super::OfferDecryptErrorKind::IdentityKeyMismatch) {
122+
tracing::warn!(
123+
%peer_id,
124+
"offer identity key mismatch: possible relay \
125+
tampering or spoofed offer"
126+
);
127+
}
121128
let dispatcher = state_context.into_dispatcher();
122129
let answer = P2pConnectionResponse::SignalDecryptionFailed;
123130
dispatcher.push(P2pChannelsSignalingExchangeAction::AnswerSend { peer_id, answer });

crates/p2p/src/connection/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,6 @@ pub enum P2pConnectionErrorResponse {
2727
SignalDecryptionFailed,
2828
#[error("internal error")]
2929
InternalError,
30+
#[error("answer not provided")]
31+
AnswerNotProvided,
3032
}

crates/p2p/src/connection/outgoing/p2p_connection_outgoing_actions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ impl redux::EnablingCondition<P2pState> for P2pConnectionOutgoingAction {
190190
}
191191
P2pConnectionOutgoingError::Rejected(_)
192192
| P2pConnectionOutgoingError::RemoteSignalDecryptionFailed
193-
| P2pConnectionOutgoingError::RemoteInternalError => {
193+
| P2pConnectionOutgoingError::RemoteInternalError
194+
| P2pConnectionOutgoingError::RemoteAnswerNotProvided => {
194195
matches!(s, P2pConnectionOutgoingState::AnswerRecvPending { .. })
195196
}
196197
P2pConnectionOutgoingError::FinalizeError(_) => {

crates/p2p/src/connection/outgoing/p2p_connection_outgoing_reducer.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,9 @@ impl P2pConnectionOutgoingState {
327327
P2pConnectionErrorResponse::InternalError => {
328328
P2pConnectionOutgoingError::RemoteInternalError
329329
}
330+
P2pConnectionErrorResponse::AnswerNotProvided => {
331+
P2pConnectionOutgoingError::RemoteAnswerNotProvided
332+
}
330333
},
331334
});
332335
Ok(())

crates/p2p/src/connection/outgoing/p2p_connection_outgoing_state.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ pub enum P2pConnectionOutgoingError {
159159
RemoteSignalDecryptionFailed,
160160
#[error("remote internal error")]
161161
RemoteInternalError,
162+
#[error("remote answer not provided")]
163+
RemoteAnswerNotProvided,
162164
#[error("finalization error: {0}")]
163165
FinalizeError(String),
164166
#[error("connection authorization error")]

0 commit comments

Comments
 (0)