Skip to content

Commit 8885844

Browse files
committed
[v13.0/forgejo] fix: prevent commit API from leaking user's hidden email address on valid GPG signed commits
1 parent a2068a4 commit 8885844

File tree

3 files changed

+130
-1
lines changed

3 files changed

+130
-1
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-
2+
id: 7
3+
owner_id: 2
4+
key_id: C21381CD63F609EE
5+
primary_key_id:
6+
content: xsBNBGjmzbQBCADPn1Cl1LibN3K7+9A+If/RjDdTfFVbGk7A8pEDpkgzkmbIcMsi+WvPypbnoTJWyc4E5rsj7MYU6NKK3HADFX4rI6h/732xLmK/R+ekeT8wv+XKO1ncTtm/YITC5qRl84xeYfg4DuGGikhCc8xj8ZKWrx5r1ekISidyvNDJrrcHFToqlqGX74w9mRSSQJm1RayX+5nDph7qOpV8ZCpZT2LS00BYkYO62OHl2Ecm+qWiLyyA4iDR4DVSR9qyLzlXdEI2qQXf28zLV4YPaHckNWdRnaqlr/Mlbd+rbZt/49IBDgXPc+EGo6OXYmZls7pyPSCyirz/ObIyM/9t0JIh8rFrABEBAAE=
7+
verified: true
8+
can_sign: true
9+
can_encrypt_comms: true
10+
can_encrypt_storage: true
11+
can_certify: true
12+
emails: "[{\"ID\":8,\"UID\":1,\"Email\":\"[email protected]\",\"LowerEmail\":\"[email protected]\",\"IsActivated\":true,\"IsPrimary\":false}]"
13+
14+
-
15+
id: 8
16+
owner_id: 2
17+
key_id: 9836974DF1195913
18+
primary_key_id: C21381CD63F609EE
19+
content: zsBNBGjmzbQBCADUa4mDg7owJwoBGi85flwoNw/QOvZew2LSZh7++dt+ClKcxryDpkAF4jmPsmKZEjhYfe4QE53E6jlYXRhprISONuR7gYPvvgCrvmEQKoh6o/tOlv0I4Ngix3SkE44u/CpFEVOrmFHBUvjgRHreOhwL3eyUao3PVte4kyAmvzZuGCmFFr7e0D48EGVbSLTGGj/GtHArnWPbs96S6qjXU/4qSvHsmNbGousgbovxXF/AxqIzyjvXheNEEQzUPjainDlOi3Z9BPJXr7T3ytt3slp48teKEXAC/uGVvFICwGN3WRCG8XsVwIQLUE23xclVdVqdzkO4Jr1D+Gtg95fYC2PvABEBAAE=
20+
verified: false
21+
can_sign: true
22+
can_encrypt_comms: true
23+
can_encrypt_storage: true
24+
can_certify: true

services/convert/convert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerifi
236236
if verif.SigningUser != nil {
237237
commitVerification.Signer = &api.PayloadUser{
238238
Name: verif.SigningUser.Name,
239-
Email: verif.SigningUser.Email,
239+
Email: verif.SigningEmail,
240240
}
241241
}
242242
return commitVerification

