entity: working commit signatures

Michael Muré created 4 years ago

Change summary

entity/dag/operation_pack_test.go | 44 +++++++++++++++++++++++++++++
repository/gogit.go               | 16 ++++++---
repository/mock_repo.go           |  2 +
repository/repo_testing.go        | 50 ++++++++++++++++++++++++++++++++
4 files changed, 105 insertions(+), 7 deletions(-)

Detailed changes

entity/dag/operation_pack_test.go

@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
+	"github.com/MichaelMure/git-bug/identity"
 )
 
 func TestOperationPackReadWrite(t *testing.T) {
@@ -42,3 +44,45 @@ func TestOperationPackReadWrite(t *testing.T) {
 	}
 	require.Equal(t, opp.Id(), opp3.Id())
 }
+
+func TestOperationPackSignedReadWrite(t *testing.T) {
+	repo, id1, _, def := makeTestContext()
+
+	err := id1.(*identity.Identity).Mutate(repo, func(orig *identity.Mutator) {
+		orig.Keys = append(orig.Keys, identity.GenerateKey())
+	})
+	require.NoError(t, err)
+
+	opp := &operationPack{
+		Author: id1,
+		Operations: []Operation{
+			newOp1(id1, "foo"),
+			newOp2(id1, "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, commit)
+	require.NoError(t, err)
+
+	require.Equal(t, opp, opp2)
+
+	// make sure we get the same Id with the same data
+	opp3 := &operationPack{
+		Author: id1,
+		Operations: []Operation{
+			newOp1(id1, "foo"),
+			newOp2(id1, "bar"),
+		},
+		CreateTime: 123,
+		EditTime:   456,
+	}
+	require.Equal(t, opp.Id(), opp3.Id())
+}

repository/gogit.go

@@ -715,12 +715,7 @@ func (repo *GoGitRepo) ListCommits(ref string) ([]Hash, error) {
 }
 
 func (repo *GoGitRepo) ReadCommit(hash Hash) (Commit, error) {
-	encoded, err := repo.r.Storer.EncodedObject(plumbing.CommitObject, plumbing.NewHash(hash.String()))
-	if err != nil {
-		return Commit{}, err
-	}
-
-	commit, err := object.DecodeCommit(repo.r.Storer, encoded)
+	commit, err := repo.r.CommitObject(plumbing.NewHash(hash.String()))
 	if err != nil {
 		return Commit{}, err
 	}
@@ -737,6 +732,15 @@ func (repo *GoGitRepo) ReadCommit(hash Hash) (Commit, error) {
 	}
 
 	if commit.PGPSignature != "" {
+		// I can't find a way to just remove the signature when reading the encoded commit so we need to
+		// re-encode the commit without signature.
+
+		encoded := &plumbing.MemoryObject{}
+		err := commit.EncodeWithoutSignature(encoded)
+		if err != nil {
+			return Commit{}, err
+		}
+
 		result.SignedData, err = encoded.Reader()
 		if err != nil {
 			return Commit{}, err

repository/mock_repo.go

@@ -299,6 +299,8 @@ func (r *mockRepoData) ReadCommit(hash Hash) (Commit, error) {
 	}
 
 	if c.sig != "" {
+		// Note: this is actually incorrect as the signed data should be the full commit (+comment, +date ...)
+		// but only the tree hash work for our purpose here.
 		result.SignedData = strings.NewReader(string(c.treeHash))
 		result.Signature = strings.NewReader(c.sig)
 	}

repository/repo_testing.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/openpgp"
 
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
@@ -47,6 +48,7 @@ func RepoTest(t *testing.T, creator RepoCreator, cleaner RepoCleaner) {
 
 			t.Run("Data", func(t *testing.T) {
 				RepoDataTest(t, repo)
+				RepoDataSignatureTest(t, repo)
 			})
 
 			t.Run("Config", func(t *testing.T) {
@@ -200,8 +202,54 @@ func RepoDataTest(t *testing.T, repo RepoData) {
 
 	err = repo.RemoveRef("refs/bugs/ref1")
 	require.NoError(t, err)
+}
+
+func RepoDataSignatureTest(t *testing.T, repo RepoData) {
+	data := randomData()
+
+	blobHash, err := repo.StoreData(data)
+	require.NoError(t, err)
+
+	treeHash, err := repo.StoreTree([]TreeEntry{
+		{
+			ObjectType: Blob,
+			Hash:       blobHash,
+			Name:       "blob",
+		},
+	})
+	require.NoError(t, err)
+
+	pgpEntity1, err := openpgp.NewEntity("", "", "", nil)
+	require.NoError(t, err)
+	keyring1 := openpgp.EntityList{pgpEntity1}
+
+	pgpEntity2, err := openpgp.NewEntity("", "", "", nil)
+	require.NoError(t, err)
+	keyring2 := openpgp.EntityList{pgpEntity2}
+
+	commitHash1, err := repo.StoreSignedCommit(treeHash, pgpEntity1)
+	require.NoError(t, err)
+
+	commit1, err := repo.ReadCommit(commitHash1)
+	require.NoError(t, err)
+
+	_, err = openpgp.CheckDetachedSignature(keyring1, commit1.SignedData, commit1.Signature)
+	require.NoError(t, err)
+
+	_, err = openpgp.CheckDetachedSignature(keyring2, commit1.SignedData, commit1.Signature)
+	require.Error(t, err)
+
+	commitHash2, err := repo.StoreSignedCommit(treeHash, pgpEntity1, commitHash1)
+	require.NoError(t, err)
+
+	commit2, err := repo.ReadCommit(commitHash2)
+	require.NoError(t, err)
+
+	_, err = openpgp.CheckDetachedSignature(keyring1, commit2.SignedData, commit2.Signature)
+	require.NoError(t, err)
 
-	// TODO: testing for commit's signature
+	_, err = openpgp.CheckDetachedSignature(keyring2, commit2.SignedData, commit2.Signature)
+	require.Error(t, err)
 }
 
 // helper to test a RepoClock