verifcid: introduce and integrate Allowlist interface

CHANGELOG.md

@@ -16,8 +16,17 @@ The following emojis are used to highlight certain changes:
 
 ### Added
 
+* The `verifycid` package has been updated with the new Allowlist interface as part of
+  reducing globals efforts. Still, existing global accessor funcs are kept for
+  backwards-compatibility.
+* The `blockservice` and `provider` packages has been updated to accommodate for 
+  changes in `verifycid`.
+
 ### Changed
 
+* 🛠 `blockservice.New` now accepts a variadic of func options following the [Functional
+  Options pattern](https://www.sohamkamani.com/golang/options-pattern/).
+
 ### Removed
 
 ### Fixed

blockservice/blockservice.go

@@ -1,4 +1,4 @@
-// package blockservice implements a BlockService interface that provides
+// Package blockservice implements a BlockService interface that provides
 // a single GetBlock/AddBlock interface that seamlessly retrieves data either
 // locally or from a remote peer through the exchange.
 package blockservice
@@ -11,11 +11,11 @@
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
 
-	blockstore "github.com/ipfs/boxo/blockstore"
-	exchange "github.com/ipfs/boxo/exchange"
+	"github.com/ipfs/boxo/blockstore"
+	"github.com/ipfs/boxo/exchange"
 	"github.com/ipfs/boxo/verifcid"
 	blocks "github.com/ipfs/go-block-format"
-	cid "github.com/ipfs/go-cid"
+	"github.com/ipfs/go-cid"
 	ipld "github.com/ipfs/go-ipld-format"
 	logging "github.com/ipfs/go-log/v2"
 
@@ -64,39 +64,65 @@
 	DeleteBlock(ctx context.Context, o cid.Cid) error
 }
 
+// BoundedBlockService is a Blockservice bounded via strict multihash Allowlist.
+type BoundedBlockService interface {
+	BlockService
+
+	Allowlist() verifcid.Allowlist
+}
+
 type blockService struct {
+	allowlist  verifcid.Allowlist
 	blockstore blockstore.Blockstore
 	exchange   exchange.Interface
 	// If checkFirst is true then first check that a block doesn't
 	// already exist to avoid republishing the block on the exchange.
 	checkFirst bool
 }
 
