From 6b07cb0d8bea293f4b1be2885905e9ff5fe227aa Mon Sep 17 00:00:00 2001 From: Wondertan Date: Sun, 9 Jul 2023 18:56:11 +0200 Subject: [PATCH] verifcid: introduce Allowlist interface --- CHANGELOG.md | 6 + blockservice/blockservice.go | 106 +++++++++++------- provider/reprovider.go | 13 ++- verifcid/allowlist.go | 69 ++++++++++++ .../{validate_test.go => allowlist_test.go} | 30 +++-- verifcid/cid.go | 33 ++++++ verifcid/validate.go | 69 ------------ 7 files changed, 201 insertions(+), 125 deletions(-) create mode 100644 verifcid/allowlist.go rename verifcid/{validate_test.go => allowlist_test.go} (69%) create mode 100644 verifcid/cid.go delete mode 100644 verifcid/validate.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b3a41d83f..04021016c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ 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 ### Removed diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 773fb5303..cdcac33cc 100644 --- a/blockservice/blockservice.go +++ b/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 @@ import ( "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,7 +64,15 @@ type BlockService interface { 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 @@ -72,33 +80,45 @@ type blockService struct { checkFirst bool } -// NewBlockService creates a BlockService with given datastore instance. -func New(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) BlockService { + if exchange == nil { logger.Debug("blockservice running in local (offline) mode.") } return &blockService{ + allowlist: verifcid.DefaultAllowlist, blockstore: bs, - exchange: rem, + exchange: exchange, checkFirst: true, } } // 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 { +func NewWriteThrough(bs blockstore.Blockstore, exchange exchange.Interface) BlockService { + if exchange == nil { logger.Debug("blockservice running in local (offline) mode.") } return &blockService{ + allowlist: verifcid.DefaultAllowlist, blockstore: bs, - exchange: rem, + exchange: exchange, checkFirst: false, } } +// NewWithAllowList creates a BlockService with customized multihash Allowlist. +func NewWithAllowList(bs blockstore.Blockstore, exchange exchange.Interface, allowlist verifcid.Allowlist) BlockService { + return &blockService{ + allowlist: allowlist, + blockstore: bs, + exchange: exchange, + checkFirst: true, + } +} + // Blockstore returns the blockstore behind this blockservice. func (s *blockService) Blockstore() blockstore.Blockstore { return s.blockstore @@ -109,27 +129,37 @@ func (s *blockService) Exchange() exchange.Interface { 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 +169,7 @@ func (s *blockService) AddBlock(ctx context.Context, o blocks.Block) error { 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 +200,7 @@ func (s *blockService) AddBlocks(ctx context.Context, bs []blocks.Block) error { // 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 +250,15 @@ func (s *blockService) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, e 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,10 +307,10 @@ func (s *blockService) GetBlocks(ctx context.Context, ks []cid.Cid) <-chan block 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() { @@ -289,7 +318,7 @@ func getBlocks(ctx context.Context, ks []cid.Cid, bs blockstore.Blockstore, fget 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 +329,7 @@ func getBlocks(ctx context.Context, ks []cid.Cid, bs blockstore.Blockstore, fget 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 +425,13 @@ type notifier interface { // 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,7 +474,7 @@ func (s *Session) GetBlock(ctx context.Context, c cid.Cid) (blocks.Block, error) 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 @@ -452,7 +482,7 @@ func (s *Session) GetBlocks(ctx context.Context, ks []cid.Cid) <-chan blocks.Blo 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) diff --git a/provider/reprovider.go b/provider/reprovider.go index 619bf8196..e95227ee0 100644 --- a/provider/reprovider.go +++ b/provider/reprovider.go @@ -13,7 +13,7 @@ import ( "github.com/ipfs/boxo/verifcid" "github.com/ipfs/go-cid" "github.com/ipfs/go-datastore" - namespace "github.com/ipfs/go-datastore/namespace" + "github.com/ipfs/go-datastore/namespace" logging "github.com/ipfs/go-log/v2" "github.com/multiformats/go-multihash" ) @@ -47,6 +47,7 @@ type reprovider struct { initalReprovideDelay time.Duration initialReprovideDelaySet bool + allowlist verifcid.Allowlist rsys Provide keyProvider KeyChanFunc @@ -102,6 +103,7 @@ var DefaultKeyPrefix = datastore.NewKey("/provider") // If provider casts to [Ready], it will wait until [Ready.Ready] is true. func New(ds datastore.Batching, opts ...Option) (System, error) { s := &reprovider{ + allowlist: verifcid.DefaultAllowlist, reprovideInterval: DefaultReproviderInterval, maxReprovideBatchSize: math.MaxUint, keyPrefix: DefaultKeyPrefix, @@ -149,6 +151,13 @@ func New(ds datastore.Batching, opts ...Option) (System, error) { return s, nil } +func Allowlist(allowlist verifcid.Allowlist) Option { + return func(system *reprovider) error { + system.allowlist = allowlist + return nil + } +} + func ReproviderInterval(duration time.Duration) Option { return func(system *reprovider) error { system.reprovideInterval = duration @@ -294,7 +303,7 @@ func (s *reprovider) run() { delete(m, c) // hash security - if err := verifcid.ValidateCid(c); err != nil { + if err := verifcid.ValidateCid(s.allowlist, c); err != nil { log.Errorf("insecure hash in reprovider, %s (%s)", c, err) continue } diff --git a/verifcid/allowlist.go b/verifcid/allowlist.go new file mode 100644 index 000000000..b572de3a6 --- /dev/null +++ b/verifcid/allowlist.go @@ -0,0 +1,69 @@ +package verifcid + +import ( + mh "github.com/multiformats/go-multihash" +) + +// DefaultAllowlist is the default list of hashes allowed in IPFS. +var DefaultAllowlist defaultAllowlist + +// Allowlist defines an interface containing list of allowed multihashes. +type Allowlist interface { + // IsAllowed checks for multihash allowance by the code. + IsAllowed(code uint64) bool +} + +// NewAllowlist constructs new [Allowlist] from the given map set. +func NewAllowlist(allowset map[uint64]bool) Allowlist { + return allowlist{allowset: allowset} +} + +// NewOverridingAllowlist is like [NewAllowlist] but it will fallback to an other [AllowList] if keys are missing. +// If override is nil it will return unsecure for unknown things. +func NewOverridingAllowlist(override Allowlist, allowset map[uint64]bool) Allowlist { + return allowlist{override, allowset} +} + +type allowlist struct { + override Allowlist + allowset map[uint64]bool +} + +func (al allowlist) IsAllowed(code uint64) bool { + if good, found := al.allowset[code]; found { + return good + } + + if al.override != nil { + return al.override.IsAllowed(code) + } + + return false +} + +type defaultAllowlist struct{} + +func (defaultAllowlist) IsAllowed(code uint64) bool { + switch code { + case mh.SHA2_256, mh.SHA2_512, + mh.SHAKE_256, + mh.DBL_SHA2_256, + mh.BLAKE3, + mh.IDENTITY, + + mh.SHA3_224, mh.SHA3_256, mh.SHA3_384, mh.SHA3_512, + mh.KECCAK_224, mh.KECCAK_256, mh.KECCAK_384, mh.KECCAK_512, + + mh.SHA1: // not really secure but still useful for git + return true + default: + if code >= mh.BLAKE2B_MIN+19 && code <= mh.BLAKE2B_MAX { + return true + } + if code >= mh.BLAKE2S_MIN+19 && code <= mh.BLAKE2S_MAX { + return true + } + + return false + } +} diff --git a/verifcid/validate_test.go b/verifcid/allowlist_test.go similarity index 69% rename from verifcid/validate_test.go rename to verifcid/allowlist_test.go index 5129b861a..ab8c415e1 100644 --- a/verifcid/validate_test.go +++ b/verifcid/allowlist_test.go @@ -5,10 +5,10 @@ import ( mh "github.com/multiformats/go-multihash" - cid "github.com/ipfs/go-cid" + "github.com/ipfs/go-cid" ) -func TestValidateCids(t *testing.T) { +func TestDefaultAllowList(t *testing.T) { assertTrue := func(v bool) { t.Helper() if !v { @@ -21,17 +21,6 @@ func TestValidateCids(t *testing.T) { t.Fatal("expected failure") } } - - assertTrue(IsGoodHash(mh.SHA2_256)) - assertTrue(IsGoodHash(mh.BLAKE2B_MIN + 32)) - assertTrue(IsGoodHash(mh.DBL_SHA2_256)) - assertTrue(IsGoodHash(mh.KECCAK_256)) - assertTrue(IsGoodHash(mh.SHA3)) - - assertTrue(IsGoodHash(mh.SHA1)) - - assertFalse(IsGoodHash(mh.BLAKE2B_MIN + 5)) - mhcid := func(code uint64, length int) cid.Cid { mhash, err := mh.Sum([]byte{}, code, length) if err != nil { @@ -40,6 +29,15 @@ func TestValidateCids(t *testing.T) { return cid.NewCidV1(cid.DagCBOR, mhash) } + allowlist := DefaultAllowlist + assertTrue(allowlist.IsAllowed(mh.SHA2_256)) + assertTrue(allowlist.IsAllowed(mh.BLAKE2B_MIN + 32)) + assertTrue(allowlist.IsAllowed(mh.DBL_SHA2_256)) + assertTrue(allowlist.IsAllowed(mh.KECCAK_256)) + assertTrue(allowlist.IsAllowed(mh.SHA3)) + assertTrue(allowlist.IsAllowed(mh.SHA1)) + assertFalse(allowlist.IsAllowed(mh.BLAKE2B_MIN + 5)) + cases := []struct { cid cid.Cid err error @@ -53,9 +51,9 @@ func TestValidateCids(t *testing.T) { } for i, cas := range cases { - if ValidateCid(cas.cid) != cas.err { + if ValidateCid(allowlist, cas.cid) != cas.err { t.Errorf("wrong result in case of %s (index %d). Expected: %s, got %s", - cas.cid, i, cas.err, ValidateCid(cas.cid)) + cas.cid, i, cas.err, ValidateCid(DefaultAllowlist, cas.cid)) } } @@ -64,7 +62,7 @@ func TestValidateCids(t *testing.T) { if err != nil { t.Fatalf("failed to produce a multihash from the long blake3 hash: %v", err) } - if ValidateCid(cid.NewCidV1(cid.DagCBOR, longBlake3Mh)) != ErrAboveMaximumHashLength { + if ValidateCid(allowlist, cid.NewCidV1(cid.DagCBOR, longBlake3Mh)) != ErrAboveMaximumHashLength { t.Errorf("a CID that was longer than the maximum hash length did not error with ErrAboveMaximumHashLength") } } diff --git a/verifcid/cid.go b/verifcid/cid.go new file mode 100644 index 000000000..334da2671 --- /dev/null +++ b/verifcid/cid.go @@ -0,0 +1,33 @@ +package verifcid + +import ( + "fmt" + + "github.com/ipfs/go-cid" + mh "github.com/multiformats/go-multihash" +) + +var ErrPossiblyInsecureHashFunction = fmt.Errorf("potentially insecure hash functions not allowed") +var ErrBelowMinimumHashLength = fmt.Errorf("hashes must be at least %d bytes long", minimumHashLength) +var ErrAboveMaximumHashLength = fmt.Errorf("hashes must be at most %d bytes long", maximumHashLength) + +const minimumHashLength = 20 +const maximumHashLength = 128 + +// ValidateCid validates multihash allowance behind given CID. +func ValidateCid(allowlist Allowlist, c cid.Cid) error { + pref := c.Prefix() + if !allowlist.IsAllowed(pref.MhType) { + return ErrPossiblyInsecureHashFunction + } + + if pref.MhType != mh.IDENTITY && pref.MhLength < minimumHashLength { + return ErrBelowMinimumHashLength + } + + if pref.MhType != mh.IDENTITY && pref.MhLength > maximumHashLength { + return ErrAboveMaximumHashLength + } + + return nil +} diff --git a/verifcid/validate.go b/verifcid/validate.go deleted file mode 100644 index 7b27debc9..000000000 --- a/verifcid/validate.go +++ /dev/null @@ -1,69 +0,0 @@ -package verifcid - -import ( - "fmt" - - cid "github.com/ipfs/go-cid" - mh "github.com/multiformats/go-multihash" -) - -var ErrPossiblyInsecureHashFunction = fmt.Errorf("potentially insecure hash functions not allowed") -var ErrBelowMinimumHashLength = fmt.Errorf("hashes must be at least %d bytes long", minimumHashLength) -var ErrAboveMaximumHashLength = fmt.Errorf("hashes must be at most %d bytes long", maximumHashLength) - -const minimumHashLength = 20 -const maximumHashLength = 128 - -var goodset = map[uint64]bool{ - mh.SHA2_256: true, - mh.SHA2_512: true, - mh.SHA3_224: true, - mh.SHA3_256: true, - mh.SHA3_384: true, - mh.SHA3_512: true, - mh.SHAKE_256: true, - mh.DBL_SHA2_256: true, - mh.KECCAK_224: true, - mh.KECCAK_256: true, - mh.KECCAK_384: true, - mh.KECCAK_512: true, - mh.BLAKE3: true, - mh.IDENTITY: true, - - mh.SHA1: true, // not really secure but still useful -} - -func IsGoodHash(code uint64) bool { - good, found := goodset[code] - if good { - return true - } - - if !found { - if code >= mh.BLAKE2B_MIN+19 && code <= mh.BLAKE2B_MAX { - return true - } - if code >= mh.BLAKE2S_MIN+19 && code <= mh.BLAKE2S_MAX { - return true - } - } - - return false -} - -func ValidateCid(c cid.Cid) error { - pref := c.Prefix() - if !IsGoodHash(pref.MhType) { - return ErrPossiblyInsecureHashFunction - } - - if pref.MhType != mh.IDENTITY && pref.MhLength < minimumHashLength { - return ErrBelowMinimumHashLength - } - - if pref.MhType != mh.IDENTITY && pref.MhLength > maximumHashLength { - return ErrAboveMaximumHashLength - } - - return nil -}