Skip to content

Commit

Permalink
updating origin ID checks to use idtranslator
Browse files Browse the repository at this point in the history
  • Loading branch information
synzhu committed Aug 21, 2021
1 parent 112c27d commit 96cf07f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 82 deletions.
26 changes: 11 additions & 15 deletions network/p2p/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/libp2p/go-libp2p-core/peerstore"
"github.com/rs/zerolog"

"github.com/onflow/flow-go/crypto"
"github.com/onflow/flow-go/engine"
"github.com/onflow/flow-go/model/flow"
"github.com/onflow/flow-go/module"
Expand Down Expand Up @@ -406,25 +405,22 @@ func (m *Middleware) Unsubscribe(channel network.Channel) error {
return nil
}

// processAuthenticatedMessage processes a message and a source (indicated by its PublicKey) and eventually passes it to the overlay
// In particular, it checks the claim of protocol authorship situated in the message against `originKey`
// The assumption is that the message has been authenticated at the network level (libp2p) to origin at the network public key `originKey`
// processAuthenticatedMessage processes a message and a source (indicated by its peer ID) and eventually passes it to the overlay
// In particular, it checks the claim of protocol authorship situated in the message against `peerID`
// The assumption is that the message has been authenticated at the network level (libp2p) to originate from the peer with ID `peerID`
// this requirement is fulfilled by e.g. the output of readConnection and readSubscription
func (m *Middleware) processAuthenticatedMessage(msg *message.Message, originKey crypto.PublicKey) {
identities := m.ov.Identities()
func (m *Middleware) processAuthenticatedMessage(msg *message.Message, peerID peer.ID) {
flowID, err := m.idTranslator.GetFlowID(peerID)
if err != nil {
m.log.Warn().Err(err).Msgf("received message from unknown peer %v, and was dropped", peerID.String())
return
}

// check the origin of the message corresponds to the one claimed in the OriginID
originID := flow.HashToID(msg.OriginID)

originIdentity, found := identities.ByNodeID(originID)
if !found {
m.log.Warn().Msgf("received message with claimed originID %x, which is not known by this node, and was dropped", originID)
return
} else if originIdentity.NetworkPubKey == nil {
m.log.Warn().Msgf("received message with claimed originID %x, wich has no network identifiers at this node, and was dropped", originID)
return
} else if !originIdentity.NetworkPubKey.Equals(originKey) {
m.log.Warn().Msgf("received message claiming to be from nodeID %v with key %x was actually signed by %x and dropped", originID, originIdentity.NetworkPubKey, originKey)
if flowID != originID {
m.log.Warn().Msgf("received message claiming to be from nodeID %v was actually from %v and dropped", originID, flowID)
return
}

Expand Down
20 changes: 6 additions & 14 deletions network/p2p/readConnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (

ggio "github.com/gogo/protobuf/io"
libp2pnetwork "github.com/libp2p/go-libp2p-core/network"
"github.com/libp2p/go-libp2p-core/peer"

"github.com/rs/zerolog"

"github.com/onflow/flow-go/crypto"
"github.com/onflow/flow-go/module"
"github.com/onflow/flow-go/module/metrics"
"github.com/onflow/flow-go/network/message"
Expand All @@ -21,17 +21,17 @@ import (
type readConnection struct {
ctx context.Context
stream libp2pnetwork.Stream
remoteKey crypto.PublicKey
remoteID peer.ID
log zerolog.Logger
metrics module.NetworkMetrics
maxMsgSize int
callback func(msg *message.Message, pk crypto.PublicKey)
callback func(msg *message.Message, peerID peer.ID)
}

// newReadConnection creates a new readConnection
func newReadConnection(ctx context.Context,
stream libp2pnetwork.Stream,
callback func(msg *message.Message, pubKey crypto.PublicKey),
callback func(msg *message.Message, peerID peer.ID),
log zerolog.Logger,
metrics module.NetworkMetrics,
maxMsgSize int) *readConnection {
Expand All @@ -40,18 +40,10 @@ func newReadConnection(ctx context.Context,
maxMsgSize = DefaultMaxUnicastMsgSize
}

remoteKey := stream.Conn().RemotePublicKey()
flowKey, err := FlowPublicKeyFromLibP2P(remoteKey)
// this should not happen if the stream was setup properly
if err != nil {
log.Err(err).Msg("failed to extract flow public key of stream libp2p key")
return nil
}

c := readConnection{
ctx: ctx,
stream: stream,
remoteKey: flowKey,
remoteID: stream.Conn().RemotePeer(),
callback: callback,
log: log,
metrics: metrics,
Expand Down Expand Up @@ -113,7 +105,7 @@ func (rc *readConnection) receiveLoop(wg *sync.WaitGroup) {
rc.metrics.NetworkMessageReceived(msg.Size(), metrics.ChannelOneToOne, msg.Type)

// call the callback
rc.callback(&msg, rc.remoteKey)
rc.callback(&msg, rc.remoteID)
}
}

Expand Down
58 changes: 5 additions & 53 deletions network/p2p/readSubscription.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,14 @@ package p2p

import (
"context"
"fmt"
"strings"
"sync"

lcrypto "github.com/libp2p/go-libp2p-core/crypto"
"github.com/libp2p/go-libp2p-core/peer"
pubsub "github.com/libp2p/go-libp2p-pubsub"

"github.com/rs/zerolog"

"github.com/onflow/flow-go/crypto"
"github.com/onflow/flow-go/module"
"github.com/onflow/flow-go/network/message"
_ "github.com/onflow/flow-go/utils/binstat"
Expand All @@ -25,13 +22,13 @@ type readSubscription struct {
log zerolog.Logger
sub *pubsub.Subscription
metrics module.NetworkMetrics
callback func(msg *message.Message, pubKey crypto.PublicKey)
callback func(msg *message.Message, peerID peer.ID)
}

// newReadSubscription reads the messages coming in on the subscription
func newReadSubscription(ctx context.Context,
sub *pubsub.Subscription,
callback func(msg *message.Message, pubKey crypto.PublicKey),
callback func(msg *message.Message, peerID peer.ID),
log zerolog.Logger,
metrics module.NetworkMetrics) *readSubscription {

Expand Down Expand Up @@ -82,16 +79,9 @@ func (r *readSubscription) receiveLoop(wg *sync.WaitGroup) {
return
}

// if pubsub.WithMessageSigning(true) and pubsub.WithStrictSignatureVerification(true),
// the emitter is authenticated
emitterKey, err := messagePubKey(rawMsg)
pid, err := peer.IDFromBytes(rawMsg.From)
if err != nil {
r.log.Err(err).Msg("failed to extract libp2p public key of message")
return
}
flowKey, err := FlowPublicKeyFromLibP2P(emitterKey)
if err != nil {
r.log.Err(err).Msg("failed to extract flow public key of libp2p key")
r.log.Err(err).Msg("failed to validate peer ID of incoming message")
return
}

Expand All @@ -109,44 +99,6 @@ func (r *readSubscription) receiveLoop(wg *sync.WaitGroup) {
r.metrics.NetworkMessageReceived(msg.Size(), msg.ChannelID, msg.Type)

// call the callback
r.callback(&msg, flowKey)
}
}

// messagePubKey extracts the public key of the envelope signer from a libp2p message.
// The location of that key depends on the type of the key, see:
// https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md
// This reproduces the exact logic of the private function doing the same decoding in libp2p:
// https://github.com/libp2p/go-libp2p-pubsub/blob/ba28f8ecfc551d4d916beb748d3384951bce3ed0/sign.go#L77
func messagePubKey(m *pubsub.Message) (lcrypto.PubKey, error) {
var pubk lcrypto.PubKey

// m.From is the original sender of the message (versus `m.ReceivedFrom` which is the last hop which sent us this message)
pid, err := peer.IDFromBytes(m.From)
if err != nil {
return nil, err
}

if m.Key == nil {
// no attached key, it must be extractable from the source ID
pubk, err = pid.ExtractPublicKey()
if err != nil {
return nil, fmt.Errorf("cannot extract signing key: %s", err.Error())
}
if pubk == nil {
return nil, fmt.Errorf("cannot extract signing key")
}
} else {
pubk, err = lcrypto.UnmarshalPublicKey(m.Key)
if err != nil {
return nil, fmt.Errorf("cannot unmarshal signing key: %s", err.Error())
}

// verify that the source ID matches the attached key
if !pid.MatchesPublicKey(pubk) {
return nil, fmt.Errorf("bad signing key; source ID %s doesn't match key", pid)
}
r.callback(&msg, pid)
}

return pubk, nil
}

0 comments on commit 96cf07f

Please sign in to comment.