Skip to content

Commit

Permalink
Revert Most of "Merge pull request #4751 from ipfs/feat/safe-cid"
Browse files Browse the repository at this point in the history
This mostly reverts commit 13887ff,
reversing changes made to a544026.

It keeps the fixed to "ipfs pin verify" and a fix to t0050-block.sh to use
a longer block size.
  • Loading branch information
kevina committed Jun 24, 2018
1 parent be20903 commit c6ee301
Show file tree
Hide file tree
Showing 11 changed files with 6 additions and 354 deletions.
39 changes: 4 additions & 35 deletions blockservice/blockservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"fmt"
"io"

"github.com/ipfs/go-ipfs/thirdparty/verifcid"

blocks "gx/ipfs/QmTRCUvZLiir12Qr6MV3HKfKMHX8Nf1Vddn6t2g5nsQSb9/go-block-format"
exchange "gx/ipfs/QmVSe7YJbPnEmkSUKD3HxSvp8HJoyCU55hQoCMRq7N1jaK/go-ipfs-exchange-interface"
cid "gx/ipfs/QmapdYm1b22Frv3k17fqrBYTFRxwiaVJkB299Mfn33edeB/go-cid"
Expand Down Expand Up @@ -131,11 +129,6 @@ func NewSession(ctx context.Context, bs BlockService) *Session {
// TODO pass a context into this if the remote.HasBlock is going to remain here.
func (s *blockService) AddBlock(o blocks.Block) error {
c := o.Cid()
// hash security
err := verifcid.ValidateCid(c)
if err != nil {
return err
}
if s.checkFirst {
if has, err := s.blockstore.Has(c); has || err != nil {
return err
Expand All @@ -157,13 +150,6 @@ func (s *blockService) AddBlock(o blocks.Block) error {
}

func (s *blockService) AddBlocks(bs []blocks.Block) error {
// hash security
for _, b := range bs {
err := verifcid.ValidateCid(b.Cid())
if err != nil {
return err
}
}
var toput []blocks.Block
if s.checkFirst {
toput = make([]blocks.Block, 0, len(bs))
Expand Down Expand Up @@ -205,15 +191,10 @@ func (s *blockService) GetBlock(ctx context.Context, c *cid.Cid) (blocks.Block,
f = s.exchange
}

return getBlock(ctx, c, s.blockstore, f) // hash security
return getBlock(ctx, c, s.blockstore, f)
}

func getBlock(ctx context.Context, c *cid.Cid, bs blockstore.Blockstore, f exchange.Fetcher) (blocks.Block, error) {
err := verifcid.ValidateCid(c) // hash security
if err != nil {
return nil, err
}

block, err := bs.Get(c)
if err == nil {
return block, nil
Expand Down Expand Up @@ -246,7 +227,7 @@ func getBlock(ctx context.Context, c *cid.Cid, bs blockstore.Blockstore, f excha
// the returned channel.
// NB: No guarantees are made about order.
func (s *blockService) GetBlocks(ctx context.Context, ks []*cid.Cid) <-chan blocks.Block {
return getBlocks(ctx, ks, s.blockstore, s.exchange) // hash security
return getBlocks(ctx, ks, s.blockstore, s.exchange)
}

func getBlocks(ctx context.Context, ks []*cid.Cid, bs blockstore.Blockstore, f exchange.Fetcher) <-chan blocks.Block {
Expand All @@ -255,18 +236,6 @@ func getBlocks(ctx context.Context, ks []*cid.Cid, bs blockstore.Blockstore, f e
go func() {
defer close(out)

k := 0
for _, c := range ks {
// hash security
if err := verifcid.ValidateCid(c); err == nil {
ks[k] = c
k++
} else {
log.Errorf("unsafe CID (%s) passed to blockService.GetBlocks: %s", c, err)
}
}
ks = ks[:k]

var misses []*cid.Cid
for _, c := range ks {
hit, err := bs.Get(c)
Expand Down Expand Up @@ -325,12 +294,12 @@ type Session struct {

// GetBlock gets a block in the context of a request session
func (s *Session) GetBlock(ctx context.Context, c *cid.Cid) (blocks.Block, error) {
return getBlock(ctx, c, s.bs, s.ses) // hash security
return getBlock(ctx, c, s.bs, s.ses)
}

// GetBlocks gets blocks in the context of a request session
func (s *Session) GetBlocks(ctx context.Context, ks []*cid.Cid) <-chan blocks.Block {
return getBlocks(ctx, ks, s.bs, s.ses) // hash security
return getBlocks(ctx, ks, s.bs, s.ses)
}

var _ BlockGetter = (*Session)(nil)
5 changes: 0 additions & 5 deletions core/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
pin "github.com/ipfs/go-ipfs/pin"
repo "github.com/ipfs/go-ipfs/repo"
cfg "github.com/ipfs/go-ipfs/repo/config"
"github.com/ipfs/go-ipfs/thirdparty/verifbs"
uio "github.com/ipfs/go-ipfs/unixfs/io"

offline "gx/ipfs/QmPf114DXfa6TqGKYhBGR7EtXRho4rCJgwyA1xkuMY5vwF/go-ipfs-exchange-offline"
Expand Down Expand Up @@ -178,9 +177,7 @@ func setupNode(ctx context.Context, n *IpfsNode, cfg *BuildCfg) error {
TempErrFunc: isTooManyFDError,
}

// hash security
bs := bstore.NewBlockstore(rds)
bs = &verifbs.VerifBS{Blockstore: bs}

opts := bstore.DefaultCacheOpts()
conf, err := n.Repo.Config()
Expand All @@ -206,10 +203,8 @@ func setupNode(ctx context.Context, n *IpfsNode, cfg *BuildCfg) error {
n.Blockstore = bstore.NewGCBlockstore(cbs, n.GCLocker)

if conf.Experimental.FilestoreEnabled {
// hash security
n.Filestore = filestore.NewFilestore(cbs, n.Repo.FileManager())
n.Blockstore = bstore.NewGCBlockstore(n.Filestore, n.GCLocker)
n.Blockstore = &verifbs.VerifBSGC{GCBlockstore: n.Blockstore}
}

rcfg, err := n.Repo.Config()
Expand Down
2 changes: 1 addition & 1 deletion core/commands/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ You can now check what blocks have been created by:
exch = offline.Exchange(addblockstore)
}

bserv := blockservice.New(addblockstore, exch) // hash security 001
bserv := blockservice.New(addblockstore, exch)
dserv := dag.NewDAGService(bserv)

outChan := make(chan interface{}, adderOutChanSize)
Expand Down
10 changes: 0 additions & 10 deletions core/commands/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
path "github.com/ipfs/go-ipfs/path"
resolver "github.com/ipfs/go-ipfs/path/resolver"
pin "github.com/ipfs/go-ipfs/pin"
"github.com/ipfs/go-ipfs/thirdparty/verifcid"
uio "github.com/ipfs/go-ipfs/unixfs/io"

u "gx/ipfs/QmPdKqUcHGFdeSpvjVoaTRPPstGif9GBZb5Q56RVw9o69A/go-ipfs-util"
Expand Down Expand Up @@ -609,15 +608,6 @@ func pinVerify(ctx context.Context, n *core.IpfsNode, opts pinVerifyOpts) <-chan
return status
}

if err := verifcid.ValidateCid(root); err != nil {
status := PinStatus{Ok: false}
if opts.explain {
status.BadNodes = []BadNode{BadNode{Cid: key, Err: err.Error()}}
}
visited[key] = status
return status
}

links, err := getLinks(ctx, root)
if err != nil {
status := PinStatus{Ok: false}
Expand Down
7 changes: 0 additions & 7 deletions exchange/reprovide/reprovide.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"
"time"

"github.com/ipfs/go-ipfs/thirdparty/verifcid"

backoff "gx/ipfs/QmPJUtEJsm5YLUWhF6imvyCH8KZXRJa9Wup7FDMwTy5Ufz/backoff"
routing "gx/ipfs/QmUV9hDAAyjeGbxbXkJ2sYqZ6dTd1DXJ2REhYEkRm178Tg/go-libp2p-routing"
cid "gx/ipfs/QmapdYm1b22Frv3k17fqrBYTFRxwiaVJkB299Mfn33edeB/go-cid"
Expand Down Expand Up @@ -85,11 +83,6 @@ func (rp *Reprovider) Reprovide() error {
return fmt.Errorf("failed to get key chan: %s", err)
}
for c := range keychan {
// hash security
if err := verifcid.ValidateCid(c); err != nil {
log.Errorf("insecure hash in reprovider, %s (%s)", c, err)
continue
}
op := func() error {
err := rp.rsys.Provide(rp.ctx, c, true)
if err != nil {
Expand Down
26 changes: 1 addition & 25 deletions pin/gc/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import (
"context"
"errors"
"fmt"
"strings"

bserv "github.com/ipfs/go-ipfs/blockservice"
dag "github.com/ipfs/go-ipfs/merkledag"
pin "github.com/ipfs/go-ipfs/pin"
"github.com/ipfs/go-ipfs/thirdparty/verifcid"

offline "gx/ipfs/QmPf114DXfa6TqGKYhBGR7EtXRho4rCJgwyA1xkuMY5vwF/go-ipfs-exchange-offline"
ipld "gx/ipfs/QmWi2BYBL5gJ3CiAiQchg6rn1A8iBsrWy51EYxvHVjFvLb/go-ipld-format"
Expand Down Expand Up @@ -131,34 +129,12 @@ func GC(ctx context.Context, bs bstore.GCBlockstore, dstor dstore.Datastore, pn
// adds them to the given cid.Set, using the provided dag.GetLinks function
// to walk the tree.
func Descendants(ctx context.Context, getLinks dag.GetLinks, set *cid.Set, roots []*cid.Cid) error {
verifyGetLinks := func(ctx context.Context, c *cid.Cid) ([]*ipld.Link, error) {
err := verifcid.ValidateCid(c)
if err != nil {
return nil, err
}

return getLinks(ctx, c)
}

verboseCidError := func(err error) error {
if strings.Contains(err.Error(), verifcid.ErrBelowMinimumHashLength.Error()) ||
strings.Contains(err.Error(), verifcid.ErrPossiblyInsecureHashFunction.Error()) {
err = fmt.Errorf("\"%s\"\nPlease run 'ipfs pin verify'"+
" to list insecure hashes. If you want to read them,"+
" please downgrade your go-ipfs to 0.4.13\n", err)
log.Error(err)
}
return err
}

for _, c := range roots {
set.Add(c)

// EnumerateChildren recursively walks the dag and adds the keys to the given set
err := dag.EnumerateChildren(ctx, verifyGetLinks, c, set.Visit)

err := dag.EnumerateChildren(ctx, getLinks, c, set.Visit)
if err != nil {
err = verboseCidError(err)
return err
}
}
Expand Down
1 change: 0 additions & 1 deletion test/sharness/t0275-cid-security-data/AFKSEBCGPUJZE.data

This file was deleted.

86 changes: 0 additions & 86 deletions test/sharness/t0275-cid-security.sh

This file was deleted.

63 changes: 0 additions & 63 deletions thirdparty/verifbs/verifbs.go

This file was deleted.

Loading

1 comment on commit c6ee301

@GitCop
Copy link

@GitCop GitCop commented on c6ee301 Jun 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were the following issues with your Pull Request

We ask for a few features in the commit message for Open Source licensing hygiene and commit message clarity.
git commit --amend can often help you quickly improve the commit message.
Guidelines and a script are available to help in the long run.
Your feedback on GitCop is welcome on this issue.


This message was auto-generated by https://gitcop.com

Please sign in to comment.