Skip to content

Commit

Permalink
Merge pull request #4751 from ipfs/feat/safe-cid
Browse files Browse the repository at this point in the history
Disallow usage of unsafe hashes for reading and adding content
  • Loading branch information
whyrusleeping authored Mar 5, 2018
2 parents a544026 + 4527279 commit 13887ff
Show file tree
Hide file tree
Showing 13 changed files with 362 additions and 23 deletions.
33 changes: 29 additions & 4 deletions blockservice/blockservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"io"

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

logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log"
blockstore "gx/ipfs/QmTVDM4LCSUMFNQzbDLL9zQwp8usE6QHymFdh3h8vL9v6b/go-ipfs-blockstore"
Expand Down Expand Up @@ -130,6 +131,11 @@ 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 @@ -149,6 +155,13 @@ 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 @@ -189,10 +202,15 @@ func (s *blockService) GetBlock(ctx context.Context, c *cid.Cid) (blocks.Block,
f = s.exchange
}

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

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 @@ -224,11 +242,18 @@ 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)
return getBlocks(ctx, ks, s.blockstore, s.exchange) // hash security
}

func getBlocks(ctx context.Context, ks []*cid.Cid, bs blockstore.Blockstore, f exchange.Fetcher) <-chan blocks.Block {
out := make(chan blocks.Block)
for _, c := range ks {
// hash security
if err := verifcid.ValidateCid(c); err != nil {
log.Errorf("unsafe CID (%s) passed to blockService.GetBlocks: %s", c, err)
}
}

go func() {
defer close(out)
var misses []*cid.Cid
Expand Down Expand Up @@ -285,12 +310,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)
return getBlock(ctx, c, s.bs, s.ses) // hash security
}

// 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)
return getBlocks(ctx, ks, s.bs, s.ses) // hash security
}

var _ BlockGetter = (*Session)(nil)
5 changes: 5 additions & 0 deletions core/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ 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"

ds "gx/ipfs/QmPpegoMqhAEqjncrzArm7KVWAkCm78rqL2DPuNjhPrshg/go-datastore"
Expand Down Expand Up @@ -170,7 +171,9 @@ func setupNode(ctx context.Context, n *IpfsNode, cfg *BuildCfg) error {
TempErrFunc: isTooManyFDError,
}

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

opts := bstore.DefaultCacheOpts()
conf, err := n.Repo.Config()
Expand All @@ -196,8 +199,10 @@ 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(bs, n.Repo.FileManager())
n.Blockstore = bstore.NewGCBlockstore(n.Filestore, n.GCLocker)
n.Blockstore = &verifbs.VerifBSGC{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)
bserv := blockservice.New(addblockstore, exch) // hash security 001
dserv := dag.NewDAGService(bserv)

outChan := make(chan interface{}, adderOutChanSize)
Expand Down
38 changes: 23 additions & 15 deletions core/commands/pin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ 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/QmNiJuT8Ja3hMVpBHXv3Q6dwmperaQ6JjLtpMQgMCD7xvx/go-ipfs-util"
Expand Down Expand Up @@ -462,25 +463,23 @@ var verifyPinCmd = &cmds.Command{
cmds.Text: func(res cmds.Response) (io.Reader, error) {
quiet, _, _ := res.Request().Option("quiet").Bool()

outChan, ok := res.Output().(<-chan interface{})
out, err := unwrapOutput(res.Output())
if err != nil {
return nil, err
}
r, ok := out.(*PinVerifyRes)
if !ok {
return nil, u.ErrCast()
return nil, e.TypeErr(r, out)
}

rdr, wtr := io.Pipe()
go func() {
defer wtr.Close()
for r0 := range outChan {
r := r0.(*PinVerifyRes)
if quiet && !r.Ok {
fmt.Fprintf(wtr, "%s\n", r.Cid)
} else if !quiet {
r.Format(wtr)
}
}
}()
buf := &bytes.Buffer{}
if quiet && !r.Ok {
fmt.Fprintf(buf, "%s\n", r.Cid)
} else if !quiet {
r.Format(buf)
}

return rdr, nil
return buf, nil
},
},
}
Expand Down Expand Up @@ -610,6 +609,15 @@ 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: 7 additions & 0 deletions exchange/reprovide/reprovide.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"time"

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

