From f165191cbd23515fb3b1be6ab92fbc4897f43732 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Tue, 13 Jun 2023 11:51:43 +0200 Subject: [PATCH] feat: replace EmbedPublicKey by option --- ipns/record.go | 54 +++--- ipns/record_test.go | 49 +++--- ipns/validation_test.go | 21 +-- namesys/ipns_resolver_validation_test.go | 208 ----------------------- namesys/publisher.go | 6 +- 5 files changed, 59 insertions(+), 279 deletions(-) delete mode 100644 namesys/ipns_resolver_validation_test.go diff --git a/ipns/record.go b/ipns/record.go index 275700d0a..0a31f302e 100644 --- a/ipns/record.go +++ b/ipns/record.go @@ -17,6 +17,7 @@ import ( "github.com/ipld/go-ipld-prime/datamodel" basicnode "github.com/ipld/go-ipld-prime/node/basic" ic "github.com/libp2p/go-libp2p/core/crypto" + ic_pb "github.com/libp2p/go-libp2p/core/crypto/pb" "github.com/libp2p/go-libp2p/core/peer" "github.com/multiformats/go-multibase" "go.uber.org/multierr" @@ -194,14 +195,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 } } @@ -243,7 +251,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 +271,17 @@ func NewRecord(sk ic.PrivKey, value path.Path, seq uint64, eol time.Time, ttl ti pb.SignatureV1 = sig1 } + // By default, embed public key if it's not a Ed25519 key. Otherwise, only if + // the user has explicitly asked for it to be embedded. + if (options.embedPublicKey == nil && sk.Type() != ic_pb.KeyType_Ed25519) || + (options.embedPublicKey != nil && *options.embedPublicKey) { + pkBytes, err := ic.MarshalPublicKey(sk.GetPublic()) + if err != nil { + return nil, err + } + pb.PubKey = pkBytes + } + return &Record{ pb: pb, node: node, @@ -396,31 +415,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..afa4ee527 100644 --- a/ipns/record_test.go +++ b/ipns/record_test.go @@ -107,7 +107,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 +116,31 @@ 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 non-ed25519 keys", func(t *testing.T) { + t.Parallel() - _, err := rec.PubKey() - require.ErrorIs(t, err, ErrPublicKeyNotFound) + for _, keyType := range []int{ic.RSA, ic.Secp256k1, 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 keys", func(t *testing.T) { + t.Parallel() - recPID, err := peer.IDFromPublicKey(recPK) - require.NoError(t, err) + sk, _, _ := mustKeyPair(t, ic.Ed25519) + 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 +148,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 +166,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) 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