bridge: verify LockupObservation signature

Final missing piece of the aggregation mechanism - signatures are now
verified before storing them in the aggregation.

ghstack-source-id: 3bb57c488623454370ad1bf2a38e1278c65ce9e0
Pull Request resolved: https://github.com/certusone/wormhole/pull/59
This commit is contained in:
Leo 2020-10-28 22:41:34 +01:00
parent 55fd671228
commit 45d10618ce
1 changed files with 41 additions and 3 deletions

View File

@ -152,12 +152,42 @@ func vaaConsensusProcessor(lockC chan *common.ChainLock, setC chan *common.Guard
state.vaaSignatures[hash].ourVAA = v
case m := <-obsvC:
// SECURITY: at this point, observations received from the p2p network are fully untrusted (all fields!)
//
// Note that observations are never tied to the (verified) p2p identity key - the p2p network
// identity is completely decoupled from the guardian identity, p2p is just transport.
logger.Info("received lockup observation",
zap.String("digest", hex.EncodeToString(m.Hash)),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(m.Addr)))
// Verify the Guardian's signature. This verifies that m.Signature matches m.Hash and recovers
// the public key that was used to sign the payload.
pk, err := crypto.Ecrecover(m.Hash, m.Signature)
if err != nil {
logger.Warn("failed to verify signature on lockup observation",
zap.String("digest", hex.EncodeToString(m.Hash)),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(m.Addr)),
zap.Error(err))
break
}
// Verify that m.Addr matches the public key that signed m.Hash.
their_addr := ethcommon.BytesToAddress(m.Addr)
signer_pk := ethcommon.BytesToAddress(crypto.Keccak256(pk[1:])[12:])
if their_addr != signer_pk {
logger.Info("invalid lockup observation - address does not match pubkey",
zap.String("digest", hex.EncodeToString(m.Hash)),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(m.Addr)),
zap.String("pk", signer_pk.Hex()))
break
}
// Verify that m.Addr is included in the current guardian set.
_, ok := gs.KeyIndex(their_addr)
if !ok {
logger.Warn("received observation by unknown guardian - is our guardian set outdated?",
@ -167,13 +197,22 @@ func vaaConsensusProcessor(lockC chan *common.ChainLock, setC chan *common.Guard
break
}
// Hooray! Now, we have verified all fields on LockupObservation and know that it includes
// a valid signature by an active guardian. We still don't fully trust them, as they may be
// byzantine, but now we know who we're dealing with.
// TODO: timeout/garbage collection for lockup state
// TODO: rebroadcast signatures for VAAs that fail to reach consensus
// []byte isn't hashable in a map. Paying a small extra cost to for encoding for easier debugging.
// []byte isn't hashable in a map. Paying a small extra cost for encoding for easier debugging.
hash := hex.EncodeToString(m.Hash)
if state.vaaSignatures[hash] == nil {
// We haven't yet seen this lockup ourselves, and therefore do not know what the VAA looks like.
// However, we have established that a valid guardian has signed it, and therefore we can
// already start aggregating signatures for it.
//
// TODO: a malicious guardian can DoS this by creating fake lockups
state.vaaSignatures[hash] = &vaaState{
firstObserved: time.Now(),
signatures: map[ethcommon.Address][]byte{},
@ -182,11 +221,10 @@ func vaaConsensusProcessor(lockC chan *common.ChainLock, setC chan *common.Guard
state.vaaSignatures[hash].signatures[their_addr] = m.Signature
// Enumerate guardian set and check for signatures
// Aggregate all valid signatures into a list of vaa.Signature and construct signed VAA.
agg := make([]bool, len(gs.Keys))
var sigs []*vaa.Signature
for i, a := range gs.Keys {
// TODO: verify signature
s, ok := state.vaaSignatures[hash].signatures[a]
if ok {