search

status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

[Part of #7] Wrap & signs messages if specified in config #19

Closed cammellos closed 5 years ago

cammellos commented 5 years ago

Currently datasync propagates the unencrypted messages. This messages might be relayed to a third party. We can't therefore rely on whisper signature, as the author might not be the same as the relayer.

This commit wraps our current message in a protobuf record with an optional signature.

If the signature is present it will be used to indicate the author of the message, otherwise whisper is used.

It's not enabled by default as this is a breaking change and should go live with v1. Decoding is backward compatible, so old messages will still be read.

I have also changed the messageID scheme to use the Keccak256 of the encoded unencrypted payload, instead of the whisper ID.

Protocol change

Messages are now wrapped in a protobuf record:

bytes payload // Encoded transit
bytes signature //Signature of encoded transit

I haven't tested it with the console client yet.

I have passed a bool for the config, but given that there's another PR that does the same we might want to have a config struct, or pass a generic v1 config, although probably having a config struct with separate fields is neater.

pedropombeiro commented 5 years ago

@cammellos I've created a featureFlags type in my PR in case you want to leverage that to add sendV1Messages.

decanus commented 5 years ago

@cammellos one thing we may want to investigate is whether it is worth using https://github.com/gogo/protobuf if we start using protobufs more often. I believe they have a more efficient marshall and unmarshall method.

oskarth commented 5 years ago

@cammellos Can we create a corresponding PR in the https://github.com/status-im/specs repo?

cammellos commented 5 years ago

@adambabik addressed feeback, I have added feature flags from @PombeirP PR. Only one point I am not sure ("Let's add a scenario when it can be already provided."). @oskarth I will open a pr against specs

cammellos commented 5 years ago

@oskarth https://github.com/status-im/specs/pull/34

cammellos commented 5 years ago

@adambabik @PombeirP could you review again please?

I have changed the ID scheme to be: sha3-256(CompressedBytesOfPK + payload) which is similar to what we do in status-react.

We can't have only Sha3-256(payload) as potentially an attacker could send us an identical message, resulting in an ID clash.

We therefore use the PK of either transport of provided with the signature.