backoff "gx/ipfs/QmPJUtEJsm5YLUWhF6imvyCH8KZXRJa9Wup7FDMwTy5Ufz/backoff"
logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log"
routing "gx/ipfs/QmTiWLZ6Fo5j4KcTVutZJ5KWRRJrbxzmxA4td8NfEdrPh7/go-libp2p-routing"
Expand Down Expand Up @@ -83,6 +85,11 @@ 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: 25 additions & 1 deletion pin/gc/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (
"context"
"errors"
"fmt"
"strings"

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

dstore "gx/ipfs/QmPpegoMqhAEqjncrzArm7KVWAkCm78rqL2DPuNjhPrshg/go-datastore"
logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log"
Expand Down Expand Up @@ -129,12 +131,34 @@ 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, getLinks, c, set.Visit)
err := dag.EnumerateChildren(ctx, verifyGetLinks, c, set.Visit)

if err != nil {
err = verboseCidError(err)
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/sharness/t0050-block.sh
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,11 @@ test_expect_success "block get output looks right" '
'

test_expect_success "can set multihash type and length on block put" '
HASH=$(echo "foooo" | ipfs block put --format=raw --mhtype=sha3 --mhlen=16)
HASH=$(echo "foooo" | ipfs block put --format=raw --mhtype=sha3 --mhlen=20)
'

test_expect_success "output looks good" '
test "z25ScPysKoxJBcPxczn9NvuHiZU5" = "$HASH"
test "z83bYcqyBkbx5fuNAcvbdv4pr5RYQiEpK" = "$HASH"
'

test_expect_success "can read block with different hash" '
Expand Down
9 changes: 9 additions & 0 deletions test/sharness/t0085-pins.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ test_pins() {
cat hashes | ipfs pin add $EXTRA_ARGS
'

test_expect_success "see if verify works" '
ipfs pin verify
'

test_expect_success "see if verify --verbose works" '
ipfs pin verify --verbose > verify_out &&
test $(cat verify_out | wc -l) > 8
'

test_expect_success "unpin those hashes" '
cat hashes | ipfs pin rm
'
Expand Down
1 change: 1 addition & 0 deletions test/sharness/t0275-cid-security-data/AFKSEBCGPUJZE.data
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testing
76 changes: 76 additions & 0 deletions test/sharness/t0275-cid-security.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#!/bin/sh
#
# Copyright (c) 2017 Jakub Sztandera
# MIT Licensed; see the LICENSE file in this repository.
#

test_description="Cid Security"

. lib/test-lib.sh

test_init_ipfs

test_expect_success "adding using unsafe function fails with error" '
echo foo | test_must_fail ipfs add --hash murmur3 2>add_out
'

test_expect_success "error reason is pointed out" '
grep "insecure hash functions not allowed" add_out
'

test_expect_success "adding using too short of a hash function gives out an error" '
echo foo | test_must_fail ipfs block put --mhlen 19 2>block_out
'

test_expect_success "error reason is pointed out" '
grep "hashes must be at 20 least bytes long" block_out
'


test_cat_get() {

test_expect_success "ipfs cat fails with unsafe hash function" '
test_must_fail ipfs cat zDvnoLcPKWR 2>ipfs_cat
'


test_expect_success "error reason is pointed out" '
grep "insecure hash functions not allowed" ipfs_cat
'


test_expect_success "ipfs get fails with too short function" '
test_must_fail ipfs get z2ba5YhCCFNFxLtxMygQwjBjYSD8nUeN 2>ipfs_get
'

test_expect_success "error reason is pointed out" '
grep "hashes must be at 20 least bytes long" ipfs_get
'
}


test_gc() {
test_expect_success "injecting insecure block" '
mkdir -p "$IPFS_PATH/blocks/JZ" &&
cp -f ../t0275-cid-security-data/AFKSEBCGPUJZE.data "$IPFS_PATH/blocks/JZ"
'

test_expect_success "gc works" 'ipfs repo gc > gc_out'
test_expect_success "gc removed bad block" '
grep zDvnoGUyhEq gc_out
'
}


# should work offline
test_cat_get
test_gc

# should work online
test_launch_ipfs_daemon
test_cat_get
test_gc
test_kill_ipfs_daemon

test_done
Loading

0 comments on commit 13887ff

Please sign in to comment.