From 429b913dc5519babfab47bf65222e32415a6c1bd Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sat, 12 Aug 2023 13:58:17 +0200 Subject: fix openpgp handling to sign/check --- entities/identity/key.go | 19 +++++------ entity/dag/common_test.go | 6 ++++ entity/dag/operation_pack.go | 55 +++++--------------------------- entity/dag/operation_pack_test.go | 66 ++++++++++++++++++++++----------------- 4 files changed, 59 insertions(+), 87 deletions(-) diff --git a/entities/identity/key.go b/entities/identity/key.go index 87271dd5..5ded31fa 100644 --- a/entities/identity/key.go +++ b/entities/identity/key.go @@ -217,18 +217,15 @@ func (k *Key) storePrivate(repo repository.RepoKeyring) error { } func (k *Key) PGPEntity() *openpgp.Entity { - uid := packet.NewUserId("", "", "") - return &openpgp.Entity{ + e := &openpgp.Entity{ PrimaryKey: k.public, PrivateKey: k.private, - Identities: map[string]*openpgp.Identity{ - uid.Id: { - Name: uid.Id, - UserId: uid, - SelfSignature: &packet.Signature{ - IsPrimaryId: func() *bool { b := true; return &b }(), - }, - }, - }, + Identities: map[string]*openpgp.Identity{}, + } + // somehow initialize the proper fields with identity, self-signature ... + err := e.AddUserId("name", "", "", nil) + if err != nil { + panic(err) } + return e } diff --git a/entity/dag/common_test.go b/entity/dag/common_test.go index 51acfa49..4c7b59f0 100644 --- a/entity/dag/common_test.go +++ b/entity/dag/common_test.go @@ -109,6 +109,12 @@ func makeTestContext() (repository.ClockedRepo, identity.Interface, identity.Int return repo, id1, id2, resolvers, def } +func makeTestContextGoGit(t *testing.T) (repository.ClockedRepo, identity.Interface, identity.Interface, entity.Resolvers, Definition) { + repo := repository.CreateGoGitTestRepo(t, false) + id1, id2, resolvers, def := makeTestContextInternal(repo) + return repo, id1, id2, resolvers, def +} + func makeTestContextRemote(t *testing.T) (repository.ClockedRepo, repository.ClockedRepo, repository.ClockedRepo, identity.Interface, identity.Interface, entity.Resolvers, Definition) { repoA := repository.CreateGoGitTestRepo(t, false) repoB := repository.CreateGoGitTestRepo(t, false) diff --git a/entity/dag/operation_pack.go b/entity/dag/operation_pack.go index c999ff23..3a871c12 100644 --- a/entity/dag/operation_pack.go +++ b/entity/dag/operation_pack.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/ProtonMail/go-crypto/openpgp" - "github.com/ProtonMail/go-crypto/openpgp/packet" "github.com/pkg/errors" "github.com/MichaelMure/git-bug/entities/identity" @@ -23,7 +22,7 @@ const createClockEntryPrefix = "create-clock-" const editClockEntryPrefix = "edit-clock-" // operationPack is a wrapper structure to store multiple operations in a single git blob. -// Additionally, it holds and store the metadata for those operations. +// Additionally, it holds and stores the metadata for those operations. type operationPack struct { // An identifier, taken from a hash of the serialized Operations. id entity.Id @@ -272,7 +271,12 @@ func readOperationPack(def Definition, repo repository.RepoData, resolvers entit // Verify signature if we expect one keys := author.ValidKeysAtTime(fmt.Sprintf(editClockPattern, def.Namespace), editTime) if len(keys) > 0 { - keyring := PGPKeyring(keys) + // this is a *very* convoluted and inefficient way to make OpenPGP accept to check a signature, but anything + // else goes against the grain and make it very unhappy. + keyring := openpgp.EntityList{} + for _, key := range keys { + keyring = append(keyring, key.PGPEntity()) + } _, err = openpgp.CheckDetachedSignature(keyring, commit.SignedData, commit.Signature, nil) if err != nil { return nil, fmt.Errorf("signature failure: %v", err) @@ -360,48 +364,3 @@ func unmarshallPack(def Definition, resolvers entity.Resolvers, data []byte) ([] return ops, author, nil } - -var _ openpgp.KeyRing = &PGPKeyring{} - -// PGPKeyring implement a openpgp.KeyRing from an slice of Key -type PGPKeyring []*identity.Key - -func (pk PGPKeyring) KeysById(id uint64) []openpgp.Key { - var result []openpgp.Key - for _, key := range pk { - if key.Public().KeyId == id { - result = append(result, openpgp.Key{ - PublicKey: key.Public(), - PrivateKey: key.Private(), - Entity: &openpgp.Entity{ - PrimaryKey: key.Public(), - PrivateKey: key.Private(), - Identities: map[string]*openpgp.Identity{ - "": {}, - }, - }, - SelfSignature: &packet.Signature{ - IsPrimaryId: func() *bool { b := true; return &b }(), - }, - }) - } - } - return result -} - -func (pk PGPKeyring) KeysByIdUsage(id uint64, requiredUsage byte) []openpgp.Key { - // the only usage we care about is the ability to sign, which all keys should already be capable of - return pk.KeysById(id) -} - -func (pk PGPKeyring) DecryptionKeys() []openpgp.Key { - // result := make([]openpgp.Key, len(pk)) - // for i, key := range pk { - // result[i] = openpgp.Key{ - // PublicKey: key.Public(), - // PrivateKey: key.Private(), - // } - // } - // return result - panic("not implemented") -} diff --git a/entity/dag/operation_pack_test.go b/entity/dag/operation_pack_test.go index bd8e8e03..09e60caf 100644 --- a/entity/dag/operation_pack_test.go +++ b/entity/dag/operation_pack_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" "github.com/MichaelMure/git-bug/entities/identity" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" ) @@ -40,37 +41,46 @@ func TestOperationPackReadWrite(t *testing.T) { } func TestOperationPackSignedReadWrite(t *testing.T) { - repo, author, _, resolver, def := makeTestContext() - - err := author.(*identity.Identity).Mutate(repo, func(orig *identity.Mutator) { - orig.Keys = append(orig.Keys, identity.GenerateKey()) - }) - require.NoError(t, err) + type makerFn func() (repository.ClockedRepo, identity.Interface, identity.Interface, entity.Resolvers, Definition) - opp := &operationPack{ - Author: author, - Operations: []Operation{ - newOp1(author, "foo"), - newOp2(author, "bar"), + for _, maker := range []makerFn{ + makeTestContext, + func() (repository.ClockedRepo, identity.Interface, identity.Interface, entity.Resolvers, Definition) { + return makeTestContextGoGit(t) }, - CreateTime: 123, - EditTime: 456, - } - - commitHash, err := opp.Write(def, repo) - require.NoError(t, err) - - commit, err := repo.ReadCommit(commitHash) - require.NoError(t, err) - - opp2, err := readOperationPack(def, repo, resolver, commit) - require.NoError(t, err) - - for _, op := range opp.Operations { - // force the creation of the id - op.Id() + } { + repo, author, _, resolver, def := maker() + + err := author.(*identity.Identity).Mutate(repo, func(orig *identity.Mutator) { + orig.Keys = append(orig.Keys, identity.GenerateKey()) + }) + require.NoError(t, err) + + opp := &operationPack{ + Author: author, + Operations: []Operation{ + newOp1(author, "foo"), + newOp2(author, "bar"), + }, + CreateTime: 123, + EditTime: 456, + } + + commitHash, err := opp.Write(def, repo) + require.NoError(t, err) + + commit, err := repo.ReadCommit(commitHash) + require.NoError(t, err) + + opp2, err := readOperationPack(def, repo, resolver, commit) + require.NoError(t, err) + + for _, op := range opp.Operations { + // force the creation of the id + op.Id() + } + require.Equal(t, opp, opp2) } - require.Equal(t, opp, opp2) } func TestOperationPackFiles(t *testing.T) { -- cgit