Merge branch 'deterministic-gpg-signature' into 'master'

fix: deterministic gpg signature

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6621

Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Adrien <adrien@huggingface.co>

8 files changed, 59 insertions, 16 deletions

diff --git a/cmd/gitaly-gpg/main.go b/cmd/gitaly-gpg/main.go
index 49e37fcfc..de0b8406c 100644
--- a/cmd/gitaly-gpg/main.go
+++ b/cmd/gitaly-gpg/main.go
@@ -6,8 +6,10 @@ import (
 	"io"
 	"log"
 	"os"
+	"time"
 
 	"github.com/urfave/cli/v2"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/signature"
 )
 
@@ -34,7 +36,18 @@ func gpgApp() *cli.App {
 				return fmt.Errorf("reading contents from stdin: %w", err)
 			}
 
-			sig, err := signingKeys.CreateSignature(contents)
+			// Signing a commit can be nondeterministic if not done at the same second
+			// We use `GIT_AUTHOR_DATE` to ensure that the signature is deterministic
+			date := time.Now()
+			authorDate, exists := os.LookupEnv("GIT_AUTHOR_DATE")
+			if exists {
+				date, err = time.ParseInLocation(git.Rfc2822DateFormat, authorDate, time.FixedZone("", 0))
+				if err != nil {
+					return fmt.Errorf("error parsing GIT_AUTHOR_DATE: %w", err)
+				}
+			}
+
+			sig, err := signingKeys.CreateSignature(contents, date)
 			if err != nil {
 				return fmt.Errorf("creating signature: %w", err)
 			}
