axelarnetwork / tofn

A threshold cryptography library in Rust
Apache License 2.0
112 stars 23 forks source link

feat(blame): authenticate-mesages #63

Closed sdaveas closed 3 years ago

ggutoski commented 3 years ago

As discussed, tests for spoofed messages require some intervention at the network routing level.

Explanation: If Alice spoofs a message from Bob then there will be 2 messages from Bob. Parties will throw an error (and abort the protocol!) when they receive 2 messages from one party. Thus, we must ensure that everyone receives only one message from Bob. (In the future we should more gracefully handle the receipt of multiple messages from a party.) For test purposes we want other parties to receive Alice's spoofed message instead of Bob's real message. This requires intervention at the network routing level.

We identified the following places to do this:

sdaveas commented 3 years ago

To sum up the new changes:

  1. We no longer take into account UnauthenticatedSender behaviour inside protocol (as we do for the rest of the behaviours). We rather replicate the actions of the criminal at the routing level, which is simulated in execute_protocol_vec. To do this, we need to pass an additional argument in this function to avoid code duplication. This is arguably an acceptable solution despite its ugliness.
  2. The spoofer is defined by two arguments: victim and status. The latter indicates the round he acts. However, he does not have access to Protocol’s private var status because we corrupt messages at the routing level. The way I handled this is to make the spoofer “smart” enough to derive the current round from the content of the message by mapping MsgTypes to Statuses. https://github.com/axelarnetwork/tofn/blob/35eeae8e1f7dbf40d36b10ff552cc2ad34f8008e/src/protocol/gg20/keygen/mod.rs#L63-L80
  3. We need two distinct Spoofer structs because the underlying MsgMeta they corrupt are of different types, keygen::MsgMeta and sign::MsgMeta, respectively. This will be reverted after refactoring the code and use a generic MsgMeta type :slightly_smiling_face:
ggutoski commented 3 years ago

Can you remember why we decided not to pull the from field from MsgMeta as suggested in #42 ?

sdaveas commented 3 years ago

Can you remember why we decided not to pull the from field from MsgMeta as suggested in #42 ?

I originally started implementing the solution you proposed in #42 but it seemed more intuitive to me to handle authentication at tofn side instead of tofnd. The rationale was to handle all malicious cases in single place, and since we didn't find any significant disadvantage in that approach, we stick with it. Of course, that was before realizing that UnauthenticatedSender is a behaviour, which, unlike all others existing ones, is actualized at the routing level. Perhaps this intuition would change the approach we eventually followed, but I still think that it is responsibility of the library to handle crimes and we can't strictly rely on client code to do that.