From b87326c283c89bf407a85b5d0b4364eb124a448a Mon Sep 17 00:00:00 2001 From: Igor Drozdov Date: Tue, 1 Aug 2023 12:50:25 +0200 Subject: [PATCH] Support key rotation for signing keys This commit allows specifying rotated signing keys in a separate `git.rotated_signing_keys` config field. It is added to prevent the following race condition: 1. An old signing key is used to create a signature 2. GetCommitSignatures is not yet called to store the values in Rails DB 3. An admin configures a new signing key 4. GetCommitSignatures is called, tries to verify the signatures using the new signing key, fails and returns SIGNER_USER instead of SIGNER_SYSTEM. Now: 3. An admin configures a new signing key and the old signing key(s) in the list (newest first) 4. GetCommitSignatures is called, and iterates over all configured signing keys, tries to verify the signature using each of them, returns SIGNER_SYSTEM if any of the verifications are successful --- cmd/gitaly-git2go/git2goutil/sign.go | 4 +- cmd/gitaly-gpg/main.go | 4 +- internal/gitaly/config/config.go | 1 + internal/gitaly/config/config_test.go | 4 +- .../service/commit/commit_signatures.go | 8 +-- .../service/commit/commit_signatures_test.go | 50 ++++++++++----- .../testdata/signing_ssh_key_ed25519.sig | 6 ++ .../signing_ssh_key_ed25519_sha256.sig | 6 ++ .../commit/testdata/signing_ssh_key_rsa | 39 ++++++++++++ .../commit/testdata/signing_ssh_key_rsa.sig | 20 ++++++ .../testdata/signing_ssh_key_rsa_sha256.sig | 20 ++++++ .../service/operations/cherry_pick_test.go | 2 +- .../gitaly/service/operations/revert_test.go | 2 +- .../gitaly/service/operations/squash_test.go | 2 +- internal/signature/signature.go | 59 +++++++++++++++++- internal/signature/signature_test.go | 41 ++++++++++++ internal/signature/testdata/signing_key.gpg | Bin 0 -> 464 bytes .../signature/testdata/signing_key.gpg.sig | 7 +++ internal/signature/testdata/signing_key.ssh | 8 +++ .../signature/testdata/signing_key.ssh.sig | 6 ++ 20 files changed, 260 insertions(+), 29 deletions(-) create mode 100644 internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519.sig create mode 100644 internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519_sha256.sig create mode 100644 internal/gitaly/service/commit/testdata/signing_ssh_key_rsa create mode 100644 internal/gitaly/service/commit/testdata/signing_ssh_key_rsa.sig create mode 100644 internal/gitaly/service/commit/testdata/signing_ssh_key_rsa_sha256.sig create mode 100644 internal/signature/signature_test.go create mode 100644 internal/signature/testdata/signing_key.gpg create mode 100644 internal/signature/testdata/signing_key.gpg.sig create mode 100644 internal/signature/testdata/signing_key.ssh create mode 100644 internal/signature/testdata/signing_key.ssh.sig diff --git a/cmd/gitaly-git2go/git2goutil/sign.go b/cmd/gitaly-git2go/git2goutil/sign.go index 596542706c..efa8d1b1d7 100644 --- a/cmd/gitaly-git2go/git2goutil/sign.go +++ b/cmd/gitaly-git2go/git2goutil/sign.go @@ -13,10 +13,10 @@ func CreateCommitSignature(signingKeyPath string, contentToSign []byte) ([]byte, return nil, nil } - signingKey, err := signature.ParseSigningKey(signingKeyPath) + signingKeys, err := signature.ParseSigningKeys(signingKeyPath) if err != nil { return nil, fmt.Errorf("failed to parse signing key: %w", err) } - return signingKey.CreateSignature(contentToSign) + return signingKeys.CreateSignature(contentToSign) } diff --git a/cmd/gitaly-gpg/main.go b/cmd/gitaly-gpg/main.go index 1c1b5b5508..ca41ed1d21 100644 --- a/cmd/gitaly-gpg/main.go +++ b/cmd/gitaly-gpg/main.go @@ -24,7 +24,7 @@ func gpgApp() *cli.App { return errors.New("expected --status-fd=2") } - signingKey, err := signature.ParseSigningKey(cCtx.Args().First()) + signingKeys, err := signature.ParseSigningKeys(cCtx.Args().First()) if err != nil { return fmt.Errorf("reading signed key file %s : %w", cCtx.Args().First(), err) } @@ -34,7 +34,7 @@ func gpgApp() *cli.App { return fmt.Errorf("reading contents from stdin: %w", err) } - sig, err := signingKey.CreateSignature(contents) + sig, err := signingKeys.CreateSignature(contents) if err != nil { return fmt.Errorf("creating signature: %w", err) } diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index 4c37b1c223..5b02d15bfe 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -240,6 +240,7 @@ type Git struct { CatfileCacheSize int `toml:"catfile_cache_size,omitempty" json:"catfile_cache_size"` Config []GitConfig `toml:"config,omitempty" json:"config"` SigningKey string `toml:"signing_key,omitempty" json:"signing_key"` + RotatedSigningKeys []string `toml:"rotated_signing_keys,omitempty" json:"rotated_signing_keys"` } // Validate runs validation on all fields and compose all found errors. diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index a1577eba9b..47bdf72935 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -408,7 +408,8 @@ func TestLoadConfigCommand(t *testing.T) { cmd := writeScript(t, `cat <<-EOF { "git": { - "signing_key": "signing_key" + "signing_key": "signing_key", + "rotated_signing_keys": ["rotated_key_1", "rotated_key_2"] } } EOF @@ -425,6 +426,7 @@ func TestLoadConfigCommand(t *testing.T) { cfg.ConfigCommand = cmd cfg.Git.BinPath = "foo/bar" cfg.Git.SigningKey = "signing_key" + cfg.Git.RotatedSigningKeys = []string{"rotated_key_1", "rotated_key_2"} }), } }, diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go index 4ac24dd903..2aa9c957da 100644 --- a/internal/gitaly/service/commit/commit_signatures.go +++ b/internal/gitaly/service/commit/commit_signatures.go @@ -39,9 +39,9 @@ func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesReques } defer cancel() - var signingKey signature.SigningKey + var signingKeys *signature.SigningKeys if s.cfg.Git.SigningKey != "" { - signingKey, err = signature.ParseSigningKey(s.cfg.Git.SigningKey) + signingKeys, err = signature.ParseSigningKeys(s.cfg.Git.SigningKey, s.cfg.Git.RotatedSigningKeys...) if err != nil { return fmt.Errorf("failed to parse signing key: %w", err) } @@ -62,8 +62,8 @@ func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesReques } signer := gitalypb.GetCommitSignaturesResponse_SIGNER_USER - if signingKey != nil { - if err := signingKey.Verify(signatureKey, commitText); err == nil { + if signingKeys != nil { + if err := signingKeys.Verify(signatureKey, commitText); err == nil { signer = gitalypb.GetCommitSignaturesResponse_SIGNER_SYSTEM } } diff --git a/internal/gitaly/service/commit/commit_signatures_test.go b/internal/gitaly/service/commit/commit_signatures_test.go index 1a5e6671ab..ba6d80ec6e 100644 --- a/internal/gitaly/service/commit/commit_signatures_test.go +++ b/internal/gitaly/service/commit/commit_signatures_test.go @@ -60,6 +60,7 @@ func testGetCommitSignatures(t *testing.T, ctx context.Context) { testcfg.BuildGitalyGPG(t, cfg) cfg.Git.SigningKey = "testdata/signing_ssh_key_ed25519" + cfg.Git.RotatedSigningKeys = []string{"testdata/signing_ssh_key_rsa"} cfg.SocketPath = startTestServices(t, cfg) client := newCommitServiceClient(t, cfg.SocketPath) @@ -302,11 +303,30 @@ func testGetCommitSignatures(t *testing.T, ctx context.Context) { }) require.NoError(t, err) + rotatedKeyCommitID, err := repo.WriteCommit(ctx, localrepo.WriteCommitConfig{ + TreeID: tree.OID, + AuthorName: gittest.DefaultCommitterName, + AuthorEmail: gittest.DefaultCommitterMail, + CommitterName: gittest.DefaultCommitterName, + CommitterEmail: gittest.DefaultCommitterMail, + AuthorDate: gittest.DefaultCommitTime, + CommitterDate: gittest.DefaultCommitTime, + Message: "rotated key commit message", + SigningKey: cfg.Git.RotatedSigningKeys[0], + }) + require.NoError(t, err) + + rsaSHA1Signature := string(testhelper.MustReadFile(t, "testdata/signing_ssh_key_rsa.sig")) + rsaSHA256Signature := string(testhelper.MustReadFile(t, "testdata/signing_ssh_key_rsa_sha256.sig")) + ed25519SHA1Signature := string(testhelper.MustReadFile(t, "testdata/signing_ssh_key_ed25519.sig")) + ed25519SHA256Signature := string(testhelper.MustReadFile(t, "testdata/signing_ssh_key_ed25519_sha256.sig")) + return setupData{ request: &gitalypb.GetCommitSignaturesRequest{ Repository: repoProto, CommitIds: []string{ commitID.String(), + rotatedKeyCommitID.String(), }, }, expectedResponses: testhelper.EnabledOrDisabledFlag(ctx, featureflag.GPGSigning, @@ -314,20 +334,8 @@ func testGetCommitSignatures(t *testing.T, ctx context.Context) { { CommitId: commitID.String(), Signature: []byte(gittest.ObjectHashDependent(t, map[string]string{ - "sha1": `-----BEGIN SSH SIGNATURE----- -U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgVzKQNpRPvHihfJQJ+Com -F8BdFuG2wuXh+LjXjbOs8IgAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3Nz -aC1lZDI1NTE5AAAAQB6uCeUpvnFGR/cowe1pQyTZiTzKsi1tnez0EO8o2LtrJr+g -k8fZo+m7jSM0TpefrL0iyHxevrbKslyXw1lJVAM= ------END SSH SIGNATURE----- -`, - "sha256": `-----BEGIN SSH SIGNATURE----- -U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgVzKQNpRPvHihfJQJ+Com -F8BdFuG2wuXh+LjXjbOs8IgAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3Nz -aC1lZDI1NTE5AAAAQKgC1TFLVZOqvVs2AqCp2lhkRAUtZsDa89RgHOOsYAC3T1kB -4lOayj2uzBahoM0gc7REITUyg5MTzfIhcIPfhAQ= ------END SSH SIGNATURE----- -`, + "sha1": ed25519SHA1Signature, + "sha256": ed25519SHA256Signature, })), SignedText: []byte(fmt.Sprintf( "tree %s\nauthor %s\ncommitter %s\n\nmessage", @@ -337,6 +345,20 @@ aC1lZDI1NTE5AAAAQKgC1TFLVZOqvVs2AqCp2lhkRAUtZsDa89RgHOOsYAC3T1kB )), Signer: gitalypb.GetCommitSignaturesResponse_SIGNER_SYSTEM, }, + { + CommitId: rotatedKeyCommitID.String(), + Signature: []byte(gittest.ObjectHashDependent(t, map[string]string{ + "sha1": rsaSHA1Signature, + "sha256": rsaSHA256Signature, + })), + SignedText: []byte(fmt.Sprintf( + "tree %s\nauthor %s\ncommitter %s\n\nrotated key commit message", + tree.OID, + gittest.DefaultCommitterSignature, + gittest.DefaultCommitterSignature, + )), + Signer: gitalypb.GetCommitSignaturesResponse_SIGNER_SYSTEM, + }, }, nil, ), diff --git a/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519.sig b/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519.sig new file mode 100644 index 0000000000..31242342db --- /dev/null +++ b/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519.sig @@ -0,0 +1,6 @@ +-----BEGIN SSH SIGNATURE----- +U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgVzKQNpRPvHihfJQJ+Com +F8BdFuG2wuXh+LjXjbOs8IgAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3Nz +aC1lZDI1NTE5AAAAQB6uCeUpvnFGR/cowe1pQyTZiTzKsi1tnez0EO8o2LtrJr+g +k8fZo+m7jSM0TpefrL0iyHxevrbKslyXw1lJVAM= +-----END SSH SIGNATURE----- diff --git a/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519_sha256.sig b/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519_sha256.sig new file mode 100644 index 0000000000..baa0da2e57 --- /dev/null +++ b/internal/gitaly/service/commit/testdata/signing_ssh_key_ed25519_sha256.sig @@ -0,0 +1,6 @@ +-----BEGIN SSH SIGNATURE----- +U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgVzKQNpRPvHihfJQJ+Com +F8BdFuG2wuXh+LjXjbOs8IgAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3Nz +aC1lZDI1NTE5AAAAQKgC1TFLVZOqvVs2AqCp2lhkRAUtZsDa89RgHOOsYAC3T1kB +4lOayj2uzBahoM0gc7REITUyg5MTzfIhcIPfhAQ= +-----END SSH SIGNATURE----- diff --git a/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa new file mode 100644 index 0000000000..09a40864dd --- /dev/null +++ b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa @@ -0,0 +1,39 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABlwAAAAdzc2gtcn +NhAAAAAwEAAQAAAYEAxsu/Ci6WyIX51Ug+utJnFBJ7CXucjnu0jG+RpJjrGqFgWDXSkXj2 +VBLe6oZP0P3MSQBZHVSRw8P9CIEv8y3WgNGE2FGhhvClwUPLJJXtrojAk9fygCjm3wSY3D +yDr+dIRvNbrpZ3NmK8U+erQLg5JfmQMRUcbqOpdRCHCX/1zz6Nz0olF+0/eSWqqqhmxQuu +bFwQQc787AdAa5s/HRZ+4NU+1pEze/HxOmrs8evYtX9r2YcWj1bKVgGy5Ggxmhe5DPVxFj +f0SoneA8wzTiFsfb5IJGOXuhLDiiKmVx6T5fz0LVa5XlXfnVf5CMsl83FKmQN3n5L4lymN +XlIe3dOakwa9sO4S8bgi8qoFUOS2FtQta16ZFIF4ZeC+OBR1dfbYzD5UxjHxMjbSfRHkp5 +Lw7OiIE37RXJv3oUgM3jeo8mDQLsbkld4pdMT7Ofor6kFmFi1IhEccMHA6ZehDaznnwznR +U3QPWWVjqEW2VkS8+nm6hvBoQjs6BBAVt6BZ4V65AAAFoNQ2nRvUNp0bAAAAB3NzaC1yc2 +EAAAGBAMbLvwoulsiF+dVIPrrSZxQSewl7nI57tIxvkaSY6xqhYFg10pF49lQS3uqGT9D9 +zEkAWR1UkcPD/QiBL/Mt1oDRhNhRoYbwpcFDyySV7a6IwJPX8oAo5t8EmNw8g6/nSEbzW6 +6WdzZivFPnq0C4OSX5kDEVHG6jqXUQhwl/9c8+jc9KJRftP3klqqqoZsULrmxcEEHO/OwH +QGubPx0WfuDVPtaRM3vx8Tpq7PHr2LV/a9mHFo9WylYBsuRoMZoXuQz1cRY39EqJ3gPMM0 +4hbH2+SCRjl7oSw4oiplcek+X89C1WuV5V351X+QjLJfNxSpkDd5+S+JcpjV5SHt3TmpMG +vbDuEvG4IvKqBVDkthbULWtemRSBeGXgvjgUdXX22Mw+VMYx8TI20n0R5KeS8OzoiBN+0V +yb96FIDN43qPJg0C7G5JXeKXTE+zn6K+pBZhYtSIRHHDBwOmXoQ2s558M50VN0D1llY6hF +tlZEvPp5uobwaEI7OgQQFbegWeFeuQAAAAMBAAEAAAGAGp4XQz6/s7O0oukcdRlM8fQTg0 +6IxM8teoxJvPc4q4UmCEmUmyPOH62zKUW4lCwXWULxq6qyJbstOyFJEU925CKpnek4LoA0 +QW9ZWNm2TGNFHcaRUrWnS/8qlHqJy1i1ZcKZ6QN+jMqlmrpvRKgmBr6mntvLxcimHOWMny +oB+LDQfgvYcZ6zm/3+HwGTWRjaTun4x0b2uIe0CXRs+/ESJfqHgmVItnTLrt24QiApEQwx +nZun2qNtThzGHi0RTyeTw/4AtKDxYWVBQRciEeEEvAACtwkvUQbY4fILwtbke/UjWoi3eT +i+ePEGxLfeYxhJlhr6xX7AdD6TPq1x49OULZnnNZ0N4wuJ3KmkfxsnYJUiHoxR/s6PfKW1 +LXz7nhKYBpVAaAFIEP0eWHN5G3S7aDW91DvyA8IYnvkFsNYzxTRFNLmxH6+ENLnIGHzxwu +I/0hZ3y3Ajg4H0rUcoh7TuENm/d+q2JlpSbNdEzkyJbyw6aLb9FlHzJCNj/GxTqjg9AAAA +wGw8O5k3Gcsb4IiO9vLT4vuARnXLLmClehn0psGIckgpLs0xaarOgt4R9781OUG6Adc75Z +/aJ3zMDXIfAvnLAaghh/omsBVdnLpdj2kuYc9Y9Yy59miPsO9uALXY7NXVyHM9TTFPV2Bj +4mOFpyTk951wiTIXQjnhXY1m5lp7+ct59vf4CcP+IDM/mQZ/SZUV1tPZVxHaJD15s5Bywx +yKx88sBjFPwAARS54X39xToTVN7wNP5SUsNHMWKpIA3qnLSgAAAMEA7Bn4J86XzJADru7T +QDAONAbh8YD4p37/LHS/YD71lCvLrjuIQI6d9auInLHkQBQbZkx5mxalUbzlbrTIvBoM5Y +vWhCGgNvrmvvfn2LDXZn2iTZqlxYCONV+cg62iEjKXdbcCn7rIRGHxggQAxAobbFTQxnMs +8gF2gabI8z0gRoxNtkcqrqkYp3o97Lm3QYz38n3w3qF9RmvoIwwoaJGNF+Z856g/Id1DAF +ekThx/MXnSlkCCRMQjy4tun15JGaxrAAAAwQDXjOMTh+fu1LIXPQgJaxe3TPPOlrNuAMrK +S5/QFR0c/oskhQFF6kYgs3p+onU4/Uym9LLLLE2VGOI/PSnAH/QLETZABDa+yC2UeswGDk +ep7yGLrfACHUrWD1vmk23r8ZajEpFVTGsO4UwPoIfRgoHc2XCzezIQVisWicLT9rHD5bXR +EtMoGayzGCDOQdHQ4HZMWEMVEEgS9pM5cQOd77DTam0VLIXDdvNfnfgyTaZhtIEER8D1pU +FlOeBtO3DGamsAAAAlaWdvcmRyb3pkb3ZASWdvcnMtTWFjQm9vay1Qcm8tMi5sb2NhbAEC +AwQFBg== +-----END OPENSSH PRIVATE KEY----- diff --git a/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa.sig b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa.sig new file mode 100644 index 0000000000..ad5bed790a --- /dev/null +++ b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa.sig @@ -0,0 +1,20 @@ +-----BEGIN SSH SIGNATURE----- +U1NIU0lHAAAAAQAAAZcAAAAHc3NoLXJzYQAAAAMBAAEAAAGBAMbLvwoulsiF+dVI +PrrSZxQSewl7nI57tIxvkaSY6xqhYFg10pF49lQS3uqGT9D9zEkAWR1UkcPD/QiB +L/Mt1oDRhNhRoYbwpcFDyySV7a6IwJPX8oAo5t8EmNw8g6/nSEbzW66WdzZivFPn +q0C4OSX5kDEVHG6jqXUQhwl/9c8+jc9KJRftP3klqqqoZsULrmxcEEHO/OwHQGub +Px0WfuDVPtaRM3vx8Tpq7PHr2LV/a9mHFo9WylYBsuRoMZoXuQz1cRY39EqJ3gPM +M04hbH2+SCRjl7oSw4oiplcek+X89C1WuV5V351X+QjLJfNxSpkDd5+S+JcpjV5S +Ht3TmpMGvbDuEvG4IvKqBVDkthbULWtemRSBeGXgvjgUdXX22Mw+VMYx8TI20n0R +5KeS8OzoiBN+0Vyb96FIDN43qPJg0C7G5JXeKXTE+zn6K+pBZhYtSIRHHDBwOmXo +Q2s558M50VN0D1llY6hFtlZEvPp5uobwaEI7OgQQFbegWeFeuQAAAANnaXQAAAAA +AAAABnNoYTUxMgAAAZQAAAAMcnNhLXNoYTItNTEyAAABgCgdx7uzvtHCshLxN/Ky +h+Lvs7jTFlIiqxqZRE6hQKOI04qsOBZeARA3mFbGQKCRUc4Qb/vT1qDPHf6/CtFE +kZ2TeK0gpEKmX29fzk2dpVvIKhtwBqUGFv7iRVI4NnyFtDOUwcjmCtlsIA6TD0X2 +8W+LpDt/Qm7H87yeviIxWegU2IiSuoG1euMvuTsRDquZAaOkkMmAtDn9/1vi9xtA +Phkwl+T7sivXPX79ncE8bdP9eBu76WEzviOUo07h32c876FDU5RHfjfYBo3xya54 +LS+PiZjw2pTULVyOQWQIwR6cP8rjCH3mNI3ScbA8AYHHCzP38QHCzdcaOBQ4n9Yi +q0XTOtXS2H67qkMC8Ge90nO+9833t7kOES1fsxGAVO7wfiUgnzT7DPzrbs9J43+P +nn4helHH9Dy17WY+UzCzRkAOLOnZxU2VXPRcGUr7fshthA2Oa5mgHnDCP3sUw7hL +WDKTjKjH4dHiw2BFbgpZuGmKkF9i1eSRLwGE8LZKHPWPcg== +-----END SSH SIGNATURE----- diff --git a/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa_sha256.sig b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa_sha256.sig new file mode 100644 index 0000000000..1c675bf5e2 --- /dev/null +++ b/internal/gitaly/service/commit/testdata/signing_ssh_key_rsa_sha256.sig @@ -0,0 +1,20 @@ +-----BEGIN SSH SIGNATURE----- +U1NIU0lHAAAAAQAAAZcAAAAHc3NoLXJzYQAAAAMBAAEAAAGBAMbLvwoulsiF+dVI +PrrSZxQSewl7nI57tIxvkaSY6xqhYFg10pF49lQS3uqGT9D9zEkAWR1UkcPD/QiB +L/Mt1oDRhNhRoYbwpcFDyySV7a6IwJPX8oAo5t8EmNw8g6/nSEbzW66WdzZivFPn +q0C4OSX5kDEVHG6jqXUQhwl/9c8+jc9KJRftP3klqqqoZsULrmxcEEHO/OwHQGub +Px0WfuDVPtaRM3vx8Tpq7PHr2LV/a9mHFo9WylYBsuRoMZoXuQz1cRY39EqJ3gPM +M04hbH2+SCRjl7oSw4oiplcek+X89C1WuV5V351X+QjLJfNxSpkDd5+S+JcpjV5S +Ht3TmpMGvbDuEvG4IvKqBVDkthbULWtemRSBeGXgvjgUdXX22Mw+VMYx8TI20n0R +5KeS8OzoiBN+0Vyb96FIDN43qPJg0C7G5JXeKXTE+zn6K+pBZhYtSIRHHDBwOmXo +Q2s558M50VN0D1llY6hFtlZEvPp5uobwaEI7OgQQFbegWeFeuQAAAANnaXQAAAAA +AAAABnNoYTUxMgAAAZQAAAAMcnNhLXNoYTItNTEyAAABgBYdJDU01OqZ/zVE6RfG +fAShwbcouhB3E1ZBIC01gDGvEwN7h/TNFlSwlZThTt07CWatDaVvAYjhHj8Vcl5m +bFbtsNxXhx+9ysL/RTy6fzi1LcHg4yEyYsOdumXn2/q5UipHgtlaar6GdfYO6M4a +fhQUjkWg+rz7xB2wzjylklpTDtZK/Nspm1ikbXWer6+B158AMt+6xFy1xwXsjM8/ +USATUlvzSnxPiBbo4HxNMyVmVO/xHyR+Yi2h8fwdMF10hlGu0jdsTTrlTcuy4eFN +HB6n+werd+a9k1IUvNAvIjopcNQsn9svsqOwXxnt8kxfEQHEdmL51vTKpA1QqFZG +PpVMj2bkkDzXRHz4JV4VuChPnbjfkPCDuHiSpzSUGgcRCq3/jPidgNp6Os+MEzAM +5HgBja9P33epLfAr46UBus26DWa2iBUm0V48a1aQ4ugRaiQHxXjnjSSYlsvggbQg +raSIKKzt/PmigE3Iyj0EpkAgBHVQG1KttLFLqpMuMARfJQ== +-----END SSH SIGNATURE----- diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go index 66ed82a88e..1b5ef721ef 100644 --- a/internal/gitaly/service/operations/cherry_pick_test.go +++ b/internal/gitaly/service/operations/cherry_pick_test.go @@ -524,7 +524,7 @@ func testServerUserCherryPickMergeCommit(t *testing.T, ctx context.Context) { gpgsig, dataWithoutGpgSig := signature.ExtractSignature(t, ctx, data) - signingKey, err := signature.ParseSigningKey("testdata/signing_ssh_key_ecdsa") + signingKey, err := signature.ParseSigningKeys("testdata/signing_ssh_key_ecdsa") require.NoError(t, err) require.NoError(t, signingKey.Verify([]byte(gpgsig), []byte(dataWithoutGpgSig))) diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go index a74b954f54..bef89503ca 100644 --- a/internal/gitaly/service/operations/revert_test.go +++ b/internal/gitaly/service/operations/revert_test.go @@ -587,7 +587,7 @@ func testServerUserRevertMergeCommit(t *testing.T, ctx context.Context) { gpgsig, dataWithoutGpgSig := signature.ExtractSignature(t, ctx, data) - signingKey, err := signature.ParseSigningKey("testdata/signing_ssh_key_rsa") + signingKey, err := signature.ParseSigningKeys("testdata/signing_ssh_key_rsa") require.NoError(t, err) require.NoError(t, signingKey.Verify([]byte(gpgsig), []byte(dataWithoutGpgSig))) diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go index 6fb6570660..96969ce77d 100644 --- a/internal/gitaly/service/operations/squash_test.go +++ b/internal/gitaly/service/operations/squash_test.go @@ -116,7 +116,7 @@ func testUserSquashSuccessful(t *testing.T, ctx context.Context) { gpgsig, dataWithoutGpgSig := signature.ExtractSignature(t, ctx, data) - signingKey, err := signature.ParseSigningKey("testdata/signing_ssh_key_ed25519") + signingKey, err := signature.ParseSigningKeys("testdata/signing_ssh_key_ed25519") require.NoError(t, err) require.NoError(t, signingKey.Verify([]byte(gpgsig), []byte(dataWithoutGpgSig))) diff --git a/internal/signature/signature.go b/internal/signature/signature.go index 087af1b0b5..6f9418aa04 100644 --- a/internal/signature/signature.go +++ b/internal/signature/signature.go @@ -6,14 +6,48 @@ import ( "os" ) -// SigningKey is the common interface interface of SSH and GPG signing keys +// SigningKey is the common interface of SSH and GPG signing keys type SigningKey interface { CreateSignature([]byte) ([]byte, error) Verify([]byte, []byte) error } -// ParseSigningKey parses a signing key and returns either GPG or SSH key -func ParseSigningKey(path string) (SigningKey, error) { +// SigningKeys represents all signing keys configured in the system. +// The primary key is used for creating signatures, the secondary +// keys are used for verification if the primary key failed to verify +// a signature +type SigningKeys struct { + primaryKey SigningKey + secondaryKeys []SigningKey +} + +// ParseSigningKeys parses a list of signing keys separated by a comma and returns +// a list of GPG or SSH keys. +// Multiple signing keys are necessary to provide proper key rotation. +// The latest signing key is specified first and used for creating a signature. The +// previous signing keys go after and are used to verify a signature. +func ParseSigningKeys(primaryPath string, secondaryPaths ...string) (*SigningKeys, error) { + primaryKey, err := parseSigningKey(primaryPath) + if err != nil { + return nil, err + } + + secondaryKeys := make([]SigningKey, 0, len(secondaryPaths)) + for _, path := range secondaryPaths { + signingKey, err := parseSigningKey(path) + if err != nil { + return nil, err + } + secondaryKeys = append(secondaryKeys, signingKey) + } + + return &SigningKeys{ + primaryKey: primaryKey, + secondaryKeys: secondaryKeys, + }, nil +} + +func parseSigningKey(path string) (SigningKey, error) { key, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("open file: %w", err) @@ -25,3 +59,22 @@ func ParseSigningKey(path string) (SigningKey, error) { return parseGpgSigningKey(key) } + +// CreateSignature uses the primary key to create a signature +func (s *SigningKeys) CreateSignature(contentToSign []byte) ([]byte, error) { + return s.primaryKey.CreateSignature(contentToSign) +} + +// Verify iterates over all signing keys and returns nil if any +// verification was successful. Otherwise, the last error is returned. +// Note: when Golang 1.19 is no longer supported, can be refactored using errors.Join +func (s *SigningKeys) Verify(signature, signedText []byte) error { + var err error + for _, signingKey := range append([]SigningKey{s.primaryKey}, s.secondaryKeys...) { + err = signingKey.Verify(signature, signedText) + if err == nil { + return nil + } + } + return err +} diff --git a/internal/signature/signature_test.go b/internal/signature/signature_test.go new file mode 100644 index 0000000000..8b25af2778 --- /dev/null +++ b/internal/signature/signature_test.go @@ -0,0 +1,41 @@ +package signature + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +var commit = []byte(` +tree 86ec18bfe87ad42a782fdabd8310f9b7ac750f51 +parent b83d6e391c22777fca1ed3012fce84f633d7fed0 +parent 4a24d82dbca5c11c61556f3b35ca472b7463187e +author User 1491906794 +0000 +committer User 1491906794 +0000 + +Update README.md to include +`) + +func TestParseSigningKeys(t *testing.T) { + primaryPath := "testdata/signing_key.ssh" + secondaryPaths := []string{"testdata/signing_key.gpg"} + + expectedSSHSignature, err := os.ReadFile("testdata/signing_key.ssh.sig") + require.NoError(t, err) + + expectedGPGSignature, err := os.ReadFile("testdata/signing_key.gpg.sig") + require.NoError(t, err) + + signingKeys, err := ParseSigningKeys(primaryPath, secondaryPaths...) + require.NoError(t, err) + require.NotNil(t, signingKeys.primaryKey) + require.Len(t, signingKeys.secondaryKeys, 1) + + signature, err := signingKeys.CreateSignature(commit) + require.NoError(t, err) + require.Equal(t, signature, expectedSSHSignature) + + require.NoError(t, signingKeys.Verify(expectedSSHSignature, commit)) + require.NoError(t, signingKeys.Verify(expectedGPGSignature, commit)) +} diff --git a/internal/signature/testdata/signing_key.gpg b/internal/signature/testdata/signing_key.gpg new file mode 100644 index 0000000000000000000000000000000000000000..11c9da480a714a2f17464cf5fdf9dcc961ae6b5a GIT binary patch literal 464 zcmbOd!IHB7Qo0zYHX9=g<1Kf7Mn-mrxst)h?(dA#`93`&?sP^*?!TLd7A;dcq0zLe zO*pZv#(;s5q2Xz^T%3^MH-5#qxlM~(COzOT75Y9|?caiu)DKPHb8ZS4Z((!KEb&Q9 z>X^(TEXKuPEvCqF%+xvj;sxf+xhLKFKZSQLPvJN-ft3klKa(^wD>o+xI}?)<6PqX( zCkHnZix?9#Ba<9ElXwFI7pDMB{lv!%|Gis_UT(-~czt)*@uq$+Z=Eeu-l*%`|D|KN zv#4^abLd@$zv5|rkDl%g{-hkxCarRH&COTdZePV77yNx