Merge pull request ipfs#114 from dirkmc/fix/remove-sigs

Remove record signature verification

dht.go

@@ -172,7 +172,7 @@ func (dht *IpfsDHT) getValueOrPeers(ctx context.Context, p peer.ID, key string)
 		log.Debug("getValueOrPeers: got value")
 
 		// make sure record is valid.
-		err = dht.verifyRecordOnline(ctx, record)
+		err = dht.Validator.VerifyRecord(record)
 		if err != nil {
 			log.Info("Received invalid record! (discarded)")
 			// return a sentinal to signify an invalid record was received
@@ -235,7 +235,7 @@ func (dht *IpfsDHT) getLocal(key string) (*recpb.Record, error) {
 		return nil, err
 	}
 
-	err = dht.verifyRecordLocally(rec)
+	err = dht.Validator.VerifyRecord(rec)
 	if err != nil {
 		log.Debugf("local record verify failed: %s (discarded)", err)
 		return nil, err

dht_test.go

@@ -49,12 +49,7 @@ func setupDHT(ctx context.Context, t *testing.T, client bool) *IpfsDHT {
 		d = NewDHT(ctx, h, dss)
 	}
 
-	d.Validator["v"] = &record.ValidChecker{
-		Func: func(*record.ValidationRecord) error {
-			return nil
-		},
-		Sign: false,
-	}
+	d.Validator["v"] = func(*record.ValidationRecord) error { return nil }
 	d.Selector["v"] = func(_ string, bs [][]byte) (int, error) { return 0, nil }
 	return d
 }
@@ -149,10 +144,7 @@ func TestValueGetSet(t *testing.T) {
 	defer dhtA.host.Close()
 	defer dhtB.host.Close()
 
-	vf := &record.ValidChecker{
-		Func: func(*record.ValidationRecord) error { return nil },
-		Sign: false,
-	}
+	vf := func(*record.ValidationRecord) error { return nil }
 	nulsel := func(_ string, bs [][]byte) (int, error) { return 0, nil }
 
 	dhtA.Validator["v"] = vf

ext_test.go

@@ -103,15 +103,7 @@ func TestGetFailures(t *testing.T) {
 		typ := pb.Message_GET_VALUE
 		str := "hello"
 
-		sk, err := d.getOwnPrivateKey()
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		rec, err := record.MakePutRecord(sk, str, []byte("blah"), true)
-		if err != nil {
-			t.Fatal(err)
-		}
+		rec := record.MakePutRecord(str, []byte("blah"))
 		req := pb.Message{
 			Type:   &typ,
 			Key:    &str,

handlers.go

@@ -119,11 +119,6 @@ func (dht *IpfsDHT) checkLocalDatastore(k string) (*recpb.Record, error) {
 		return nil, err
 	}
 
-	// if its our record, dont bother checking the times on it
-	if peer.ID(rec.GetAuthor()) == dht.self {
-		return rec, nil
-	}
-
 	var recordIsBad bool
 	recvtime, err := u.ParseRFC3339(rec.GetTimeReceived())
 	if err != nil {
@@ -156,12 +151,6 @@ func (dht *IpfsDHT) checkLocalDatastore(k string) (*recpb.Record, error) {
 func cleanRecord(rec *recpb.Record) {
 	rec.XXX_unrecognized = nil
 	rec.TimeReceived = nil
-
-	// Only include the author if there's a signature (otherwise, it's
-	// unvalidated and could be anything).
-	if len(rec.Signature) == 0 {
-		rec.Author = nil
-	}
 }
 
 // Store a value in this peer local storage
@@ -183,8 +172,8 @@ func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Mess
 	}
 	cleanRecord(rec)
 
-	if err = dht.verifyRecordLocally(rec); err != nil {
-		log.Warningf("Bad dht record in PUT from: %s. %s", peer.ID(pmes.GetRecord().GetAuthor()), err)
+	if err = dht.Validator.VerifyRecord(rec); err != nil {
+		log.Warningf("Bad dht record in PUT from: %s. %s", p.Pretty(), err)
 		return nil, err
 	}
 

handlers_test.go

@@ -12,8 +12,6 @@ func TestCleanRecordSigned(t *testing.T) {
 	actual := new(recpb.Record)
 	actual.TimeReceived = proto.String("time")
 	actual.XXX_unrecognized = []byte("extra data")
-	actual.Signature = []byte("signature")
-	actual.Author = proto.String("author")
 	actual.Value = []byte("value")
 	actual.Key = proto.String("key")
 
@@ -24,8 +22,6 @@ func TestCleanRecordSigned(t *testing.T) {
 	}
 
 	expected := new(recpb.Record)
-	expected.Signature = []byte("signature")
-	expected.Author = proto.String("author")
 	expected.Value = []byte("value")
 	expected.Key = proto.String("key")
 	expectedBytes, err := proto.Marshal(expected)

package.json

@@ -84,9 +84,9 @@
     },
     {
       "author": "whyrusleeping",
-      "hash": "QmUpttFinNDmNPgFwKN8sZK6BUtBmA68Y4KdSBDXa8t9sJ",
+      "hash": "QmcBSi3Zxa6ytDQxig2iMv4VMfiKKy7v4tibi1Sq6Z5u2x",
       "name": "go-libp2p-record",
-      "version": "3.0.2"
+      "version": "4.0.0"
     },
     {
       "author": "whyrusleeping",

records.go

@@ -8,8 +8,6 @@ import (
 	ctxfrac "github.com/jbenet/go-context/frac"
 	ci "github.com/libp2p/go-libp2p-crypto"
 	peer "github.com/libp2p/go-libp2p-peer"
-	record "github.com/libp2p/go-libp2p-record"
-	recpb "github.com/libp2p/go-libp2p-record/pb"
 	routing "github.com/libp2p/go-libp2p-routing"
 )
 
@@ -109,51 +107,3 @@ func (dht *IpfsDHT) getPublicKeyFromNode(ctx context.Context, p peer.ID) (ci.Pub
 	log.Debugf("DHT got public key from node itself.")
 	return pk, nil
 }
-
-// verifyRecordLocally attempts to verify a record. if we do not have the public
-// key, we fail. we do not search the dht.
-func (dht *IpfsDHT) verifyRecordLocally(r *recpb.Record) error {
-	if r == nil {
-		log.Error("nil record passed into verifyRecordLocally")
-		return fmt.Errorf("nil record")
-	}
-
-	if len(r.Signature) > 0 {
-		// First, validate the signature
-		p := peer.ID(r.GetAuthor())
-		pk := dht.peerstore.PubKey(p)
-		if pk == nil {
-			return fmt.Errorf("do not have public key for %s", p)
-		}
-
-		if err := record.CheckRecordSig(r, pk); err != nil {
-			return err
-		}
-	}
-
-	return dht.Validator.VerifyRecord(r)
-}
-
-// verifyRecordOnline verifies a record, searching the DHT for the public key
-// if necessary. The reason there is a distinction in the functions is that
-// retrieving arbitrary public keys from the DHT as a result of passively
-// receiving records (e.g. through a PUT_VALUE or ADD_PROVIDER) can cause a
-// massive amplification attack on the dht. Use with care.
-func (dht *IpfsDHT) verifyRecordOnline(ctx context.Context, r *recpb.Record) error {
-
-	if len(r.Signature) > 0 {
-		// get the public key, search for it if necessary.
-		p := peer.ID(r.GetAuthor())
-		pk, err := dht.GetPublicKey(ctx, p)
-		if err != nil {
-			return err
-		}
-
-		err = record.CheckRecordSig(r, pk)
-		if err != nil {
-			return err
-		}
-	}
-
-	return dht.Validator.VerifyRecord(r)
-}

routing.go

@@ -43,22 +43,8 @@ func (dht *IpfsDHT) PutValue(ctx context.Context, key string, value []byte) (err
 		eip.Done()
 	}()
 	log.Debugf("PutValue %s", key)
-	sk, err := dht.getOwnPrivateKey()
-	if err != nil {
-		return err
-	}
-
-	sign, err := dht.Validator.IsSigned(key)
-	if err != nil {
-		return err
-	}
-
-	rec, err := record.MakePutRecord(sk, key, value, sign)
-	if err != nil {
-		log.Debug("creation of record failed!")
-		return err
-	}
 
+	rec := record.MakePutRecord(key, value)
 	err = dht.putLocal(key, rec)
 	if err != nil {
 		return err
@@ -128,13 +114,7 @@ func (dht *IpfsDHT) GetValue(ctx context.Context, key string) (_ []byte, err err
 		return nil, routing.ErrNotFound
 	}
 
-	fixupRec, err := record.MakePutRecord(dht.peerstore.PrivKey(dht.self), key, best, true)
-	if err != nil {
-		// probably shouldnt actually 'error' here as we have found a value we like,
-		// but this call failing probably isnt something we want to ignore
-		return nil, err
-	}
-
+	fixupRec := record.MakePutRecord(key, best)
 	for _, v := range vals {
 		// if someone sent us a different 'less-valid' record, lets correct them
 		if !bytes.Equal(v.Val, best) {