From 353daaee5bf099647db744572c7f7cabf965d42a Mon Sep 17 00:00:00 2001 From: Adrien Carreira Date: Tue, 23 Jan 2024 15:12:14 +0000 Subject: signature: extend CreateSignature to accept timestamp Since the gpg signature is time-dependent, aligning the timestamp with the author's date makes the gpg signature consistent across all gitaly nodes. --- cmd/gitaly-gpg/main.go | 15 ++++++++++++++- internal/git/localrepo/commit_test.go | 14 +++++++++++--- internal/git/signature.go | 6 +++--- internal/git/signature_test.go | 2 +- internal/signature/gpg.go | 9 +++++++-- internal/signature/signature.go | 7 ++++--- internal/signature/signature_test.go | 19 +++++++++++++++++-- internal/signature/ssh.go | 3 ++- 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") -- cgit v1.2.3