From 601463727550a5d4c64e2885e94081738f3bd0e5 Mon Sep 17 00:00:00 2001 From: Yusef Napora Date: Fri, 15 Nov 2019 13:11:04 -0500 Subject: [PATCH] make envelope fields private & validate on unmarshal --- crypto/envelope.go | 99 +++++++++++++++++++++++++---------------- crypto/envelope_test.go | 97 ++++++++++++++++++---------------------- peerstore/peerstore.go | 18 +++++--- routing/state.go | 11 +++-- 4 files changed, 123 insertions(+), 102 deletions(-) diff --git a/crypto/envelope.go b/crypto/envelope.go index 926d8ee..5474ee2 100644 --- a/crypto/envelope.go +++ b/crypto/envelope.go @@ -16,14 +16,13 @@ import ( type SignedEnvelope struct { // The public key that can be used to verify the signature and derive the peer id of the signer. - PublicKey PubKey + publicKey PubKey // A binary identifier that indicates what kind of data is contained in the payload. // TODO(yusef): enforce multicodec prefix - PayloadType []byte + payloadType []byte - // The envelope payload. This is private to discourage accessing the payload without verifying the signature. - // To access, use the Open method. + // The envelope payload. payload []byte // The signature of the domain string, type hint, and payload. @@ -31,6 +30,7 @@ type SignedEnvelope struct { } var errEmptyDomain = errors.New("envelope domain must not be empty") +var errInvalidSignature = errors.New("invalid signature or incorrect domain") // MakeEnvelope constructs a new SignedEnvelope using the given privateKey. // @@ -38,27 +38,42 @@ var errEmptyDomain = errors.New("envelope domain must not be empty") // and must be supplied when verifying the signature. // // The 'payloadType' field indicates what kind of data is contained and may be empty. -func MakeEnvelope(privateKey PrivKey, domain string, payloadType []byte, contents []byte) (*SignedEnvelope, error) { +func MakeEnvelope(privateKey PrivKey, domain string, payloadType []byte, payload []byte) (*SignedEnvelope, error) { if len(domain) == 0 { return nil, errEmptyDomain } - toSign := makeSigBuffer(domain, payloadType, contents) + toSign := makeSigBuffer(domain, payloadType, payload) sig, err := privateKey.Sign(toSign) if err != nil { return nil, err } return &SignedEnvelope{ - PublicKey: privateKey.GetPublic(), - PayloadType: payloadType, - payload: contents, + publicKey: privateKey.GetPublic(), + payloadType: payloadType, + payload: payload, signature: sig, }, nil } -// UnmarshalEnvelope converts a serialized protobuf representation of an envelope -// into a SignedEnvelope struct. -func UnmarshalEnvelope(serializedEnvelope []byte) (*SignedEnvelope, error) { +// OpenEnvelope unmarshals a serialized SignedEnvelope, validating its signature +// using the provided 'domain' string. +func OpenEnvelope(envelopeBytes []byte, domain string) (*SignedEnvelope, error) { + e, err := UnmarshalEnvelopeWithoutValidating(envelopeBytes) + if err != nil { + return nil, err + } + err = e.validate(domain) + if err != nil { + return nil, err + } + return e, nil +} + +// UnmarshalEnvelopeWithoutValidating unmarshals a serialized SignedEnvelope protobuf message, +// without validating its contents. Should not be used unless you have a very good reason +// (e.g. testing)! +func UnmarshalEnvelopeWithoutValidating(serializedEnvelope []byte) (*SignedEnvelope, error) { e := pb.SignedEnvelope{} if err := proto.Unmarshal(serializedEnvelope, &e); err != nil { return nil, err @@ -68,57 +83,63 @@ func UnmarshalEnvelope(serializedEnvelope []byte) (*SignedEnvelope, error) { return nil, err } return &SignedEnvelope{ - PublicKey: key, - PayloadType: e.PayloadType, + publicKey: key, + payloadType: e.PayloadType, payload: e.Payload, signature: e.Signature, }, nil } -// Validate returns true if the envelope signature is valid for the given 'domain', -// or false if it is invalid. May return an error if signature validation fails. -func (e *SignedEnvelope) Validate(domain string) (bool, error) { - toVerify := makeSigBuffer(domain, e.PayloadType, e.payload) - return e.PublicKey.Verify(toVerify, e.signature) +// PublicKey returns the public key that can be used to verify the signature and derive the peer id of the signer. +func (e *SignedEnvelope) PublicKey() PubKey { + return e.publicKey +} + +// PayloadType returns a binary identifier that indicates what kind of data is contained in the payload. +func (e *SignedEnvelope) PayloadType() []byte { + return e.payloadType +} + +// Payload returns the binary payload of a SignedEnvelope. +func (e *SignedEnvelope) Payload() []byte { + return e.payload } -// Marshal returns a []byte containing a serialized protobuf representation of -// the SignedEnvelope. func (e *SignedEnvelope) Marshal() ([]byte, error) { - key, err := PublicKeyToProto(e.PublicKey) + key, err := PublicKeyToProto(e.publicKey) if err != nil { return nil, err } msg := pb.SignedEnvelope{ PublicKey: key, - PayloadType: e.PayloadType, + PayloadType: e.payloadType, Payload: e.payload, Signature: e.signature, } return proto.Marshal(&msg) } -// Open validates the signature (within the given 'domain') and returns -// the payload of the envelope. Will fail with an error if the signature -// is invalid. -func (e *SignedEnvelope) Open(domain string) ([]byte, error) { - valid, err := e.Validate(domain) - if err != nil { - return nil, err - } - if !valid { - return nil, errors.New("invalid signature or incorrect domain") - } - return e.payload, nil -} - func (e *SignedEnvelope) Equals(other *SignedEnvelope) bool { - return e.PublicKey.Equals(other.PublicKey) && - bytes.Compare(e.PayloadType, other.PayloadType) == 0 && + return e.publicKey.Equals(other.publicKey) && + bytes.Compare(e.payloadType, other.payloadType) == 0 && bytes.Compare(e.payload, other.payload) == 0 && bytes.Compare(e.signature, other.signature) == 0 } +// validate returns true if the envelope signature is valid for the given 'domain', +// or false if it is invalid. May return an error if signature validation fails. +func (e *SignedEnvelope) validate(domain string) error { + toVerify := makeSigBuffer(domain, e.payloadType, e.payload) + valid, err := e.publicKey.Verify(toVerify, e.signature) + if err != nil { + return err + } + if !valid { + return errInvalidSignature + } + return nil +} + // makeSigBuffer is a helper function that prepares a buffer to sign or verify. func makeSigBuffer(domain string, typeHint []byte, content []byte) []byte { b := bytes.Buffer{} diff --git a/crypto/envelope_test.go b/crypto/envelope_test.go index 549f5bd..e6d164e 100644 --- a/crypto/envelope_test.go +++ b/crypto/envelope_test.go @@ -2,7 +2,8 @@ package crypto_test import ( "bytes" - "github.com/gogo/protobuf/proto" + "github.com/golang/protobuf/proto" + . "github.com/libp2p/go-libp2p-core/crypto" pb "github.com/libp2p/go-libp2p-core/crypto/pb" "github.com/libp2p/go-libp2p-core/test" @@ -23,39 +24,27 @@ func TestEnvelopeHappyPath(t *testing.T) { t.Errorf("error constructing envelope: %v", err) } - if !envelope.PublicKey.Equals(pub) { + if !envelope.PublicKey().Equals(pub) { t.Error("envelope has unexpected public key") } - if bytes.Compare(payloadType, envelope.PayloadType) != 0 { + if bytes.Compare(payloadType, envelope.PayloadType()) != 0 { t.Error("PayloadType does not match payloadType used to construct envelope") } - valid, err := envelope.Validate(domain) - if err != nil { - t.Errorf("error validating envelope: %v", err) - } - if !valid { - t.Error("envelope should be valid, but Valid returns false") - } - - c, err := envelope.Open(domain) - if err != nil { - t.Errorf("error opening envelope: %v", err) - } - if bytes.Compare(c, payload) != 0 { - t.Error("payload of envelope do not match input") - } - serialized, err := envelope.Marshal() if err != nil { t.Errorf("error serializing envelope: %v", err) } - deserialized, err := UnmarshalEnvelope(serialized) + deserialized, err := OpenEnvelope(serialized, domain) if err != nil { t.Errorf("error deserializing envelope: %v", err) } + if bytes.Compare(deserialized.Payload(), payload) != 0 { + t.Error("payload of envelope does not match input") + } + if !envelope.Equals(deserialized) { t.Error("round-trip serde results in unequal envelope structures") } @@ -73,13 +62,11 @@ func TestEnvelopeValidateFailsForDifferentDomain(t *testing.T) { if err != nil { t.Errorf("error constructing envelope: %v", err) } - - valid, err := envelope.Validate("other-domain") - if err != nil { - t.Errorf("error validating envelope: %v", err) - } - if valid { - t.Error("envelope should be invalid, but Valid returns true") + serialized, err := envelope.Marshal() + // try to open our modified envelope + _, err = OpenEnvelope(serialized, "wrong-domain") + if err == nil { + t.Error("should not be able to open envelope with incorrect domain") } } @@ -95,13 +82,13 @@ func TestEnvelopeValidateFailsIfTypeHintIsAltered(t *testing.T) { if err != nil { t.Errorf("error constructing envelope: %v", err) } - envelope.PayloadType = []byte("foo") - valid, err := envelope.Validate("other-domain") - if err != nil { - t.Errorf("error validating envelope: %v", err) - } - if valid { - t.Error("envelope should be invalid, but Valid returns true") + serialized := alterMessageAndMarshal(t, envelope, func(msg *pb.SignedEnvelope) { + msg.PayloadType = []byte("foo") + }) + // try to open our modified envelope + _, err = OpenEnvelope(serialized, domain) + if err == nil { + t.Error("should not be able to open envelope with modified payloadType") } } @@ -118,33 +105,35 @@ func TestEnvelopeValidateFailsIfContentsAreAltered(t *testing.T) { t.Errorf("error constructing envelope: %v", err) } - // since the payload field is private, we'll serialize and alter the - // serialized protobuf + serialized := alterMessageAndMarshal(t, envelope, func(msg *pb.SignedEnvelope) { + msg.Payload = []byte("totally legit, trust me") + }) + // try to open our modified envelope + _, err = OpenEnvelope(serialized, domain) + if err == nil { + t.Error("should not be able to open envelope with modified payload") + } +} + +// Since we're outside of the crypto package (to avoid import cycles with test package), +// we can't alter the fields in a SignedEnvelope directly. This helper marshals +// the envelope to a protobuf and calls the alterMsg function, which should +// alter the protobuf message. +// Returns the serialized altered protobuf message. +func alterMessageAndMarshal(t *testing.T, envelope *SignedEnvelope, alterMsg func(*pb.SignedEnvelope)) []byte { serialized, err := envelope.Marshal() if err != nil { - t.Errorf("error serializing envelope: %v", err) + t.Errorf("error marshaling envelope: %v", err) } - msg := pb.SignedEnvelope{} err = proto.Unmarshal(serialized, &msg) if err != nil { - t.Errorf("error deserializing envelope: %v", err) + t.Errorf("error unmarshaling envelope: %v", err) } - msg.Payload = []byte("totally legit, trust me") - serialized, err = proto.Marshal(&msg) - - // unmarshal our altered envelope - deserialized, err := UnmarshalEnvelope(serialized) + alterMsg(&msg) + serialized, err = msg.Marshal() if err != nil { - t.Errorf("error deserializing envelope: %v", err) - } - - // verify that it's now invalid - valid, err := deserialized.Validate(domain) - if err != nil { - t.Errorf("error validating envelope: %v", err) - } - if valid { - t.Error("envelope should be invalid, but Valid returns true") + t.Errorf("error marshaling envelope: %v", err) } + return serialized } diff --git a/peerstore/peerstore.go b/peerstore/peerstore.go index ed45f38..b12b62f 100644 --- a/peerstore/peerstore.go +++ b/peerstore/peerstore.go @@ -97,8 +97,8 @@ type AddrBook interface { AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duration) // AddCertifiedAddrs adds addresses from a routing.RoutingState record - // contained in the given SignedEnvelope. - AddCertifiedAddrs(envelope *ic.SignedEnvelope, ttl time.Duration) error + // contained in a serialized SignedEnvelope. + AddCertifiedAddrs(envelopeBytes []byte, ttl time.Duration) error // SetAddr calls mgr.SetAddrs(p, addr, ttl) SetAddr(p peer.ID, addr ma.Multiaddr, ttl time.Duration) @@ -130,9 +130,17 @@ type AddrBook interface { // PeersWithAddrs returns all of the peer IDs stored in the AddrBook PeersWithAddrs() peer.IDSlice - // SignedRoutingState returns a SignedEnvelope containing a RoutingState - // record, if one exists for the given peer. - SignedRoutingState(p peer.ID) *ic.SignedEnvelope + // SignedRoutingState returns a signed RoutingState record for the + // given peer id, if one exists in the peerstore. The record is + // returned as a byte slice containing a serialized SignedEnvelope. + // Returns nil if no routing state exists for the peer. + SignedRoutingState(p peer.ID) []byte + + // SignedRoutingStates returns signed RoutingState records for each of + // the given peer ids, if one exists in the peerstore. + // Returns a map of peer ids to serialized SignedEnvelope messages. If + // no routing state exists for a peer, their map entry will be nil. + SignedRoutingStates(peers ...peer.ID) map[peer.ID][]byte } // KeyBook tracks the keys of Peers. diff --git a/routing/state.go b/routing/state.go index 91f4fa6..8bdb02a 100644 --- a/routing/state.go +++ b/routing/state.go @@ -75,16 +75,19 @@ func UnmarshalRoutingState(serialized []byte) (*RoutingState, error) { // RoutingStateFromEnvelope unwraps a peer RoutingState record from a SignedEnvelope. // This method will fail if the signature is invalid, or if the record // belongs to a peer other than the one that signed the envelope. -func RoutingStateFromEnvelope(envelope *crypto.SignedEnvelope) (*RoutingState, error) { - msgBytes, err := envelope.Open(StateEnvelopeDomain) +func RoutingStateFromEnvelope(envelopeBytes []byte) (*RoutingState, error) { + envelope, err := crypto.OpenEnvelope(envelopeBytes, StateEnvelopeDomain) if err != nil { return nil, err } - state, err := UnmarshalRoutingState(msgBytes) + if bytes.Compare(envelope.PayloadType(), StateEnvelopePayloadType) != 0 { + return nil, errors.New("unexpected envelope payload type") + } + state, err := UnmarshalRoutingState(envelope.Payload()) if err != nil { return nil, err } - if !state.PeerID.MatchesPublicKey(envelope.PublicKey) { + if !state.PeerID.MatchesPublicKey(envelope.PublicKey()) { return nil, errors.New("peer id in routing state record does not match signing key") } return state, nil