-// NewBlockService creates a BlockService with given datastore instance.
-func New(bs blockstore.Blockstore, rem exchange.Interface) BlockService {
-	if rem == nil {
-		logger.Debug("blockservice running in local (offline) mode.")
+type Option func(*blockService)
+
+// WriteThrough disable cache checks for writes and make them go straight to
+// the blockstore.
+func WriteThrough() Option {
+	return func(bs *blockService) {
+		bs.checkFirst = false
 	}
+}
 
-	return &blockService{
-		blockstore: bs,
-		exchange:   rem,
-		checkFirst: true,
+// WithAllowlist sets a custom [verifcid.Allowlist] which will be used
+func WithAllowlist(allowlist verifcid.Allowlist) Option {
+	return func(bs *blockService) {
+		bs.allowlist = allowlist
 	}
 }
 
-// NewWriteThrough creates a BlockService that guarantees writes will go
-// through to the blockstore and are not skipped by cache checks.
-func NewWriteThrough(bs blockstore.Blockstore, rem exchange.Interface) BlockService {
-	if rem == nil {
+// New creates a BlockService with given datastore instance.
+func New(bs blockstore.Blockstore, exchange exchange.Interface, opts ...Option) BlockService {
+	if exchange == nil {
 		logger.Debug("blockservice running in local (offline) mode.")
 	}
 
-	return &blockService{
+	service := &blockService{
+		allowlist:  verifcid.DefaultAllowlist,
 		blockstore: bs,
-		exchange:   rem,
-		checkFirst: false,
+		exchange:   exchange,
+		checkFirst: true,
 	}
+
+	for _, opt := range opts {
+		opt(service)
+	}
+
+	return service
+}
+
+// NewWriteThrough creates a BlockService that guarantees writes will go
+// through to the blockstore and are not skipped by cache checks.
+//
+// Deprecated: Use [New] with the [WriteThrough] option.
+func NewWriteThrough(bs blockstore.Blockstore, exchange exchange.Interface) BlockService {
+	return New(bs, exchange, WriteThrough())
 }
 
 // Blockstore returns the blockstore behind this blockservice.
@@ -109,27 +135,37 @@
 	return s.exchange
 }
 
+func (s *blockService) Allowlist() verifcid.Allowlist {
+	return s.allowlist
+}
+
 // NewSession creates a new session that allows for
 // controlled exchange of wantlists to decrease the bandwidth overhead.
 // If the current exchange is a SessionExchange, a new exchange
 // session will be created. Otherwise, the current exchange will be used
 // directly.
 func NewSession(ctx context.Context, bs BlockService) *Session {
+	allowlist := verifcid.Allowlist(verifcid.DefaultAllowlist)
+	if bbs, ok := bs.(BoundedBlockService); ok {
+		allowlist = bbs.Allowlist()
+	}
 	exch := bs.Exchange()
 	if sessEx, ok := exch.(exchange.SessionExchange); ok {
 		return &Session{
-			sessCtx:  ctx,
-			ses:      nil,
-			sessEx:   sessEx,
-			bs:       bs.Blockstore(),
-			notifier: exch,
+			allowlist: allowlist,
+			sessCtx:   ctx,
+			ses:       nil,
+			sessEx:    sessEx,
+			bs:        bs.Blockstore(),
+			notifier:  exch,
 		}
 	}
 	return &Session{
-		ses:      exch,
-		sessCtx:  ctx,
-		bs:       bs.Blockstore(),
-		notifier: exch,
+		allowlist: allowlist,
+		ses:       exch,
+		sessCtx:   ctx,
+		bs:        bs.Blockstore(),
+		notifier:  exch,
 	}
 }
 
@@ -139,8 +175,7 @@
 	defer span.End()
 
 	c := o.Cid()
-	// hash security
-	err := verifcid.ValidateCid(c)
+	err := verifcid.ValidateCid(s.allowlist, c) // hash security
 	if err != nil {
 		return err
 	}
@@ -171,7 +206,7 @@
 
 	// hash security
 	for _, b := range bs {
-		err := verifcid.ValidateCid(b.Cid())
+		err := verifcid.ValidateCid(s.allowlist, b.Cid())
 		if err != nil {
 			return err
 		}
@@ -221,15 +256,15 @@
 		f = s.getExchange
 	}
 
-	return getBlock(ctx, c, s.blockstore, f) // hash security
+	return getBlock(ctx, c, s.blockstore, s.allowlist, f)
 }
 
 func (s *blockService) getExchange() notifiableFetcher {
 	return s.exchange
 }
 
-func getBlock(ctx context.Context, c cid.Cid, bs blockstore.Blockstore, fget func() notifiableFetcher) (blocks.Block, error) {
-	err := verifcid.ValidateCid(c) // hash security
+func getBlock(ctx context.Context, c cid.Cid, bs blockstore.Blockstore, allowlist verifcid.Allowlist, fget func() notifiableFetcher) (blocks.Block, error) {
+	err := verifcid.ValidateCid(allowlist, c) // hash security
 	if err != nil {
 		return nil, err
 	}
@@ -278,18 +313,18 @@
 		f = s.getExchange
 	}
 
-	return getBlocks(ctx, ks, s.blockstore, f) // hash security
+	return getBlocks(ctx, ks, s.blockstore, s.allowlist, f)
 }
 
-func getBlocks(ctx context.Context, ks []cid.Cid, bs blockstore.Blockstore, fget func() notifiableFetcher) <-chan blocks.Block {
+func getBlocks(ctx context.Context, ks []cid.Cid, bs blockstore.Blockstore, allowlist verifcid.Allowlist, fget func() notifiableFetcher) <-chan blocks.Block {
 	out := make(chan blocks.Block)
 
 	go func() {
 		defer close(out)
 
 		allValid := true
 		for _, c := range ks {
-			if err := verifcid.ValidateCid(c); err != nil {
+			if err := verifcid.ValidateCid(allowlist, c); err != nil {
 				allValid = false
 				break
 			}
@@ -300,7 +335,7 @@
 			ks2 := make([]cid.Cid, 0, len(ks))
 			for _, c := range ks {
 				// hash security
-				if err := verifcid.ValidateCid(c); err == nil {
+				if err := verifcid.ValidateCid(allowlist, c); err == nil {
 					ks2 = append(ks2, c)
 				} else {
 					logger.Errorf("unsafe CID (%s) passed to blockService.GetBlocks: %s", c, err)
@@ -396,12 +431,13 @@
 
 // Session is a helper type to provide higher level access to bitswap sessions
 type Session struct {
-	bs       blockstore.Blockstore
-	ses      exchange.Fetcher
-	sessEx   exchange.SessionExchange
-	sessCtx  context.Context
-	notifier notifier
-	lk       sync.Mutex
+	allowlist verifcid.Allowlist
+	bs        blockstore.Blockstore
+	ses       exchange.Fetcher
+	sessEx    exchange.SessionExchange
+	sessCtx   context.Context
+	notifier  notifier
+	lk        sync.Mutex
 }
 
 type notifiableFetcher interface {
@@ -444,15 +480,15 @@
 	ctx, span := internal.StartSpan(ctx, "Session.GetBlock", trace.WithAttributes(attribute.Stringer("CID", c)))
 	defer span.End()
 
-	return getBlock(ctx, c, s.bs, s.getFetcherFactory()) // hash security
+	return getBlock(ctx, c, s.bs, s.allowlist, s.getFetcherFactory())
 }
 
 // GetBlocks gets blocks in the context of a request session
 func (s *Session) GetBlocks(ctx context.Context, ks []cid.Cid) <-chan blocks.Block {
 	ctx, span := internal.StartSpan(ctx, "Session.GetBlocks")
 	defer span.End()
 
-	return getBlocks(ctx, ks, s.bs, s.getFetcherFactory()) // hash security
+	return getBlocks(ctx, ks, s.bs, s.allowlist, s.getFetcherFactory())
 }
 
 var _ BlockGetter = (*Session)(nil)

blockservice/blockservice_test.go

@@ -7,15 +7,20 @@ import (
 	blockstore "github.com/ipfs/boxo/blockstore"
 	exchange "github.com/ipfs/boxo/exchange"
 	offline "github.com/ipfs/boxo/exchange/offline"
+	"github.com/ipfs/boxo/verifcid"
 	blocks "github.com/ipfs/go-block-format"
 	cid "github.com/ipfs/go-cid"
 	ds "github.com/ipfs/go-datastore"
 	dssync "github.com/ipfs/go-datastore/sync"
 	butil "github.com/ipfs/go-ipfs-blocksutil"
 	ipld "github.com/ipfs/go-ipld-format"
+	"github.com/multiformats/go-multihash"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestWriteThroughWorks(t *testing.T) {
+	t.Parallel()
+
 	bstore := &PutCountingBlockstore{
 		blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore())),
 		0,
@@ -46,6 +51,8 @@ func TestWriteThroughWorks(t *testing.T) {
 }
 
 func TestExchangeWrite(t *testing.T) {
+	t.Parallel()
+
 	bstore := &PutCountingBlockstore{
 		blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore())),
 		0,
@@ -117,6 +124,8 @@ func TestExchangeWrite(t *testing.T) {
 }
 
 func TestLazySessionInitialization(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
@@ -215,6 +224,8 @@ func (fe *fakeSessionExchange) NewSession(ctx context.Context) exchange.Fetcher
 }
 
 func TestNilExchange(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
@@ -241,3 +252,39 @@ func TestNilExchange(t *testing.T) {
 		t.Fatal("got the wrong block")
 	}
 }
+
+func TestAllowlist(t *testing.T) {
+	t.Parallel()
+	a := assert.New(t)
+
+	ctx := context.Background()
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	bgen := butil.NewBlockGenerator()
+	block := bgen.Next()
+
+	data := []byte("this is some blake3 block")
+	mh, err := multihash.Sum(data, multihash.BLAKE3, -1)
+	a.NoError(err)
+	blake3 := cid.NewCidV1(cid.Raw, mh)
+
+	bs := blockstore.NewBlockstore(dssync.MutexWrap(ds.NewMapDatastore()))
+	a.NoError(bs.Put(ctx, block))
+	b, err := blocks.NewBlockWithCid(data, blake3)
+	a.NoError(err)
+	a.NoError(bs.Put(ctx, b))
+
+	check := func(getBlock func(context.Context, cid.Cid) (blocks.Block, error)) {
+		_, err := getBlock(ctx, block.Cid())
+		a.Error(err)
+		a.ErrorIs(err, verifcid.ErrPossiblyInsecureHashFunction)
+
+		_, err = getBlock(ctx, blake3)
+		a.NoError(err)
+	}
+
+	blockservice := New(bs, nil, WithAllowlist(verifcid.NewAllowlist(map[uint64]bool{multihash.BLAKE3: true})))
+	check(blockservice.GetBlock)
+	check(NewSession(ctx, blockservice).GetBlock)
+}