Skip to content

Commit 90919bb

Browse files
zeripathzeripath
authored
Show Signer in commit lists and add basic trust (#10425)
* Show Signer in commit lists and add basic trust Show the avatar of the signer in the commit list pages as we do not enforce that the signer is an author or committer. This makes it clearer who has signed the commit. Also display commits signed by non-members differently from members and in particular make it clear when a non-member signer is different from the committer to help reduce the risk of spoofing. Signed-off-by: Andrew Thornton <[email protected]> * ensure orange text and background is available Signed-off-by: Andrew Thornton <[email protected]> * Update gpg_key.go * Update models/gpg_key.go * Apply suggestions from code review * Require team collaborators to have access to UnitTypeCode * as per @6543 * fix position of sha as per @silverwind * as per @guillep2k
1 parent 858aebc commit 90919bb

File tree

16 files changed

+416
-70
lines changed

16 files changed

+416
-70
lines changed

‎docs/content/doc/features/comparison.en-us.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ _Symbols used in table:_
6060
| Git LFS 2.0 | ✓ | ✘ | ✓ | ✓ | ✓ | ⁄ | ✓ |
6161
| Group Milestones | ✘ | ✘ | ✘ | ✓ | ✓ | ✘ | ✘ |
6262
| Granular user roles (Code, Issues, Wiki etc) | ✓ | ✘ | ✘ | ✓ | ✓ | ✘ | ✘ |
63-
| Verified Committer | ✘ | ✘ | ? | ✓ | ✓ | ✓ | ✘ |
63+
| Verified Committer | ⁄ | ✘ | ? | ✓ | ✓ | ✓ | ✘ |
6464
| GPG Signed Commits | ✓ | ✘ | ✓ | ✓ | ✓ | ✓ | ✓ |
6565
| Reject unsigned commits | [✓](https://github.com/go-gitea/gitea/pull/9708) | ✘ | ✓ | ✓ | ✓ | ✘ | ✓ |
6666
| Repository Activity page | ✓ | ✘ | ✓ | ✓ | ✓ | ✓ | ✓ |

‎models/gpg_key.go

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ type CommitVerification struct {
374374
CommittingUser *User
375375
SigningEmail string
376376
SigningKey *GPGKey
377+
TrustStatus string
377378
}
378379

379380
// SignCommit represents a commit with validation of signature.
@@ -759,18 +760,54 @@ func verifyWithGPGSettings(gpgSettings *git.GPGSettings, sig *packet.Signature,
759760
}
760761

761762
// ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys.
762-
func ParseCommitsWithSignature(oldCommits *list.List) *list.List {
763+
func ParseCommitsWithSignature(oldCommits *list.List, repository *Repository) *list.List {
763764
var (
764765
newCommits = list.New()
765766
e = oldCommits.Front()
766767
)
768+
memberMap := map[int64]bool{}
769+
767770
for e != nil {
768771
c := e.Value.(UserCommit)
769-
newCommits.PushBack(SignCommit{
772+
signCommit := SignCommit{
770773
UserCommit: &c,
771774
Verification: ParseCommitWithSignature(c.Commit),
772-
})
775+
}
776+
777+
_ = CalculateTrustStatus(signCommit.Verification, repository, &memberMap)
778+
779+
newCommits.PushBack(signCommit)
773780
e = e.Next()
774781
}
775782
return newCommits
776783
}
784+
785+
// CalculateTrustStatus will calculate the TrustStatus for a commit verification within a repository
786+
func CalculateTrustStatus(verification *CommitVerification, repository *Repository, memberMap *map[int64]bool) (err error) {
787+
if verification.Verified {
788+
verification.TrustStatus = "trusted"
789+
if verification.SigningUser.ID != 0 {
790+
var isMember bool
791+
if memberMap != nil {
792+
var has bool
793+
isMember, has = (*memberMap)[verification.SigningUser.ID]
794+
if !has {
795+
isMember, err = repository.IsOwnerMemberCollaborator(verification.SigningUser.ID)
796+
(*memberMap)[verification.SigningUser.ID] = isMember
797+
}
798+
} else {
799+
isMember, err = repository.IsOwnerMemberCollaborator(verification.SigningUser.ID)
800+
}
801+
802+
if !isMember {
803+
verification.TrustStatus = "untrusted"
804+
if verification.CommittingUser.ID != verification.SigningUser.ID {
805+
// The committing user and the signing user are not the same and are not the default key
806+
// This should be marked as questionable unless the signing user is a collaborator/team member etc.
807+
verification.TrustStatus = "unmatched"
808+
}
809+
}
810+
}
811+
}
812+
return
813+
}

‎models/repo_collaboration.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,23 @@ func (repo *Repository) getRepoTeams(e Engine) (teams []*Team, err error) {
210210
func (repo *Repository) GetRepoTeams() ([]*Team, error) {
211211
return repo.getRepoTeams(x)
212212
}
213+
214+
// IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository
215+
func (repo *Repository) IsOwnerMemberCollaborator(userID int64) (bool, error) {
216+
if repo.OwnerID == userID {
217+
return true, nil
218+
}
219+
teamMember, err := x.Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id").
220+
Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id").
221+
Where("team_repo.repo_id = ?", repo.ID).
222+
And("team_unit.`type` = ?", UnitTypeCode).
223+
And("team_user.uid = ?", userID).Table("team_user").Exist(&TeamUser{})
224+
if err != nil {
225+
return false, err
226+
}
227+
if teamMember {
228+
return true, nil
229+
}
230+
231+
return x.Get(&Collaboration{RepoID: repo.ID, UserID: userID})
232+
}

‎options/locale/locale_en-US.ini

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,8 @@ commits.date = Date
809809
commits.older = Older
810810
commits.newer = Newer
811811
commits.signed_by = Signed by
812+
commits.signed_by_untrusted_user = Signed by untrusted user
813+
commits.signed_by_untrusted_user_unmatched = Signed by untrusted user who does not match committer
812814
commits.gpg_key_id = GPG Key ID
813815
814816
ext_issues = Ext. Issues

‎routers/private/hook.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,8 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error {
9090
if err != nil {
9191
return err
9292
}
93-
log.Info("have commit %s", commit.ID.String())
9493
verification := models.ParseCommitWithSignature(commit)
9594
if !verification.Verified {
96-
log.Info("unverified commit %s", commit.ID.String())
9795
cancel()
9896
return &errUnverifiedCommit{
9997
commit.ID.String(),

‎routers/repo/commit.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func Commits(ctx *context.Context) {
7070
return
7171
}
7272
commits = models.ValidateCommitsWithEmails(commits)
73-
commits = models.ParseCommitsWithSignature(commits)
73+
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
7474
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
7575
ctx.Data["Commits"] = commits
7676

@@ -139,7 +139,7 @@ func SearchCommits(ctx *context.Context) {
139139
return
140140
}
141141
commits = models.ValidateCommitsWithEmails(commits)
142-
commits = models.ParseCommitsWithSignature(commits)
142+
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
143143
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
144144
ctx.Data["Commits"] = commits
145145

@@ -185,7 +185,7 @@ func FileHistory(ctx *context.Context) {
185185
return
186186
}
187187
commits = models.ValidateCommitsWithEmails(commits)
188-
commits = models.ParseCommitsWithSignature(commits)
188+
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
189189
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
190190
ctx.Data["Commits"] = commits
191191

@@ -269,12 +269,18 @@ func Diff(ctx *context.Context) {
269269
setPathsCompareContext(ctx, parentCommit, commit, headTarget)
270270
ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
271271
ctx.Data["Commit"] = commit
272-
ctx.Data["Verification"] = models.ParseCommitWithSignature(commit)
272+
verification := models.ParseCommitWithSignature(commit)
273+
ctx.Data["Verification"] = verification
273274
ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
274275
ctx.Data["Diff"] = diff
275276
ctx.Data["Parents"] = parents
276277
ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0
277278

279+
if err := models.CalculateTrustStatus(verification, ctx.Repo.Repository, nil); err != nil {
280+
ctx.ServerError("CalculateTrustStatus", err)
281+
return
282+
}
283+
278284
note := &git.Note{}
279285
err = git.GetNote(ctx.Repo.GitRepo, commitID, note)
280286
if err == nil {

‎routers/repo/compare.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func PrepareCompareDiff(
316316
}
317317

318318
compareInfo.Commits = models.ValidateCommitsWithEmails(compareInfo.Commits)
319-
compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits)
319+
compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits, headRepo)
320320
compareInfo.Commits = models.ParseCommitsWithStatus(compareInfo.Commits, headRepo)
321321
ctx.Data["Commits"] = compareInfo.Commits
322322
ctx.Data["CommitCount"] = compareInfo.Commits.Len()

‎routers/repo/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ func ViewPullCommits(ctx *context.Context) {
495495
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
496496
commits = prInfo.Commits
497497
commits = models.ValidateCommitsWithEmails(commits)
498-
commits = models.ParseCommitsWithSignature(commits)
498+
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
499499
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
500500
ctx.Data["Commits"] = commits
501501
ctx.Data["CommitCount"] = commits.Len()

‎routers/repo/view.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,14 @@ func renderDirectory(ctx *context.Context, treeLink string) {
333333
// Show latest commit info of repository in table header,
334334
// or of directory if not in root directory.
335335
ctx.Data["LatestCommit"] = latestCommit
336-
ctx.Data["LatestCommitVerification"] = models.ParseCommitWithSignature(latestCommit)
336+
verification := models.ParseCommitWithSignature(latestCommit)
337+
338+
if err := models.CalculateTrustStatus(verification, ctx.Repo.Repository, nil); err != nil {
339+
ctx.ServerError("CalculateTrustStatus", err)
340+
return
341+
}
342+
ctx.Data["LatestCommitVerification"] = verification
343+
337344
ctx.Data["LatestCommitUser"] = models.ValidateCommitWithEmail(latestCommit)
338345

339346
statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, ctx.Repo.Commit.ID.String(), 0)

‎routers/repo/wiki.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
284284
return nil, nil
285285
}
286286
commitsHistory = models.ValidateCommitsWithEmails(commitsHistory)
287-
commitsHistory = models.ParseCommitsWithSignature(commitsHistory)
287+
commitsHistory = models.ParseCommitsWithSignature(commitsHistory, ctx.Repo.Repository)
288288

289289
ctx.Data["Commits"] = commitsHistory
290290

‎templates/repo/commit_page.tmpl

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,22 @@
22
<div class="repository diff">
33
{{template "repo/header" .}}
44
<div class="ui container {{if .IsSplitStyle}}fluid padded{{end}}">
5-
<div class="ui top attached info clearing segment {{if .Commit.Signature}} isSigned {{if .Verification.Verified }} isVerified {{end}}{{end}}">
5+
{{$class := ""}}
6+
{{if .Commit.Signature}}
7+
{{$class = (printf "%s%s" $class " isSigned")}}
8+
{{if .Verification.Verified}}
9+
{{if eq .Verification.TrustStatus "trusted"}}
10+
{{$class = (printf "%s%s" $class " isVerified")}}
11+
{{else if eq .Verification.TrustStatus "untrusted"}}
12+
{{$class = (printf "%s%s" $class " isVerifiedUntrusted")}}
13+
{{else}}
14+
{{$class = (printf "%s%s" $class " isVerifiedUnmatched")}}
15+
{{end}}
16+
{{else if .Verification.Warning}}
17+
{{$class = (printf "%s%s" $class " isWarning")}}
18+
{{end}}
19+
{{end}}
20+
<div class="ui top attached info clearing segment {{$class}}">
621
<a class="ui floated right blue tiny button" href="{{EscapePound .SourcePath}}">
722
{{.i18n.Tr "repo.diff.browse_source"}}
823
</a>
@@ -12,15 +27,15 @@
1227
{{end}}
1328
<span class="text grey">{{svg "octicon-git-branch" 16}}{{.BranchName}}</span>
1429
</div>
15-
<div class="ui attached info segment {{if .Commit.Signature}} isSigned {{if .Verification.Verified }} isVerified {{end}}{{end}}">
30+
<div class="ui attached info segment {{$class}}">
1631
<div class="ui stackable grid">
1732
<div class="nine wide column">
1833
{{if .Author}}
1934
<img class="ui avatar image" src="{{.Author.RelAvatarLink}}" />
2035
{{if .Author.FullName}}
21-
<a href="{{.Author.HomeLink}}"><strong>{{.Author.FullName}}</strong></a> {{if .IsSigned}}<{{.Commit.Author.Email}}>{{end}}
36+
<a href="{{.Author.HomeLink}}"><strong>{{.Author.FullName}}</strong> {{if .IsSigned}}&lt;{{.Commit.Author.Email}}&gt;{{end}}</a>
2237
{{else}}
23-
<a href="{{.Author.HomeLink}}"><strong>{{.Commit.Author.Name}}</strong></a> {{if .IsSigned}}<{{.Commit.Author.Email}}>{{end}}
38+
<a href="{{.Author.HomeLink}}"><strong>{{.Commit.Author.Name}}</strong> {{if .IsSigned}}&lt;{{.Commit.Author.Email}}&gt;{{end}}</a>
2439
{{end}}
2540
{{else}}
2641
<img class="ui avatar image" src="{{AvatarLink .Commit.Author.Email}}" />
@@ -30,7 +45,7 @@
3045
<span> </span>
3146
{{if ne .Verification.CommittingUser.ID 0}}
3247
<img class="ui avatar image" src="{{.Verification.CommittingUser.RelAvatarLink}}" />
33-
<a href="{{.Verification.CommittingUser.HomeLink}}"><strong>{{.Commit.Committer.Name}}</strong></a> <{{.Commit.Committer.Email}}>
48+
<a href="{{.Verification.CommittingUser.HomeLink}}"><strong>{{.Commit.Committer.Name}}</strong> &lt;{{.Commit.Committer.Email}}&gt;</a>
3449
{{else}}
3550
<img class="ui avatar image" src="{{AvatarLink .Commit.Committer.Email}}" />
3651
<strong>{{.Commit.Committer.Name}}</strong>
@@ -58,40 +73,42 @@
5873
</div><!-- end grid -->
5974
</div>
6075
{{if .Commit.Signature}}
61-
{{if .Verification.Verified }}
62-
<div class="ui bottom attached positive message">
76+
<div class="ui bottom attached message {{$class}}">
77+
{{if .Verification.Verified }}
6378
{{if ne .Verification.SigningUser.ID 0}}
64-
<i class="green lock icon"></i>
65-
<span>{{.i18n.Tr "repo.commits.signed_by"}}:</span>
79+
<i class="lock icon"></i>
80+
{{if eq .Verification.TrustStatus "trusted"}}
81+
<span class="ui text">{{.i18n.Tr "repo.commits.signed_by"}}:</span>
82+
{{else if eq .Verification.TrustStatus "untrusted"}}
83+
<span class="ui text">{{.i18n.Tr "repo.commits.signed_by_untrusted_user"}}:</span>
84+
{{else}}
85+
<span class="ui text">{{.i18n.Tr "repo.commits.signed_by_untrusted_user_unmatched"}}:</span>
86+
{{end}}
6687
<img class="ui avatar image" src="{{.Verification.SigningUser.RelAvatarLink}}" />
67-
<a href="{{.Verification.SigningUser.HomeLink}}"><strong>{{.Verification.SigningUser.Name}}</strong></a> <{{.Verification.SigningEmail}}>
68-
<span class="pull-right"><span>{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> {{.Verification.SigningKey.KeyID}}</span>
88+
<a href="{{.Verification.SigningUser.HomeLink}}"><strong>{{.Verification.SigningUser.Name}}</strong> <{{.Verification.SigningEmail}}></a>
89+
<span class="pull-right"><span class="ui text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> {{.Verification.SigningKey.KeyID}}</span>
6990
{{else}}
7091
<i class="icons" title="{{.i18n.Tr "gpg.default_key"}}">
71-
<i class="green lock icon"></i>
92+
<i class="lock icon"></i>
7293
<i class="tiny inverted cog icon centerlock"></i>
7394
</i>
74-
<span>{{.i18n.Tr "repo.commits.signed_by"}}:</span>
95+
<span class="ui text">{{.i18n.Tr "repo.commits.signed_by"}}:</span>
7596
<img class="ui avatar image" src="{{AvatarLink .Verification.SigningEmail}}" />
7697
<strong>{{.Verification.SigningUser.Name}}</strong> <{{.Verification.SigningEmail}}>
77-
<span class="pull-right"><span>{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="cogs icon" title="{{.i18n.Tr "gpg.default_key"}}"></i>{{.Verification.SigningKey.KeyID}}</span>
98+
<span class="pull-right"><span class="ui text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="cogs icon" title="{{.i18n.Tr "gpg.default_key"}}"></i>{{.Verification.SigningKey.KeyID}}</span>
7899
{{end}}
79-
</div>
80-
{{else if .Verification.Warning}}
81-
<div class="ui bottom attached message">
82-
<i class="red unlock icon"></i>
83-
<span class="red text">{{.i18n.Tr .Verification.Reason}}</span>
84-
<span class="pull-right"><span class="red text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="red warning icon"></i>{{.Verification.SigningKey.KeyID}}</span>
85-
</div>
86-
{{else}}
87-
<div class="ui bottom attached message">
88-
<i class="grey unlock icon"></i>
100+
{{else if .Verification.Warning}}
101+
<i class="unlock icon"></i>
102+
<span class="ui text">{{.i18n.Tr .Verification.Reason}}</span>
103+
<span class="pull-right"><span class="ui text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="warning icon"></i>{{.Verification.SigningKey.KeyID}}</span>
104+
{{else}}
105+
<i class="unlock icon"></i>
89106
{{.i18n.Tr .Verification.Reason}}
90107
{{if and .Verification.SigningKey (ne .Verification.SigningKey.KeyID "")}}
91-
<span class="pull-right"><span class="red text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="red warning icon"></i>{{.Verification.SigningKey.KeyID}}</span>
108+
<span class="pull-right"><span class="ui text">{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> <i class="warning icon"></i>{{.Verification.SigningKey.KeyID}}</span>
92109
{{end}}
93-
</div>
94-
{{end}}
110+
{{end}}
111+
</div>
95112
{{end}}
96113
{{if .Note}}
97114
<div class="ui top attached info segment message git-notes">

‎templates/repo/commits_list.tmpl

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@
2828
{{if .Signature}}
2929
{{$class = (printf "%s%s" $class " isSigned")}}
3030
{{if .Verification.Verified}}
31-
{{$class = (printf "%s%s" $class " isVerified")}}
31+
{{if eq .Verification.TrustStatus "trusted"}}
32+
{{$class = (printf "%s%s" $class " isVerified")}}
33+
{{else if eq .Verification.TrustStatus "untrusted"}}
34+
{{$class = (printf "%s%s" $class " isVerifiedUntrusted")}}
35+
{{else}}
36+
{{$class = (printf "%s%s" $class " isVerifiedUnmatched")}}
37+
{{end}}
3238
{{else if .Verification.Warning}}
3339
{{$class = (printf "%s%s" $class " isWarning")}}
3440
{{end}}
@@ -38,20 +44,22 @@
3844
{{else}}
3945
<span class="{{$class}}">
4046
{{end}}
41-
{{ShortSha .ID.String}}
47+
<span class="shortsha">{{ShortSha .ID.String}}</span>
4248
{{if .Signature}}
4349
<div class="ui detail icon button">
4450
{{if .Verification.Verified}}
45-
{{if ne .Verification.SigningUser.ID 0}}
46-
<i title="{{.Verification.Reason}}" class="lock green icon"></i>
47-
{{else}}
48-
<i title="{{.Verification.Reason}}" class="icons">
49-
<i class="green lock icon"></i>
50-
<i class="tiny inverted cog icon centerlock"></i>
51-
</i>
52-
{{end}}
53-
{{else if .Verification.Warning}}
54-
<i title="{{$.i18n.Tr .Verification.Reason}}" class="red unlock icon"></i>
51+
<div title="{{if eq .Verification.TrustStatus "trusted"}}{{else if eq .Verification.TrustStatus "untrusted"}}{{$.i18n.Tr "repo.commits.signed_by_untrusted_user"}}: {{else}}{{$.i18n.Tr "repo.commits.signed_by_untrusted_user_unmatched"}}: {{end}}{{.Verification.Reason}}">
52+
{{if ne .Verification.SigningUser.ID 0}}
53+
<i class="lock icon"></i>
54+
<img class="ui signature avatar image" src="{{.Verification.SigningUser.RelAvatarLink}}" />
55+
{{else}}
56+
<i title="{{.Verification.Reason}}" class="icons">
57+
<i class="lock icon"></i>
58+
<i class="tiny inverted cog icon centerlock"></i>
59+
</i>
60+
<img class="ui signature avatar image" src="{{AvatarLink .Verification.SigningEmail}}" />
61+
{{end}}
62+
</div>
5563
{{else}}
5664
<i title="{{$.i18n.Tr .Verification.Reason}}" class="unlock icon"></i>
5765
{{end}}

0 commit comments

Comments
 (0)