Skip to content

Commit

Permalink
fix(dht): discard encrypted message with no destination (#3472)
Browse files Browse the repository at this point in the history
Description
---

Discards inbound DHT messages that are encrypted but have no explicit NodeDestination  and bans the peer that sent it

Motivation and Context
---
In order to support encrypted messages without a destination the message needs to be flooded on the network.
This change disallows this on the DHT network so that it cannot be used to flood the network. 

How Has This Been Tested?
---
New unit tests, existing tests updated, backward compatibility checked manually (base node, no bans)
  • Loading branch information
sdbondi committed Oct 19, 2021
1 parent d1b3523 commit 6ca3424
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
6 changes: 3 additions & 3 deletions comms/dht/src/dht.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ mod test {
DhtMessageFlags::ENCRYPTED,
true,
MessageTag::new(),
false,
true,
);

let inbound_message = make_comms_inbound_message(&node_identity, dht_envelope.to_encoded_bytes().into());
Expand Down Expand Up @@ -593,12 +593,12 @@ mod test {
let ecdh_key = crypt::generate_ecdh_secret(node_identity2.secret_key(), node_identity2.public_key());
let encrypted_bytes = crypt::encrypt(&ecdh_key, &msg.to_encoded_bytes()).unwrap();
let dht_envelope = make_dht_envelope(
&node_identity,
&node_identity2,
encrypted_bytes,
DhtMessageFlags::ENCRYPTED,
true,
MessageTag::new(),
false,
true,
);

let origin_mac = dht_envelope.header.as_ref().unwrap().origin_mac.clone();
Expand Down
43 changes: 41 additions & 2 deletions comms/dht/src/inbound/decryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ enum DecryptionError {
EnvelopeBodyDecodeFailed,
#[error("Failed to decrypt message body")]
MessageBodyDecryptionFailed,
#[error("Encrypted message without a destination is invalid")]
EncryptedMessageNoDestination,
}

/// This layer is responsible for attempting to decrypt inbound messages.
Expand Down Expand Up @@ -168,6 +170,7 @@ where S: Service<DecryptedDhtMessage, Response = (), Error = PipelineError>

Err(err @ OriginMacNotProvided) |
Err(err @ EphemeralKeyNotProvided) |
Err(err @ EncryptedMessageNoDestination) |
Err(err @ OriginMacInvalidSignature) => {
// This message should not have been propagated, or has been manipulated in some way. Ban the source of
// this message.
Expand Down Expand Up @@ -208,6 +211,11 @@ where S: Service<DecryptedDhtMessage, Response = (), Error = PipelineError>
message.dht_header.message_tag
);

// Check if there is no destination specified and discard
if dht_header.destination.is_unknown() {
return Err(DecryptionError::EncryptedMessageNoDestination);
}

let e_pk = dht_header
.ephemeral_public_key
.as_ref()
Expand Down Expand Up @@ -449,7 +457,7 @@ mod test {
plain_text_msg.to_encoded_bytes(),
DhtMessageFlags::ENCRYPTED,
true,
false,
true,
);

block_on(service.call(inbound_msg)).unwrap();
Expand Down Expand Up @@ -479,7 +487,7 @@ mod test {
some_secret,
DhtMessageFlags::ENCRYPTED,
true,
false,
true,
);

block_on(service.call(inbound_msg.clone())).unwrap();
Expand Down Expand Up @@ -514,4 +522,35 @@ mod test {
unpack_enum!(DecryptionError::MessageRejectDecryptionFailed = err);
assert!(result.lock().unwrap().is_none());
}

#[runtime::test]
async fn decrypt_inbound_fail_no_destination() {
let _ = env_logger::try_init();
let (connectivity, mock) = create_connectivity_mock();
mock.spawn();
let result = Arc::new(Mutex::new(None));
let service = service_fn({
let result = result.clone();
move |msg: DecryptedDhtMessage| {
*result.lock().unwrap() = Some(msg);
future::ready(Result::<(), PipelineError>::Ok(()))
}
});
let node_identity = make_node_identity();
let mut service = DecryptionService::new(Default::default(), node_identity.clone(), connectivity, service);

let plain_text_msg = b"Secret message to nowhere".to_vec();
let inbound_msg = make_dht_inbound_message(
&node_identity,
plain_text_msg.to_encoded_bytes(),
DhtMessageFlags::ENCRYPTED,
true,
false,
);

let err = service.call(inbound_msg).await.unwrap_err();
let err = err.downcast::<DecryptionError>().unwrap();
unpack_enum!(DecryptionError::EncryptedMessageNoDestination = err);
assert!(result.lock().unwrap().is_none());
}
}
2 changes: 1 addition & 1 deletion comms/dht/tests/dht.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ async fn dht_propagate_dedup() {
.dht
.outbound_requester()
.propagate(
NodeDestination::Unknown,
node_D.node_identity().node_id().clone().into(),
OutboundEncryption::encrypt_for(node_D.node_identity().public_key().clone()),
vec![],
out_msg,
Expand Down

0 comments on commit 6ca3424

Please sign in to comment.