diff --git a/models/asymkey/ssh_key_fingerprint.go b/models/asymkey/ssh_key_fingerprint.go index 4dcfe1f27925a..b666469ae87b4 100644 --- a/models/asymkey/ssh_key_fingerprint.go +++ b/models/asymkey/ssh_key_fingerprint.go @@ -13,9 +13,9 @@ import ( "xorm.io/builder" ) -// The database is used in checkKeyFingerprint however most of these functions probably belong in a module +// The database is used in checkKeyFingerprint. However, most of these functions probably belong in a module -// checkKeyFingerprint only checks if key fingerprint has been used as public key, +// checkKeyFingerprint only checks if key fingerprint has been used as a public key, // it is OK to use same key as deploy key for multiple repositories/users. func checkKeyFingerprint(ctx context.Context, fingerprint string) error { has, err := db.Exist[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) diff --git a/models/fixtures/public_key.yml b/models/fixtures/public_key.yml index ae620ee2d19da..856b0e3fb2976 100644 --- a/models/fixtures/public_key.yml +++ b/models/fixtures/public_key.yml @@ -9,3 +9,4 @@ created_unix: 1559593109 updated_unix: 1565224552 login_source_id: 0 + verified: false diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 773e7ca83c869..54ef052a507a9 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -36,8 +36,9 @@ func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model } // ParseCommitWithSignatureCommitter parses a commit's GPG or SSH signature. +// The caller guarantees that the committer user is related to the commit by checking its activated email addresses or no-reply address. // If the commit is singed by an instance key, then committer can be nil. -// If the signature exists, even if committer is nil, the returned CommittingUser will be a non-nil fake user. +// If the signature exists, even if committer is nil, the returned CommittingUser will be a non-nil fake user (e.g.: instance key) func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { // If no signature, just report the committer if c.Signature == nil { @@ -114,20 +115,11 @@ func parseCommitWithGPGSignature(ctx context.Context, c *git.Commit, committer * } } - committerEmailAddresses, _ := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committer.ID, user_model.GetEmailAddresses) - activated := false - for _, e := range committerEmailAddresses { - if e.IsActivated && strings.EqualFold(e.Email, c.Committer.Email) { - activated = true - break - } - } - for _, k := range keys { // Pre-check (& optimization) that emails attached to key can be attached to the committer email and can validate canValidate := false email := "" - if k.Verified && activated { + if k.Verified { canValidate = true email = c.Committer.Email } @@ -217,8 +209,8 @@ func checkKeyEmails(ctx context.Context, email string, keys ...*asymkey_model.GP return true, e.Email } } - if user.KeepEmailPrivate && strings.EqualFold(email, user.GetEmail()) { - return true, user.GetEmail() + if user != nil && strings.EqualFold(email, user.GetPlaceholderEmail()) { + return true, user.GetPlaceholderEmail() } } } @@ -388,21 +380,8 @@ func parseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUs } } - committerEmailAddresses, err := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committerUser.ID, user_model.GetEmailAddresses) - if err != nil { - log.Error("GetEmailAddresses: %v", err) - } - - activated := false - for _, e := range committerEmailAddresses { - if e.IsActivated && strings.EqualFold(e.Email, c.Committer.Email) { - activated = true - break - } - } - for _, k := range keys { - if k.Verified && activated { + if k.Verified { commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, k, committerUser, committerUser, c.Committer.Email) if commitVerification != nil { return commitVerification diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go index 6bcb6997f4376..6edba1e90aff3 100644 --- a/services/asymkey/commit_test.go +++ b/services/asymkey/commit_test.go @@ -7,6 +7,9 @@ import ( "strings" "testing" + asymkey_model "code.gitea.io/gitea/models/asymkey" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" @@ -17,7 +20,49 @@ import ( ) func TestParseCommitWithSSHSignature(t *testing.T) { - // Here we only test the TrustedSSHKeys. The complete signing test is in tests/integration/gpg_ssh_git_test.go + assert.NoError(t, unittest.PrepareTestDatabase()) + + // Here we only need to do some tests that "tests/integration/gpg_ssh_git_test.go" doesn't cover + + // -----BEGIN OPENSSH PRIVATE KEY----- + // b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW + // QyNTUxOQAAACC6T6zF0oPak8dOIzzT1kXB7LrcsVo04SKc3GjuvMllZwAAAJgy08upMtPL + // qQAAAAtzc2gtZWQyNTUxOQAAACC6T6zF0oPak8dOIzzT1kXB7LrcsVo04SKc3GjuvMllZw + // AAAEDWqPHTH51xb4hy1y1f1VeWL/2A9Q0b6atOyv5fx8x5prpPrMXSg9qTx04jPNPWRcHs + // utyxWjThIpzcaO68yWVnAAAAEXVzZXIyQGV4YW1wbGUuY29tAQIDBA== + // -----END OPENSSH PRIVATE KEY----- + sshPubKey, err := asymkey_model.AddPublicKey(t.Context(), 999, "user-ssh-key-any-name", "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILpPrMXSg9qTx04jPNPWRcHsutyxWjThIpzcaO68yWVn", 0) + require.NoError(t, err) + _, err = db.GetEngine(t.Context()).ID(sshPubKey.ID).Cols("verified").Update(&asymkey_model.PublicKey{Verified: true}) + require.NoError(t, err) + + t.Run("UserSSHKey", func(t *testing.T) { + commit, err := git.CommitFromReader(nil, git.Sha1ObjectFormat.EmptyObjectID(), strings.NewReader(`tree a3b1fad553e0f9a2b4a58327bebde36c7da75aa2 +author user2 1752194028 -0700 +committer user2 1752194028 -0700 +gpgsig -----BEGIN SSH SIGNATURE----- + U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAguk+sxdKD2pPHTiM809ZFwey63L + FaNOEinNxo7rzJZWcAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 + AAAAQBfX+6mcKZBnXckwHcBFqRuXMD3vTKi1yv5wgrqIxTyr2LWB97xxmO92cvjsr0POQ2 + 2YA7mQS510Cg2s1uU1XAk= + -----END SSH SIGNATURE----- + +init project +`)) + require.NoError(t, err) + + // the committingUser is guaranteed by the caller, parseCommitWithSSHSignature doesn't do any more checks + committingUser := &user_model.User{ID: 999, Name: "user-x"} + ret := parseCommitWithSSHSignature(t.Context(), commit, committingUser) + require.NotNil(t, ret) + assert.True(t, ret.Verified) + assert.Equal(t, committingUser.Name+" / "+sshPubKey.Fingerprint, ret.Reason) + assert.False(t, ret.Warning) + assert.Equal(t, committingUser, ret.SigningUser) + assert.Equal(t, committingUser, ret.CommittingUser) + assert.Equal(t, sshPubKey.ID, ret.SigningSSHKey.ID) + }) + t.Run("TrustedSSHKey", func(t *testing.T) { defer test.MockVariableValue(&setting.Repository.Signing.SigningName, "gitea")() defer test.MockVariableValue(&setting.Repository.Signing.SigningEmail, "gitea@fake.local")()