From a9c59f3f154aa586e7303505f90b5f5fa40b96a3 Mon Sep 17 00:00:00 2001 From: Dirk McCormick Date: Thu, 1 Feb 2018 15:09:57 -0500 Subject: [PATCH] Remove record signature verification --- dht.go | 4 ++-- dht_test.go | 12 ++---------- ext_test.go | 10 +--------- handlers.go | 15 ++------------- handlers_test.go | 4 ---- package.json | 4 ++-- records.go | 50 ------------------------------------------------ routing.go | 24 ++--------------------- 8 files changed, 11 insertions(+), 112 deletions(-) diff --git a/dht.go b/dht.go index 5e8c2870430..15166e175e7 100644 --- a/dht.go +++ b/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 diff --git a/dht_test.go b/dht_test.go index cee61a6d5ed..c4ec262cf45 100644 --- a/dht_test.go +++ b/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 diff --git a/ext_test.go b/ext_test.go index 1a05cc0082b..2a5340b32f5 100644 --- a/ext_test.go +++ b/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, diff --git a/handlers.go b/handlers.go index 4d5e77a5c7f..a93b83d9664 100644 --- a/handlers.go +++ b/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 } diff --git a/handlers_test.go b/handlers_test.go index 9d931700507..986bf5bfb0a 100644 --- a/handlers_test.go +++ b/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) diff --git a/package.json b/package.json index 145cd425ad0..413f101d84a 100644 --- a/package.json +++ b/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", diff --git a/records.go b/records.go index 4a544675b69..b272c705625 100644 --- a/records.go +++ b/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) -} diff --git a/routing.go b/routing.go index 9976415a9ce..f94846d4efb 100644 --- a/routing.go +++ b/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) {