services/convert/convert_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright 2025 The Forgejo Authors. All rights reserved.
2+
// SPDX-License-Identifier: GPL-3.0-or-later
3+
4+
package convert
5+
6+
import (
7+
"testing"
8+
9+
"forgejo.org/models/db"
10+
"forgejo.org/models/unittest"
11+
user_model "forgejo.org/models/user"
12+
"forgejo.org/modules/git"
13+
api "forgejo.org/modules/structs"
14+
15+
"github.com/stretchr/testify/assert"
16+
"github.com/stretchr/testify/require"
17+
)
18+
19+
func TestToVerification(t *testing.T) {
20+
defer unittest.OverrideFixtures("models/fixtures/TestParseCommitWithSSHSignature")()
21+
require.NoError(t, unittest.PrepareTestDatabase())
22+
23+
// Change the user's primary email address to ensure this value isn't ambiguous with any other return value from
24+
// signature verification.
25+
userModel := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
26+
userModel.Email = "[email protected]"
27+
db.GetEngine(t.Context()).ID(userModel.ID).Cols("email").Update(userModel)
28+
29+
t.Run("SSH Key Signature", func(t *testing.T) {
30+
commit := &git.Commit{
31+
Committer: &git.Signature{
32+
33+
},
34+
Signature: &git.ObjectSignature{
35+
Payload: `tree 853694aae8816094a0d875fee7ea26278dbf5d0f
36+
parent c2780d5c313da2a947eae22efd7dacf4213f4e7f
37+
author user2 <[email protected]> 1699707877 +0100
38+
committer user2 <[email protected]> 1699707877 +0100
39+
40+
Add content
41+
`,
42+
Signature: `-----BEGIN SSH SIGNATURE-----
43+
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgoGSe9Zy7Ez9bSJcaTNjh/Y7p95
44+
f5DujjqkpzFRtw6CEAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
45+
AAAAQBe2Fwk/FKY3SBCnG6jSYcO6ucyahp2SpQ/0P+otslzIHpWNW8cQ0fGLdhhaFynJXQ
46+
fs9cMpZVM9BfIKNUSO8QY=
47+
-----END SSH SIGNATURE-----
48+
`,
49+
},
50+
}
51+
commitVerification := ToVerification(t.Context(), commit)
52+
require.NotNil(t, commitVerification)
53+
assert.Equal(t, &api.PayloadCommitVerification{
54+
Verified: true,
55+
Reason: "user2 / SHA256:TKfwbZMR7e9OnlV2l1prfah1TXH8CmqR0PvFEXVCXA4",
56+
Signature: "-----BEGIN SSH SIGNATURE-----\nU1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgoGSe9Zy7Ez9bSJcaTNjh/Y7p95\nf5DujjqkpzFRtw6CEAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5\nAAAAQBe2Fwk/FKY3SBCnG6jSYcO6ucyahp2SpQ/0P+otslzIHpWNW8cQ0fGLdhhaFynJXQ\nfs9cMpZVM9BfIKNUSO8QY=\n-----END SSH SIGNATURE-----\n",
57+
Signer: &api.PayloadUser{
58+
Name: "user2",
59+
Email: "[email protected]", // expected email will match the commit's committer's email, regardless of `KeepEmailPrivate`.
60+
},
61+
Payload: "tree 853694aae8816094a0d875fee7ea26278dbf5d0f\nparent c2780d5c313da2a947eae22efd7dacf4213f4e7f\nauthor user2 <[email protected]> 1699707877 +0100\ncommitter user2 <[email protected]> 1699707877 +0100\n\nAdd content\n",
62+
}, commitVerification)
63+
})
64+
65+
t.Run("GPG Signature", func(t *testing.T) {
66+
commit := &git.Commit{
67+
ID: git.MustIDFromString("e20aa0bcd2878f65a93de68a3eed9045d6efdd74"),
68+
Committer: &git.Signature{
69+
70+
},
71+
Signature: &git.ObjectSignature{
72+
Payload: `tree e20aa0bcd2878f65a93de68a3eed9045d6efdd74
73+
parent 5cd9b9847563eb730d63d23c1f1b84868e52ae7d
74+
author user2 <[email protected]> 1759956520 -0600
75+
committer user2 <[email protected]> 1759956520 -0600
76+
77+
Add content
78+
`,
79+
Signature: `-----BEGIN PGP SIGNATURE-----
80+
81+
iQEzBAABCgAdFiEEdlqhn25IEoMmvK5vmDaXTfEZWRMFAmjmzigACgkQmDaXTfEZ
82+
WROC4ggAs8mD8csA6FV5e2v/4HcxuaZKCN+D8Gvku2JUigODQCA+NOX0FF2jDnCh
83+
tXylBPB4HJw1spKkDLtOpnCUSOniBdl9NcZjnBt6sP/OSnEfLznXFra+9fCHzsu0
84+
9uhDn3Wn1iHWXQ2ZglUwVS0ja6pNgEip8wNZBysv8+XbO1CEEW0m7zQA6tunzIwp
85+
yiPZDUJrKtpKAK0+v19EccT2VjYAa+Vo+p3/E0piaTYNbsTqtFRy63tdjDkf+mo+
86+
l/PaPhrMqdnbxv3/sd/63VCNdvPH3f0+OuydcC7mXyysmvap99EC+QKnpsrm7RAP
87+
uf51WIBywxztet6vi+jYJK1jFoY4iA==
88+
=Lnrt
89+
-----END PGP SIGNATURE-----`,
90+
},
91+
}
92+
commitVerification := ToVerification(t.Context(), commit)
93+
require.NotNil(t, commitVerification)
94+
assert.Equal(t, &api.PayloadCommitVerification{
95+
Verified: true,
96+
Reason: "user2 / 9836974DF1195913",
97+
Signature: "-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCgAdFiEEdlqhn25IEoMmvK5vmDaXTfEZWRMFAmjmzigACgkQmDaXTfEZ\nWROC4ggAs8mD8csA6FV5e2v/4HcxuaZKCN+D8Gvku2JUigODQCA+NOX0FF2jDnCh\ntXylBPB4HJw1spKkDLtOpnCUSOniBdl9NcZjnBt6sP/OSnEfLznXFra+9fCHzsu0\n9uhDn3Wn1iHWXQ2ZglUwVS0ja6pNgEip8wNZBysv8+XbO1CEEW0m7zQA6tunzIwp\nyiPZDUJrKtpKAK0+v19EccT2VjYAa+Vo+p3/E0piaTYNbsTqtFRy63tdjDkf+mo+\nl/PaPhrMqdnbxv3/sd/63VCNdvPH3f0+OuydcC7mXyysmvap99EC+QKnpsrm7RAP\nuf51WIBywxztet6vi+jYJK1jFoY4iA==\n=Lnrt\n-----END PGP SIGNATURE-----",
98+
Signer: &api.PayloadUser{
99+
Name: "user2",
100+
Email: "[email protected]", // expected email will match the signing key's email
101+
},
102+
Payload: "tree e20aa0bcd2878f65a93de68a3eed9045d6efdd74\nparent 5cd9b9847563eb730d63d23c1f1b84868e52ae7d\nauthor user2 <[email protected]> 1759956520 -0600\ncommitter user2 <[email protected]> 1759956520 -0600\n\nAdd content\n",
103+
}, commitVerification)
104+
})
105+
}

0 commit comments

Comments
 (0)