Merge pull request #1975 from OpenBazaar/failing-listing-sign-verify

Fix failing signature validation

Author: placer14

Godeps/Godeps.json

@@ -590,7 +590,7 @@ func (n *OpenBazaarNode) createContractWithOrder(data *repo.PurchaseData) (*pb.R
 			return nil, errors.New("listing does not accept the selected currency")
 		}
 
-		ser, err := listing.MarshalJSON()
+		ser, err := listing.MarshalProtobuf()
 		if err != nil {
 			return nil, err
 		}

core/order.go

@@ -962,13 +962,19 @@ func (l *Listing) GetShippingRegions() ([]string, []string) {
 	return returnShipTo, returnFreeShipTo
 }
 
+// MarshalProtobuf returns the byte serialization of the underlying protobuf
+func (l *Listing) MarshalProtobuf() ([]byte, error) {
+	return proto.Marshal(l.listingProto)
+}
+
 type listingSigner interface {
 	TestNetworkEnabled() bool
 	RegressionNetworkEnabled() bool
 	GetNodeID() (*pb.ID, error)
 	Sign([]byte) ([]byte, error)
 }
 
+// MarshalJSON returns the json serialization of the underlying protobuf
 func (l *Listing) MarshalJSON() ([]byte, error) {
 	m := jsonpb.Marshaler{
 		EnumsAsInts:  false,
@@ -1030,7 +1036,7 @@ func (l *Listing) Sign(n listingSigner) (*SignedListing, error) {
 	l.listingProto.VendorID = id
 
 	// Sign listing
-	serializedListing, err := l.MarshalJSON()
+	serializedListing, err := l.MarshalProtobuf()
 	if err != nil {
 		return nil, fmt.Errorf("serializing listing: %s", err.Error())
 	}

repo/listing.go

@@ -115,15 +115,16 @@ func (l SignedListing) ValidateListing(isTestnet bool) error {
 // VerifySignature checks the listings signature was produced by the vendor's
 // Identity key and that the key was derived from the vendor's peerID
 func (l SignedListing) VerifySignature() error {
-	ser, err := proto.Marshal(l.GetListing().listingProto)
+	ser, err := l.GetListing().MarshalProtobuf()
 	if err != nil {
 		return fmt.Errorf("marshaling listing: %s", err.Error())
 	}
+
 	pubkey, err := l.GetListing().GetVendorID().IdentityKey()
 	if err != nil {
 		return fmt.Errorf("getting identity pubkey: %s", err.Error())
 	}
-	valid, err := pubkey.Verify(ser, l.signedListingProto.GetSignature())
+	valid, err := pubkey.Verify(ser, l.GetSignature())
 	if err != nil {
 		return fmt.Errorf("verifying signature: %s", err.Error())
 	}

repo/signed_listing.go

@@ -1,10 +1,15 @@
 package repo_test
 
 import (
+	"bytes"
+	"crypto/rand"
+	crypto "gx/ipfs/QmTW4SdgBWq9GjsBsHeUx8WuGxzhgzAf88UMH2w62PC8yK/go-libp2p-crypto"
+	peer "gx/ipfs/QmYVXrKrKHDC9FobgmcmshCDyWwdrfwfanNQN4oxJ9Fk3h/go-libp2p-peer"
 	"math/big"
 	"testing"
 
 	"github.com/OpenBazaar/openbazaar-go/repo"
+	"github.com/OpenBazaar/openbazaar-go/test"
 	"github.com/OpenBazaar/openbazaar-go/test/factory"
 )
 
@@ -192,3 +197,72 @@ func TestSignedListingAttributes(t *testing.T) {
 		}
 	}
 }
+
+func TestSignAndVerifyListing(t *testing.T) {
+	testnode, err := test.NewNode()
+	if err != nil {
+		t.Fatalf("create test node: %v", err)
+	}
+
+	priv, pub, err := crypto.GenerateEd25519Key(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pid, err := peer.IDFromPublicKey(pub)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testnode.IpfsNode.Identity = pid
+	testnode.IpfsNode.PrivateKey = priv
+
+	lpb := factory.NewListing("test")
+	l, err := repo.NewListingFromProtobuf(lpb)
+	if err != nil {
+		t.Fatalf("create repo listing: %v", err)
+	}
+
+	sl, err := l.Sign(testnode)
+	if err != nil {
+		t.Fatalf("sign listing: %v", err)
+	}
+
+	// get identities from node and listing and compare
+	nodeID, err := testnode.GetNodeID()
+	if err != nil {
+		t.Fatalf("get node id: %v", err)
+	}
+	nodeIdentityBytes := nodeID.GetPubkeys().GetIdentity()
+	listingIdentityBytes := sl.GetListing().GetVendorID().IdentityKeyBytes()
+	if !bytes.Equal(nodeIdentityBytes, listingIdentityBytes) {
+		t.Fatal("expected listing and node identity bytes to match, but did not")
+	}
+
+	// get peer IDs from node and listing and compare
+	listingPeerHash, err := sl.GetListing().GetVendorID().Hash()
+	if err != nil {
+		t.Fatalf("get listing peer hash: %v", err)
+	}
+	if listingPeerHash != nodeID.PeerID {
+		t.Errorf("expected listing has to be (%s), but was (%s)", nodeID.PeerID, listingPeerHash)
+	}
+
+	// get listing signature and ensure it's as expected
+	serializedListing, err := l.MarshalProtobuf()
+	if err != nil {
+		t.Fatalf("serialize listing: %v", err)
+	}
+	expectedSig, err := testnode.IpfsNode.PrivateKey.Sign(serializedListing)
+	if err != nil {
+		t.Fatalf("sign listing: %v", err)
+	}
+	actualSig := sl.GetSignature()
+	if !bytes.Equal(expectedSig, actualSig) {
+		t.Fatal("expected signature on listing to match generated signature, but did not")
+	}
+
+	if err := sl.VerifySignature(); err != nil {
+		t.Errorf("verify signed listing: %v", err)
+	}
+}