Merge pull request #1029 from wlynch/signer-fix

Signer: fix usage of crypto.Signer interface

4 files changed, 94 insertions, 35 deletions

diff --git a/options.go b/options.go
index 0a6eb94..4f913dc 100644
--- a/options.go
+++ b/options.go
@@ -1,7 +1,6 @@
 package git
 
 import (
-	"crypto"
 	"errors"
 	"fmt"
 	"regexp"
@@ -516,7 +515,7 @@ type CommitOptions struct {
 	// Signer denotes a cryptographic signer to sign the commit with.
 	// A nil value here means the commit will not be signed.
 	// Takes precedence over SignKey.
-	Signer crypto.Signer
+	Signer Signer
 	// Amend will create a new commit object and replace the commit that HEAD currently
 	// points to. Cannot be used with All nor Parents.
 	Amend bool
diff --git a/signer.go b/signer.go
new file mode 100644
index 0000000..e3ef7eb
--- /dev/null
+++ b/signer.go
@@ -0,0 +1,33 @@
+package git
+
+import (
+	"io"
+
+	"github.com/go-git/go-git/v5/plumbing"
+)
+
+// signableObject is an object which can be signed.
+type signableObject interface {
+	EncodeWithoutSignature(o plumbing.EncodedObject) error
+}
+
+// Signer is an interface for signing git objects.
+// message is a reader containing the encoded object to be signed.
+// Implementors should return the encoded signature and an error if any.
+// See https://git-scm.com/docs/gitformat-signature for more information.
+type Signer interface {
+	Sign(message io.Reader) ([]byte, error)
+}
+
+func signObject(signer Signer, obj signableObject) ([]byte, error) {
+	encoded := &plumbing.MemoryObject{}
+	if err := obj.EncodeWithoutSignature(encoded); err != nil {
+		return nil, err
+	}
+	r, err := encoded.Reader()
+	if err != nil {
+		return nil, err
+	}
+
+	return signer.Sign(r)
+}
diff --git a/signer_test.go b/signer_test.go
new file mode 100644
index 0000000..eba0922
--- /dev/null
+++ b/signer_test.go
@@ -0,0 +1,56 @@
+package git
+
+import (
+	"encoding/base64"
+	"fmt"
+	"io"
+	"time"
+
+	"github.com/go-git/go-billy/v5/memfs"
+	"github.com/go-git/go-git/v5/plumbing/object"
+	"github.com/go-git/go-git/v5/storage/memory"
+)
+
+type b64signer struct{}
+
+// This is not secure, and is only used as an example for testing purposes.
+// Please don't do this.
+func (b64signer) Sign(message io.Reader) ([]byte, error) {
+	b, err := io.ReadAll(message)
+	if err != nil {
+		return nil, err
+	}
+	out := make([]byte, base64.StdEncoding.EncodedLen(len(b)))
+	base64.StdEncoding.Encode(out, b)
+	return out, nil
+}
+
+func ExampleSigner() {
+	repo, err := Init(memory.NewStorage(), memfs.New())
+	if err != nil {
+		panic(err)
+	}
+	w, err := repo.Worktree()
+	if err != nil {
+		panic(err)
+	}
+	commit, err := w.Commit("example commit", &CommitOptions{
+		Author: &object.Signature{
+			Name:  "John Doe",
+			Email: "john@example.com",
+			When:  time.UnixMicro(1234567890).UTC(),
+		},
+		Signer:            b64signer{},
+		AllowEmptyCommits: true,
+	})
+	if err != nil {
+		panic(err)
+	}
+
+	obj, err := repo.CommitObject(commit)
+	if err != nil {
+		panic(err)
+	}
+	fmt.Println(obj.PGPSignature)
+	// Output: dHJlZSA0YjgyNWRjNjQyY2I2ZWI5YTA2MGU1NGJmOGQ2OTI4OGZiZWU0OTA0CmF1dGhvciBKb2huIERvZSA8am9obkBleGFtcGxlLmNvbT4gMTIzNCArMDAwMApjb21taXR0ZXIgSm9obiBEb2UgPGpvaG5AZXhhbXBsZS5jb20+IDEyMzQgKzAwMDAKCmV4YW1wbGUgY29tbWl0
+}
diff --git a/worktree_commit.go b/worktree_commit.go
index 18002f2..7945af1 100644
--- a/worktree_commit.go
+++ b/worktree_commit.go
@@ -2,8 +2,6 @@ package git
 
 import (
 	"bytes"
-	"crypto"
-	"crypto/rand"
 	"errors"
 	"io"
 	"path"
@@ -135,7 +133,7 @@ func (w *Worktree) buildCommitObject(msg string, opts *CommitOptions, tree plumb
 		signer = &gpgSigner{key: opts.SignKey}
 	}
 	if signer != nil {
-		sig, err := w.buildCommitSignature(commit, signer)
+		sig, err := signObject(signer, commit)
 		if err != nil {
 			return plumbing.ZeroHash, err
 		}
@@ -151,44 +149,17 @@ func (w *Worktree) buildCommitObject(msg string, opts *CommitOptions, tree plumb
 
 type gpgSigner struct {
 	key *openpgp.Entity
+	cfg *packet.Config
 }
 
-func (s *gpgSigner) Public() crypto.PublicKey {
-	return s.key.PrimaryKey
-}
-
-func (s *gpgSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
-	var cfg *packet.Config
-	if opts != nil {
-		cfg = &packet.Config{
-			DefaultHash: opts.HashFunc(),
-		}
-	}
-
+func (s *gpgSigner) Sign(message io.Reader) ([]byte, error) {
 	var b bytes.Buffer
-	if err := openpgp.ArmoredDetachSign(&b, s.key, bytes.NewReader(digest), cfg); err != nil {
+	if err := openpgp.ArmoredDetachSign(&b, s.key, message, s.cfg); err != nil {
 		return nil, err
 	}
 	return b.Bytes(), nil
 }
 
-func (w *Worktree) buildCommitSignature(commit *object.Commit, signer crypto.Signer) ([]byte, error) {
-	encoded := &plumbing.MemoryObject{}
-	if err := commit.Encode(encoded); err != nil {
-		return nil, err
-	}
-	r, err := encoded.Reader()
-	if err != nil {
-		return nil, err
-	}
-	b, err := io.ReadAll(r)
-	if err != nil {
-		return nil, err
-	}
-
-	return signer.Sign(rand.Reader, b, nil)
-}
-
 // buildTreeHelper converts a given index.Index file into multiple git objects
 // reading the blobs from the given filesystem and creating the trees from the
 // index structure. The created objects are pushed to a given Storer.