Skip to content

Commit

Permalink
Node: Unknown guardian tweaks (#4076)
Browse files Browse the repository at this point in the history 
* Node: Unknown guardian tweaks

* Code review rework
  • Loading branch information
@bruce-riley
bruce-riley authored Aug 14, 2024
1 parent 5a1e235 commit 8b9f9b5
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 53 deletions.
60 changes: 36 additions & 24 deletions node/pkg/p2p/p2p.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -760,24 +760,30 @@ func Run(params *RunParams) func(ctx context.Context) error {
gs := params.gst.Get()
if gs == nil {
// No valid guardian set yet - dropping heartbeat
logger.Log(params.components.SignedHeartbeatLogLevel, "skipping heartbeat - no guardian set",
zap.Any("value", s),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "skipping heartbeat - no guardian set",
zap.Any("value", s),
zap.String("from", envelope.GetFrom().String()))
}
break
}
if heartbeat, err := processSignedHeartbeat(envelope.GetFrom(), s, gs, params.gst, params.disableHeartbeatVerify); err != nil {
p2pMessagesReceived.WithLabelValues("invalid_heartbeat").Inc()
logger.Log(params.components.SignedHeartbeatLogLevel, "invalid signed heartbeat received",
zap.Error(err),
zap.Any("payload", msg.Message),
zap.Any("value", s),
zap.Binary("raw", envelope.Data),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "invalid signed heartbeat received",
zap.Error(err),
zap.Any("payload", msg.Message),
zap.Any("value", s),
zap.Binary("raw", envelope.Data),
zap.String("from", envelope.GetFrom().String()))
}
} else {
p2pMessagesReceived.WithLabelValues("valid_heartbeat").Inc()
logger.Log(params.components.SignedHeartbeatLogLevel, "valid signed heartbeat received",
zap.Any("value", heartbeat),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "valid signed heartbeat received",
zap.Any("value", heartbeat),
zap.String("from", envelope.GetFrom().String()))
}

func() {
if len(heartbeat.P2PNodeId) != 0 {
Expand Down Expand Up @@ -981,24 +987,30 @@ func Run(params *RunParams) func(ctx context.Context) error {
gs := params.gst.Get()
if gs == nil {
// No valid guardian set yet - dropping heartbeat
logger.Log(params.components.SignedHeartbeatLogLevel, "skipping heartbeat - no guardian set",
zap.Any("value", s),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "skipping heartbeat - no guardian set",
zap.Any("value", s),
zap.String("from", envelope.GetFrom().String()))
}
break
}
if heartbeat, err := processSignedHeartbeat(envelope.GetFrom(), s, gs, params.gst, params.disableHeartbeatVerify); err != nil {
p2pMessagesReceived.WithLabelValues("invalid_heartbeat").Inc()
logger.Log(params.components.SignedHeartbeatLogLevel, "invalid signed heartbeat received",
zap.Error(err),
zap.Any("payload", msg.Message),
zap.Any("value", s),
zap.Binary("raw", envelope.Data),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "invalid signed heartbeat received",
zap.Error(err),
zap.Any("payload", msg.Message),
zap.Any("value", s),
zap.Binary("raw", envelope.Data),
zap.String("from", envelope.GetFrom().String()))
}
} else {
p2pMessagesReceived.WithLabelValues("valid_heartbeat").Inc()
logger.Log(params.components.SignedHeartbeatLogLevel, "valid signed heartbeat received",
zap.Any("value", heartbeat),
zap.String("from", envelope.GetFrom().String()))
if logger.Level().Enabled(params.components.SignedHeartbeatLogLevel) {
logger.Log(params.components.SignedHeartbeatLogLevel, "valid signed heartbeat received",
zap.Any("value", heartbeat),
zap.String("from", envelope.GetFrom().String()))
}

func() {
if len(heartbeat.P2PNodeId) != 0 {
Expand Down
58 changes: 29 additions & 29 deletions node/pkg/processor/observation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func (p *Processor) handleSingleObservation(addr []byte, m *gossipv1.Observation
start := time.Now()
observationsReceivedTotal.Inc()

their_addr := common.BytesToAddress(addr)
hash := hex.EncodeToString(m.Hash)
s := p.state.signatures[hash]
if s != nil && s.submitted {
Expand All @@ -116,35 +117,6 @@ func (p *Processor) handleSingleObservation(addr []byte, m *gossipv1.Observation
)
}

// 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 {
p.logger.Warn("failed to verify signature on observation",
zap.String("messageId", m.MessageId),
zap.String("digest", hash),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(addr)),
zap.Error(err))
observationsFailedTotal.WithLabelValues("invalid_signature").Inc()
return
}

// Verify that addr matches the public key that signed m.Hash.
their_addr := common.BytesToAddress(addr)
signer_pk := common.BytesToAddress(crypto.Keccak256(pk[1:])[12:])

if their_addr != signer_pk {
p.logger.Info("invalid observation - address does not match pubkey",
zap.String("messageId", m.MessageId),
zap.String("digest", hash),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(addr)),
zap.String("pk", signer_pk.Hex()))
observationsFailedTotal.WithLabelValues("pubkey_mismatch").Inc()
return
}

// Determine which guardian set to use. The following cases are possible:
//
// - We have already seen the message and generated ourObservation. In this case, use the guardian set valid at the time,
Expand Down Expand Up @@ -197,6 +169,34 @@ func (p *Processor) handleSingleObservation(addr []byte, m *gossipv1.Observation
return
}

// 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 {
p.logger.Warn("failed to verify signature on observation",
zap.String("messageId", m.MessageId),
zap.String("digest", hash),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(addr)),
zap.Error(err))
observationsFailedTotal.WithLabelValues("invalid_signature").Inc()
return
}

// Verify that addr matches the public key that signed m.Hash.
signer_pk := common.BytesToAddress(crypto.Keccak256(pk[1:])[12:])

if their_addr != signer_pk {
p.logger.Info("invalid observation - address does not match pubkey",
zap.String("messageId", m.MessageId),
zap.String("digest", hash),
zap.String("signature", hex.EncodeToString(m.Signature)),
zap.String("addr", hex.EncodeToString(addr)),
zap.String("pk", signer_pk.Hex()))
observationsFailedTotal.WithLabelValues("pubkey_mismatch").Inc()
return
}

// Hooray! Now, we have verified all fields on SignedObservation 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.
Expand Down

0 comments on commit 8b9f9b5

Please sign in to comment.