From de175e3b06e623280058278cdb0a62de7443cd86 Mon Sep 17 00:00:00 2001 From: Exploding Dragon Date: Tue, 6 Aug 2024 21:03:33 +0800 Subject: [PATCH 1/2] Add signature support for the RPM module (#27069) close #27031 If the rpm package does not contain a matching gpg signature, the installation will fail. See (#27031) , now auto-signing rpm uploads. This option is turned off by default for compatibility. --- custom/conf/app.example.ini | 3 +- modules/setting/packages.go | 3 ++ routers/api/packages/rpm/rpm.go | 16 ++++++++- services/packages/rpm/repository.go | 38 ++++++++++++++++++++-- tests/integration/api_packages_rpm_test.go | 27 +++++++++++++++ 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 89d04d381c..62edef597c 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2555,7 +2555,8 @@ LEVEL = Info ;LIMIT_SIZE_SWIFT = -1 ;; Maximum size of a Vagrant upload (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`) ;LIMIT_SIZE_VAGRANT = -1 - +;; Enable RPM re-signing by default. (It will overwrite the old signature ,using v4 format, not compatible with CentOS 6 or older) +;DEFAULT_RPM_SIGN_ENABLED = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; default storage for attachments, lfs and avatars diff --git a/modules/setting/packages.go b/modules/setting/packages.go index 00fba67b39..bc093e7ea6 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -42,6 +42,8 @@ var ( LimitSizeRubyGems int64 LimitSizeSwift int64 LimitSizeVagrant int64 + + DefaultRPMSignEnabled bool }{ Enabled: true, LimitTotalOwnerCount: -1, @@ -97,6 +99,7 @@ func loadPackagesFrom(rootCfg ConfigProvider) (err error) { Packages.LimitSizeRubyGems = mustBytes(sec, "LIMIT_SIZE_RUBYGEMS") Packages.LimitSizeSwift = mustBytes(sec, "LIMIT_SIZE_SWIFT") Packages.LimitSizeVagrant = mustBytes(sec, "LIMIT_SIZE_VAGRANT") + Packages.DefaultRPMSignEnabled = sec.Key("DEFAULT_RPM_SIGN_ENABLED").MustBool(false) return nil } diff --git a/routers/api/packages/rpm/rpm.go b/routers/api/packages/rpm/rpm.go index 11d7729eec..4c822e0999 100644 --- a/routers/api/packages/rpm/rpm.go +++ b/routers/api/packages/rpm/rpm.go @@ -133,6 +133,21 @@ func UploadPackageFile(ctx *context.Context) { } defer buf.Close() + // if rpm sign enabled + if setting.Packages.DefaultRPMSignEnabled || ctx.FormBool("sign") { + pri, _, err := rpm_service.GetOrCreateKeyPair(ctx, ctx.Package.Owner.ID) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + buf, err = rpm_service.SignPackage(buf, pri) + if err != nil { + // Not in rpm format, parsing failed. + apiError(ctx, http.StatusBadRequest, err) + return + } + } + pck, err := rpm_module.ParsePackage(buf) if err != nil { if errors.Is(err, util.ErrInvalidArgument) { @@ -142,7 +157,6 @@ func UploadPackageFile(ctx *context.Context) { } return } - if _, err := buf.Seek(0, io.SeekStart); err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/services/packages/rpm/repository.go b/services/packages/rpm/repository.go index c52c8a5dd9..19968f9b30 100644 --- a/services/packages/rpm/repository.go +++ b/services/packages/rpm/repository.go @@ -21,14 +21,16 @@ import ( rpm_model "code.gitea.io/gitea/models/packages/rpm" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/log" packages_module "code.gitea.io/gitea/modules/packages" rpm_module "code.gitea.io/gitea/modules/packages/rpm" "code.gitea.io/gitea/modules/util" packages_service "code.gitea.io/gitea/services/packages" - "github.com/keybase/go-crypto/openpgp" - "github.com/keybase/go-crypto/openpgp/armor" - "github.com/keybase/go-crypto/openpgp/packet" + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" + "github.com/ProtonMail/go-crypto/openpgp/packet" + "github.com/sassoftware/go-rpmutils" ) // GetOrCreateRepositoryVersion gets or creates the internal repository package @@ -641,3 +643,33 @@ func addDataAsFileToRepo(ctx context.Context, pv *packages_model.PackageVersion, OpenSize: wc.Written(), }, nil } + +func SignPackage(rpm *packages_module.HashedBuffer, privateKey string) (*packages_module.HashedBuffer, error) { + keyring, err := openpgp.ReadArmoredKeyRing(bytes.NewReader([]byte(privateKey))) + if err != nil { + // failed to parse key + return nil, err + } + entity := keyring[0] + h, err := rpmutils.SignRpmStream(rpm, entity.PrivateKey, nil) + if err != nil { + // error signing rpm + return nil, err + } + signBlob, err := h.DumpSignatureHeader(false) + if err != nil { + // error writing sig header + return nil, err + } + if len(signBlob)%8 != 0 { + log.Info("incorrect padding: got %d bytes, expected a multiple of 8", len(signBlob)) + return nil, err + } + + // move fp to sign end + if _, err := rpm.Seek(int64(h.OriginalSignatureHeaderSize()), io.SeekStart); err != nil { + return nil, err + } + // create signed rpm buf + return packages_module.CreateHashedBufferFromReader(io.MultiReader(bytes.NewReader(signBlob), rpm)) +} diff --git a/tests/integration/api_packages_rpm_test.go b/tests/integration/api_packages_rpm_test.go index 1dcec6099e..6feceaeb78 100644 --- a/tests/integration/api_packages_rpm_test.go +++ b/tests/integration/api_packages_rpm_test.go @@ -24,7 +24,10 @@ import ( "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/sassoftware/go-rpmutils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPackageRpm(t *testing.T) { @@ -431,6 +434,30 @@ gpgkey=%sapi/packages/%s/rpm/repository.key`, AddBasicAuth(user.Name) MakeRequest(t, req, http.StatusNotFound) }) + + t.Run("UploadSign", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + url := groupURL + "/upload?sign=true" + req := NewRequestWithBody(t, "PUT", url, bytes.NewReader(content)). + AddBasicAuth(user.Name) + MakeRequest(t, req, http.StatusCreated) + + gpgReq := NewRequest(t, "GET", rootURL+"/repository.key") + gpgResp := MakeRequest(t, gpgReq, http.StatusOK) + pub, err := openpgp.ReadArmoredKeyRing(gpgResp.Body) + require.NoError(t, err) + + req = NewRequest(t, "GET", fmt.Sprintf("%s/package/%s/%s/%s", groupURL, packageName, packageVersion, packageArchitecture)) + resp := MakeRequest(t, req, http.StatusOK) + + _, sigs, err := rpmutils.Verify(resp.Body, pub) + require.NoError(t, err) + require.NotEmpty(t, sigs) + + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/package/%s/%s/%s", groupURL, packageName, packageVersion, packageArchitecture)). + AddBasicAuth(user.Name) + MakeRequest(t, req, http.StatusNoContent) + }) }) } } From df7f1c2eadac24f64adf3d823a7642a6eccf77f3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 6 Aug 2024 21:32:49 +0800 Subject: [PATCH 2/2] Fix protected branch files detection on pre_receive hook (#31778) Fix #31738 When pushing a new branch, the old commit is zero. Most git commands cannot recognize the zero commit id. To get the changed files in the push, we need to get the first diverge commit of this branch. In most situations, we could check commits one by one until one commit is contained by another branch. Then we will think that commit is the diverge point. And in a pre-receive hook, this will be more difficult because all commits haven't been merged and they actually stored in a temporary place by git. So we need to bring some envs to let git know the commit exist. --- modules/git/commit_test.go | 17 ++++++++++ modules/git/diff.go | 12 ++++++- modules/git/repo_commit.go | 49 ++++++++++++++++++++++++++--- modules/git/repo_commit_test.go | 3 +- routers/private/hook_pre_receive.go | 4 +-- services/pull/patch.go | 10 +++--- 6 files changed, 81 insertions(+), 14 deletions(-) diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index a33e7df31a..0ddeb182ef 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -4,6 +4,8 @@ package git import ( + "context" + "os" "path/filepath" "strings" "testing" @@ -345,3 +347,18 @@ func TestGetCommitFileStatusMerges(t *testing.T) { assert.Equal(t, commitFileStatus.Removed, expected.Removed) assert.Equal(t, commitFileStatus.Modified, expected.Modified) } + +func Test_GetCommitBranchStart(t *testing.T) { + bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") + repo, err := OpenRepository(context.Background(), bareRepo1Path) + assert.NoError(t, err) + defer repo.Close() + commit, err := repo.GetBranchCommit("branch1") + assert.NoError(t, err) + assert.EqualValues(t, "2839944139e0de9737a044f78b0e4b40d989a9e3", commit.ID.String()) + + startCommitID, err := repo.GetCommitBranchStart(os.Environ(), "branch1", commit.ID.String()) + assert.NoError(t, err) + assert.NotEmpty(t, startCommitID) + assert.EqualValues(t, "9c9aef8dd84e02bc7ec12641deb4c930a7c30185", startCommitID) +} diff --git a/modules/git/diff.go b/modules/git/diff.go index 10ef3d83fb..833f6220f9 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -271,7 +271,17 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi } // GetAffectedFiles returns the affected files between two commits -func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []string) ([]string, error) { +func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID string, env []string) ([]string, error) { + if oldCommitID == emptySha1ObjectID.String() || oldCommitID == emptySha256ObjectID.String() { + startCommitID, err := repo.GetCommitBranchStart(env, branchName, newCommitID) + if err != nil { + return nil, err + } + if startCommitID == "" { + return nil, fmt.Errorf("cannot find the start commit of %s", newCommitID) + } + oldCommitID = startCommitID + } stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { log.Error("Unable to create os.Pipe for %s", repo.Path) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 8c3285769e..9405634df1 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -7,6 +7,7 @@ package git import ( "bytes" "io" + "os" "strconv" "strings" @@ -414,7 +415,7 @@ func (repo *Repository) commitsBefore(id ObjectID, limit int) ([]*Commit, error) commits := make([]*Commit, 0, len(formattedLog)) for _, commit := range formattedLog { - branches, err := repo.getBranches(commit, 2) + branches, err := repo.getBranches(os.Environ(), commit.ID.String(), 2) if err != nil { return nil, err } @@ -437,12 +438,15 @@ func (repo *Repository) getCommitsBeforeLimit(id ObjectID, num int) ([]*Commit, return repo.commitsBefore(id, num) } -func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) { +func (repo *Repository) getBranches(env []string, commitID string, limit int) ([]string, error) { if DefaultFeatures().CheckVersionAtLeast("2.7.0") { stdout, _, err := NewCommand(repo.Ctx, "for-each-ref", "--format=%(refname:strip=2)"). AddOptionFormat("--count=%d", limit). - AddOptionValues("--contains", commit.ID.String(), BranchPrefix). - RunStdString(&RunOpts{Dir: repo.Path}) + AddOptionValues("--contains", commitID, BranchPrefix). + RunStdString(&RunOpts{ + Dir: repo.Path, + Env: env, + }) if err != nil { return nil, err } @@ -451,7 +455,10 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) return branches, nil } - stdout, _, err := NewCommand(repo.Ctx, "branch").AddOptionValues("--contains", commit.ID.String()).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "branch").AddOptionValues("--contains", commitID).RunStdString(&RunOpts{ + Dir: repo.Path, + Env: env, + }) if err != nil { return nil, err } @@ -513,3 +520,35 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error } return nil } + +func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) { + cmd := NewCommand(repo.Ctx, "log", prettyLogFormat) + cmd.AddDynamicArguments(endCommitID) + + stdout, _, runErr := cmd.RunStdBytes(&RunOpts{ + Dir: repo.Path, + Env: env, + }) + if runErr != nil { + return "", runErr + } + + parts := bytes.Split(bytes.TrimSpace(stdout), []byte{'\n'}) + + var startCommitID string + for _, commitID := range parts { + branches, err := repo.getBranches(env, string(commitID), 2) + if err != nil { + return "", err + } + for _, b := range branches { + if b != branch { + return startCommitID, nil + } + } + + startCommitID = string(commitID) + } + + return "", nil +} diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index fee145e924..19983b47b1 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -4,6 +4,7 @@ package git import ( + "os" "path/filepath" "testing" @@ -31,7 +32,7 @@ func TestRepository_GetCommitBranches(t *testing.T) { for _, testCase := range testCases { commit, err := bareRepo1.GetCommit(testCase.CommitID) assert.NoError(t, err) - branches, err := bareRepo1.getBranches(commit, 2) + branches, err := bareRepo1.getBranches(os.Environ(), commit.ID.String(), 2) assert.NoError(t, err) assert.Equal(t, testCase.ExpectedBranches, branches) } diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 8853875014..73fe9b886c 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -235,7 +235,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r globs := protectBranch.GetProtectedFilePatterns() if len(globs) > 0 { - _, err := pull_service.CheckFileProtection(gitRepo, oldCommitID, newCommitID, globs, 1, ctx.env) + _, err := pull_service.CheckFileProtection(gitRepo, branchName, oldCommitID, newCommitID, globs, 1, ctx.env) if err != nil { if !models.IsErrFilePathProtected(err) { log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) @@ -293,7 +293,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r // Allow commits that only touch unprotected files globs := protectBranch.GetUnprotectedFilePatterns() if len(globs) > 0 { - unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(gitRepo, oldCommitID, newCommitID, globs, ctx.env) + unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(gitRepo, branchName, oldCommitID, newCommitID, globs, ctx.env) if err != nil { log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) ctx.JSON(http.StatusInternalServerError, private.Response{ diff --git a/services/pull/patch.go b/services/pull/patch.go index e391a7f9d0..0934a86c89 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -503,11 +503,11 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * } // CheckFileProtection check file Protection -func CheckFileProtection(repo *git.Repository, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) { +func CheckFileProtection(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, limit int, env []string) ([]string, error) { if len(patterns) == 0 { return nil, nil } - affectedFiles, err := git.GetAffectedFiles(repo, oldCommitID, newCommitID, env) + affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env) if err != nil { return nil, err } @@ -533,11 +533,11 @@ func CheckFileProtection(repo *git.Repository, oldCommitID, newCommitID string, } // CheckUnprotectedFiles check if the commit only touches unprotected files -func CheckUnprotectedFiles(repo *git.Repository, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) { +func CheckUnprotectedFiles(repo *git.Repository, branchName, oldCommitID, newCommitID string, patterns []glob.Glob, env []string) (bool, error) { if len(patterns) == 0 { return false, nil } - affectedFiles, err := git.GetAffectedFiles(repo, oldCommitID, newCommitID, env) + affectedFiles, err := git.GetAffectedFiles(repo, branchName, oldCommitID, newCommitID, env) if err != nil { return false, err } @@ -574,7 +574,7 @@ func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, return nil } - pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ()) + pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.HeadBranch, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ()) if err != nil && !models.IsErrFilePathProtected(err) { return err }