diff --git a/models/branches.go b/models/branches.go index b6398f5694..75f5c0a3a7 100644 --- a/models/branches.go +++ b/models/branches.go @@ -46,6 +46,7 @@ type ProtectedBranch struct { RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"` BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"` DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` + RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` diff --git a/models/error.go b/models/error.go index f0d5699aad..fe9af70f3a 100644 --- a/models/error.go +++ b/models/error.go @@ -916,6 +916,22 @@ func (err ErrUserDoesNotHaveAccessToRepo) Error() string { return fmt.Sprintf("user doesn't have acces to repo [user_id: %d, repo_name: %s]", err.UserID, err.RepoName) } +// ErrWontSign explains the first reason why a commit would not be signed +// There may be other reasons - this is just the first reason found +type ErrWontSign struct { + Reason signingMode +} + +func (e *ErrWontSign) Error() string { + return fmt.Sprintf("wont sign: %s", e.Reason) +} + +// IsErrWontSign checks if an error is a ErrWontSign +func IsErrWontSign(err error) bool { + _, ok := err.(*ErrWontSign) + return ok +} + // __________ .__ // \______ \____________ ____ ____ | |__ // | | _/\_ __ \__ \ / \_/ ___\| | \ diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6bdec1dfba..edea36cf79 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -298,6 +298,8 @@ var migrations = []Migration{ NewMigration("Add owner_name on table repository", addOwnerNameOnRepository), // v121 -> v122 NewMigration("add is_restricted column for users table", addIsRestricted), + // v122 -> v123 + NewMigration("Add Require Signed Commits to ProtectedBranch", addRequireSignedCommits), } // Migrate database to current version diff --git a/models/migrations/v122.go b/models/migrations/v122.go new file mode 100644 index 0000000000..e28adc1d82 --- /dev/null +++ b/models/migrations/v122.go @@ -0,0 +1,18 @@ +// Copyright 2020 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 migrations + +import ( + "xorm.io/xorm" +) + +func addRequireSignedCommits(x *xorm.Engine) error { + + type ProtectedBranch struct { + RequireSignedCommits bool `xorm:"NOT NULL DEFAULT false"` + } + + return x.Sync2(new(ProtectedBranch)) +} diff --git a/models/pull.go b/models/pull.go index 0435311e4e..1edd890035 100644 --- a/models/pull.go +++ b/models/pull.go @@ -152,16 +152,18 @@ func (pr *PullRequest) LoadProtectedBranch() (err error) { } func (pr *PullRequest) loadProtectedBranch(e Engine) (err error) { - if pr.BaseRepo == nil { - if pr.BaseRepoID == 0 { - return nil - } - pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID) - if err != nil { - return + if pr.ProtectedBranch == nil { + if pr.BaseRepo == nil { + if pr.BaseRepoID == 0 { + return nil + } + pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID) + if err != nil { + return + } } + pr.ProtectedBranch, err = getProtectedBranchBy(e, pr.BaseRepo.ID, pr.BaseBranch) } - pr.ProtectedBranch, err = getProtectedBranchBy(e, pr.BaseRepo.ID, pr.BaseBranch) return } diff --git a/models/pull_sign.go b/models/pull_sign.go index 19d8907c3d..1d3474abe7 100644 --- a/models/pull_sign.go +++ b/models/pull_sign.go @@ -11,16 +11,16 @@ import ( ) // 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) { +func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string, error) { if err := pr.GetBaseRepo(); err != nil { log.Error("Unable to get Base Repo for pull request") - return false, "" + return false, "", err } repo := pr.BaseRepo signingKey := signingKey(repo.RepoPath()) if signingKey == "" { - return false, "" + return false, "", &ErrWontSign{noKey} } rules := signingModeFromStrings(setting.Repository.Signing.Merges) @@ -30,92 +30,101 @@ func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit st for _, rule := range rules { switch rule { case never: - return false, "" + return false, "", &ErrWontSign{never} case always: break case pubkey: keys, err := ListGPGKeys(u.ID) - if err != nil || len(keys) == 0 { - return false, "" + if err != nil { + return false, "", err + } + if len(keys) == 0 { + return false, "", &ErrWontSign{pubkey} } case twofa: - twofa, err := GetTwoFactorByUID(u.ID) - if err != nil || twofa == nil { - return false, "" + twofaModel, err := GetTwoFactorByUID(u.ID) + if err != nil { + return false, "", err + } + if twofaModel == nil { + return false, "", &ErrWontSign{twofa} } case approved: protectedBranch, err := GetProtectedBranchBy(repo.ID, pr.BaseBranch) - if err != nil || protectedBranch == nil { - return false, "" + if err != nil { + return false, "", err + } + if protectedBranch == nil { + return false, "", &ErrWontSign{approved} } if protectedBranch.GetGrantedApprovalsCount(pr) < 1 { - return false, "" + return false, "", &ErrWontSign{approved} } case baseSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(tmpBasePath) if err != nil { - return false, "" + return false, "", err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(baseCommit) if err != nil { - return false, "" + return false, "", err } verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{baseSigned} } case headSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(tmpBasePath) if err != nil { - return false, "" + return false, "", err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, "" + return false, "", err } verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{headSigned} } case commitsSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(tmpBasePath) if err != nil { - return false, "" + return false, "", err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, "" + return false, "", err } verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{commitsSigned} } // need to work out merge-base mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) if err != nil { - return false, "" + return false, "", err } commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) if err != nil { - return false, "" + return false, "", err } for e := commitList.Front(); e != nil; e = e.Next() { commit = e.Value.(*git.Commit) verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{commitsSigned} } } } } - return true, signingKey + return true, signingKey, nil } diff --git a/models/repo_sign.go b/models/repo_sign.go index a684efb55f..64f70ac7bd 100644 --- a/models/repo_sign.go +++ b/models/repo_sign.go @@ -25,6 +25,7 @@ const ( headSigned signingMode = "headsigned" commitsSigned signingMode = "commitssigned" approved signingMode = "approved" + noKey signingMode = "nokey" ) func signingModeFromStrings(modeStrings []string) []signingMode { @@ -95,122 +96,140 @@ func PublicSigningKey(repoPath string) (string, error) { } // SignInitialCommit determines if we should sign the initial commit to this repository -func SignInitialCommit(repoPath string, u *User) (bool, string) { +func SignInitialCommit(repoPath string, u *User) (bool, string, error) { rules := signingModeFromStrings(setting.Repository.Signing.InitialCommit) signingKey := signingKey(repoPath) if signingKey == "" { - return false, "" + return false, "", &ErrWontSign{noKey} } for _, rule := range rules { switch rule { case never: - return false, "" + return false, "", &ErrWontSign{never} case always: break case pubkey: keys, err := ListGPGKeys(u.ID) - if err != nil || len(keys) == 0 { - return false, "" + if err != nil { + return false, "", err + } + if len(keys) == 0 { + return false, "", &ErrWontSign{pubkey} } case twofa: - twofa, err := GetTwoFactorByUID(u.ID) - if err != nil || twofa == nil { - return false, "" + twofaModel, err := GetTwoFactorByUID(u.ID) + if err != nil { + return false, "", err + } + if twofaModel == nil { + return false, "", &ErrWontSign{twofa} } } } - return true, signingKey + return true, signingKey, nil } // SignWikiCommit determines if we should sign the commits to this repository wiki -func (repo *Repository) SignWikiCommit(u *User) (bool, string) { +func (repo *Repository) SignWikiCommit(u *User) (bool, string, error) { rules := signingModeFromStrings(setting.Repository.Signing.Wiki) signingKey := signingKey(repo.WikiPath()) if signingKey == "" { - return false, "" + return false, "", &ErrWontSign{noKey} } for _, rule := range rules { switch rule { case never: - return false, "" + return false, "", &ErrWontSign{never} case always: break case pubkey: keys, err := ListGPGKeys(u.ID) - if err != nil || len(keys) == 0 { - return false, "" + if err != nil { + return false, "", err + } + if len(keys) == 0 { + return false, "", &ErrWontSign{pubkey} } case twofa: - twofa, err := GetTwoFactorByUID(u.ID) - if err != nil || twofa == nil { - return false, "" + twofaModel, err := GetTwoFactorByUID(u.ID) + if err != nil { + return false, "", err + } + if twofaModel == nil { + return false, "", &ErrWontSign{twofa} } case parentSigned: gitRepo, err := git.OpenRepository(repo.WikiPath()) if err != nil { - return false, "" + return false, "", err } defer gitRepo.Close() commit, err := gitRepo.GetCommit("HEAD") if err != nil { - return false, "" + return false, "", err } if commit.Signature == nil { - return false, "" + return false, "", &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{parentSigned} } } } - return true, signingKey + return true, signingKey, nil } // SignCRUDAction determines if we should sign a CRUD commit to this repository -func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string) (bool, string) { +func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string) (bool, string, error) { rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions) signingKey := signingKey(repo.RepoPath()) if signingKey == "" { - return false, "" + return false, "", &ErrWontSign{noKey} } for _, rule := range rules { switch rule { case never: - return false, "" + return false, "", &ErrWontSign{never} case always: break case pubkey: keys, err := ListGPGKeys(u.ID) - if err != nil || len(keys) == 0 { - return false, "" + if err != nil { + return false, "", err + } + if len(keys) == 0 { + return false, "", &ErrWontSign{pubkey} } case twofa: - twofa, err := GetTwoFactorByUID(u.ID) - if err != nil || twofa == nil { - return false, "" + twofaModel, err := GetTwoFactorByUID(u.ID) + if err != nil { + return false, "", err + } + if twofaModel == nil { + return false, "", &ErrWontSign{twofa} } case parentSigned: gitRepo, err := git.OpenRepository(tmpBasePath) if err != nil { - return false, "" + return false, "", err } defer gitRepo.Close() commit, err := gitRepo.GetCommit(parentCommit) if err != nil { - return false, "" + return false, "", err } if commit.Signature == nil { - return false, "" + return false, "", &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(commit) if !verification.Verified { - return false, "" + return false, "", &ErrWontSign{parentSigned} } } } - return true, signingKey + return true, signingKey, nil } diff --git a/modules/auth/repo_form.go b/modules/auth/repo_form.go index 0086419b65..a5071de47e 100644 --- a/modules/auth/repo_form.go +++ b/modules/auth/repo_form.go @@ -173,6 +173,7 @@ type ProtectBranchForm struct { ApprovalsWhitelistTeams string BlockOnRejectedReviews bool DismissStaleApprovals bool + RequireSignedCommits bool } // Validate validates the fields diff --git a/modules/context/repo.go b/modules/context/repo.go index 86c7df2b05..66700a6937 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -74,14 +74,57 @@ func RepoMustNotBeArchived() macaron.Handler { } } +// CanCommitToBranchResults represents the results of CanCommitToBranch +type CanCommitToBranchResults struct { + CanCommitToBranch bool + EditorEnabled bool + UserCanPush bool + RequireSigned bool + WillSign bool + SigningKey string + WontSignReason string +} + // CanCommitToBranch returns true if repository is editable and user has proper access level // and branch is not protected for push -func (r *Repository) CanCommitToBranch(doer *models.User) (bool, error) { - protectedBranch, err := r.Repository.IsProtectedBranchForPush(r.BranchName, doer) +func (r *Repository) CanCommitToBranch(doer *models.User) (CanCommitToBranchResults, error) { + protectedBranch, err := models.GetProtectedBranchBy(r.Repository.ID, r.BranchName) + if err != nil { - return false, err + return CanCommitToBranchResults{}, err } - return r.CanEnableEditor() && !protectedBranch, nil + userCanPush := true + requireSigned := false + if protectedBranch != nil { + userCanPush = protectedBranch.CanUserPush(doer.ID) + requireSigned = protectedBranch.RequireSignedCommits + } + + sign, keyID, err := r.Repository.SignCRUDAction(doer, r.Repository.RepoPath(), git.BranchPrefix+r.BranchName) + + canCommit := r.CanEnableEditor() && userCanPush + if requireSigned { + canCommit = canCommit && sign + } + wontSignReason := "" + if err != nil { + if models.IsErrWontSign(err) { + wontSignReason = string(err.(*models.ErrWontSign).Reason) + err = nil + } else { + wontSignReason = "error" + } + } + + return CanCommitToBranchResults{ + CanCommitToBranch: canCommit, + EditorEnabled: r.CanEnableEditor(), + UserCanPush: userCanPush, + RequireSigned: requireSigned, + WillSign: sign, + SigningKey: keyID, + WontSignReason: wontSignReason, + }, err } // CanUseTimetracker returns whether or not a user can use the timetracker. diff --git a/modules/git/command.go b/modules/git/command.go index 33143dbd75..3cb676c8da 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -97,7 +97,7 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura // RunInDirTimeoutEnvFullPipelineFunc executes the command in given directory with given timeout, // it pipes stdout and stderr to given io.Writer and passes in an io.Reader as stdin. Between cmd.Start and cmd.Wait the passed in function is run. -func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc)) error { +func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.Duration, dir string, stdout, stderr io.Writer, stdin io.Reader, fn func(context.Context, context.CancelFunc) error) error { if timeout == -1 { timeout = DefaultCommandExecutionTimeout @@ -135,7 +135,11 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time. defer process.GetManager().Remove(pid) if fn != nil { - fn(ctx, cancel) + err := fn(ctx, cancel) + if err != nil { + cancel() + return err + } } if err := cmd.Wait(); err != nil && ctx.Err() != context.DeadlineExceeded { diff --git a/modules/git/commit.go b/modules/git/commit.go index dfb7adcd1a..9646d56063 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -33,7 +33,7 @@ type Commit struct { CommitMessage string Signature *CommitGPGSignature - parents []SHA1 // SHA1 strings + Parents []SHA1 // SHA1 strings submoduleCache *ObjectCache } @@ -94,7 +94,7 @@ func convertCommit(c *object.Commit) *Commit { Committer: &c.Committer, Author: &c.Author, Signature: convertPGPSignature(c), - parents: c.ParentHashes, + Parents: c.ParentHashes, } } @@ -111,10 +111,10 @@ func (c *Commit) Summary() string { // ParentID returns oid of n-th parent (0-based index). // It returns nil if no such parent exists. func (c *Commit) ParentID(n int) (SHA1, error) { - if n >= len(c.parents) { + if n >= len(c.Parents) { return SHA1{}, ErrNotExist{"", ""} } - return c.parents[n], nil + return c.Parents[n], nil } // Parent returns n-th parent (0-based index) of the commit. @@ -133,7 +133,7 @@ func (c *Commit) Parent(n int) (*Commit, error) { // ParentCount returns number of parents of the commit. // 0 if this is the root commit, otherwise 1,2, etc. func (c *Commit) ParentCount() int { - return len(c.parents) + return len(c.Parents) } func isImageFile(data []byte) (string, bool) { diff --git a/modules/git/commit_reader.go b/modules/git/commit_reader.go new file mode 100644 index 0000000000..06d8f426d7 --- /dev/null +++ b/modules/git/commit_reader.go @@ -0,0 +1,108 @@ +// Copyright 2020 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 git + +import ( + "bufio" + "bytes" + "io" + "strings" + + "gopkg.in/src-d/go-git.v4/plumbing" +) + +// CommitFromReader will generate a Commit from a provided reader +// We will need this to interpret commits from cat-file +func CommitFromReader(gitRepo *Repository, sha plumbing.Hash, reader io.Reader) (*Commit, error) { + commit := &Commit{ + ID: sha, + } + + payloadSB := new(strings.Builder) + signatureSB := new(strings.Builder) + messageSB := new(strings.Builder) + message := false + pgpsig := false + + scanner := bufio.NewScanner(reader) + // Split by '\n' but include the '\n' + scanner.Split(func(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\n'); i >= 0 { + // We have a full newline-terminated line. + return i + 1, data[0 : i+1], nil + } + // If we're at EOF, we have a final, non-terminated line. Return it. + if atEOF { + return len(data), data, nil + } + // Request more data. + return 0, nil, nil + }) + + for scanner.Scan() { + line := scanner.Bytes() + if pgpsig { + if len(line) > 0 && line[0] == ' ' { + _, _ = signatureSB.Write(line[1:]) + continue + } else { + pgpsig = false + } + } + + if !message { + // This is probably not correct but is copied from go-gits interpretation... + trimmed := bytes.TrimSpace(line) + if len(trimmed) == 0 { + message = true + _, _ = payloadSB.Write(line) + continue + } + + split := bytes.SplitN(trimmed, []byte{' '}, 2) + var data []byte + if len(split) > 1 { + data = split[1] + } + + switch string(split[0]) { + case "tree": + commit.Tree = *NewTree(gitRepo, plumbing.NewHash(string(data))) + _, _ = payloadSB.Write(line) + case "parent": + commit.Parents = append(commit.Parents, plumbing.NewHash(string(data))) + _, _ = payloadSB.Write(line) + case "author": + commit.Author = &Signature{} + commit.Author.Decode(data) + _, _ = payloadSB.Write(line) + case "committer": + commit.Committer = &Signature{} + commit.Committer.Decode(data) + _, _ = payloadSB.Write(line) + case "gpgsig": + _, _ = signatureSB.Write(data) + _ = signatureSB.WriteByte('\n') + pgpsig = true + } + } else { + _, _ = messageSB.Write(line) + } + } + commit.CommitMessage = messageSB.String() + _, _ = payloadSB.WriteString(commit.CommitMessage) + commit.Signature = &CommitGPGSignature{ + Signature: signatureSB.String(), + Payload: payloadSB.String(), + } + if len(commit.Signature.Signature) == 0 { + commit.Signature = nil + } + + return commit, scanner.Err() +} diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index c91f597f9a..c1689b0be0 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -55,9 +55,26 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo BranchName: opts.NewBranch, } } - } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{ - UserName: doer.LowerName, + } else { + protectedBranch, err := repo.GetBranchProtection(opts.OldBranch) + if err != nil { + return nil, err + } + if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + if protectedBranch != nil && protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } } } diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index f9ea4ba155..a1cc37e8c6 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -219,7 +219,7 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(author, committer *models // Determine if we should sign if version.Compare(binVersion, "1.7.9", ">=") { - sign, keyID := t.repo.SignCRUDAction(author, t.basePath, "HEAD") + sign, keyID, _ := t.repo.SignCRUDAction(author, t.basePath, "HEAD") if sign { args = append(args, "-S"+keyID) } else if version.Compare(binVersion, "2.0.0", ">=") { @@ -268,7 +268,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { var finalErr error if err := git.NewCommand("diff-index", "--cached", "-p", "HEAD"). - RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) { + RunInDirTimeoutEnvFullPipelineFunc(nil, 30*time.Second, t.basePath, stdoutWriter, stderr, nil, func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() diff, finalErr = gitdiff.ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader) if finalErr != nil { @@ -276,6 +276,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { cancel() } _ = stdoutReader.Close() + return finalErr }); err != nil { if finalErr != nil { log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr) diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index e22a2062a0..430a83093d 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -151,8 +151,27 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up if err != nil && !git.IsErrBranchNotExist(err) { return nil, err } - } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{UserName: doer.LowerName} + } else { + protectedBranch, err := repo.GetBranchProtection(opts.OldBranch) + if err != nil { + return nil, err + } + if protectedBranch != nil && !protectedBranch.CanUserPush(doer.ID) { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + if protectedBranch != nil && protectedBranch.RequireSignedCommits { + _, _, err := repo.SignCRUDAction(doer, repo.RepoPath(), opts.OldBranch) + if err != nil { + if !models.IsErrWontSign(err) { + return nil, err + } + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } + } } // If FromTreePath is not set, set it to the opts.TreePath diff --git a/modules/repository/init.go b/modules/repository/init.go index a65b335174..9d0beb1138 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -130,7 +130,7 @@ func initRepoCommit(tmpPath string, repo *models.Repository, u *models.User) (er } if version.Compare(binVersion, "1.7.9", ">=") { - sign, keyID := models.SignInitialCommit(tmpPath, u) + sign, keyID, _ := models.SignInitialCommit(tmpPath, u) if sign { args = append(args, "-S"+keyID) } else if version.Compare(binVersion, "2.0.0", ">=") { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 38db43a57c..140c1bd2e3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -748,6 +748,7 @@ editor.name_your_file = Name your file… editor.filename_help = Add a directory by typing its name followed by a slash ('/'). Remove a directory by typing backspace at the beginning of the input field. editor.or = or editor.cancel_lower = Cancel +editor.commit_signed_changes = Commit Signed Changes editor.commit_changes = Commit Changes editor.add_tmpl = Add '' editor.add = Add '%s' @@ -780,6 +781,9 @@ editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v editor.upload_file_is_locked = File '%s' is locked by %s. editor.upload_files_to_dir = Upload files to '%s' editor.cannot_commit_to_protected_branch = Cannot commit to protected branch '%s'. +editor.no_commit_to_branch = Unable to commit directly to branch because: +editor.user_no_push_to_branch = User cannot push to branch +editor.require_signed_commit = Branch requires a signed commit commits.desc = Browse source code change history. commits.commits = Commits @@ -1068,6 +1072,7 @@ pulls.merge_pull_request = Merge Pull Request pulls.rebase_merge_pull_request = Rebase and Merge pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff) pulls.squash_merge_pull_request = Squash and Merge +pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed pulls.invalid_merge_option = You cannot use this merge option for this pull request. pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s
%[2]s
Hint: Try a different strategy pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s
%[2]s
%[3]s
Hint:Try a different strategy @@ -1109,6 +1114,19 @@ milestones.filter_sort.most_complete = Most complete milestones.filter_sort.most_issues = Most issues milestones.filter_sort.least_issues = Least issues +signing.will_sign = This commit will be signed with key '%s' +signing.wont_sign.error = There was an error whilst checking if the commit could be signed +signing.wont_sign.nokey = There is no key available to sign this commit +signing.wont_sign.never = Commits are never signed +signing.wont_sign.always = Commits are always signed +signing.wont_sign.pubkey = The commit will not be signed because you do not have a public key associated with your account +signing.wont_sign.twofa = You must have two factor authentication enabled to have commits signed +signing.wont_sign.parentsigned = The commit will not be signed as the parent commit is not signed +signing.wont_sign.basesigned = The merge will not be signed as the base commit is not signed +signing.wont_sign.headsigned = The merge will not be signed as the head commit is not signed +signing.wont_sign.commitssigned = The merge will not be signed as all the associated commits are not signed +signing.wont_sign.approved = The merge will not be signed as the PR is not approved + ext_wiki = Ext. Wiki ext_wiki.desc = Link to an external wiki. @@ -1416,6 +1434,8 @@ settings.protect_approvals_whitelist_users = Whitelisted reviewers: settings.protect_approvals_whitelist_teams = Whitelisted teams for reviews: settings.dismiss_stale_approvals = Dismiss stale approvals settings.dismiss_stale_approvals_desc = When new commits that change the content of the pull request are pushed to the branch, old approvals will be dismissed. +settings.require_signed_commits = Require Signed Commits +settings.require_signed_commits_desc = Reject pushes to this branch if they are unsigned or unverifiable settings.add_protected_branch = Enable protection settings.delete_protected_branch = Disable protection settings.update_protect_branch_success = Branch protection for branch '%s' has been updated. diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 6b643371e5..bca756aea1 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -639,6 +639,15 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { } } + if _, err := pull_service.IsSignedIfRequired(pr, ctx.User); err != nil { + if !models.IsErrWontSign(err) { + ctx.Error(http.StatusInternalServerError, "IsSignedIfRequired", err) + return + } + ctx.Error(http.StatusMethodNotAllowed, fmt.Sprintf("Protected branch %s requires signed commits but this merge would not be signed", pr.BaseBranch), err) + return + } + if len(form.Do) == 0 { form.Do = string(models.MergeStyleMerge) } diff --git a/routers/private/hook.go b/routers/private/hook.go index b4626fddf4..6a07de15ff 100644 --- a/routers/private/hook.go +++ b/routers/private/hook.go @@ -6,7 +6,10 @@ package private import ( + "bufio" + "context" "fmt" + "io" "net/http" "os" "strings" @@ -18,10 +21,101 @@ import ( "code.gitea.io/gitea/modules/repofiles" "code.gitea.io/gitea/modules/util" pull_service "code.gitea.io/gitea/services/pull" + "gopkg.in/src-d/go-git.v4/plumbing" "gitea.com/macaron/macaron" ) +func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error { + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + log.Error("Unable to create os.Pipe for %s", repo.Path) + return err + } + defer func() { + _ = stdoutReader.Close() + _ = stdoutWriter.Close() + }() + + err = git.NewCommand("rev-list", oldCommitID+"..."+newCommitID). + RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path, + stdoutWriter, nil, nil, + func(ctx context.Context, cancel context.CancelFunc) error { + _ = stdoutWriter.Close() + err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env) + if err != nil { + log.Error("%v", err) + cancel() + } + _ = stdoutReader.Close() + return err + }) + if err != nil && !isErrUnverifiedCommit(err) { + log.Error("Unable to check commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err) + } + return err +} + +func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository, env []string) error { + scanner := bufio.NewScanner(input) + for scanner.Scan() { + line := scanner.Text() + err := readAndVerifyCommit(line, repo, env) + if err != nil { + log.Error("%v", err) + return err + } + } + return scanner.Err() +} + +func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + log.Error("Unable to create pipe for %s: %v", repo.Path, err) + return err + } + defer func() { + _ = stdoutReader.Close() + _ = stdoutWriter.Close() + }() + hash := plumbing.NewHash(sha) + + return git.NewCommand("cat-file", "commit", sha). + RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path, + stdoutWriter, nil, nil, + func(ctx context.Context, cancel context.CancelFunc) error { + _ = stdoutWriter.Close() + commit, err := git.CommitFromReader(repo, hash, stdoutReader) + if err != nil { + return err + } + log.Info("have commit %s", commit.ID.String()) + verification := models.ParseCommitWithSignature(commit) + if !verification.Verified { + log.Info("unverified commit %s", commit.ID.String()) + cancel() + return &errUnverifiedCommit{ + commit.ID.String(), + } + } + return nil + }) +} + +type errUnverifiedCommit struct { + sha string +} + +func (e *errUnverifiedCommit) Error() string { + return fmt.Sprintf("Unverified commit: %s", e.sha) +} + +func isErrUnverifiedCommit(err error) bool { + _, ok := err.(*errUnverifiedCommit) + return ok +} + // HookPreReceive checks whether a individual commit is acceptable func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { ownerName := ctx.Params(":owner") @@ -35,6 +129,30 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { return } repo.OwnerName = ownerName + gitRepo, err := git.OpenRepository(repo.RepoPath()) + if err != nil { + log.Error("Unable to get git repository for: %s/%s Error: %v", ownerName, repoName, err) + ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ + "err": err.Error(), + }) + return + } + defer gitRepo.Close() + + // Generate git environment for checking commits + env := os.Environ() + if opts.GitAlternativeObjectDirectories != "" { + env = append(env, + private.GitAlternativeObjectDirectories+"="+opts.GitAlternativeObjectDirectories) + } + if opts.GitObjectDirectory != "" { + env = append(env, + private.GitObjectDirectory+"="+opts.GitObjectDirectory) + } + if opts.GitQuarantinePath != "" { + env = append(env, + private.GitQuarantinePath+"="+opts.GitQuarantinePath) + } for i := range opts.OldCommitIDs { oldCommitID := opts.OldCommitIDs[i] @@ -51,7 +169,7 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { return } if protectBranch != nil && protectBranch.IsProtected() { - // check and deletion + // detect and prevent deletion if newCommitID == git.EmptySHA { log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo) ctx.JSON(http.StatusForbidden, map[string]interface{}{ @@ -62,20 +180,6 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { // detect force push if git.EmptySHA != oldCommitID { - env := os.Environ() - if opts.GitAlternativeObjectDirectories != "" { - env = append(env, - private.GitAlternativeObjectDirectories+"="+opts.GitAlternativeObjectDirectories) - } - if opts.GitObjectDirectory != "" { - env = append(env, - private.GitObjectDirectory+"="+opts.GitObjectDirectory) - } - if opts.GitQuarantinePath != "" { - env = append(env, - private.GitQuarantinePath+"="+opts.GitQuarantinePath) - } - output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env) if err != nil { log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err) @@ -92,6 +196,27 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { } } + + // Require signed commits + if protectBranch.RequireSignedCommits { + err := verifyCommits(oldCommitID, newCommitID, gitRepo, env) + if err != nil { + if !isErrUnverifiedCommit(err) { + log.Error("Unable to check commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) + ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ + "err": fmt.Sprintf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err), + }) + return + } + unverifiedCommit := err.(*errUnverifiedCommit).sha + log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, unverifiedCommit) + ctx.JSON(http.StatusForbidden, map[string]interface{}{ + "err": fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, unverifiedCommit), + }) + return + } + } + canPush := false if opts.IsDeployKey { canPush = protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) diff --git a/routers/repo/editor.go b/routers/repo/editor.go index 82c74ba75f..8d4f1f8827 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -36,12 +36,13 @@ const ( ) func renderCommitRights(ctx *context.Context) bool { - canCommit, err := ctx.Repo.CanCommitToBranch(ctx.User) + canCommitToBranch, err := ctx.Repo.CanCommitToBranch(ctx.User) if err != nil { log.Error("CanCommitToBranch: %v", err) } - ctx.Data["CanCommitToBranch"] = canCommit - return canCommit + ctx.Data["CanCommitToBranch"] = canCommitToBranch + + return canCommitToBranch.CanCommitToBranch } // getParentTreeFields returns list of parent tree names and corresponding tree paths diff --git a/routers/repo/issue.go b/routers/repo/issue.go index ce3eb5bd2c..afc115c6e2 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -971,6 +971,21 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["GrantedApprovals"] = cnt + ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits + } + ctx.Data["WillSign"] = false + if ctx.User != nil { + sign, key, err := pull.SignMerge(ctx.User, pull.BaseRepo.RepoPath(), pull.BaseBranch, pull.GetGitRefName()) + ctx.Data["WillSign"] = sign + ctx.Data["SigningKey"] = key + if err != nil { + if models.IsErrWontSign(err) { + ctx.Data["WontSignReason"] = err.(*models.ErrWontSign).Reason + } else { + ctx.Data["WontSignReason"] = "error" + log.Error("Error whilst checking if could sign pr %d in repo %s. Error: %v", pull.ID, pull.BaseRepo.FullName(), err) + } + } } ctx.Data["IsPullBranchDeletable"] = canDelete && pull.HeadRepo != nil && diff --git a/routers/repo/setting_protected_branch.go b/routers/repo/setting_protected_branch.go index da28ac50be..e8902ed8ac 100644 --- a/routers/repo/setting_protected_branch.go +++ b/routers/repo/setting_protected_branch.go @@ -246,6 +246,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) } protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews protectBranch.DismissStaleApprovals = f.DismissStaleApprovals + protectBranch.RequireSignedCommits = f.RequireSignedCommits err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ UserIDs: whitelistUsers, diff --git a/services/pull/merge.go b/services/pull/merge.go index e825c3fdd1..f6f0abe836 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -158,7 +158,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.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", ">=") { @@ -470,6 +470,21 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { return out.String(), nil } +// IsSignedIfRequired check if merge will be signed if required +func IsSignedIfRequired(pr *models.PullRequest, doer *models.User) (bool, error) { + if err := pr.LoadProtectedBranch(); err != nil { + return false, err + } + + if pr.ProtectedBranch == nil || !pr.ProtectedBranch.RequireSignedCommits { + return true, nil + } + + sign, _, err := pr.SignMerge(doer, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName()) + + return sign, err +} + // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections func IsUserAllowedToMerge(pr *models.PullRequest, p models.Permission, user *models.User) (bool, error) { if p.IsAdmin() { diff --git a/services/pull/patch.go b/services/pull/patch.go index 1dbeb81c01..815263e898 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -162,7 +162,7 @@ func TestPatch(pr *models.PullRequest) error { RunInDirTimeoutEnvFullPipelineFunc( nil, -1, tmpBasePath, nil, stderrWriter, nil, - func(ctx context.Context, cancel context.CancelFunc) { + func(ctx context.Context, cancel context.CancelFunc) error { _ = stderrWriter.Close() const prefix = "error: patch failed:" const errorPrefix = "error: " @@ -199,6 +199,7 @@ func TestPatch(pr *models.PullRequest) error { } } _ = stderrReader.Close() + return nil }) if err != nil { diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index 58af203cfd..e2b04ade77 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -184,7 +184,7 @@ func updateWikiPage(doer *models.User, repo *models.Repository, oldWikiName, new Message: message, } - sign, signingKey := repo.SignWikiCommit(doer) + sign, signingKey, _ := repo.SignWikiCommit(doer) if sign { commitTreeOpts.KeyID = signingKey } else { @@ -298,7 +298,7 @@ func DeleteWikiPage(doer *models.User, repo *models.Repository, wikiName string) Parents: []string{"HEAD"}, } - sign, signingKey := repo.SignWikiCommit(doer) + sign, signingKey, _ := repo.SignWikiCommit(doer) if sign { commitTreeOpts.KeyID = signingKey } else { diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 2ff08e3931..1915e9be21 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -1,7 +1,11 @@
-

{{.i18n.Tr "repo.editor.commit_changes"}}

+

{{- if .CanCommitToBranch.WillSign}} + {{.i18n.Tr "repo.editor.commit_signed_changes"}} + {{- else}} + {{.i18n.Tr "repo.editor.commit_changes"}} + {{- end}}

@@ -10,11 +14,20 @@
-
+
diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 7073312b1f..1bf0fa7864 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -48,6 +48,7 @@ {{else if .IsBlockedByApprovals}}red {{else if .IsBlockedByRejection}}red {{else if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}}red + {{else if and .RequireSigned (not .WillSign)}}}red {{else if .Issue.PullRequest.IsChecking}}yellow {{else if .Issue.PullRequest.CanAutoMerge}}green {{else}}red{{end}}"> @@ -93,49 +94,69 @@
{{else if .IsPullRequestBroken}}
- + {{$.i18n.Tr "repo.pulls.data_broken"}}
{{else if .IsPullWorkInProgress}}
- + {{$.i18n.Tr "repo.pulls.cannot_merge_work_in_progress" .WorkInProgressPrefix | Str2html}}
{{else if .Issue.PullRequest.IsChecking}}
- + {{$.i18n.Tr "repo.pulls.is_checking"}}
{{else if .Issue.PullRequest.CanAutoMerge}} {{if .IsBlockedByApprovals}}
- + {{$.i18n.Tr "repo.pulls.blocked_by_approvals" .GrantedApprovals .Issue.PullRequest.ProtectedBranch.RequiredApprovals}}
{{else if .IsBlockedByRejection}}
- + {{$.i18n.Tr "repo.pulls.blocked_by_rejection"}}
{{else if and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess)}}
- + {{$.i18n.Tr "repo.pulls.required_status_check_failed"}}
+ {{else if and .RequireSigned (not .WillSign)}} +
+ + {{$.i18n.Tr "repo.pulls.require_signed_wont_sign"}} +
+
+ + {{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }} +
{{end}} - {{$notAllOk := or .IsBlockedByApprovals .IsBlockedByRejection (and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess))}} - {{if or $.IsRepoAdmin (not $notAllOk)}} + {{$notAllOk := or .IsBlockedByApprovals .IsBlockedByRejection (and .RequireSigned (not .WillSign)) (and .EnableStatusCheck (not .IsRequiredStatusCheckSuccess))}} + {{if and (or $.IsRepoAdmin (not $notAllOk)) (or (not .RequireSigned) .WillSign)}} {{if $notAllOk}}
- + {{$.i18n.Tr "repo.pulls.required_status_check_administrator"}}
{{else}}
- + {{$.i18n.Tr "repo.pulls.can_auto_merge_desc"}}
{{end}} + {{if .WillSign}} +
+ + {{$.i18n.Tr "repo.signing.will_sign" .SigningKey}} +
+ {{else}} +
+ + {{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }} +
+ {{end}} {{if .AllowMerge}} {{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}} {{$approvers := .Issue.PullRequest.GetApprovers}} @@ -282,6 +303,11 @@ {{$.i18n.Tr "repo.pulls.required_status_check_failed"}}
+ {{else if and .RequireSigned (not .WillSign)}} +
+ + {{$.i18n.Tr "repo.pulls.require_signed_wont_sign"}} +
{{else}}
diff --git a/templates/repo/settings/protected_branch.tmpl b/templates/repo/settings/protected_branch.tmpl index c6701ce8a9..b4c65f7830 100644 --- a/templates/repo/settings/protected_branch.tmpl +++ b/templates/repo/settings/protected_branch.tmpl @@ -210,7 +210,7 @@

{{.i18n.Tr "repo.settings.block_rejected_reviews_desc"}}

-
+
@@ -218,6 +218,13 @@

{{.i18n.Tr "repo.settings.dismiss_stale_approvals_desc"}}

+
+
+ + +

{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}

+
+
diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index cd35b88f3b..27a0698f7b 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -652,6 +652,9 @@ margin-left: 10px; margin-top: 10px; } + .icon-octicon { + padding-left: 2px; + } } .review-item {