diff --git a/internal/git/localrepo/commit_test.go b/internal/git/localrepo/commit_test.go
index c92cb6f9f..25202f351 100644
--- a/internal/git/localrepo/commit_test.go
+++ b/internal/git/localrepo/commit_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 func TestRepo_ReadCommit(t *testing.T) {
@@ -382,14 +383,16 @@ func testWriteCommit(t *testing.T, ctx context.Context) {
 			tb.Skip()
 		}
 
+		commitDate := time.Date(2023, time.November, 10, 23, 0, 0, 0, time.UTC)
+
 		cfg := WriteCommitConfig{
 			TreeID:         treeA.OID,
 			AuthorName:     gittest.DefaultCommitterName,
 			AuthorEmail:    gittest.DefaultCommitterMail,
 			CommitterName:  gittest.DefaultCommitterName,
 			CommitterEmail: gittest.DefaultCommitterMail,
-			AuthorDate:     gittest.DefaultCommitTime,
-			CommitterDate:  gittest.DefaultCommitTime,
+			AuthorDate:     commitDate,
+			CommitterDate:  commitDate,
 			Message:        "my custom message",
 			GitConfig: config.Git{
 				SigningKey: "testdata/signing_gpg_key",
@@ -402,7 +405,12 @@ func testWriteCommit(t *testing.T, ctx context.Context) {
 		commit, err := repo.ReadCommit(ctx, git.Revision(oid))
 		require.NoError(t, err)
 
-		require.Equal(t, gittest.DefaultCommitAuthor, commit.Author)
+		require.Equal(t, &gitalypb.CommitAuthor{
+			Name:     []byte(gittest.DefaultCommitterName),
+			Email:    []byte(gittest.DefaultCommitterMail),
+			Date:     timestamppb.New(commitDate),
+			Timezone: []byte("+0000"),
+		}, commit.Author)
 		require.Equal(t, "my custom message", string(commit.Body))
 
 		data, err := repo.ReadObject(ctx, oid)
diff --git a/internal/git/signature.go b/internal/git/signature.go
index 6c1754272..300ad7a83 100644
--- a/internal/git/signature.go
+++ b/internal/git/signature.go
@@ -10,8 +10,8 @@ import (
 )
 
 const (
-	// rfc2822DateFormat is the date format that Git typically uses for dates.
-	rfc2822DateFormat = "Mon Jan 02 2006 15:04:05 -0700"
+	// Rfc2822DateFormat is the date format that Git typically uses for dates.
+	Rfc2822DateFormat = "Mon Jan 02 2006 15:04:05 -0700"
 )
 
 var signatureSanitizer = strings.NewReplacer("\n", "", "<", "", ">", "")
@@ -47,7 +47,7 @@ func NewSignature(name, email string, when time.Time) Signature {
 // If you need to format a time to be used in signatures directly, e.g. because it is passed to git-hash-object(1), you
 // can use `FormatSignatureTime()` instead.
 func FormatTime(t time.Time) string {
-	return t.Format(rfc2822DateFormat)
+	return t.Format(Rfc2822DateFormat)
 }
 
 // FormatSignatureTime formats a time such that it can be embedded into a tag or commit object directly.
diff --git a/internal/git/signature_test.go b/internal/git/signature_test.go
index 6a98d64d7..8fc1f9294 100644
--- a/internal/git/signature_test.go
+++ b/internal/git/signature_test.go
@@ -114,7 +114,7 @@ func TestFormatTime(t *testing.T) {
 
 				// We use `time.ParseInLocation()` here such that Go won't automatically translate e.g. `+0200`
 				// into "CEST" or `time.Local`.
-				actualParsedTime, err := time.ParseInLocation(rfc2822DateFormat, actualString, time.FixedZone("", 0))
+				actualParsedTime, err := time.ParseInLocation(Rfc2822DateFormat, actualString, time.FixedZone("", 0))
 
 				require.NoError(t, err)
 				require.Equal(t, tc.expectedParsedTime, actualParsedTime)
diff --git a/internal/signature/gpg.go b/internal/signature/gpg.go
index e614b1988..9c313ba37 100644
--- a/internal/signature/gpg.go
+++ b/internal/signature/gpg.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
@@ -24,13 +25,17 @@ func parseGpgSigningKey(key []byte) (*GpgSigningKey, error) {
 }
 
 // CreateSignature creates a gpg signature
-func (sk *GpgSigningKey) CreateSignature(contentToSign []byte) ([]byte, error) {
+func (sk *GpgSigningKey) CreateSignature(contentToSign []byte, date time.Time) ([]byte, error) {
 	var sigBuf strings.Builder
 	if err := openpgp.ArmoredDetachSignText(
 		&sigBuf,
 		sk.Entity,
 		bytes.NewReader(contentToSign),
-		&packet.Config{},
+		&packet.Config{
+			Time: func() time.Time {
+				return date
+			},
+		},
 	); err != nil {
 		return nil, fmt.Errorf("sign commit: %w", err)
 	}
diff --git a/internal/signature/signature.go b/internal/signature/signature.go
index 6f9418aa0..ce2a7f78e 100644
--- a/internal/signature/signature.go
+++ b/internal/signature/signature.go
@@ -4,11 +4,12 @@ import (
 	"bytes"
 	"fmt"
 	"os"
+	"time"
 )
 
 // SigningKey is the common interface of SSH and GPG signing keys
 type SigningKey interface {
-	CreateSignature([]byte) ([]byte, error)
+	CreateSignature([]byte, time.Time) ([]byte, error)
 	Verify([]byte, []byte) error
 }
 
@@ -61,8 +62,8 @@ func parseSigningKey(path string) (SigningKey, error) {
 }
 
 // CreateSignature uses the primary key to create a signature
-func (s *SigningKeys) CreateSignature(contentToSign []byte) ([]byte, error) {
-	return s.primaryKey.CreateSignature(contentToSign)
+func (s *SigningKeys) CreateSignature(contentToSign []byte, date time.Time) ([]byte, error) {
+	return s.primaryKey.CreateSignature(contentToSign, date)
 }
 
 // Verify iterates over all signing keys and returns nil if any
diff --git a/internal/signature/signature_test.go b/internal/signature/signature_test.go
index 8b25af277..28a22184a 100644
--- a/internal/signature/signature_test.go
+++ b/internal/signature/signature_test.go
@@ -3,6 +3,7 @@ package signature
 import (
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 )
@@ -32,10 +33,24 @@ func TestParseSigningKeys(t *testing.T) {
 	require.NotNil(t, signingKeys.primaryKey)
 	require.Len(t, signingKeys.secondaryKeys, 1)
 
-	signature, err := signingKeys.CreateSignature(commit)
+	signature, err := signingKeys.CreateSignature(commit, time.Now())
 	require.NoError(t, err)
-	require.Equal(t, signature, expectedSSHSignature)
+	require.Equal(t, expectedSSHSignature, signature)
 
 	require.NoError(t, signingKeys.Verify(expectedSSHSignature, commit))
 	require.NoError(t, signingKeys.Verify(expectedGPGSignature, commit))
 }
+
+func TestGPGSignatureDeterministic(t *testing.T) {
+	primaryPath := "testdata/signing_key.gpg"
+	signingKeys, err := ParseSigningKeys(primaryPath)
+	require.NoError(t, err)
+	require.NotNil(t, signingKeys.primaryKey)
+
+	expectedGPGSignature, err := os.ReadFile("testdata/signing_key.gpg.sig")
+	require.NoError(t, err)
+
+	signature, err := signingKeys.CreateSignature(commit, time.Unix(1691162414, 0))
+	require.NoError(t, err)
+	require.Equal(t, expectedGPGSignature, signature)
+}
diff --git a/internal/signature/ssh.go b/internal/signature/ssh.go
index 83e6c6551..b7fe27d12 100644
--- a/internal/signature/ssh.go
+++ b/internal/signature/ssh.go
@@ -5,6 +5,7 @@ import (
 	"crypto/sha512"
 	"encoding/pem"
 	"fmt"
+	"time"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -56,7 +57,7 @@ func parseSSHSigningKey(key []byte) (*SSHSigningKey, error) {
 }
 
 // CreateSignature creates an SSH signature
-func (sk *SSHSigningKey) CreateSignature(contentToSign []byte) ([]byte, error) {
+func (sk *SSHSigningKey) CreateSignature(contentToSign []byte, _ time.Time) ([]byte, error) {
 	signer, ok := sk.PrivateKey.(ssh.AlgorithmSigner)
 	if !ok {
 		return nil, fmt.Errorf("wrong type of the private key")