diff --git a/CHANGELOG.md b/CHANGELOG.md index c7bf962..bd990b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ The following emojis are used to highlight certain changes: ### Fixed +- `GetIPNS` no longer returns an IPNS record whose EOL has already passed. An expired record is cryptographically invalid, so it is treated as not found, and when multiple routers answer the first non-expired record is returned. + ### Security ## [v0.13.0] - 2026-05-26 diff --git a/server_routers.go b/server_routers.go index ce7806f..04f0b8a 100644 --- a/server_routers.go +++ b/server_routers.go @@ -244,12 +244,48 @@ func (mi *manyIter[T]) Close() error { return err } +// isExpiredIPNSRecord reports whether an EOL-type IPNS record has passed its +// validity. A record past its EOL is cryptographically invalid, so returning it +// only hands the caller a record that fails validation. Records with a non-EOL +// validity type or an unreadable validity are treated as not expired. +// +// Every backend already validates the record it returns, so this is a +// re-check rather than the first line of defense. We still do it because a +// signature is immutable and verified once, but EOL is time-varying: the +// longer a record lingers (a cache hit, a stored copy, clock drift between +// our infrastructure and the producer), the more likely it has expired +// between the backend's check and the moment we hand it to the user. +// parallelRouter is first-result-wins with no revalidation, so without this +// the aggregator could let an expired record win the race. See +// https://github.com/ipfs/boxo/pull/1166 for the upstream cache-control fix. +func isExpiredIPNSRecord(rec *ipns.Record) bool { + if rec == nil { + return false + } + validityType, err := rec.ValidityType() + if err != nil || validityType != ipns.ValidityEOL { + return false + } + validity, err := rec.Validity() + if err != nil { + return false + } + return time.Now().After(validity) +} + func (r parallelRouter) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, error) { switch len(r.routers) { case 0: return nil, routing.ErrNotFound case 1: - return r.routers[0].GetIPNS(ctx, name) + rec, err := r.routers[0].GetIPNS(ctx, name) + if err != nil { + return nil, err + } + if isExpiredIPNSRecord(rec) { + return nil, routing.ErrNotFound + } + return rec, nil } ctx, cancel := context.WithCancel(ctx) @@ -282,6 +318,11 @@ func (r parallelRouter) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Reco case res := <-results: switch res.err { case nil: + // An expired record is invalid and must not be served; treat it + // as not found and keep waiting for another router to answer. + if isExpiredIPNSRecord(res.val) { + continue + } return res.val, nil case routing.ErrNotFound, routing.ErrNotSupported: continue diff --git a/server_routers_test.go b/server_routers_test.go index 3523ab8..d51a5bb 100644 --- a/server_routers_test.go +++ b/server_routers_test.go @@ -89,6 +89,17 @@ func makeIPNSRecord(t *testing.T, sk crypto.PrivKey, opts ...ipns.Option) (*ipns return record, rawRecord } +func makeExpiredIPNSRecord(t *testing.T, sk crypto.PrivKey) *ipns.Record { + cid, err := cid.Decode("bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4") + require.NoError(t, err) + + // EOL in the past so the record is already expired + record, err := ipns.NewRecord(sk, path.FromCid(cid), 1, time.Now().Add(-time.Hour), time.Second*20) + require.NoError(t, err) + + return record +} + func TestGetIPNS(t *testing.T) { t.Parallel() @@ -158,6 +169,47 @@ func TestGetIPNS(t *testing.T) { _, err := r.GetIPNS(ctx, name) require.ErrorIs(t, err, routing.ErrNotFound) }) + + t.Run("Expired Record Treated As Not Found", func(t *testing.T) { + ctx := context.Background() + + expired := makeExpiredIPNSRecord(t, sk) + + mr1 := &mockRouter{} + mr1.On("GetIPNS", mock.Anything, name).Return(expired, nil) + + r := parallelRouter{ + routers: []router{ + composableRouter{ipns: mr1}, + }, + } + + _, err := r.GetIPNS(ctx, name) + require.ErrorIs(t, err, routing.ErrNotFound) + }) + + t.Run("Skips Expired Record For Valid One", func(t *testing.T) { + ctx := context.Background() + + expired := makeExpiredIPNSRecord(t, sk) + + mr1 := &mockRouter{} + mr1.On("GetIPNS", mock.Anything, name).Return(expired, nil) + + mr2 := &mockRouter{} + mr2.On("GetIPNS", mock.Anything, name).Return(rec, nil) + + r := parallelRouter{ + routers: []router{ + composableRouter{ipns: mr1}, + composableRouter{ipns: mr2}, + }, + } + + getRec, err := r.GetIPNS(ctx, name) + require.NoError(t, err) + require.EqualValues(t, rec, getRec) + }) } func TestPutIPNS(t *testing.T) {