From e643269a65043bc12ce14b7a04af9d61c4a67b3e Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 1 Mar 2018 21:21:11 +0100 Subject: [PATCH] Enforce Cid security rules for getting and adding blocks License: MIT Signed-off-by: Jakub Sztandera --- blockservice/blockservice.go | 33 ++++++++++++++++--- core/builder.go | 5 +++ core/commands/add.go | 2 +- removeme/verifbs/verifbs.go | 63 ++++++++++++++++++++++++++++++++++++ test/sharness/t0050-block.sh | 4 +-- 5 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 removeme/verifbs/verifbs.go diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index c6d99c97d4f..db7aad51540 100644 --- a/blockservice/blockservice.go +++ b/blockservice/blockservice.go @@ -10,6 +10,7 @@ import ( "io" exchange "github.com/ipfs/go-ipfs/exchange" + "github.com/ipfs/go-ipfs/removeme/verifcid" logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log" blockstore "gx/ipfs/QmTVDM4LCSUMFNQzbDLL9zQwp8usE6QHymFdh3h8vL9v6b/go-ipfs-blockstore" @@ -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 @@ -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)) @@ -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) + if err != nil { + return nil, err + } + block, err := bs.Get(c) if err == nil { return block, nil @@ -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 @@ -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) diff --git a/core/builder.go b/core/builder.go index 32006b24e05..4e73ebb39da 100644 --- a/core/builder.go +++ b/core/builder.go @@ -15,6 +15,7 @@ import ( dag "github.com/ipfs/go-ipfs/merkledag" resolver "github.com/ipfs/go-ipfs/path/resolver" pin "github.com/ipfs/go-ipfs/pin" + "github.com/ipfs/go-ipfs/removeme/verifbs" repo "github.com/ipfs/go-ipfs/repo" cfg "github.com/ipfs/go-ipfs/repo/config" uio "github.com/ipfs/go-ipfs/unixfs/io" @@ -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() @@ -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() diff --git a/core/commands/add.go b/core/commands/add.go index 22171d9a597..f7f7a4215aa 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -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) diff --git a/removeme/verifbs/verifbs.go b/removeme/verifbs/verifbs.go new file mode 100644 index 00000000000..fec59743a57 --- /dev/null +++ b/removeme/verifbs/verifbs.go @@ -0,0 +1,63 @@ +package verifbs + +import ( + "github.com/ipfs/go-ipfs/removeme/verifcid" + + bstore "gx/ipfs/QmTVDM4LCSUMFNQzbDLL9zQwp8usE6QHymFdh3h8vL9v6b/go-ipfs-blockstore" + cid "gx/ipfs/QmcZfnkapfECQGcLZaf9B79NRg7cRa9EnZh4LSbkCzwNvY/go-cid" + blocks "gx/ipfs/Qmej7nf81hi2x2tvjRBF3mcp74sQyuDH4VMYDGd1YtXjb2/go-block-format" +) + +type VerifBSGC struct { + bstore.GCBlockstore +} + +func (bs *VerifBSGC) Put(b blocks.Block) error { + if err := verifcid.ValidateCid(b.Cid()); err != nil { + return err + } + return bs.GCBlockstore.Put(b) +} + +func (bs *VerifBSGC) PutMany(blks []blocks.Block) error { + for _, b := range blks { + if err := verifcid.ValidateCid(b.Cid()); err != nil { + return err + } + } + return bs.GCBlockstore.PutMany(blks) +} + +func (bs *VerifBSGC) Get(c *cid.Cid) (blocks.Block, error) { + if err := verifcid.ValidateCid(c); err != nil { + return nil, err + } + return bs.GCBlockstore.Get(c) +} + +type VerifBS struct { + bstore.Blockstore +} + +func (bs *VerifBS) Put(b blocks.Block) error { + if err := verifcid.ValidateCid(b.Cid()); err != nil { + return err + } + return bs.Blockstore.Put(b) +} + +func (bs *VerifBS) PutMany(blks []blocks.Block) error { + for _, b := range blks { + if err := verifcid.ValidateCid(b.Cid()); err != nil { + return err + } + } + return bs.Blockstore.PutMany(blks) +} + +func (bs *VerifBS) Get(c *cid.Cid) (blocks.Block, error) { + if err := verifcid.ValidateCid(c); err != nil { + return nil, err + } + return bs.Blockstore.Get(c) +} diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index 285d8ad94b5..f88edf2a7f1 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -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" '