diff --git a/ipns/record.go b/ipns/record.go index 275700d0a..474db7d32 100644 --- a/ipns/record.go +++ b/ipns/record.go @@ -194,14 +194,21 @@ const ( ) type options struct { - compatibleWithV1 bool + v1Compatibility bool + embedPublicKey *bool } type Option func(*options) -func CompatibleWithV1(compatible bool) Option { - return func(opts *options) { - opts.compatibleWithV1 = compatible +func WithV1Compatibility(compatible bool) Option { + return func(o *options) { + o.v1Compatibility = compatible + } +} + +func WithPublicKey(embedded bool) Option { + return func(o *options) { + o.embedPublicKey = &embedded } } @@ -214,7 +221,9 @@ func processOptions(opts ...Option) *options { } // NewRecord creates a new IPNS [Record] and signs it with the given private key. -// This function does not embed the public key. To do so, call [EmbedPublicKey]. +// By default, we embed the public key for key types whose peer IDs do not encode +// the public key, such as RSA and ECDSA key types. This can be changed with the +// option [WithPublicKey]. func NewRecord(sk ic.PrivKey, value path.Path, seq uint64, eol time.Time, ttl time.Duration, opts ...Option) (*Record, error) { options := processOptions(opts...) @@ -243,7 +252,7 @@ func NewRecord(sk ic.PrivKey, value path.Path, seq uint64, eol time.Time, ttl ti SignatureV2: sig2, } - if options.compatibleWithV1 { + if options.v1Compatibility { pb.Value = []byte(value) typ := ipns_pb.IpnsEntry_EOL pb.ValidityType = &typ @@ -263,6 +272,24 @@ func NewRecord(sk ic.PrivKey, value path.Path, seq uint64, eol time.Time, ttl ti pb.SignatureV1 = sig1 } + embedPublicKey := false + if options.embedPublicKey == nil { + embedPublicKey, err = needToEmbedPublicKey(sk.GetPublic()) + if err != nil { + return nil, err + } + } else { + embedPublicKey = *options.embedPublicKey + } + + if embedPublicKey { + pkBytes, err := ic.MarshalPublicKey(sk.GetPublic()) + if err != nil { + return nil, err + } + pb.PubKey = pkBytes + } + return &Record{ pb: pb, node: node, @@ -342,6 +369,26 @@ func recordDataForSignatureV2(data []byte) ([]byte, error) { return dataForSig, nil } +func needToEmbedPublicKey(pk ic.PubKey) (bool, error) { + // First try extracting the peer ID from the public key. + pid, err := peer.IDFromPublicKey(pk) + if err != nil { + return false, fmt.Errorf("cannot convert public key to peer ID: %w", err) + } + + _, err = pid.ExtractPublicKey() + if err == nil { + // Can be extracted, therefore no need to embed the public key. + return false, nil + } + + if errors.Is(err, peer.ErrNoPublicKey) { + return true, nil + } + + return false, fmt.Errorf("cannot extract ID from public key: %w", err) +} + // compare compares two IPNS Records. It returns: // // - -1 if a is older than b @@ -396,31 +443,6 @@ func compare(a, b *Record) (int, error) { return 0, nil } -// EmbedPublicKey embeds the given public key in the given [Record]. While not -// strictly required, some nodes (e.g., DHT servers), may reject IPNS Records -// that do not embed their public keys as they may not be able to validate them -// efficiently. -func EmbedPublicKey(r *Record, pk ic.PubKey) error { - // Try extracting the public key from the ID. If we can, do not embed it. - pid, err := peer.IDFromPublicKey(pk) - if err != nil { - return err - } - if _, err := pid.ExtractPublicKey(); err != peer.ErrNoPublicKey { - // Either a *real* error or nil. - return err - } - - // We failed to extract the public key from the peer ID, embed it. - pkBytes, err := ic.MarshalPublicKey(pk) - if err != nil { - return err - } - - r.pb.PubKey = pkBytes - return nil -} - // ExtractPublicKey extracts a [crypto.PubKey] matching the given [peer.ID] from // the IPNS Record, if possible. func ExtractPublicKey(r *Record, pid peer.ID) (ic.PubKey, error) { diff --git a/ipns/record_test.go b/ipns/record_test.go index 0eced4595..493487a08 100644 --- a/ipns/record_test.go +++ b/ipns/record_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/gogo/protobuf/proto" ipns_pb "github.com/ipfs/boxo/ipns/pb" "github.com/ipfs/boxo/path" "github.com/ipfs/boxo/util" @@ -107,7 +108,7 @@ func TestNewRecord(t *testing.T) { t.Run("V1+V2 with option", func(t *testing.T) { t.Parallel() - rec := mustNewRecord(t, sk, testPath, seq, eol, ttl, CompatibleWithV1(true)) + rec := mustNewRecord(t, sk, testPath, seq, eol, ttl, WithV1Compatibility(true)) require.NotEmpty(t, rec.pb.SignatureV1) _, err := rec.PubKey() @@ -116,32 +117,33 @@ func TestNewRecord(t *testing.T) { fieldsMatch(t, rec, testPath, seq, eol, ttl) fieldsMatchV1(t, rec, testPath, seq, eol, ttl) }) -} - -func TestEmbedPublicKey(t *testing.T) { - t.Parallel() - - sk, pk, pid := mustKeyPair(t, ic.RSA) - - seq := uint64(0) - eol := time.Now().Add(time.Hour) - ttl := time.Minute * 10 - rec := mustNewRecord(t, sk, testPath, seq, eol, ttl) + t.Run("Public key embedded by default for RSA and ECDSA keys", func(t *testing.T) { + t.Parallel() - _, err := rec.PubKey() - require.ErrorIs(t, err, ErrPublicKeyNotFound) + for _, keyType := range []int{ic.RSA, ic.ECDSA} { + sk, _, _ := mustKeyPair(t, keyType) + rec := mustNewRecord(t, sk, testPath, seq, eol, ttl) + fieldsMatch(t, rec, testPath, seq, eol, ttl) - err = EmbedPublicKey(rec, pk) - require.NoError(t, err) + pk, err := rec.PubKey() + require.NoError(t, err) + require.True(t, pk.Equals(sk.GetPublic())) + } + }) - recPK, err := rec.PubKey() - require.NoError(t, err) + t.Run("Public key not embedded by default for Ed25519 and Secp256k1 keys", func(t *testing.T) { + t.Parallel() - recPID, err := peer.IDFromPublicKey(recPK) - require.NoError(t, err) + for _, keyType := range []int{ic.Ed25519, ic.Secp256k1} { + sk, _, _ := mustKeyPair(t, keyType) + rec := mustNewRecord(t, sk, testPath, seq, eol, ttl) + fieldsMatch(t, rec, testPath, seq, eol, ttl) - require.Equal(t, pid, recPID) + _, err := rec.PubKey() + require.ErrorIs(t, err, ErrPublicKeyNotFound) + } + }) } func TestExtractPublicKey(t *testing.T) { @@ -149,18 +151,16 @@ func TestExtractPublicKey(t *testing.T) { t.Run("Returns expected public key when embedded in Peer ID", func(t *testing.T) { sk, pk, pid := mustKeyPair(t, ic.Ed25519) - rec := mustNewRecord(t, sk, testPath, 0, time.Now().Add(time.Hour), time.Minute*10) + rec := mustNewRecord(t, sk, testPath, 0, time.Now().Add(time.Hour), time.Minute*10, WithPublicKey(false)) pk2, err := ExtractPublicKey(rec, pid) require.Nil(t, err) require.Equal(t, pk, pk2) }) - t.Run("Returns expected public key when embedded in Record", func(t *testing.T) { + t.Run("Returns expected public key when embedded in Record (by default)", func(t *testing.T) { sk, pk, pid := mustKeyPair(t, ic.RSA) rec := mustNewRecord(t, sk, testPath, 0, time.Now().Add(time.Hour), time.Minute*10) - err := EmbedPublicKey(rec, pk) - require.Nil(t, err) pk2, err := ExtractPublicKey(rec, pid) require.Nil(t, err) @@ -169,7 +169,7 @@ func TestExtractPublicKey(t *testing.T) { t.Run("Errors when not embedded in Record or Peer ID", func(t *testing.T) { sk, _, pid := mustKeyPair(t, ic.RSA) - rec := mustNewRecord(t, sk, testPath, 0, time.Now().Add(time.Hour), time.Minute*10) + rec := mustNewRecord(t, sk, testPath, 0, time.Now().Add(time.Hour), time.Minute*10, WithPublicKey(false)) pk, err := ExtractPublicKey(rec, pid) require.Error(t, err) @@ -248,3 +248,52 @@ func TestCBORDataSerialization(t *testing.T) { assert.Equal(t, expected, f) } } + +func TestUnmarshal(t *testing.T) { + t.Parallel() + + t.Run("Errors on invalid bytes", func(t *testing.T) { + _, err := UnmarshalRecord([]byte("blah blah blah")) + require.ErrorIs(t, err, ErrBadRecord) + }) + + t.Run("Errors if record is too long", func(t *testing.T) { + data := make([]byte, MaxRecordSize+1) + _, err := UnmarshalRecord(data) + require.ErrorIs(t, err, ErrRecordSize) + }) + + t.Run("Errors with V1-only records", func(t *testing.T) { + pb := ipns_pb.IpnsEntry{} + data, err := proto.Marshal(&pb) + require.NoError(t, err) + _, err = UnmarshalRecord(data) + require.ErrorIs(t, err, ErrDataMissing) + }) + + t.Run("Errors on bad data", func(t *testing.T) { + pb := ipns_pb.IpnsEntry{ + Data: []byte("definitely not cbor"), + } + data, err := proto.Marshal(&pb) + require.NoError(t, err) + _, err = UnmarshalRecord(data) + require.ErrorIs(t, err, ErrBadRecord) + }) +} + +func TestKey(t *testing.T) { + for _, v := range [][]string{ + {"RSA", "QmRp2LvtSQtCkUWCpi92ph5MdQyRtfb9jHbkNgZzGExGuG", "/ipns/k2k4r8kpauqq30hoj9oktej5btbgz1jeos16d3te36xd78trvak0jcor"}, + {"Ed25519", "12D3KooWSzRuSFHgLsKr6jJboAPdP7xMga2YBgBspYuErxswcgvt", "/ipns/k51qzi5uqu5dmjjgoe7s21dncepi970722cn30qlhm9qridas1c9ktkjb6ejux"}, + {"ECDSA", "QmSBUTocZ9LxE53Br9PDDcPWnR1FJQRv94U96Wkt8eypAw", "/ipns/k2k4r8ku8cnc1sl2h5xn7i07dma9abfnkqkxi4a6nd1xq0knoxe7b0y4"}, + {"Secp256k1", "16Uiu2HAmUymv6JpFwNZppdKUMxGJuHsTeicXgHGKbBasu4Ruj3K1", "/ipns/kzwfwjn5ji4puw3jc1qw4b073j74xvq21iziuqw4rem21pr7f0l4dj8i9yb978s"}, + } { + t.Run(v[0], func(t *testing.T) { + pid, err := peer.Decode(v[1]) + require.NoError(t, err) + key := Key(pid) + require.Equal(t, v[2], key) + }) + } +} diff --git a/ipns/validation_test.go b/ipns/validation_test.go index 3aa83dfcb..875116e1d 100644 --- a/ipns/validation_test.go +++ b/ipns/validation_test.go @@ -70,12 +70,12 @@ func TestOrdering(t *testing.T) { func TestValidator(t *testing.T) { t.Parallel() - check := func(t *testing.T, sk ic.PrivKey, keybook peerstore.KeyBook, key string, val []byte, eol time.Time, exp error) { + check := func(t *testing.T, sk ic.PrivKey, keybook peerstore.KeyBook, key string, val []byte, eol time.Time, exp error, opts ...Option) { validator := Validator{keybook} data := val if data == nil { // do not call mustNewRecord because that validates the record! - rec, err := NewRecord(sk, testPath, 1, eol, 0) + rec, err := NewRecord(sk, testPath, 1, eol, 0, opts...) require.NoError(t, err) data = mustMarshal(t, rec) } @@ -99,9 +99,10 @@ func TestValidator(t *testing.T) { check(t, sk, kb, RoutingKey(pid), nil, ts.Add(time.Hour*-1), ErrExpiredRecord) check(t, sk, kb, RoutingKey(pid), []byte("bad data"), ts.Add(time.Hour), ErrBadRecord) check(t, sk, kb, "/ipns/"+"bad key", nil, ts.Add(time.Hour), ErrKeyFormat) - check(t, sk, emptyKB, RoutingKey(pid), nil, ts.Add(time.Hour), ErrPublicKeyNotFound) - check(t, sk2, kb, RoutingKey(pid2), nil, ts.Add(time.Hour), ErrPublicKeyNotFound) - check(t, sk2, kb, RoutingKey(pid), nil, ts.Add(time.Hour), ErrSignature) + check(t, sk, emptyKB, RoutingKey(pid), nil, ts.Add(time.Hour), ErrPublicKeyNotFound, WithPublicKey(false)) + check(t, sk2, kb, RoutingKey(pid2), nil, ts.Add(time.Hour), ErrPublicKeyNotFound, WithPublicKey(false)) + check(t, sk2, kb, RoutingKey(pid), nil, ts.Add(time.Hour), ErrPublicKeyMismatch) + check(t, sk2, kb, RoutingKey(pid), nil, ts.Add(time.Hour), ErrSignature, WithPublicKey(false)) check(t, sk, kb, "//"+string(pid), nil, ts.Add(time.Hour), ErrInvalidPath) check(t, sk, kb, "/wrong/"+string(pid), nil, ts.Add(time.Hour), ErrInvalidPath) }) @@ -128,14 +129,14 @@ func TestValidator(t *testing.T) { kb, err := pstoremem.NewPeerstore() require.NoError(t, err) - sk, pk, pid := mustKeyPair(t, ic.RSA) - rec := mustNewRecord(t, sk, testPath, 1, eol, 0) + sk, _, pid := mustKeyPair(t, ic.RSA) + rec := mustNewRecord(t, sk, testPath, 1, eol, 0, WithPublicKey(false)) // Fails with RSA key without embedded public key. check(t, sk, kb, RoutingKey(pid), mustMarshal(t, rec), eol, ErrPublicKeyNotFound) // Embeds public key, must work now. - require.NoError(t, EmbedPublicKey(rec, pk)) + rec = mustNewRecord(t, sk, testPath, 1, eol, 0) check(t, sk, kb, RoutingKey(pid), mustMarshal(t, rec), eol, nil) // Force bad public key. Validation fails. @@ -163,8 +164,8 @@ func TestValidate(t *testing.T) { v := Validator{} - rec1 := mustNewRecord(t, sk, path.FromString("/path/1"), 1, eol, 0, CompatibleWithV1(true)) - rec2 := mustNewRecord(t, sk, path.FromString("/path/2"), 2, eol, 0, CompatibleWithV1(true)) + rec1 := mustNewRecord(t, sk, path.FromString("/path/1"), 1, eol, 0, WithV1Compatibility(true)) + rec2 := mustNewRecord(t, sk, path.FromString("/path/2"), 2, eol, 0, WithV1Compatibility(true)) best, err := v.Select(ipnsRoutingKey, [][]byte{mustMarshal(t, rec1), mustMarshal(t, rec2)}) require.NoError(t, err) diff --git a/namesys/ipns_resolver_validation_test.go b/namesys/ipns_resolver_validation_test.go deleted file mode 100644 index ac8143b37..000000000 --- a/namesys/ipns_resolver_validation_test.go +++ /dev/null @@ -1,208 +0,0 @@ -package namesys - -import ( - "context" - "testing" - "time" - - opts "github.com/ipfs/boxo/coreiface/options/namesys" - "github.com/ipfs/boxo/ipns" - "github.com/ipfs/boxo/path" - mockrouting "github.com/ipfs/boxo/routing/mock" - "github.com/ipfs/boxo/routing/offline" - ds "github.com/ipfs/go-datastore" - dssync "github.com/ipfs/go-datastore/sync" - record "github.com/libp2p/go-libp2p-record" - testutil "github.com/libp2p/go-libp2p-testing/net" - ci "github.com/libp2p/go-libp2p/core/crypto" - "github.com/libp2p/go-libp2p/core/peer" - pstore "github.com/libp2p/go-libp2p/core/peerstore" - "github.com/libp2p/go-libp2p/core/routing" - "github.com/libp2p/go-libp2p/core/test" - "github.com/libp2p/go-libp2p/p2p/host/peerstore/pstoremem" -) - -func TestResolverValidation(t *testing.T) { - t.Run("RSA", - func(t *testing.T) { - testResolverValidation(t, ci.RSA) - }) - t.Run("Ed25519", - func(t *testing.T) { - testResolverValidation(t, ci.Ed25519) - }) - t.Run("ECDSA", - func(t *testing.T) { - testResolverValidation(t, ci.ECDSA) - }) - t.Run("Secp256k1", - func(t *testing.T) { - testResolverValidation(t, ci.Secp256k1) - }) -} - -func testResolverValidation(t *testing.T, keyType int) { - ctx := context.Background() - rid := testutil.RandIdentityOrFatal(t) - dstore := dssync.MutexWrap(ds.NewMapDatastore()) - peerstore, err := pstoremem.NewPeerstore() - if err != nil { - t.Fatal(err) - } - - vstore := newMockValueStore(rid, dstore, peerstore) - resolver := NewIpnsResolver(vstore) - - nvVstore := offline.NewOfflineRouter(dstore, mockrouting.MockValidator{}) - - // Create entry with expiry in one hour - priv, id, _, ipnsDHTPath := genKeys(t, keyType) - ts := time.Now() - p := path.Path("/ipfs/QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG") - rec, err := createIPNSRecordWithEmbeddedPublicKey(priv, p, 1, ts.Add(time.Hour), 0) - if err != nil { - t.Fatal(err) - } - - // Publish entry - err = PublishEntry(ctx, vstore, ipnsDHTPath, rec) - if err != nil { - t.Fatal(err) - } - - // Resolve entry - resp, err := resolve(ctx, resolver, id.Pretty(), opts.DefaultResolveOpts()) - if err != nil { - t.Fatal(err) - } - if resp != path.Path(p) { - t.Fatalf("Mismatch between published path %s and resolved path %s", p, resp) - } - // Create expired entry - expiredEntry, err := createIPNSRecordWithEmbeddedPublicKey(priv, p, 1, ts.Add(-1*time.Hour), 0) - if err != nil { - t.Fatal(err) - } - - // Publish entry - err = PublishEntry(ctx, nvVstore, ipnsDHTPath, expiredEntry) - if err != nil { - t.Fatal(err) - } - - // Record should fail validation because entry is expired - _, err = resolve(ctx, resolver, id.Pretty(), opts.DefaultResolveOpts()) - if err == nil { - t.Fatal("ValidateIpnsRecord should have returned error") - } - - // Create IPNS record path with a different private key - priv2, id2, _, ipnsDHTPath2 := genKeys(t, keyType) - - // Publish entry - err = PublishEntry(ctx, nvVstore, ipnsDHTPath2, rec) - if err != nil { - t.Fatal(err) - } - - // Record should fail validation because public key defined by - // ipns path doesn't match record signature - _, err = resolve(ctx, resolver, id2.Pretty(), opts.DefaultResolveOpts()) - if err == nil { - t.Fatal("ValidateIpnsRecord should have failed signature verification") - } - - // Try embedding the incorrect private key inside the entry - if err := ipns.EmbedPublicKey(rec, priv2.GetPublic()); err != nil { - t.Fatal(err) - } - - // Publish entry - err = PublishEntry(ctx, nvVstore, ipnsDHTPath2, rec) - if err != nil { - t.Fatal(err) - } - - // Record should fail validation because public key defined by - // ipns path doesn't match record signature - _, err = resolve(ctx, resolver, id2.Pretty(), opts.DefaultResolveOpts()) - if err == nil { - t.Fatal("ValidateIpnsRecord should have failed signature verification") - } -} - -func genKeys(t *testing.T, keyType int) (ci.PrivKey, peer.ID, string, string) { - bits := 0 - if keyType == ci.RSA { - bits = 2048 - } - - sk, pk, err := test.RandTestKeyPair(keyType, bits) - if err != nil { - t.Fatal(err) - } - id, err := peer.IDFromPublicKey(pk) - if err != nil { - t.Fatal(err) - } - return sk, id, PkKeyForID(id), ipns.RoutingKey(id) -} - -func createIPNSRecordWithEmbeddedPublicKey(sk ci.PrivKey, val path.Path, seq uint64, eol time.Time, ttl time.Duration) (*ipns.Record, error) { - rec, err := ipns.NewRecord(sk, val, seq, eol, ttl) - if err != nil { - return nil, err - } - if err := ipns.EmbedPublicKey(rec, sk.GetPublic()); err != nil { - return nil, err - } - - return rec, nil -} - -type mockValueStore struct { - r routing.ValueStore - kbook pstore.KeyBook -} - -func newMockValueStore(id testutil.Identity, dstore ds.Datastore, kbook pstore.KeyBook) *mockValueStore { - return &mockValueStore{ - r: offline.NewOfflineRouter(dstore, record.NamespacedValidator{ - "ipns": ipns.Validator{KeyBook: kbook}, - "pk": record.PublicKeyValidator{}, - }), - kbook: kbook, - } -} - -func (m *mockValueStore) GetValue(ctx context.Context, k string, opts ...routing.Option) ([]byte, error) { - return m.r.GetValue(ctx, k, opts...) -} - -func (m *mockValueStore) SearchValue(ctx context.Context, k string, opts ...routing.Option) (<-chan []byte, error) { - return m.r.SearchValue(ctx, k, opts...) -} - -func (m *mockValueStore) GetPublicKey(ctx context.Context, p peer.ID) (ci.PubKey, error) { - pk := m.kbook.PubKey(p) - if pk != nil { - return pk, nil - } - - pkkey := routing.KeyForPublicKey(p) - val, err := m.GetValue(ctx, pkkey) - if err != nil { - return nil, err - } - - pk, err = ci.UnmarshalPublicKey(val) - if err != nil { - return nil, err - } - - return pk, m.kbook.AddPubKey(p, pk) -} - -func (m *mockValueStore) PutValue(ctx context.Context, k string, d []byte, opts ...routing.Option) error { - return m.r.PutValue(ctx, k, d, opts...) -} diff --git a/namesys/publisher.go b/namesys/publisher.go index fef38390e..1db9f16a8 100644 --- a/namesys/publisher.go +++ b/namesys/publisher.go @@ -178,7 +178,7 @@ func (p *IpnsPublisher) updateRecord(ctx context.Context, k crypto.PrivKey, valu opts := opts.ProcessPublishOptions(options) // Create record - r, err := ipns.NewRecord(k, value, seqno, opts.EOL, opts.TTL, ipns.CompatibleWithV1(opts.CompatibleWithV1)) + r, err := ipns.NewRecord(k, value, seqno, opts.EOL, opts.TTL, ipns.WithV1Compatibility(opts.CompatibleWithV1)) if err != nil { return nil, err } @@ -211,10 +211,6 @@ func PutRecordToRouting(ctx context.Context, r routing.ValueStore, k crypto.PubK errs := make(chan error, 2) // At most two errors (IPNS, and public key) - if err := ipns.EmbedPublicKey(rec, k); err != nil { - return err - } - id, err := peer.IDFromPublicKey(k) if err != nil { return err