Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sign protected branches #8993

Merged
merged 14 commits into from
Dec 15, 2019
Merged
1 change: 1 addition & 0 deletions custom/conf/app.ini.sample
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ WIKI = never
; Determines when to sign on merges
; - basesigned: require that the parent of commit on the base repo is signed.
; - commitssigned: require that all the commits in the head branch are signed.
; - approved: only sign when merging an approved pr to a protected branch
MERGES = pubkey, twofa, basesigned, commitssigned

[cors]
Expand Down
3 changes: 2 additions & 1 deletion docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
- `CRUD_ACTIONS`: **pubkey, twofa, parentsigned**: \[never, pubkey, twofa, parentsigned, always\]: Sign CRUD actions.
- Options as above, with the addition of:
- `parentsigned`: Only sign if the parent commit is signed.
- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, basesigned, commitssigned, always\]: Sign merges.
- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, approved, basesigned, commitssigned, always\]: Sign merges.
- `approved`: Only sign approved merges to a protected branch.
- `basesigned`: Only sign if the parent commit in the base repo is signed.
- `headsigned`: Only sign if the head commit in the head branch is signed.
- `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/signing.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ The possible options are:
* `basesigned`: Only sign if the parent commit in the base repo is signed.
* `headsigned`: Only sign if the head commit in the head branch is signed.
* `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.
* `approved`: Only sign approved merges to a protected branch.
* `always`: Always sign

Options other than `never` and `always` can be combined as a comma
Expand Down
121 changes: 121 additions & 0 deletions models/pull_sign.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package models

import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)

// SignMerge determines if we should sign a PR merge commit to the base repository
func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about move these new codes to services/pull?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So SignInitialCommit has to currently be in models and given the rest of this code is in models - it probably makes more sense to keep it there until we can get SignInitialCommit out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can move them step by step so that it will be easy to review. There is some intermediate state that some parts in models, some parts in pull but I think that's OK.

if err := pr.GetBaseRepo(); err != nil {
log.Error("Unable to get Base Repo for pull request")
return false, ""
}
repo := pr.BaseRepo

signingKey := signingKey(repo.RepoPath())
if signingKey == "" {
lafriks marked this conversation as resolved.
Show resolved Hide resolved
return false, ""
}
rules := signingModeFromStrings(setting.Repository.Signing.Merges)

var gitRepo *git.Repository
var err error

for _, rule := range rules {
switch rule {
case never:
return false, ""
case always:
break
case pubkey:
keys, err := ListGPGKeys(u.ID)
if err != nil || len(keys) == 0 {
return false, ""
}
case twofa:
twofa, err := GetTwoFactorByUID(u.ID)
if err != nil || twofa == nil {
return false, ""
}
case approved:
protectedBranch, err := GetProtectedBranchBy(repo.ID, pr.BaseBranch)
if err != nil || protectedBranch == nil {
return false, ""
}
if protectedBranch.GetGrantedApprovalsCount(pr) < 1 {
lafriks marked this conversation as resolved.
Show resolved Hide resolved
return false, ""
}
case baseSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(baseCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
case headSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(headCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
case commitsSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(headCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
// need to work out merge-base
mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
if err != nil {
return false, ""
}
commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
if err != nil {
return false, ""
}
for e := commitList.Front(); e != nil; e = e.Next() {
commit = e.Value.(*git.Commit)
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
}
}
}
return true, signingKey
}
98 changes: 3 additions & 95 deletions models/repo_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const (
baseSigned signingMode = "basesigned"
headSigned signingMode = "headsigned"
commitsSigned signingMode = "commitssigned"
approved signingMode = "approved"
)

func signingModeFromStrings(modeStrings []string) []signingMode {
Expand All @@ -45,6 +46,8 @@ func signingModeFromStrings(modeStrings []string) []signingMode {
fallthrough
case headSigned:
fallthrough
case approved:
fallthrough
case commitsSigned:
returnable = append(returnable, signMode)
}
Expand Down Expand Up @@ -211,98 +214,3 @@ func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string
}
return true, signingKey
}

// SignMerge determines if we should sign a merge commit to this repository
func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
rules := signingModeFromStrings(setting.Repository.Signing.Merges)
signingKey := signingKey(repo.RepoPath())
if signingKey == "" {
return false, ""
}
var gitRepo *git.Repository
var err error

for _, rule := range rules {
switch rule {
case never:
return false, ""
case always:
break
case pubkey:
keys, err := ListGPGKeys(u.ID)
if err != nil || len(keys) == 0 {
return false, ""
}
case twofa:
twofa, err := GetTwoFactorByUID(u.ID)
if err != nil || twofa == nil {
return false, ""
}
case baseSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(baseCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
case headSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(headCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
case commitsSigned:
if gitRepo == nil {
gitRepo, err = git.OpenRepository(tmpBasePath)
if err != nil {
return false, ""
}
defer gitRepo.Close()
}
commit, err := gitRepo.GetCommit(headCommit)
if err != nil {
return false, ""
}
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
// need to work out merge-base
mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
if err != nil {
return false, ""
}
commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
if err != nil {
return false, ""
}
for e := commitList.Front(); e != nil; e = e.Next() {
commit = e.Value.(*git.Commit)
verification := ParseCommitWithSignature(commit)
if !verification.Verified {
return false, ""
}
}
}
}
return true, signingKey
}
2 changes: 1 addition & 1 deletion services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
// Determine if we should sign
signArg := ""
if version.Compare(binVersion, "1.7.9", ">=") {
sign, keyID := pr.BaseRepo.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
sign, keyID := pr.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
if sign {
signArg = "-S" + keyID
} else if version.Compare(binVersion, "2.0.0", ">=") {
Expand Down