diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f1403511..5b5bcd655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The following emojis are used to highlight certain changes: ### Fixed - `gateway`: `X-Ipfs-Path` values no longer carry bytes that are invalid in an HTTP field value (Section 5.5 of RFC 9110). The header used to echo raw UnixFS file names, so non-ASCII paths arrived garbled or broke strict clients; when the header is enabled, it is now omitted for such paths, which only the percent-encoded `Ipfs-Uri` can carry. [#1209](https://github.com/ipfs/boxo/pull/1209) +- `bootstrap`: the saved backup peer list is no longer dialed when no bootstrap peers are configured. [#1213](https://github.com/ipfs/boxo/pull/1213) ### Security diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 9b7ed0b9b..51ce70778 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -299,11 +299,17 @@ func bootstrapRound(ctx context.Context, host host.Host, cfg BootstrapConfig) er // Retrieving them here makes sure we remain observant of changes to client configuration. peers := cfg.BootstrapPeers() - if len(peers) > 0 { - numToDial -= int(peersConnect(ctx, host, peers, numToDial, true)) - if numToDial <= 0 { - return nil - } + if len(peers) == 0 { + // The backup list is a fallback for when configured bootstrap peers + // are unreachable (ipfs/kubo#8856). With no configured peers there is + // nothing to fall back from. + log.Debugf("%s bootstrap skipped -- no bootstrap peers configured", id) + return nil + } + + numToDial -= int(peersConnect(ctx, host, peers, numToDial, true)) + if numToDial <= 0 { + return nil } if cfg.loadBackupBootstrapPeers == nil { diff --git a/bootstrap/bootstrap_test.go b/bootstrap/bootstrap_test.go index c0b71e5ee..411a312e5 100644 --- a/bootstrap/bootstrap_test.go +++ b/bootstrap/bootstrap_test.go @@ -254,3 +254,77 @@ func TestHasCircuitProtocol(t *testing.T) { }) } } + +// TestBootstrapRoundSkipsBackupWhenNoBootstrapPeers verifies that bootstrapRound +// does not consult the backup peer list when no bootstrap peers are configured. +// The backup list exists only as a recovery mechanism for when configured +// bootstrap peers are down; with no configured peers there is nothing to +// recover from, so dialing stale backup peers from previous runs would be +// unwanted network traffic. See ipfs/kubo#11452. +func TestBootstrapRoundSkipsBackupWhenNoBootstrapPeers(t *testing.T) { + backupCalled := false + loadFunc := func(_ context.Context) []peer.AddrInfo { + backupCalled = true + return nil + } + saveFunc := func(_ context.Context, _ []peer.AddrInfo) {} + + bootCfg := BootstrapConfigWithPeers(nil, WithBackupPeers(loadFunc, saveFunc)) + bootCfg.MinPeerThreshold = 2 + + priv, _, err := crypto.GenerateEd25519Key(rand.Reader) + if err != nil { + t.Fatal(err) + } + p2pHost, err := libp2p.New(libp2p.Identity(priv)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = p2pHost.Close() }) + + if err := bootstrapRound(t.Context(), p2pHost, bootCfg); err != nil { + t.Fatalf("bootstrapRound returned error: %v", err) + } + if backupCalled { + t.Fatal("bootstrapRound consulted the backup peer list despite no bootstrap peers being configured") + } +} + +// TestBootstrapRoundDialsBackupWhenBootstrapPeersPresent confirms the backup +// list is still consulted when configured bootstrap peers fail to connect, +// preserving the recovery mechanism from ipfs/kubo#8856. +func TestBootstrapRoundDialsBackupWhenBootstrapPeersPresent(t *testing.T) { + backupCalled := false + loadFunc := func(_ context.Context) []peer.AddrInfo { + backupCalled = true + return nil + } + saveFunc := func(_ context.Context, _ []peer.AddrInfo) {} + + // A fake, undialable bootstrap peer so peersConnect attempts and fails + // rather than short-circuiting on an empty list. + fakeID, err := test.RandPeerID() + if err != nil { + t.Fatal(err) + } + fakePeer := peer.AddrInfo{ID: fakeID} + bootCfg := BootstrapConfigWithPeers([]peer.AddrInfo{fakePeer}, WithBackupPeers(loadFunc, saveFunc)) + bootCfg.MinPeerThreshold = 2 + // Keep the round snappy: the dial will fail fast against a random peer ID. + bootCfg.ConnectionTimeout = 500 * time.Millisecond + + priv, _, err := crypto.GenerateEd25519Key(rand.Reader) + if err != nil { + t.Fatal(err) + } + p2pHost, err := libp2p.New(libp2p.Identity(priv)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = p2pHost.Close() }) + + _ = bootstrapRound(t.Context(), p2pHost, bootCfg) + if !backupCalled { + t.Fatal("bootstrapRound did not consult the backup peer list despite configured bootstrap peers failing to connect") + } +}