From fc8fc5a4c092636676e3db45f9ea4549335586d7 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 2 May 2025 11:17:04 +0200 Subject: [PATCH 01/45] Improve instance wide ssh commit signing * Signed SSH commits can look like on GitHub * No user account of the committer needed * SSH format can be added in gitea config * No gitconfig changes needed * Set gpg.format git key for signing command * Previously only the default gpg key had global trust in Gitea * SSH Signing worked before with DEFAULT_TRUST_MODEL=committer, but not with model default and manually changing the .gitconfig e.g. the following is all needed ``` [repository.signing] SIGNING_KEY = /data/id_ed25519.pub SIGNING_NAME = Gitea SIGNING_EMAIL = git@domain.com SIGNING_FORMAT = ssh INITIAL_COMMIT = always CRUD_ACTIONS = always WIKI = always MERGES = always ``` `TRUSTED_SSH_KEYS` can be a list of additional ssh public keys to trust for every user of this instance --- models/asymkey/key.go | 6 + modules/git/command.go | 12 +- modules/git/repo.go | 1 + modules/git/repo_gpg.go | 12 ++ modules/git/repo_tree.go | 4 + modules/setting/repository.go | 5 + routers/api/v1/api.go | 1 + routers/api/v1/misc/signing.go | 61 +++++++++- routers/web/repo/setting/setting.go | 4 +- services/asymkey/commit.go | 70 ++++++++++- services/asymkey/sign.go | 162 ++++++++++++++----------- services/context/repo.go | 3 +- services/pull/merge.go | 7 +- services/pull/merge_prepare.go | 5 +- services/pull/merge_squash.go | 7 +- services/repository/files/temp_repo.go | 12 +- services/repository/init.go | 7 +- services/wiki/wiki.go | 6 +- templates/swagger/v1_json.tmpl | 56 +++++++++ 19 files changed, 348 insertions(+), 93 deletions(-) create mode 100644 models/asymkey/key.go diff --git a/models/asymkey/key.go b/models/asymkey/key.go new file mode 100644 index 0000000000000..4e7c50f437b70 --- /dev/null +++ b/models/asymkey/key.go @@ -0,0 +1,6 @@ +package asymkey + +type SigningKey struct { + KeyID string + Format string +} diff --git a/modules/git/command.go b/modules/git/command.go index eaaa4969d0bb1..7910f597f2696 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -47,6 +47,7 @@ type Command struct { globalArgsLength int brokenArgs []string cmd *exec.Cmd // for debug purpose only + configArgs []string } func logArgSanitize(arg string) string { @@ -196,6 +197,15 @@ func (c *Command) AddDashesAndList(list ...string) *Command { return c } +func (c *Command) AddConfig(key string, value string) *Command { + kv := key + "=" + value + if !isSafeArgumentValue(kv) { + c.brokenArgs = append(c.brokenArgs, key) + } + c.configArgs = append(c.configArgs, "-c", kv) + return c +} + // ToTrustedCmdArgs converts a list of strings (trusted as argument) to TrustedCmdArgs // In most cases, it shouldn't be used. Use NewCommand().AddXxx() function instead func ToTrustedCmdArgs(args []string) TrustedCmdArgs { @@ -321,7 +331,7 @@ func (c *Command) run(ctx context.Context, skip int, opts *RunOpts) error { startTime := time.Now() - cmd := exec.CommandContext(ctx, c.prog, c.args...) + cmd := exec.CommandContext(ctx, c.prog, append(append([]string{}, c.configArgs...), c.args...)...) c.cmd = cmd // for debug purpose only if opts.Env == nil { cmd.Env = os.Environ() diff --git a/modules/git/repo.go b/modules/git/repo.go index 45937a8d5fa54..239866fe9d613 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -28,6 +28,7 @@ type GPGSettings struct { Email string Name string PublicKeyContent string + Format string } const prettyLogFormat = `--pretty=format:%H` diff --git a/modules/git/repo_gpg.go b/modules/git/repo_gpg.go index 8f91b4dce558b..0a2ceb92187a7 100644 --- a/modules/git/repo_gpg.go +++ b/modules/git/repo_gpg.go @@ -6,6 +6,7 @@ package git import ( "fmt" + "os" "strings" "code.gitea.io/gitea/modules/process" @@ -13,6 +14,14 @@ import ( // LoadPublicKeyContent will load the key from gpg func (gpgSettings *GPGSettings) LoadPublicKeyContent() error { + if gpgSettings.Format == "ssh" { + content, err := os.ReadFile(gpgSettings.KeyID) + if err != nil { + return fmt.Errorf("unable to read SSH public key file: %s, %w", gpgSettings.KeyID, err) + } + gpgSettings.PublicKeyContent = string(content) + return nil + } content, stderr, err := process.GetManager().Exec( "gpg -a --export", "gpg", "-a", "--export", gpgSettings.KeyID) @@ -44,6 +53,9 @@ func (repo *Repository) GetDefaultPublicGPGKey(forceUpdate bool) (*GPGSettings, signingKey, _, _ := NewCommand("config", "--get", "user.signingkey").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.KeyID = strings.TrimSpace(signingKey) + format, _, _ := NewCommand("config", "--get", "gpg.format").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) + gpgSettings.Format = strings.TrimSpace(format) + defaultEmail, _, _ := NewCommand("config", "--get", "user.email").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.Email = strings.TrimSpace(defaultEmail) diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 70e5aee02353f..064b1317f07ad 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -16,6 +16,7 @@ type CommitTreeOpts struct { Parents []string Message string KeyID string + KeyFormat string NoGPGSign bool AlwaysSign bool } @@ -44,6 +45,9 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString("\n") if opts.KeyID != "" || opts.AlwaysSign { + if opts.KeyFormat != "" { + cmd.AddConfig("gpg.format", opts.KeyFormat) + } cmd.AddOptionFormat("-S%s", opts.KeyID) } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index c6bdc65b3218e..c8268eaf4460a 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -100,11 +100,13 @@ var ( SigningKey string SigningName string SigningEmail string + SigningFormat string InitialCommit []string CRUDActions []string `ini:"CRUD_ACTIONS"` Merges []string Wiki []string DefaultTrustModel string + TrustedSSHKeys []string `ini:"TRUSTED_SSH_KEYS"` } `ini:"repository.signing"` }{ DetectedCharsetsOrder: []string{ @@ -242,11 +244,13 @@ var ( SigningKey string SigningName string SigningEmail string + SigningFormat string InitialCommit []string CRUDActions []string `ini:"CRUD_ACTIONS"` Merges []string Wiki []string DefaultTrustModel string + TrustedSSHKeys []string `ini:"TRUSTED_SSH_KEYS"` }{ SigningKey: "default", SigningName: "", @@ -256,6 +260,7 @@ var ( Merges: []string{"pubkey", "twofa", "basesigned", "commitssigned"}, Wiki: []string{"never"}, DefaultTrustModel: "collaborator", + TrustedSSHKeys: []string{}, }, } RepoRootPath string diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index b98863b418dc4..3b1447dc68a09 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -972,6 +972,7 @@ func Routes() *web.Router { m.Group("", func() { m.Get("/version", misc.Version) m.Get("/signing-key.gpg", misc.SigningKey) + m.Get("/signing-key.pub", misc.SigningKeySSH) m.Post("/markup", reqToken(), bind(api.MarkupOption{}), misc.Markup) m.Post("/markdown", reqToken(), bind(api.MarkdownOption{}), misc.Markdown) m.Post("/markdown/raw", reqToken(), misc.MarkdownRaw) diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index 667396e39ca36..4135aa664eaf2 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -50,11 +50,70 @@ func SigningKey(ctx *context.APIContext) { path = ctx.Repo.Repository.RepoPath() } - content, err := asymkey_service.PublicSigningKey(ctx, path) + content, format, err := asymkey_service.PublicSigningKey(ctx, path) if err != nil { ctx.APIErrorInternal(err) return } + if format == "ssh" { + ctx.APIErrorNotFound(fmt.Errorf("SSH keys are used for signing, not GPG")) + return + } + _, err = ctx.Write([]byte(content)) + if err != nil { + ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err)) + } +} + +// SigningKey returns the public key of the default signing key if it exists +func SigningKeySSH(ctx *context.APIContext) { + // swagger:operation GET /signing-key.pub miscellaneous getSigningKeySSH + // --- + // summary: Get default signing-key.pub + // produces: + // - text/plain + // responses: + // "200": + // description: "ssh public key" + // schema: + // type: string + + // swagger:operation GET /repos/{owner}/{repo}/signing-key.pub repository repoSigningKeySSH + // --- + // summary: Get signing-key.pub for given repository + // produces: + // - text/plain + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // responses: + // "200": + // description: "ssh public key" + // schema: + // type: string + + path := "" + if ctx.Repo != nil && ctx.Repo.Repository != nil { + path = ctx.Repo.Repository.RepoPath() + } + + content, format, err := asymkey_service.PublicSigningKey(ctx, path) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if format != "ssh" { + ctx.APIErrorNotFound(fmt.Errorf("GPG keys are used for signing, not SSH")) + return + } _, err = ctx.Write([]byte(content)) if err != nil { ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err)) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 5552a8726cc89..bebb4867bc4c3 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -62,7 +62,7 @@ func SettingsCtxData(ctx *context.Context) { ctx.Data["CanConvertFork"] = ctx.Repo.Repository.IsFork && ctx.Doer.CanCreateRepoIn(ctx.Repo.Repository.Owner) signing, _ := asymkey_service.SigningKey(ctx, ctx.Repo.Repository.RepoPath()) - ctx.Data["SigningKeyAvailable"] = len(signing) > 0 + ctx.Data["SigningKeyAvailable"] = len(signing.KeyID) > 0 ctx.Data["SigningSettings"] = setting.Repository.Signing ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled @@ -105,7 +105,7 @@ func SettingsPost(ctx *context.Context) { ctx.Data["MinimumMirrorInterval"] = setting.Mirror.MinInterval signing, _ := asymkey_service.SigningKey(ctx, ctx.Repo.Repository.RepoPath()) - ctx.Data["SigningKeyAvailable"] = len(signing) > 0 + ctx.Data["SigningKeyAvailable"] = len(signing.KeyID) > 0 ctx.Data["SigningSettings"] = setting.Repository.Signing ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 105782a93a57c..6de4611389ae8 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -398,11 +398,79 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } } + // Trust more than one key for every User + for _, k := range setting.Repository.Signing.TrustedSSHKeys { + fingerprint, _ := asymkey_model.CalcFingerprint(k) + commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + Verified: true, + Content: k, + Fingerprint: fingerprint, + HasUsed: true, + }, committer, committer, c.Committer.Email) + if commitVerification != nil { + return commitVerification + } + } + + defaultReason := asymkey_model.NoKeyFound + + if setting.Repository.Signing.SigningFormat == "ssh" && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { + // OK we should try the default key + gpgSettings := git.GPGSettings{ + Sign: true, + KeyID: setting.Repository.Signing.SigningKey, + Name: setting.Repository.Signing.SigningName, + Email: setting.Repository.Signing.SigningEmail, + Format: setting.Repository.Signing.SigningFormat, + } + if err := gpgSettings.LoadPublicKeyContent(); err != nil { + log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) + } + fingerprint, _ := asymkey_model.CalcFingerprint(gpgSettings.PublicKeyContent) + if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + Verified: true, + Content: gpgSettings.PublicKeyContent, + Fingerprint: fingerprint, + HasUsed: true, + }, committer, committer, committer.Email); commitVerification != nil { + if commitVerification.Reason == asymkey_model.BadSignature { + defaultReason = asymkey_model.BadSignature + } else { + return commitVerification + } + } + } + + defaultGPGSettings, err := c.GetRepositoryDefaultPublicGPGKey(false) + if defaultGPGSettings.Format == "ssh" { + if err != nil { + log.Error("Error getting default public gpg key: %v", err) + } else if defaultGPGSettings == nil { + log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) + } else if defaultGPGSettings.Sign { + if err := defaultGPGSettings.LoadPublicKeyContent(); err != nil { + log.Error("Error getting default signing key: %s %v", defaultGPGSettings.KeyID, err) + } + fingerprint, _ := asymkey_model.CalcFingerprint(defaultGPGSettings.PublicKeyContent) + if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + Verified: true, + Content: defaultGPGSettings.PublicKeyContent, + Fingerprint: fingerprint, + HasUsed: true, + }, committer, committer, committer.Email); commitVerification != nil { + if commitVerification.Reason == asymkey_model.BadSignature { + defaultReason = asymkey_model.BadSignature + } else { + return commitVerification + } + } + } + } return &asymkey_model.CommitVerification{ CommittingUser: committer, Verified: false, - Reason: asymkey_model.NoKeyFound, + Reason: defaultReason, } } diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 2216bca54ae67..57eb6782c5738 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -6,6 +6,7 @@ package asymkey import ( "context" "fmt" + "os" "strings" asymkey_model "code.gitea.io/gitea/models/asymkey" @@ -85,9 +86,9 @@ func IsErrWontSign(err error) bool { } // SigningKey returns the KeyID and git Signature for the repo -func SigningKey(ctx context.Context, repoPath string) (string, *git.Signature) { +func SigningKey(ctx context.Context, repoPath string) (asymkey_model.SigningKey, *git.Signature) { if setting.Repository.Signing.SigningKey == "none" { - return "", nil + return asymkey_model.SigningKey{}, nil } if setting.Repository.Signing.SigningKey == "default" || setting.Repository.Signing.SigningKey == "" { @@ -95,53 +96,68 @@ func SigningKey(ctx context.Context, repoPath string) (string, *git.Signature) { value, _, _ := git.NewCommand("config", "--get", "commit.gpgsign").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) sign, valid := git.ParseBool(strings.TrimSpace(value)) if !sign || !valid { - return "", nil + return asymkey_model.SigningKey{}, nil } + format, _, _ := git.NewCommand("config", "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingKey, _, _ := git.NewCommand("config", "--get", "user.signingkey").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingName, _, _ := git.NewCommand("config", "--get", "user.name").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingEmail, _, _ := git.NewCommand("config", "--get", "user.email").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) - return strings.TrimSpace(signingKey), &git.Signature{ - Name: strings.TrimSpace(signingName), - Email: strings.TrimSpace(signingEmail), - } + return asymkey_model.SigningKey{ + KeyID: strings.TrimSpace(signingKey), + Format: strings.TrimSpace(format), + }, &git.Signature{ + Name: strings.TrimSpace(signingName), + Email: strings.TrimSpace(signingEmail), + } } - return setting.Repository.Signing.SigningKey, &git.Signature{ - Name: setting.Repository.Signing.SigningName, - Email: setting.Repository.Signing.SigningEmail, - } + return asymkey_model.SigningKey{ + KeyID: setting.Repository.Signing.SigningKey, + Format: setting.Repository.Signing.SigningFormat, + }, &git.Signature{ + Name: setting.Repository.Signing.SigningName, + Email: setting.Repository.Signing.SigningEmail, + } } // PublicSigningKey gets the public signing key within a provided repository directory -func PublicSigningKey(ctx context.Context, repoPath string) (string, error) { +func PublicSigningKey(ctx context.Context, repoPath string) (string, string, error) { signingKey, _ := SigningKey(ctx, repoPath) - if signingKey == "" { - return "", nil + if signingKey.KeyID == "" { + return "", signingKey.Format, nil + } + if signingKey.Format == "ssh" { + content, err := os.ReadFile(signingKey.KeyID) + if err != nil { + log.Error("Unable to read SSH public key file in %s: %s, %v", repoPath, signingKey, err) + return "", signingKey.Format, err + } + return string(content), signingKey.Format, nil } content, stderr, err := process.GetManager().ExecDir(ctx, -1, repoPath, - "gpg --export -a", "gpg", "--export", "-a", signingKey) + "gpg --export -a", "gpg", "--export", "-a", signingKey.KeyID) if err != nil { log.Error("Unable to get default signing key in %s: %s, %s, %v", repoPath, signingKey, stderr, err) - return "", err + return "", signingKey.Format, err } - return content, nil + return content, signingKey.Format, nil } // SignInitialCommit determines if we should sign the initial commit to this repository -func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, string, *git.Signature, error) { +func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, asymkey_model.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.InitialCommit) signingKey, sig := SigningKey(ctx, repoPath) - if signingKey == "" { - return false, "", nil, &ErrWontSign{noKey} + if signingKey.KeyID == "" { + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, "", nil, &ErrWontSign{never} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -150,18 +166,18 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if len(keys) == 0 { - return false, "", nil, &ErrWontSign{pubkey} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if twofaModel == nil { - return false, "", nil, &ErrWontSign{twofa} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} } } } @@ -169,19 +185,19 @@ Loop: } // SignWikiCommit determines if we should sign the commits to this repository wiki -func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, string, *git.Signature, error) { +func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, asymkey_model.SigningKey, *git.Signature, error) { repoWikiPath := repo.WikiPath() rules := signingModeFromStrings(setting.Repository.Signing.Wiki) signingKey, sig := SigningKey(ctx, repoWikiPath) - if signingKey == "" { - return false, "", nil, &ErrWontSign{noKey} + if signingKey.KeyID == "" { + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, "", nil, &ErrWontSign{never} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -190,35 +206,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if len(keys) == 0 { - return false, "", nil, &ErrWontSign{pubkey} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if twofaModel == nil { - return false, "", nil, &ErrWontSign{twofa} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo()) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit("HEAD") if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if commit.Signature == nil { - return false, "", nil, &ErrWontSign{parentSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{parentSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} } } } @@ -226,18 +242,18 @@ Loop: } // SignCRUDAction determines if we should sign a CRUD commit to this repository -func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, string, *git.Signature, error) { +func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, asymkey_model.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions) signingKey, sig := SigningKey(ctx, repoPath) - if signingKey == "" { - return false, "", nil, &ErrWontSign{noKey} + if signingKey.KeyID == "" { + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, "", nil, &ErrWontSign{never} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -246,35 +262,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if len(keys) == 0 { - return false, "", nil, &ErrWontSign{pubkey} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if twofaModel == nil { - return false, "", nil, &ErrWontSign{twofa} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit(parentCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if commit.Signature == nil { - return false, "", nil, &ErrWontSign{parentSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{parentSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} } } } @@ -282,16 +298,16 @@ Loop: } // SignMerge determines if we should sign a PR merge commit to the base repository -func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, string, *git.Signature, error) { +func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, asymkey_model.SigningKey, *git.Signature, error) { if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("Unable to get Base Repo for pull request") - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } repo := pr.BaseRepo signingKey, signer := SigningKey(ctx, repo.RepoPath()) - if signingKey == "" { - return false, "", nil, &ErrWontSign{noKey} + if signingKey.KeyID == "" { + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} } rules := signingModeFromStrings(setting.Repository.Signing.Merges) @@ -302,7 +318,7 @@ Loop: for _, rule := range rules { switch rule { case never: - return false, "", nil, &ErrWontSign{never} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -311,91 +327,91 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if len(keys) == 0 { - return false, "", nil, &ErrWontSign{pubkey} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if twofaModel == nil { - return false, "", nil, &ErrWontSign{twofa} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} } case approved: protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, pr.BaseBranch) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } if protectedBranch == nil { - return false, "", nil, &ErrWontSign{approved} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{approved} } if issues_model.GetGrantedApprovalsCount(ctx, protectedBranch, pr) < 1 { - return false, "", nil, &ErrWontSign{approved} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{approved} } case baseSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(baseCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{baseSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{baseSigned} } case headSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{headSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{headSigned} } case commitsSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{commitsSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{commitsSigned} } // need to work out merge-base mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) if err != nil { - return false, "", nil, err + return false, asymkey_model.SigningKey{}, nil, err } for _, commit := range commitList { verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, "", nil, &ErrWontSign{commitsSigned} + return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{commitsSigned} } } } diff --git a/services/context/repo.go b/services/context/repo.go index 127d31325867a..3521b7cd7bebf 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -14,6 +14,7 @@ import ( "path" "strings" + asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" @@ -101,7 +102,7 @@ type CanCommitToBranchResults struct { UserCanPush bool RequireSigned bool WillSign bool - SigningKey string + SigningKey asymkey_model.SigningKey WontSignReason string } diff --git a/services/pull/merge.go b/services/pull/merge.go index 256db847ef39b..42b1871a9f44f 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -396,10 +396,13 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use func commitAndSignNoAuthor(ctx *mergeContext, message string) error { cmdCommit := git.NewCommand("commit").AddOptionFormat("--message=%s", message) - if ctx.signKeyID == "" { + if ctx.signKey.KeyID == "" { cmdCommit.AddArguments("--no-gpg-sign") } else { - cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID) + if ctx.signKey.Format != "" { + cmdCommit.AddConfig("gpg.format", ctx.signKey.Format) + } + cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID) } if err := cmdCommit.Run(ctx, ctx.RunOpts()); err != nil { log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 719cc6b965605..093ea0ed379b0 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -14,6 +14,7 @@ import ( "strings" "time" + asymkey_model "code.gitea.io/gitea/models/asymkey" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -27,7 +28,7 @@ type mergeContext struct { doer *user_model.User sig *git.Signature committer *git.Signature - signKeyID string // empty for no-sign, non-empty to sign + signKey asymkey_model.SigningKey // empty for no-sign, non-empty to sign env []string } @@ -101,7 +102,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque // Determine if we should sign sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch) if sign { - mergeCtx.signKeyID = keyID + mergeCtx.signKey = keyID if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { mergeCtx.committer = signer } diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 72660cd3c57d4..6de319ca2e444 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -74,10 +74,13 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { cmdCommit := git.NewCommand("commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). AddOptionFormat("--message=%s", message) - if ctx.signKeyID == "" { + if ctx.signKey.KeyID == "" { cmdCommit.AddArguments("--no-gpg-sign") } else { - cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID) + if ctx.signKey.Format != "" { + cmdCommit.AddConfig("gpg.format", ctx.signKey.Format) + } + cmdCommit.AddOptionFormat("-S%s", ctx.signKey.KeyID) } if err := cmdCommit.Run(ctx, ctx.RunOpts()); err != nil { log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 493ff9998d0e7..2f7a1c8b3d89b 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -14,6 +14,7 @@ import ( "strings" "time" + asymkey_model "code.gitea.io/gitea/models/asymkey" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -293,15 +294,18 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit } var sign bool - var keyID string + var key asymkey_model.SigningKey var signer *git.Signature if opts.ParentCommitID != "" { - sign, keyID, signer, _ = asymkey_service.SignCRUDAction(ctx, t.repo.RepoPath(), opts.DoerUser, t.basePath, opts.ParentCommitID) + sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, t.repo.RepoPath(), opts.DoerUser, t.basePath, opts.ParentCommitID) } else { - sign, keyID, signer, _ = asymkey_service.SignInitialCommit(ctx, t.repo.RepoPath(), opts.DoerUser) + sign, key, signer, _ = asymkey_service.SignInitialCommit(ctx, t.repo.RepoPath(), opts.DoerUser) } if sign { - cmdCommitTree.AddOptionFormat("-S%s", keyID) + if key.Format != "" { + cmdCommitTree.AddConfig("gpg.format", key.Format) + } + cmdCommitTree.AddOptionFormat("-S%s", key.KeyID) if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email { // Add trailers diff --git a/services/repository/init.go b/services/repository/init.go index bd777b8a2fc04..1eeeb4aa4faa8 100644 --- a/services/repository/init.go +++ b/services/repository/init.go @@ -42,9 +42,12 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi cmd := git.NewCommand("commit", "--message=Initial commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email) - sign, keyID, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) + sign, key, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) if sign { - cmd.AddOptionFormat("-S%s", keyID) + if key.Format != "" { + cmd.AddConfig("gpg.format", key.Format) + } + cmd.AddOptionFormat("-S%s", key.KeyID) if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { // need to set the committer to the KeyID owner diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index 45a08dc5d686d..b42c896740aa2 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -194,7 +194,8 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer) if sign { - commitTreeOpts.KeyID = signingKey + commitTreeOpts.KeyID = signingKey.KeyID + commitTreeOpts.KeyFormat = signingKey.Format if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } @@ -316,7 +317,8 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer) if sign { - commitTreeOpts.KeyID = signingKey + commitTreeOpts.KeyID = signingKey.KeyID + commitTreeOpts.KeyFormat = signingKey.Format if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 223a2e84103f6..eb876050af94b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -15103,6 +15103,42 @@ } } }, + "/repos/{owner}/{repo}/signing-key.pub": { + "get": { + "produces": [ + "text/plain" + ], + "tags": [ + "repository" + ], + "summary": "Get signing-key.pub for given repository", + "operationId": "repoSigningKeySSH", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "ssh public key", + "schema": { + "type": "string" + } + } + } + } + }, "/repos/{owner}/{repo}/stargazers": { "get": { "produces": [ @@ -16936,6 +16972,26 @@ } } }, + "/signing-key.pub": { + "get": { + "produces": [ + "text/plain" + ], + "tags": [ + "miscellaneous" + ], + "summary": "Get default signing-key.pub", + "operationId": "getSigningKeySSH", + "responses": { + "200": { + "description": "ssh public key", + "schema": { + "type": "string" + } + } + } + } + }, "/teams/{id}": { "get": { "produces": [ From 0a35694007c15a54ff9b4b63e7bb60da1d7150ae Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 2 May 2025 11:32:37 +0200 Subject: [PATCH 02/45] fix code style --- modules/git/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index 7910f597f2696..dfc58fb3f5053 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -197,7 +197,7 @@ func (c *Command) AddDashesAndList(list ...string) *Command { return c } -func (c *Command) AddConfig(key string, value string) *Command { +func (c *Command) AddConfig(key, value string) *Command { kv := key + "=" + value if !isSafeArgumentValue(kv) { c.brokenArgs = append(c.brokenArgs, key) From 999860c2319fbcc996ea2b08cf02a1b53f91fafd Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 2 May 2025 11:44:24 +0200 Subject: [PATCH 03/45] fix error style --- routers/api/v1/misc/signing.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index 4135aa664eaf2..78ac1bf02a07c 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -4,6 +4,7 @@ package misc import ( + "errors" "fmt" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -56,7 +57,7 @@ func SigningKey(ctx *context.APIContext) { return } if format == "ssh" { - ctx.APIErrorNotFound(fmt.Errorf("SSH keys are used for signing, not GPG")) + ctx.APIErrorNotFound(errors.New("SSH keys are used for signing, not GPG")) return } _, err = ctx.Write([]byte(content)) @@ -111,7 +112,7 @@ func SigningKeySSH(ctx *context.APIContext) { return } if format != "ssh" { - ctx.APIErrorNotFound(fmt.Errorf("GPG keys are used for signing, not SSH")) + ctx.APIErrorNotFound(errors.New("GPG keys are used for signing, not SSH")) return } _, err = ctx.Write([]byte(content)) From 6a29629d2f924418ab1e7dd0ea7432f43b227490 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 2 May 2025 11:53:34 +0200 Subject: [PATCH 04/45] add copyright --- models/asymkey/key.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/asymkey/key.go b/models/asymkey/key.go index 4e7c50f437b70..db01934346621 100644 --- a/models/asymkey/key.go +++ b/models/asymkey/key.go @@ -1,3 +1,6 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package asymkey type SigningKey struct { From 6957ba958cd68742cf6dd072012bb0345c9690cb Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 3 May 2025 01:24:19 +0200 Subject: [PATCH 05/45] set default key format to openpgp --- models/asymkey/key.go | 9 -- modules/git/key.go | 16 ++++ modules/git/repo_gpg.go | 4 +- modules/setting/repository.go | 1 + routers/api/v1/misc/signing.go | 5 +- services/asymkey/commit.go | 4 +- services/asymkey/sign.go | 118 ++++++++++++------------- services/context/repo.go | 3 +- services/pull/merge_prepare.go | 3 +- services/repository/files/temp_repo.go | 3 +- 10 files changed, 86 insertions(+), 80 deletions(-) delete mode 100644 models/asymkey/key.go create mode 100644 modules/git/key.go diff --git a/models/asymkey/key.go b/models/asymkey/key.go deleted file mode 100644 index db01934346621..0000000000000 --- a/models/asymkey/key.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright 2025 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package asymkey - -type SigningKey struct { - KeyID string - Format string -} diff --git a/modules/git/key.go b/modules/git/key.go new file mode 100644 index 0000000000000..1653c9e49793e --- /dev/null +++ b/modules/git/key.go @@ -0,0 +1,16 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +const ( + // KeyTypeOpenPGP is the key type for GPG keys + KeyTypeOpenPGP = "openpgp" + // KeyTypeSSH is the key type for SSH keys + KeyTypeSSH = "ssh" +) + +type SigningKey struct { + KeyID string + Format string +} diff --git a/modules/git/repo_gpg.go b/modules/git/repo_gpg.go index 0a2ceb92187a7..e226757af4c87 100644 --- a/modules/git/repo_gpg.go +++ b/modules/git/repo_gpg.go @@ -14,7 +14,7 @@ import ( // LoadPublicKeyContent will load the key from gpg func (gpgSettings *GPGSettings) LoadPublicKeyContent() error { - if gpgSettings.Format == "ssh" { + if gpgSettings.Format == KeyTypeOpenPGP { content, err := os.ReadFile(gpgSettings.KeyID) if err != nil { return fmt.Errorf("unable to read SSH public key file: %s, %w", gpgSettings.KeyID, err) @@ -53,7 +53,7 @@ func (repo *Repository) GetDefaultPublicGPGKey(forceUpdate bool) (*GPGSettings, signingKey, _, _ := NewCommand("config", "--get", "user.signingkey").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.KeyID = strings.TrimSpace(signingKey) - format, _, _ := NewCommand("config", "--get", "gpg.format").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) + format, _, _ := NewCommand("config", "--default", KeyTypeOpenPGP, "--get", "gpg.format").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.Format = strings.TrimSpace(format) defaultEmail, _, _ := NewCommand("config", "--get", "user.email").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index c8268eaf4460a..7cb427f71e281 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -255,6 +255,7 @@ var ( SigningKey: "default", SigningName: "", SigningEmail: "", + SigningFormat: "openpgp", InitialCommit: []string{"always"}, CRUDActions: []string{"pubkey", "twofa", "parentsigned"}, Merges: []string{"pubkey", "twofa", "basesigned", "commitssigned"}, diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index 78ac1bf02a07c..e66a1f70df9d0 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" + "code.gitea.io/gitea/modules/git" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/context" ) @@ -56,7 +57,7 @@ func SigningKey(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } - if format == "ssh" { + if format != git.KeyTypeOpenPGP { ctx.APIErrorNotFound(errors.New("SSH keys are used for signing, not GPG")) return } @@ -111,7 +112,7 @@ func SigningKeySSH(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } - if format != "ssh" { + if format != git.KeyTypeSSH { ctx.APIErrorNotFound(errors.New("GPG keys are used for signing, not SSH")) return } diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 6de4611389ae8..93cea85b37355 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -414,7 +414,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * defaultReason := asymkey_model.NoKeyFound - if setting.Repository.Signing.SigningFormat == "ssh" && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { + if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { // OK we should try the default key gpgSettings := git.GPGSettings{ Sign: true, @@ -442,7 +442,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } defaultGPGSettings, err := c.GetRepositoryDefaultPublicGPGKey(false) - if defaultGPGSettings.Format == "ssh" { + if defaultGPGSettings.Format == git.KeyTypeSSH { if err != nil { log.Error("Error getting default public gpg key: %v", err) } else if defaultGPGSettings == nil { diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 57eb6782c5738..90ce63c33b40e 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -86,9 +86,9 @@ func IsErrWontSign(err error) bool { } // SigningKey returns the KeyID and git Signature for the repo -func SigningKey(ctx context.Context, repoPath string) (asymkey_model.SigningKey, *git.Signature) { +func SigningKey(ctx context.Context, repoPath string) (git.SigningKey, *git.Signature) { if setting.Repository.Signing.SigningKey == "none" { - return asymkey_model.SigningKey{}, nil + return git.SigningKey{}, nil } if setting.Repository.Signing.SigningKey == "default" || setting.Repository.Signing.SigningKey == "" { @@ -96,14 +96,14 @@ func SigningKey(ctx context.Context, repoPath string) (asymkey_model.SigningKey, value, _, _ := git.NewCommand("config", "--get", "commit.gpgsign").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) sign, valid := git.ParseBool(strings.TrimSpace(value)) if !sign || !valid { - return asymkey_model.SigningKey{}, nil + return git.SigningKey{}, nil } - format, _, _ := git.NewCommand("config", "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + format, _, _ := git.NewCommand("config", "--default", git.KeyTypeOpenPGP, "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingKey, _, _ := git.NewCommand("config", "--get", "user.signingkey").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingName, _, _ := git.NewCommand("config", "--get", "user.name").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingEmail, _, _ := git.NewCommand("config", "--get", "user.email").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) - return asymkey_model.SigningKey{ + return git.SigningKey{ KeyID: strings.TrimSpace(signingKey), Format: strings.TrimSpace(format), }, &git.Signature{ @@ -112,7 +112,7 @@ func SigningKey(ctx context.Context, repoPath string) (asymkey_model.SigningKey, } } - return asymkey_model.SigningKey{ + return git.SigningKey{ KeyID: setting.Repository.Signing.SigningKey, Format: setting.Repository.Signing.SigningFormat, }, &git.Signature{ @@ -127,7 +127,7 @@ func PublicSigningKey(ctx context.Context, repoPath string) (string, string, err if signingKey.KeyID == "" { return "", signingKey.Format, nil } - if signingKey.Format == "ssh" { + if signingKey.Format == git.KeyTypeSSH { content, err := os.ReadFile(signingKey.KeyID) if err != nil { log.Error("Unable to read SSH public key file in %s: %s, %v", repoPath, signingKey, err) @@ -146,18 +146,18 @@ func PublicSigningKey(ctx context.Context, repoPath string) (string, string, err } // SignInitialCommit determines if we should sign the initial commit to this repository -func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, asymkey_model.SigningKey, *git.Signature, error) { +func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, git.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.InitialCommit) signingKey, sig := SigningKey(ctx, repoPath) if signingKey.KeyID == "" { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} + return false, git.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} + return false, git.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -166,18 +166,18 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if len(keys) == 0 { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if twofaModel == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} + return false, git.SigningKey{}, nil, &ErrWontSign{twofa} } } } @@ -185,19 +185,19 @@ Loop: } // SignWikiCommit determines if we should sign the commits to this repository wiki -func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, asymkey_model.SigningKey, *git.Signature, error) { +func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, git.SigningKey, *git.Signature, error) { repoWikiPath := repo.WikiPath() rules := signingModeFromStrings(setting.Repository.Signing.Wiki) signingKey, sig := SigningKey(ctx, repoWikiPath) if signingKey.KeyID == "" { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} + return false, git.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} + return false, git.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -206,35 +206,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if len(keys) == 0 { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if twofaModel == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} + return false, git.SigningKey{}, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo()) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit("HEAD") if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if commit.Signature == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} } } } @@ -242,18 +242,18 @@ Loop: } // SignCRUDAction determines if we should sign a CRUD commit to this repository -func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, asymkey_model.SigningKey, *git.Signature, error) { +func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, git.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions) signingKey, sig := SigningKey(ctx, repoPath) if signingKey.KeyID == "" { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} + return false, git.SigningKey{}, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} + return false, git.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -262,35 +262,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if len(keys) == 0 { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if twofaModel == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} + return false, git.SigningKey{}, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit(parentCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if commit.Signature == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} } } } @@ -298,16 +298,16 @@ Loop: } // SignMerge determines if we should sign a PR merge commit to the base repository -func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, asymkey_model.SigningKey, *git.Signature, error) { +func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, git.SigningKey, *git.Signature, error) { if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("Unable to get Base Repo for pull request") - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } repo := pr.BaseRepo signingKey, signer := SigningKey(ctx, repo.RepoPath()) if signingKey.KeyID == "" { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{noKey} + return false, git.SigningKey{}, nil, &ErrWontSign{noKey} } rules := signingModeFromStrings(setting.Repository.Signing.Merges) @@ -318,7 +318,7 @@ Loop: for _, rule := range rules { switch rule { case never: - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{never} + return false, git.SigningKey{}, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -327,91 +327,91 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if len(keys) == 0 { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if twofaModel == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{twofa} + return false, git.SigningKey{}, nil, &ErrWontSign{twofa} } case approved: protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, pr.BaseBranch) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } if protectedBranch == nil { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{approved} + return false, git.SigningKey{}, nil, &ErrWontSign{approved} } if issues_model.GetGrantedApprovalsCount(ctx, protectedBranch, pr) < 1 { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{approved} + return false, git.SigningKey{}, nil, &ErrWontSign{approved} } case baseSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(baseCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{baseSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{baseSigned} } case headSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{headSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{headSigned} } case commitsSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{commitsSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{commitsSigned} } // need to work out merge-base mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) if err != nil { - return false, asymkey_model.SigningKey{}, nil, err + return false, git.SigningKey{}, nil, err } for _, commit := range commitList { verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, asymkey_model.SigningKey{}, nil, &ErrWontSign{commitsSigned} + return false, git.SigningKey{}, nil, &ErrWontSign{commitsSigned} } } } diff --git a/services/context/repo.go b/services/context/repo.go index 3521b7cd7bebf..1a5951c2ccaa8 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -14,7 +14,6 @@ import ( "path" "strings" - asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" @@ -102,7 +101,7 @@ type CanCommitToBranchResults struct { UserCanPush bool RequireSigned bool WillSign bool - SigningKey asymkey_model.SigningKey + SigningKey git.SigningKey WontSignReason string } diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 093ea0ed379b0..e28ca49659b53 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -14,7 +14,6 @@ import ( "strings" "time" - asymkey_model "code.gitea.io/gitea/models/asymkey" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -28,7 +27,7 @@ type mergeContext struct { doer *user_model.User sig *git.Signature committer *git.Signature - signKey asymkey_model.SigningKey // empty for no-sign, non-empty to sign + signKey git.SigningKey // empty for no-sign, non-empty to sign env []string } diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 2f7a1c8b3d89b..c0f21731372f8 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -14,7 +14,6 @@ import ( "strings" "time" - asymkey_model "code.gitea.io/gitea/models/asymkey" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -294,7 +293,7 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit } var sign bool - var key asymkey_model.SigningKey + var key git.SigningKey var signer *git.Signature if opts.ParentCommitID != "" { sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, t.repo.RepoPath(), opts.DoerUser, t.basePath, opts.ParentCommitID) From 785934613b957d1146db1e99dcde562ce6dee9d3 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 3 May 2025 10:44:48 +0200 Subject: [PATCH 06/45] Cleanup CommitTreeOpts --- modules/git/repo_tree.go | 11 +++++------ services/wiki/wiki.go | 6 ++---- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 064b1317f07ad..0d25b9d3a1835 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -15,8 +15,7 @@ import ( type CommitTreeOpts struct { Parents []string Message string - KeyID string - KeyFormat string + Key SigningKey NoGPGSign bool AlwaysSign bool } @@ -44,11 +43,11 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString(opts.Message) _, _ = messageBytes.WriteString("\n") - if opts.KeyID != "" || opts.AlwaysSign { - if opts.KeyFormat != "" { - cmd.AddConfig("gpg.format", opts.KeyFormat) + if opts.Key.KeyID != "" || opts.AlwaysSign { + if opts.Key.Format != "" { + cmd.AddConfig("gpg.format", opts.Key.Format) } - cmd.AddOptionFormat("-S%s", opts.KeyID) + cmd.AddOptionFormat("-S%s", opts.Key.KeyID) } if opts.NoGPGSign { diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index b42c896740aa2..6f028c881239c 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -194,8 +194,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer) if sign { - commitTreeOpts.KeyID = signingKey.KeyID - commitTreeOpts.KeyFormat = signingKey.Format + commitTreeOpts.Key = signingKey if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } @@ -317,8 +316,7 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model sign, signingKey, signer, _ := asymkey_service.SignWikiCommit(ctx, repo, doer) if sign { - commitTreeOpts.KeyID = signingKey.KeyID - commitTreeOpts.KeyFormat = signingKey.Format + commitTreeOpts.Key = signingKey if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { committer = signer } From 3c363c5cf7ec3a462d883e91d33ab9a483f7c910 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 3 May 2025 10:48:20 +0200 Subject: [PATCH 07/45] add missing docs --- modules/git/key.go | 3 ++- modules/setting/repository.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/git/key.go b/modules/git/key.go index 1653c9e49793e..a6ea543ef1c5b 100644 --- a/modules/git/key.go +++ b/modules/git/key.go @@ -3,8 +3,9 @@ package git +// Based on https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat const ( - // KeyTypeOpenPGP is the key type for GPG keys + // KeyTypeOpenPGP is the key type for GPG keys, expected default of git cli KeyTypeOpenPGP = "openpgp" // KeyTypeSSH is the key type for SSH keys KeyTypeSSH = "ssh" diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 7cb427f71e281..bf0ae923d4459 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -255,7 +255,7 @@ var ( SigningKey: "default", SigningName: "", SigningEmail: "", - SigningFormat: "openpgp", + SigningFormat: "openpgp", // git.KeyTypeOpenPGP InitialCommit: []string{"always"}, CRUDActions: []string{"pubkey", "twofa", "parentsigned"}, Merges: []string{"pubkey", "twofa", "basesigned", "commitssigned"}, From 3e992a0cd369951a5464b5ee544ceacd63ed9206 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 3 May 2025 13:27:59 +0200 Subject: [PATCH 08/45] add missing endpoint --- routers/api/v1/api.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 3b1447dc68a09..8ec7d4f6c3c98 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1426,6 +1426,7 @@ func Routes() *web.Router { Get(repo.GetFileContentsGet). Post(bind(api.GetFilesOptions{}), repo.GetFileContentsPost) // POST method requires "write" permission, so we also support "GET" method above m.Get("/signing-key.gpg", misc.SigningKey) + m.Get("/signing-key.pub", misc.SigningKeySSH) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). Put(reqToken(), reqAdmin(), bind(api.RepoTopicOptions{}), repo.UpdateTopics) From a73ccb46bcf2b8b53bb7c442d5679b384be0a523 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sun, 11 May 2025 10:39:43 +0200 Subject: [PATCH 09/45] Update modules/git/repo_gpg.go Co-authored-by: Lunny Xiao --- modules/git/repo_gpg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_gpg.go b/modules/git/repo_gpg.go index e226757af4c87..6eff247f66480 100644 --- a/modules/git/repo_gpg.go +++ b/modules/git/repo_gpg.go @@ -14,7 +14,7 @@ import ( // LoadPublicKeyContent will load the key from gpg func (gpgSettings *GPGSettings) LoadPublicKeyContent() error { - if gpgSettings.Format == KeyTypeOpenPGP { + if gpgSettings.Format == KeyTypeSSH { content, err := os.ReadFile(gpgSettings.KeyID) if err != nil { return fmt.Errorf("unable to read SSH public key file: %s, %w", gpgSettings.KeyID, err) From 8007fba4b14cc40010d0b6c0f56e81bff0153921 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 11 May 2025 11:04:26 +0200 Subject: [PATCH 10/45] update app.example.ini --- custom/conf/app.example.ini | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index a7476ad1be818..13c0e6bc6dbbb 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1190,17 +1190,21 @@ LEVEL = Info ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; -;; GPG key to use to sign commits, Defaults to the default - that is the value of git config --get user.signingkey +;; GPG or SSH key to use to sign commits, Defaults to the default - that is the value of git config --get user.signingkey +;; Depending on the value of SIGNING_FORMAT this is either: +;; - the GPG key ID +;; - the path to the ssh public key "/path/to/key.pub": where "/path/to/key" is the private key, use ssh-keygen -t ed25519 to generate a new key pair without password ;; run in the context of the RUN_USER ;; Switch to none to stop signing completely ;SIGNING_KEY = default ;; -;; If a SIGNING_KEY ID is provided and is not set to default, use the provided Name and Email address as the signer. +;; If a SIGNING_KEY ID is provided and is not set to default, use the provided Name and Email address as the signer and the signing format. ;; These should match a publicized name and email address for the key. (When SIGNING_KEY is default these are set to -;; the results of git config --get user.name and git config --get user.email respectively and can only be overridden +;; the results of git config --get user.name, git config --get user.email and git config --default openpgp --get gpg.format respectively and can only be overridden ;; by setting the SIGNING_KEY ID to the correct ID.) ;SIGNING_NAME = ;SIGNING_EMAIL = +;SIGNING_FORMAT = openpgp ;; ;; Sets the default trust model for repositories. Options are: collaborator, committer, collaboratorcommitter ;DEFAULT_TRUST_MODEL = collaborator @@ -1227,6 +1231,12 @@ LEVEL = Info ;; - 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 +;; +;; Determines which additional ssh keys are trusted for all signed commits regardless of the user +;; This is useful for ssh signing key rotation. +;; Multiple keys should be comma separated. +;; E.g."ssh- ". or "ssh- , ssh- ". +;TRUSTED_SSH_KEYS = ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From 6ae80f9d21e759498dda255f0a5e744fecec4b48 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sun, 11 May 2025 21:56:07 +0200 Subject: [PATCH 11/45] Update modules/git/command.go Co-authored-by: Lunny Xiao --- modules/git/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index dfc58fb3f5053..3598f978af80b 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -331,7 +331,7 @@ func (c *Command) run(ctx context.Context, skip int, opts *RunOpts) error { startTime := time.Now() - cmd := exec.CommandContext(ctx, c.prog, append(append([]string{}, c.configArgs...), c.args...)...) + cmd := exec.CommandContext(ctx, c.prog, append(c.configArgs, c.args...)...) c.cmd = cmd // for debug purpose only if opts.Env == nil { cmd.Env = os.Environ() From 6c0feaa420e8ee7f1c5a569a37edf6e975ff605c Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Mon, 12 May 2025 10:56:17 +0200 Subject: [PATCH 12/45] Update modules/git/command.go Co-authored-by: Lunny Xiao --- modules/git/command.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index 3598f978af80b..51c225e3b1863 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -201,8 +201,9 @@ func (c *Command) AddConfig(key, value string) *Command { kv := key + "=" + value if !isSafeArgumentValue(kv) { c.brokenArgs = append(c.brokenArgs, key) + } else { + c.configArgs = append(c.configArgs, "-c", kv) } - c.configArgs = append(c.configArgs, "-c", kv) return c } From 98bcc7cb1cfad261a0cb02fc6a70a5ad354b88d1 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Mon, 12 May 2025 11:31:44 +0200 Subject: [PATCH 13/45] fix indent --- modules/git/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index 51c225e3b1863..22f1d02339148 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -202,7 +202,7 @@ func (c *Command) AddConfig(key, value string) *Command { if !isSafeArgumentValue(kv) { c.brokenArgs = append(c.brokenArgs, key) } else { - c.configArgs = append(c.configArgs, "-c", kv) + c.configArgs = append(c.configArgs, "-c", kv) } return c } From dce379767c8d6fe3a6c5c33ee66e4a54d3d1e261 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:03:06 +0200 Subject: [PATCH 14/45] reuse gpg tests and expand them to ssh --- services/asymkey/commit.go | 10 +- .../{gpg_git_test.go => gpg_ssh_git_test.go} | 97 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) rename tests/integration/{gpg_git_test.go => gpg_ssh_git_test.go} (78%) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 93cea85b37355..745517907edec 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -432,7 +432,10 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * Content: gpgSettings.PublicKeyContent, Fingerprint: fingerprint, HasUsed: true, - }, committer, committer, committer.Email); commitVerification != nil { + }, committer, &user_model.User{ + Name: gpgSettings.Name, + Email: gpgSettings.Email, + }, gpgSettings.Email); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -457,7 +460,10 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * Content: defaultGPGSettings.PublicKeyContent, Fingerprint: fingerprint, HasUsed: true, - }, committer, committer, committer.Email); commitVerification != nil { + }, committer, &user_model.User{ + Name: defaultGPGSettings.Name, + Email: defaultGPGSettings.Email, + }, defaultGPGSettings.Email); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { diff --git a/tests/integration/gpg_git_test.go b/tests/integration/gpg_ssh_git_test.go similarity index 78% rename from tests/integration/gpg_git_test.go rename to tests/integration/gpg_ssh_git_test.go index 32de200f63d66..346af588aba96 100644 --- a/tests/integration/gpg_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -4,7 +4,10 @@ package integration import ( + "crypto/ed25519" + "crypto/rand" "encoding/base64" + "encoding/pem" "fmt" "net/url" "os" @@ -18,6 +21,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" + "golang.org/x/crypto/ssh" "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/armor" @@ -42,6 +46,99 @@ func TestGPGGit(t *testing.T) { defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() + testGitSigning(t) +} + +func TestGPGGitDefaults(t *testing.T) { + tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + err := os.Chmod(tmpDir, 0o700) + assert.NoError(t, err) + + t.Setenv("GNUPGHOME", tmpDir) + + // Need to create a root key + rootKeyPair, err := importTestingKey() + require.NoError(t, err, "importTestingKey") + + os.WriteFile(tmpDir+"/.gitconfig", []byte(` +[user] + name = gitea + email = gitea@fake.local + signingKey = `+rootKeyPair.PrimaryKey.KeyIdShortString()+` +[commit] + gpgsign = true +`), 0o600) + + defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() + defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() + defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() + defer test.MockVariableValue(&setting.Git.HomePath, tmpDir)() + + testGitSigning(t) +} + +func TestSSHGit(t *testing.T) { + tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + err := os.Chmod(tmpDir, 0o700) + assert.NoError(t, err) + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err, "ed25519.GenerateKey") + sshPubKey, err := ssh.NewPublicKey(pub) + require.NoError(t, err, "ssh.NewPublicKey") + _, err = ssh.NewSignerFromKey(priv) + require.NoError(t, err, "ssh.NewSignerFromKey") + + os.WriteFile(tmpDir+"/id_ed25519.pub", []byte(ssh.MarshalAuthorizedKey(sshPubKey)), 0o600) + block, err := ssh.MarshalPrivateKey(priv, "") + os.WriteFile(tmpDir+"/id_ed25519", []byte(pem.EncodeToMemory(block)), 0o600) + + defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, tmpDir+"/id_ed25519.pub")() + defer test.MockVariableValue(&setting.Repository.Signing.SigningName, "gitea")() + defer test.MockVariableValue(&setting.Repository.Signing.SigningEmail, "gitea@fake.local")() + defer test.MockVariableValue(&setting.Repository.Signing.SigningFormat, "ssh")() + defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() + defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() + + testGitSigning(t) +} + +func TestSSHGitDefaults(t *testing.T) { + tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + err := os.Chmod(tmpDir, 0o700) + assert.NoError(t, err) + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err, "ed25519.GenerateKey") + sshPubKey, err := ssh.NewPublicKey(pub) + require.NoError(t, err, "ssh.NewPublicKey") + _, err = ssh.NewSignerFromKey(priv) + require.NoError(t, err, "ssh.NewSignerFromKey") + + os.WriteFile(tmpDir+"/id_ed25519.pub", []byte(ssh.MarshalAuthorizedKey(sshPubKey)), 0o600) + block, err := ssh.MarshalPrivateKey(priv, "") + os.WriteFile(tmpDir+"/id_ed25519", []byte(pem.EncodeToMemory(block)), 0o600) + os.WriteFile(tmpDir+"/.gitconfig", []byte(` +[user] + name = gitea + email = gitea@fake.local + signingKey = `+tmpDir+`/id_ed25519.pub +[gpg] + format = ssh +[commit] + gpgsign = true +`), 0o600) + + defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() + defer test.MockVariableValue(&setting.Repository.Signing.SigningFormat, "ssh")() + defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() + defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() + defer test.MockVariableValue(&setting.Git.HomePath, tmpDir)() + + testGitSigning(t) +} + +func testGitSigning(t *testing.T) { username := "user2" user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: username}) baseAPITestContext := NewAPITestContext(t, username, "repo1") From 48a1d6dc2a11de27251c66bafe2ede58de5572e9 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:04:11 +0200 Subject: [PATCH 15/45] rename keyID to key --- services/pull/merge_prepare.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index e28ca49659b53..c1b606ed2ea19 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -99,9 +99,9 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque mergeCtx.committer = mergeCtx.sig // Determine if we should sign - sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch) + sign, key, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch) if sign { - mergeCtx.signKey = keyID + mergeCtx.signKey = key if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { mergeCtx.committer = signer } From e749a14910ecc04fb51738bff6a0243706626f4b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:04:40 +0200 Subject: [PATCH 16/45] update comment that signKey.KeyID may be empty --- services/pull/merge_prepare.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index c1b606ed2ea19..eb9aeb08eb558 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -27,7 +27,7 @@ type mergeContext struct { doer *user_model.User sig *git.Signature committer *git.Signature - signKey git.SigningKey // empty for no-sign, non-empty to sign + signKey git.SigningKey // signKey.KeyID empty for no-sign, non-empty to sign env []string } From 984557de979b875c26a452b87f29b308448c37d2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:06:30 +0200 Subject: [PATCH 17/45] improve example description --- custom/conf/app.example.ini | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 13c0e6bc6dbbb..5b3c069154eaa 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1192,8 +1192,8 @@ LEVEL = Info ;; ;; GPG or SSH key to use to sign commits, Defaults to the default - that is the value of git config --get user.signingkey ;; Depending on the value of SIGNING_FORMAT this is either: -;; - the GPG key ID -;; - the path to the ssh public key "/path/to/key.pub": where "/path/to/key" is the private key, use ssh-keygen -t ed25519 to generate a new key pair without password +;; - openpgp: the GPG key ID +;; - ssh: the path to the ssh public key "/path/to/key.pub": where "/path/to/key" is the private key, use ssh-keygen -t ed25519 to generate a new key pair without password ;; run in the context of the RUN_USER ;; Switch to none to stop signing completely ;SIGNING_KEY = default @@ -1204,6 +1204,9 @@ LEVEL = Info ;; by setting the SIGNING_KEY ID to the correct ID.) ;SIGNING_NAME = ;SIGNING_EMAIL = +;; SIGNING_FORMAT can be one of: +;; - openpgp (default): use GPG to sign commits +;; - ssh: use SSH to sign commits ;SIGNING_FORMAT = openpgp ;; ;; Sets the default trust model for repositories. Options are: collaborator, committer, collaboratorcommitter From da2c19eea5a5fc490781bb78f0e51d0db3434999 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:13:53 +0200 Subject: [PATCH 18/45] format test code --- tests/integration/gpg_ssh_git_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 346af588aba96..5193a2d38aeaf 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -21,12 +21,12 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" - "golang.org/x/crypto/ssh" "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/armor" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" ) func TestGPGGit(t *testing.T) { From 39eda94a0d66f5907703dec986cb9b99c35379d8 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:24:40 +0200 Subject: [PATCH 19/45] fix error handling in test --- tests/integration/gpg_ssh_git_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 5193a2d38aeaf..1305741574213 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -89,9 +89,12 @@ func TestSSHGit(t *testing.T) { _, err = ssh.NewSignerFromKey(priv) require.NoError(t, err, "ssh.NewSignerFromKey") - os.WriteFile(tmpDir+"/id_ed25519.pub", []byte(ssh.MarshalAuthorizedKey(sshPubKey)), 0o600) + err = os.WriteFile(tmpDir+"/id_ed25519.pub", ssh.MarshalAuthorizedKey(sshPubKey), 0o600) + require.NoError(t, err, "os.WriteFile id_ed25519.pub") block, err := ssh.MarshalPrivateKey(priv, "") - os.WriteFile(tmpDir+"/id_ed25519", []byte(pem.EncodeToMemory(block)), 0o600) + require.NoError(t, err, "ssh.MarshalPrivateKey") + err = os.WriteFile(tmpDir+"/id_ed25519", pem.EncodeToMemory(block), 0o600) + require.NoError(t, err, "os.WriteFile id_ed25519") defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, tmpDir+"/id_ed25519.pub")() defer test.MockVariableValue(&setting.Repository.Signing.SigningName, "gitea")() @@ -115,9 +118,12 @@ func TestSSHGitDefaults(t *testing.T) { _, err = ssh.NewSignerFromKey(priv) require.NoError(t, err, "ssh.NewSignerFromKey") - os.WriteFile(tmpDir+"/id_ed25519.pub", []byte(ssh.MarshalAuthorizedKey(sshPubKey)), 0o600) + err = os.WriteFile(tmpDir+"/id_ed25519.pub", ssh.MarshalAuthorizedKey(sshPubKey), 0o600) + require.NoError(t, err, "os.WriteFile id_ed25519.pub") block, err := ssh.MarshalPrivateKey(priv, "") - os.WriteFile(tmpDir+"/id_ed25519", []byte(pem.EncodeToMemory(block)), 0o600) + require.NoError(t, err, "ssh.MarshalPrivateKey") + err = os.WriteFile(tmpDir+"/id_ed25519", pem.EncodeToMemory(block), 0o600) + require.NoError(t, err, "os.WriteFile id_ed25519") os.WriteFile(tmpDir+"/.gitconfig", []byte(` [user] name = gitea From e45c6fb642d47187149999a09bb8be3802812c50 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:27:02 +0200 Subject: [PATCH 20/45] handle another error in test --- tests/integration/gpg_ssh_git_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 1305741574213..44b6e617ed180 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -60,7 +60,7 @@ func TestGPGGitDefaults(t *testing.T) { rootKeyPair, err := importTestingKey() require.NoError(t, err, "importTestingKey") - os.WriteFile(tmpDir+"/.gitconfig", []byte(` + err = os.WriteFile(tmpDir+"/.gitconfig", []byte(` [user] name = gitea email = gitea@fake.local @@ -68,6 +68,7 @@ func TestGPGGitDefaults(t *testing.T) { [commit] gpgsign = true `), 0o600) + require.NoError(t, err, "os.WriteFile .gitconfig") defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() @@ -124,7 +125,7 @@ func TestSSHGitDefaults(t *testing.T) { require.NoError(t, err, "ssh.MarshalPrivateKey") err = os.WriteFile(tmpDir+"/id_ed25519", pem.EncodeToMemory(block), 0o600) require.NoError(t, err, "os.WriteFile id_ed25519") - os.WriteFile(tmpDir+"/.gitconfig", []byte(` + err = os.WriteFile(tmpDir+"/.gitconfig", []byte(` [user] name = gitea email = gitea@fake.local @@ -134,6 +135,7 @@ func TestSSHGitDefaults(t *testing.T) { [commit] gpgsign = true `), 0o600) + require.NoError(t, err, "os.WriteFile .gitconfig") defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() defer test.MockVariableValue(&setting.Repository.Signing.SigningFormat, "ssh")() From 66194b226d051a6d9bd1c738758db2ca0e8fe69b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 1 Jun 2025 01:39:46 +0200 Subject: [PATCH 21/45] remove dead code --- tests/integration/gpg_ssh_git_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 44b6e617ed180..97a1bbbf293ce 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -87,8 +87,6 @@ func TestSSHGit(t *testing.T) { require.NoError(t, err, "ed25519.GenerateKey") sshPubKey, err := ssh.NewPublicKey(pub) require.NoError(t, err, "ssh.NewPublicKey") - _, err = ssh.NewSignerFromKey(priv) - require.NoError(t, err, "ssh.NewSignerFromKey") err = os.WriteFile(tmpDir+"/id_ed25519.pub", ssh.MarshalAuthorizedKey(sshPubKey), 0o600) require.NoError(t, err, "os.WriteFile id_ed25519.pub") @@ -116,8 +114,6 @@ func TestSSHGitDefaults(t *testing.T) { require.NoError(t, err, "ed25519.GenerateKey") sshPubKey, err := ssh.NewPublicKey(pub) require.NoError(t, err, "ssh.NewPublicKey") - _, err = ssh.NewSignerFromKey(priv) - require.NoError(t, err, "ssh.NewSignerFromKey") err = os.WriteFile(tmpDir+"/id_ed25519.pub", ssh.MarshalAuthorizedKey(sshPubKey), 0o600) require.NoError(t, err, "os.WriteFile id_ed25519.pub") From d3af10fc5874f2d72c448eacdd294bc2661b3a4c Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 16:09:51 +0200 Subject: [PATCH 22/45] fix error handling and add comments about what function verifies which config --- services/asymkey/commit.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 745517907edec..3a00127ba44ea 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -414,6 +414,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * defaultReason := asymkey_model.NoKeyFound + // Covers ssh verification for the default SSH signing key specifed in gitea config if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { // OK we should try the default key gpgSettings := git.GPGSettings{ @@ -425,9 +426,9 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } if err := gpgSettings.LoadPublicKeyContent(); err != nil { log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) - } - fingerprint, _ := asymkey_model.CalcFingerprint(gpgSettings.PublicKeyContent) - if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + } else if fingerprint, err := asymkey_model.CalcFingerprint(gpgSettings.PublicKeyContent); err != nil { + log.Error("Error calculating the fingerprint public key: %s %v", gpgSettings.KeyID, err) + } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ Verified: true, Content: gpgSettings.PublicKeyContent, Fingerprint: fingerprint, @@ -444,18 +445,19 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } + // Covers ssh verification for the default SSH signing key specifed in the .gitconfig file in Git.HomePath setting defaultGPGSettings, err := c.GetRepositoryDefaultPublicGPGKey(false) if defaultGPGSettings.Format == git.KeyTypeSSH { if err != nil { - log.Error("Error getting default public gpg key: %v", err) + log.Error("Error getting default public ssh key: %v", err) } else if defaultGPGSettings == nil { log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) } else if defaultGPGSettings.Sign { if err := defaultGPGSettings.LoadPublicKeyContent(); err != nil { log.Error("Error getting default signing key: %s %v", defaultGPGSettings.KeyID, err) - } - fingerprint, _ := asymkey_model.CalcFingerprint(defaultGPGSettings.PublicKeyContent) - if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + } else if fingerprint, err := asymkey_model.CalcFingerprint(defaultGPGSettings.PublicKeyContent); err != nil { + log.Error("Error calculating the fingerprint public key: %s %v", defaultGPGSettings.KeyID, err) + } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ Verified: true, Content: defaultGPGSettings.PublicKeyContent, Fingerprint: fingerprint, From c0b65efa94e4ef3e48b9b94edb3d6a324616e3e9 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 16:15:13 +0200 Subject: [PATCH 23/45] apply suggestion do not write error to already written response --- routers/api/v1/misc/signing.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index e66a1f70df9d0..ff82494f33124 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -5,7 +5,6 @@ package misc import ( "errors" - "fmt" "code.gitea.io/gitea/modules/git" asymkey_service "code.gitea.io/gitea/services/asymkey" @@ -58,13 +57,10 @@ func SigningKey(ctx *context.APIContext) { return } if format != git.KeyTypeOpenPGP { - ctx.APIErrorNotFound(errors.New("SSH keys are used for signing, not GPG")) + ctx.APIErrorNotFound("SSH keys are used for signing, not GPG") return } - _, err = ctx.Write([]byte(content)) - if err != nil { - ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err)) - } + _, _ = ctx.Write([]byte(content)) } // SigningKey returns the public key of the default signing key if it exists @@ -116,8 +112,5 @@ func SigningKeySSH(ctx *context.APIContext) { ctx.APIErrorNotFound(errors.New("GPG keys are used for signing, not SSH")) return } - _, err = ctx.Write([]byte(content)) - if err != nil { - ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err)) - } + _, _ = ctx.Write([]byte(content)) } From 64729ae1b699c93a83216fb7ec9a23cbe5cbf749 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 16:22:32 +0200 Subject: [PATCH 24/45] update comments --- tests/integration/gpg_ssh_git_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 97a1bbbf293ce..4f41e58ab118e 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -50,7 +50,7 @@ func TestGPGGit(t *testing.T) { } func TestGPGGitDefaults(t *testing.T) { - tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring and git config err := os.Chmod(tmpDir, 0o700) assert.NoError(t, err) @@ -79,7 +79,7 @@ func TestGPGGitDefaults(t *testing.T) { } func TestSSHGit(t *testing.T) { - tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + tmpDir := t.TempDir() // use a temp dir to store the SSH keys err := os.Chmod(tmpDir, 0o700) assert.NoError(t, err) @@ -106,7 +106,7 @@ func TestSSHGit(t *testing.T) { } func TestSSHGitDefaults(t *testing.T) { - tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring + tmpDir := t.TempDir() // use a temp dir to store the SSH keys and git config err := os.Chmod(tmpDir, 0o700) assert.NoError(t, err) From 5af804af67a17e0160b142e8e1d63859bc233e09 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 16:44:19 +0200 Subject: [PATCH 25/45] do not ignore fingerprint calc error --- services/asymkey/commit.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 3a00127ba44ea..e4732393c54d7 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -400,14 +400,14 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } // Trust more than one key for every User for _, k := range setting.Repository.Signing.TrustedSSHKeys { - fingerprint, _ := asymkey_model.CalcFingerprint(k) - commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ + if fingerprint, err := asymkey_model.CalcFingerprint(k); err != nil { + log.Error("Error calculating the fingerprint public key: %s %v", k, err) + } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ Verified: true, Content: k, Fingerprint: fingerprint, HasUsed: true, - }, committer, committer, c.Committer.Email) - if commitVerification != nil { + }, committer, committer, c.Committer.Email); commitVerification != nil { return commitVerification } } From 0f413c54fc7c3bf972fd3edfd1c83832e19ea1dd Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 20:06:51 +0200 Subject: [PATCH 26/45] Add TestTrustedSSHKeys Test via mocked repo --- .../user2/repo-test-trusted-ssh-keys.git/HEAD | 1 + .../repo-test-trusted-ssh-keys.git/config | 6 ++++++ .../description | 1 + .../info/exclude | 6 ++++++ .../9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e | Bin 0 -> 55 bytes .../b7/fca3aa0d80144f9718b0486681944f9c587e33 | 4 ++++ .../b9/04b3dcaada8c26236f096fe337fd3c4cd8048a | Bin 0 -> 36 bytes .../packed-refs | 1 + .../refs/heads/main | 1 + tests/integration/gpg_ssh_git_test.go | 20 ++++++++++++++++++ 10 files changed, 40 insertions(+) create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs create mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD new file mode 100644 index 0000000000000..b870d82622c1a --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/main diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config new file mode 100644 index 0000000000000..e6da231579bcc --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config @@ -0,0 +1,6 @@ +[core] + repositoryformatversion = 0 + filemode = true + bare = true + ignorecase = true + precomposeunicode = true diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description new file mode 100644 index 0000000000000..498b267a8c781 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude new file mode 100644 index 0000000000000..a5196d1be8fb5 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude @@ -0,0 +1,6 @@ +# git ls-files --others --exclude-from=.git/info/exclude +# Lines that start with '#' are comments. +# For a project mostly in C, the following would be a good set of +# exclude patterns (uncomment them if you want to use them): +# *.[oa] +# *~ diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e new file mode 100644 index 0000000000000000000000000000000000000000..bcf0704cd0cbba115cb93c01e583249df6ed4f46 GIT binary patch literal 55 zcmV-70LcG%0V^p=O;s?qU@$Z=Ff%bxD9%jJOHI)$sVHIC$+G#*s#`s3%K4o6kInzu N_}pOW0sv$I5hQpJ8C(DW literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 new file mode 100644 index 0000000000000..6d64c797ca642 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 @@ -0,0 +1,4 @@ +x
��[o�0��ٿb�QW����� $�@��˛�`�ͥ���&�k_z���|ҙ�NZ�Ϫ
�55��� +�f�pl9L��ۤ�k�� +ۙ u3�H$��PVײ�VŸ�s䟼��.��<��1]B1�.0�u���t���r��*��^�`2X��� +'���S���c\L�^�n�E�2Z?���X�S��L��x�Iʭ��_�/�hxzl����1��.1'�r�'�)>M�x��&&Aw-����dL�M��b�ぅ��KC��m]��4��f��׹x��I+�g,�n��Y�Ҷ��Z8��rN�k����v�It���[�������}�0��2PxQ�|Uo�9@��E�!S�D?J�� \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a new file mode 100644 index 0000000000000000000000000000000000000000..ee5dcf6ba398b5e76d1df5e90ee6d0ba63ddfcbe GIT binary patch literal 36 ucmV+<0Nej~0ZYosPf{>4VhG8|ELH%bM1|ta^t{v*g|y6^R6PKbBMHGIZxA~G literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs new file mode 100644 index 0000000000000..250f187384963 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs @@ -0,0 +1 @@ +# pack-refs with: peeled fully-peeled sorted diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main new file mode 100644 index 0000000000000..8b4ec6bcd860c --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main @@ -0,0 +1 @@ +b7fca3aa0d80144f9718b0486681944f9c587e33 diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 4f41e58ab118e..84f5e072cd339 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -366,3 +366,23 @@ func importTestingKey() (*openpgp.Entity, error) { // There should only be one entity in this file. return keyring[0], nil } + +func TestTrustedSSHKeys(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Repository.Signing.TrustedSSHKeys, []string{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIH6Y4idVaW3E+bLw1uqoAfJD7o5Siu+HqS51E9oQLPE9"})() + + username := "user2" + testCtx := NewAPITestContext(t, username, "repo-test-trusted-ssh-keys", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("CheckMainBranchSignedVerified", doAPIGetBranch(testCtx, "main", func(t *testing.T, branch api.Branch) { + require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) + require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) + require.True(t, branch.Commit.Verification.Verified) + })) + + setting.Repository.Signing.TrustedSSHKeys = []string{} + t.Run("CheckMainBranchSignedUnverified", doAPIGetBranch(testCtx, "main", func(t *testing.T, branch api.Branch) { + require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) + require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) + require.False(t, branch.Commit.Verification.Verified) + })) +} From d8e123aecbc298123cc966b656ebe3f42cf184d2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 20:11:54 +0200 Subject: [PATCH 27/45] fix typo --- services/asymkey/commit.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index e4732393c54d7..d648571b70e98 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -414,7 +414,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * defaultReason := asymkey_model.NoKeyFound - // Covers ssh verification for the default SSH signing key specifed in gitea config + // Covers ssh verification for the default SSH signing key specified in gitea config if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { // OK we should try the default key gpgSettings := git.GPGSettings{ @@ -445,7 +445,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } - // Covers ssh verification for the default SSH signing key specifed in the .gitconfig file in Git.HomePath setting + // Covers ssh verification for the default SSH signing key specified in the .gitconfig file in Git.HomePath setting defaultGPGSettings, err := c.GetRepositoryDefaultPublicGPGKey(false) if defaultGPGSettings.Format == git.KeyTypeSSH { if err != nil { From 6f18abc7e9d1745ae6975447242d62a5f9b6ee28 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 20:15:32 +0200 Subject: [PATCH 28/45] fix invalid APIErrorNotFound usage --- routers/api/v1/misc/signing.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index ff82494f33124..e2779fc966a64 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -4,8 +4,6 @@ package misc import ( - "errors" - "code.gitea.io/gitea/modules/git" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/context" @@ -109,7 +107,7 @@ func SigningKeySSH(ctx *context.APIContext) { return } if format != git.KeyTypeSSH { - ctx.APIErrorNotFound(errors.New("GPG keys are used for signing, not SSH")) + ctx.APIErrorNotFound("GPG keys are used for signing, not SSH") return } _, _ = ctx.Write([]byte(content)) From 3da66460fe0f83874d77fcc801f5b5f2216a69f9 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 20:55:37 +0200 Subject: [PATCH 29/45] add missing fixtures --- models/fixtures/branch.yml | 12 ++++++++++++ models/fixtures/repo_unit.yml | 7 +++++++ models/fixtures/repository.yml | 31 +++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 6536e1dda7b84..32ea21ebf69fc 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -201,3 +201,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 25 + repo_id: 63 + name: 'main' + commit_id: 'b7fca3aa0d80144f9718b0486681944f9c587e33' + commit_message: "Initial commit with signed file" + commit_time: 1602935385 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index f6b6252da1f88..59ca712885f49 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -733,3 +733,10 @@ type: 3 config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}" created_unix: 946684810 + +- + id: 111 + repo_id: 63 + type: 1 + config: "{}" + created_unix: 946684810 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 552a78cbd2773..8a618e0a72d60 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1786,3 +1786,34 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + +- + id: 63 + owner_id: 2 + owner_name: user2 + lower_name: repo-test-trusted-ssh-keys + name: repo-test-trusted-ssh-keys + default_branch: main + num_watches: 4 + num_stars: 0 + num_forks: 0 + num_issues: 2 + num_closed_issues: 1 + num_pulls: 3 + num_closed_pulls: 0 + num_milestones: 3 + num_closed_milestones: 1 + num_projects: 1 + num_closed_projects: 0 + is_private: false + is_empty: false + is_archived: false + is_mirror: false + status: 0 + is_fork: false + fork_id: 0 + is_template: false + template_id: 0 + size: 0 + is_fsck_enabled: true + close_issues_via_commit_in_any_branch: false From 9c8837bd995e0fbffa1814499c6226daee86599a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 21:09:44 +0200 Subject: [PATCH 30/45] Use pointer to git.SigningKey --- modules/git/repo_tree.go | 6 +- routers/web/repo/setting/setting.go | 4 +- services/asymkey/sign.go | 135 +++++++++++++------------ services/context/repo.go | 2 +- services/pull/merge.go | 2 +- services/pull/merge_prepare.go | 2 +- services/pull/merge_squash.go | 2 +- services/repository/files/temp_repo.go | 2 +- 8 files changed, 83 insertions(+), 72 deletions(-) diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 0d25b9d3a1835..309a73d759544 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -15,7 +15,7 @@ import ( type CommitTreeOpts struct { Parents []string Message string - Key SigningKey + Key *SigningKey NoGPGSign bool AlwaysSign bool } @@ -43,11 +43,13 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString(opts.Message) _, _ = messageBytes.WriteString("\n") - if opts.Key.KeyID != "" || opts.AlwaysSign { + if opts.Key != nil { if opts.Key.Format != "" { cmd.AddConfig("gpg.format", opts.Key.Format) } cmd.AddOptionFormat("-S%s", opts.Key.KeyID) + } else if opts.AlwaysSign { + cmd.AddOptionFormat("-S") } if opts.NoGPGSign { diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index bebb4867bc4c3..6602685e94888 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -62,7 +62,7 @@ func SettingsCtxData(ctx *context.Context) { ctx.Data["CanConvertFork"] = ctx.Repo.Repository.IsFork && ctx.Doer.CanCreateRepoIn(ctx.Repo.Repository.Owner) signing, _ := asymkey_service.SigningKey(ctx, ctx.Repo.Repository.RepoPath()) - ctx.Data["SigningKeyAvailable"] = len(signing.KeyID) > 0 + ctx.Data["SigningKeyAvailable"] = signing != nil ctx.Data["SigningSettings"] = setting.Repository.Signing ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled @@ -105,7 +105,7 @@ func SettingsPost(ctx *context.Context) { ctx.Data["MinimumMirrorInterval"] = setting.Mirror.MinInterval signing, _ := asymkey_service.SigningKey(ctx, ctx.Repo.Repository.RepoPath()) - ctx.Data["SigningKeyAvailable"] = len(signing.KeyID) > 0 + ctx.Data["SigningKeyAvailable"] = signing != nil ctx.Data["SigningSettings"] = setting.Repository.Signing ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 90ce63c33b40e..93e5c691a180a 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -86,9 +86,9 @@ func IsErrWontSign(err error) bool { } // SigningKey returns the KeyID and git Signature for the repo -func SigningKey(ctx context.Context, repoPath string) (git.SigningKey, *git.Signature) { +func SigningKey(ctx context.Context, repoPath string) (*git.SigningKey, *git.Signature) { if setting.Repository.Signing.SigningKey == "none" { - return git.SigningKey{}, nil + return nil, nil } if setting.Repository.Signing.SigningKey == "default" || setting.Repository.Signing.SigningKey == "" { @@ -96,14 +96,19 @@ func SigningKey(ctx context.Context, repoPath string) (git.SigningKey, *git.Sign value, _, _ := git.NewCommand("config", "--get", "commit.gpgsign").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) sign, valid := git.ParseBool(strings.TrimSpace(value)) if !sign || !valid { - return git.SigningKey{}, nil + return nil, nil } format, _, _ := git.NewCommand("config", "--default", git.KeyTypeOpenPGP, "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingKey, _, _ := git.NewCommand("config", "--get", "user.signingkey").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingName, _, _ := git.NewCommand("config", "--get", "user.name").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingEmail, _, _ := git.NewCommand("config", "--get", "user.email").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) - return git.SigningKey{ + + if strings.TrimSpace(signingKey) == "" { + return nil, nil + } + + return &git.SigningKey{ KeyID: strings.TrimSpace(signingKey), Format: strings.TrimSpace(format), }, &git.Signature{ @@ -112,7 +117,11 @@ func SigningKey(ctx context.Context, repoPath string) (git.SigningKey, *git.Sign } } - return git.SigningKey{ + if setting.Repository.Signing.SigningKey == "" { + return nil, nil + } + + return &git.SigningKey{ KeyID: setting.Repository.Signing.SigningKey, Format: setting.Repository.Signing.SigningFormat, }, &git.Signature{ @@ -124,8 +133,8 @@ func SigningKey(ctx context.Context, repoPath string) (git.SigningKey, *git.Sign // PublicSigningKey gets the public signing key within a provided repository directory func PublicSigningKey(ctx context.Context, repoPath string) (string, string, error) { signingKey, _ := SigningKey(ctx, repoPath) - if signingKey.KeyID == "" { - return "", signingKey.Format, nil + if signingKey == nil { + return "", "", nil } if signingKey.Format == git.KeyTypeSSH { content, err := os.ReadFile(signingKey.KeyID) @@ -146,18 +155,18 @@ func PublicSigningKey(ctx context.Context, repoPath string) (string, string, err } // SignInitialCommit determines if we should sign the initial commit to this repository -func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, git.SigningKey, *git.Signature, error) { +func SignInitialCommit(ctx context.Context, repoPath string, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.InitialCommit) signingKey, sig := SigningKey(ctx, repoPath) - if signingKey.KeyID == "" { - return false, git.SigningKey{}, nil, &ErrWontSign{noKey} + if signingKey == nil { + return false, nil, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, git.SigningKey{}, nil, &ErrWontSign{never} + return false, nil, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -166,18 +175,18 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if len(keys) == 0 { - return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, nil, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if twofaModel == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{twofa} + return false, nil, nil, &ErrWontSign{twofa} } } } @@ -185,19 +194,19 @@ Loop: } // SignWikiCommit determines if we should sign the commits to this repository wiki -func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, git.SigningKey, *git.Signature, error) { +func SignWikiCommit(ctx context.Context, repo *repo_model.Repository, u *user_model.User) (bool, *git.SigningKey, *git.Signature, error) { repoWikiPath := repo.WikiPath() rules := signingModeFromStrings(setting.Repository.Signing.Wiki) signingKey, sig := SigningKey(ctx, repoWikiPath) - if signingKey.KeyID == "" { - return false, git.SigningKey{}, nil, &ErrWontSign{noKey} + if signingKey == nil { + return false, nil, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, git.SigningKey{}, nil, &ErrWontSign{never} + return false, nil, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -206,35 +215,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if len(keys) == 0 { - return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, nil, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if twofaModel == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{twofa} + return false, nil, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo()) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit("HEAD") if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if commit.Signature == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, nil, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, nil, nil, &ErrWontSign{parentSigned} } } } @@ -242,18 +251,18 @@ Loop: } // SignCRUDAction determines if we should sign a CRUD commit to this repository -func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, git.SigningKey, *git.Signature, error) { +func SignCRUDAction(ctx context.Context, repoPath string, u *user_model.User, tmpBasePath, parentCommit string) (bool, *git.SigningKey, *git.Signature, error) { rules := signingModeFromStrings(setting.Repository.Signing.CRUDActions) signingKey, sig := SigningKey(ctx, repoPath) - if signingKey.KeyID == "" { - return false, git.SigningKey{}, nil, &ErrWontSign{noKey} + if signingKey == nil { + return false, nil, nil, &ErrWontSign{noKey} } Loop: for _, rule := range rules { switch rule { case never: - return false, git.SigningKey{}, nil, &ErrWontSign{never} + return false, nil, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -262,35 +271,35 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if len(keys) == 0 { - return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, nil, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if twofaModel == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{twofa} + return false, nil, nil, &ErrWontSign{twofa} } case parentSigned: gitRepo, err := git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } defer gitRepo.Close() commit, err := gitRepo.GetCommit(parentCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if commit.Signature == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, nil, nil, &ErrWontSign{parentSigned} } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{parentSigned} + return false, nil, nil, &ErrWontSign{parentSigned} } } } @@ -298,16 +307,16 @@ Loop: } // SignMerge determines if we should sign a PR merge commit to the base repository -func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, git.SigningKey, *git.Signature, error) { +func SignMerge(ctx context.Context, pr *issues_model.PullRequest, u *user_model.User, tmpBasePath, baseCommit, headCommit string) (bool, *git.SigningKey, *git.Signature, error) { if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("Unable to get Base Repo for pull request") - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } repo := pr.BaseRepo signingKey, signer := SigningKey(ctx, repo.RepoPath()) - if signingKey.KeyID == "" { - return false, git.SigningKey{}, nil, &ErrWontSign{noKey} + if signingKey == nil { + return false, nil, nil, &ErrWontSign{noKey} } rules := signingModeFromStrings(setting.Repository.Signing.Merges) @@ -318,7 +327,7 @@ Loop: for _, rule := range rules { switch rule { case never: - return false, git.SigningKey{}, nil, &ErrWontSign{never} + return false, nil, nil, &ErrWontSign{never} case always: break Loop case pubkey: @@ -327,91 +336,91 @@ Loop: IncludeSubKeys: true, }) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if len(keys) == 0 { - return false, git.SigningKey{}, nil, &ErrWontSign{pubkey} + return false, nil, nil, &ErrWontSign{pubkey} } case twofa: twofaModel, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if twofaModel == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{twofa} + return false, nil, nil, &ErrWontSign{twofa} } case approved: protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, pr.BaseBranch) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } if protectedBranch == nil { - return false, git.SigningKey{}, nil, &ErrWontSign{approved} + return false, nil, nil, &ErrWontSign{approved} } if issues_model.GetGrantedApprovalsCount(ctx, protectedBranch, pr) < 1 { - return false, git.SigningKey{}, nil, &ErrWontSign{approved} + return false, nil, nil, &ErrWontSign{approved} } case baseSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(baseCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{baseSigned} + return false, nil, nil, &ErrWontSign{baseSigned} } case headSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{headSigned} + return false, nil, nil, &ErrWontSign{headSigned} } case commitsSigned: if gitRepo == nil { gitRepo, err = git.OpenRepository(ctx, tmpBasePath) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{commitsSigned} + return false, nil, nil, &ErrWontSign{commitsSigned} } // need to work out merge-base mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit) if err != nil { - return false, git.SigningKey{}, nil, err + return false, nil, nil, err } for _, commit := range commitList { verification := ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return false, git.SigningKey{}, nil, &ErrWontSign{commitsSigned} + return false, nil, nil, &ErrWontSign{commitsSigned} } } } diff --git a/services/context/repo.go b/services/context/repo.go index f7f3b66c0dafd..dafa398dc851b 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -101,7 +101,7 @@ type CanCommitToBranchResults struct { UserCanPush bool RequireSigned bool WillSign bool - SigningKey git.SigningKey + SigningKey *git.SigningKey WontSignReason string } diff --git a/services/pull/merge.go b/services/pull/merge.go index 42b1871a9f44f..06e00664dc676 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -396,7 +396,7 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use func commitAndSignNoAuthor(ctx *mergeContext, message string) error { cmdCommit := git.NewCommand("commit").AddOptionFormat("--message=%s", message) - if ctx.signKey.KeyID == "" { + if ctx.signKey == nil { cmdCommit.AddArguments("--no-gpg-sign") } else { if ctx.signKey.Format != "" { diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index eb9aeb08eb558..31a1e137344e9 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -27,7 +27,7 @@ type mergeContext struct { doer *user_model.User sig *git.Signature committer *git.Signature - signKey git.SigningKey // signKey.KeyID empty for no-sign, non-empty to sign + signKey *git.SigningKey env []string } diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 6de319ca2e444..bc9bdcebe5689 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -74,7 +74,7 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { cmdCommit := git.NewCommand("commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). AddOptionFormat("--message=%s", message) - if ctx.signKey.KeyID == "" { + if ctx.signKey == nil { cmdCommit.AddArguments("--no-gpg-sign") } else { if ctx.signKey.Format != "" { diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index c0f21731372f8..1cf30edc7b548 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -293,7 +293,7 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit } var sign bool - var key git.SigningKey + var key *git.SigningKey var signer *git.Signature if opts.ParentCommitID != "" { sign, key, signer, _ = asymkey_service.SignCRUDAction(ctx, t.repo.RepoPath(), opts.DoerUser, t.basePath, opts.ParentCommitID) From 6f6ddd04b0dbd1e25f316464c7db3da15651bb98 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 21:12:13 +0200 Subject: [PATCH 31/45] fix fixture --- models/fixtures/repository.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 8a618e0a72d60..2adc2342e3054 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1794,16 +1794,16 @@ lower_name: repo-test-trusted-ssh-keys name: repo-test-trusted-ssh-keys default_branch: main - num_watches: 4 + num_watches: 0 num_stars: 0 num_forks: 0 - num_issues: 2 - num_closed_issues: 1 - num_pulls: 3 + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 num_closed_pulls: 0 - num_milestones: 3 - num_closed_milestones: 1 - num_projects: 1 + num_milestones: 0 + num_closed_milestones: 0 + num_projects: 0 num_closed_projects: 0 is_private: false is_empty: false From 59eed16514e686364badd5473425435f5d439f8d Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 23:02:01 +0200 Subject: [PATCH 32/45] . --- models/fixtures/user.yml | 2 +- tests/integration/api_repo_test.go | 6 +++--- tests/integration/oauth_test.go | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 976a236011cc9..b3bece5589c9b 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -67,7 +67,7 @@ num_followers: 2 num_following: 1 num_stars: 2 - num_repos: 14 + num_repos: 15 num_teams: 0 num_members: 0 visibility: 0 diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 672c2a2c8bf9b..df8de083ca087 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -94,9 +94,9 @@ func TestAPISearchRepo(t *testing.T) { }{ { name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{ - nil: {count: 36}, - user: {count: 36}, - user2: {count: 36}, + nil: {count: 37}, + user: {count: 37}, + user2: {count: 37}, }, }, { diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index d2228bae7992a..50c15021ee938 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -691,6 +691,10 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) { FullRepoName: "user2/commitsonpr", Private: false, }, + { + FullRepoName: "user2/repo-test-trusted-ssh-keys", + Private: false, + }, } assert.Equal(t, reposExpected, reposCaptured) From 82e7e2d267ba440513ffbc3e6bdae34712b3f321 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 23:25:07 +0200 Subject: [PATCH 33/45] fix more tests --- models/repo/repo_list_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index ca6007f6c7882..6b1bb39b85410 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -138,27 +138,27 @@ func getTestCases() []struct { { name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: optional.Some(false)}, - count: 34, + count: 35, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: optional.Some(false)}, - count: 39, + count: 40, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", opts: &repo_model.SearchRepoOptions{Keyword: "test", ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true}, - count: 15, + count: 16, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUser2IncludingCollaborativeByName", opts: &repo_model.SearchRepoOptions{Keyword: "test", ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 18, Private: true, AllPublic: true}, - count: 13, + count: 14, }, { name: "AllPublic/PublicRepositoriesOfOrganization", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: optional.Some(false), Template: optional.Some(false)}, - count: 34, + count: 35, }, { name: "AllTemplates", From 452f58bab1ef6e48776db38984aad33d9468588a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 6 Jun 2025 23:47:00 +0200 Subject: [PATCH 34/45] finally fix another test --- models/fixtures/branch.yml | 2 +- models/fixtures/repo_unit.yml | 34 +++++++++++++++++++ models/fixtures/repository.yml | 2 +- .../user2/repo-test-trusted-ssh-keys.git/HEAD | 2 +- .../refs/heads/{main => master} | 0 5 files changed, 37 insertions(+), 3 deletions(-) rename tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/{main => master} (100%) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 32ea21ebf69fc..c27ac71bf42d3 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -205,7 +205,7 @@ - id: 25 repo_id: 63 - name: 'main' + name: 'master' commit_id: 'b7fca3aa0d80144f9718b0486681944f9c587e33' commit_message: "Initial commit with signed file" commit_time: 1602935385 diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index 59ca712885f49..cd8c3a973fd96 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -737,6 +737,40 @@ - id: 111 repo_id: 63 + type: 4 + config: "{}" + created_unix: 946684810 + +- + id: 112 + repo_id: 63 + type: 5 + config: "{}" + created_unix: 946684810 + +- + id: 113 + repo_id: 63 type: 1 config: "{}" created_unix: 946684810 + +- + id: 114 + repo_id: 63 + type: 2 + config: "{}" + created_unix: 946684810 + +- + id: 115 + repo_id: 63 + type: 8 + created_unix: 946684810 + +- + id: 116 + repo_id: 63 + type: 3 + config: "{}" + created_unix: 946684810 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 2adc2342e3054..93048e44d0a8d 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1793,7 +1793,7 @@ owner_name: user2 lower_name: repo-test-trusted-ssh-keys name: repo-test-trusted-ssh-keys - default_branch: main + default_branch: master num_watches: 0 num_stars: 0 num_forks: 0 diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD index b870d82622c1a..cb089cd89a7d7 100644 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD +++ b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD @@ -1 +1 @@ -ref: refs/heads/main +ref: refs/heads/master diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master similarity index 100% rename from tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/main rename to tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master From 494921a78b029ebd9033b9ae7bfd71a8dd9cdfd8 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sat, 7 Jun 2025 00:01:02 +0200 Subject: [PATCH 35/45] fix my own test to use master branch --- tests/integration/gpg_ssh_git_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 84f5e072cd339..8d964c7b2da6f 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -373,14 +373,14 @@ func TestTrustedSSHKeys(t *testing.T) { username := "user2" testCtx := NewAPITestContext(t, username, "repo-test-trusted-ssh-keys", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) - t.Run("CheckMainBranchSignedVerified", doAPIGetBranch(testCtx, "main", func(t *testing.T, branch api.Branch) { + t.Run("CheckMasterBranchSignedVerified", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) { require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) require.True(t, branch.Commit.Verification.Verified) })) setting.Repository.Signing.TrustedSSHKeys = []string{} - t.Run("CheckMainBranchSignedUnverified", doAPIGetBranch(testCtx, "main", func(t *testing.T, branch api.Branch) { + t.Run("CheckMasterBranchSignedUnverified", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) { require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) require.False(t, branch.Commit.Verification.Verified) From 3824905e1ea00a32897bc79cb3a671a405be2a77 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 7 Jun 2025 17:01:38 +0200 Subject: [PATCH 36/45] remove verify of GetRepositoryDefaultPublicGPGKey for ssh and test for gpg --- services/asymkey/commit.go | 30 ------------ tests/integration/gpg_ssh_git_test.go | 66 --------------------------- 2 files changed, 96 deletions(-) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index d648571b70e98..04dbf09d4a320 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -445,36 +445,6 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * } } - // Covers ssh verification for the default SSH signing key specified in the .gitconfig file in Git.HomePath setting - defaultGPGSettings, err := c.GetRepositoryDefaultPublicGPGKey(false) - if defaultGPGSettings.Format == git.KeyTypeSSH { - if err != nil { - log.Error("Error getting default public ssh key: %v", err) - } else if defaultGPGSettings == nil { - log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) - } else if defaultGPGSettings.Sign { - if err := defaultGPGSettings.LoadPublicKeyContent(); err != nil { - log.Error("Error getting default signing key: %s %v", defaultGPGSettings.KeyID, err) - } else if fingerprint, err := asymkey_model.CalcFingerprint(defaultGPGSettings.PublicKeyContent); err != nil { - log.Error("Error calculating the fingerprint public key: %s %v", defaultGPGSettings.KeyID, err) - } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ - Verified: true, - Content: defaultGPGSettings.PublicKeyContent, - Fingerprint: fingerprint, - HasUsed: true, - }, committer, &user_model.User{ - Name: defaultGPGSettings.Name, - Email: defaultGPGSettings.Email, - }, defaultGPGSettings.Email); commitVerification != nil { - if commitVerification.Reason == asymkey_model.BadSignature { - defaultReason = asymkey_model.BadSignature - } else { - return commitVerification - } - } - } - } - return &asymkey_model.CommitVerification{ CommittingUser: committer, Verified: false, diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index 8d964c7b2da6f..a367720795395 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -49,35 +49,6 @@ func TestGPGGit(t *testing.T) { testGitSigning(t) } -func TestGPGGitDefaults(t *testing.T) { - tmpDir := t.TempDir() // use a temp dir to avoid messing with the user's GPG keyring and git config - err := os.Chmod(tmpDir, 0o700) - assert.NoError(t, err) - - t.Setenv("GNUPGHOME", tmpDir) - - // Need to create a root key - rootKeyPair, err := importTestingKey() - require.NoError(t, err, "importTestingKey") - - err = os.WriteFile(tmpDir+"/.gitconfig", []byte(` -[user] - name = gitea - email = gitea@fake.local - signingKey = `+rootKeyPair.PrimaryKey.KeyIdShortString()+` -[commit] - gpgsign = true -`), 0o600) - require.NoError(t, err, "os.WriteFile .gitconfig") - - defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() - defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() - defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() - defer test.MockVariableValue(&setting.Git.HomePath, tmpDir)() - - testGitSigning(t) -} - func TestSSHGit(t *testing.T) { tmpDir := t.TempDir() // use a temp dir to store the SSH keys err := os.Chmod(tmpDir, 0o700) @@ -105,43 +76,6 @@ func TestSSHGit(t *testing.T) { testGitSigning(t) } -func TestSSHGitDefaults(t *testing.T) { - tmpDir := t.TempDir() // use a temp dir to store the SSH keys and git config - err := os.Chmod(tmpDir, 0o700) - assert.NoError(t, err) - - pub, priv, err := ed25519.GenerateKey(rand.Reader) - require.NoError(t, err, "ed25519.GenerateKey") - sshPubKey, err := ssh.NewPublicKey(pub) - require.NoError(t, err, "ssh.NewPublicKey") - - err = os.WriteFile(tmpDir+"/id_ed25519.pub", ssh.MarshalAuthorizedKey(sshPubKey), 0o600) - require.NoError(t, err, "os.WriteFile id_ed25519.pub") - block, err := ssh.MarshalPrivateKey(priv, "") - require.NoError(t, err, "ssh.MarshalPrivateKey") - err = os.WriteFile(tmpDir+"/id_ed25519", pem.EncodeToMemory(block), 0o600) - require.NoError(t, err, "os.WriteFile id_ed25519") - err = os.WriteFile(tmpDir+"/.gitconfig", []byte(` -[user] - name = gitea - email = gitea@fake.local - signingKey = `+tmpDir+`/id_ed25519.pub -[gpg] - format = ssh -[commit] - gpgsign = true -`), 0o600) - require.NoError(t, err, "os.WriteFile .gitconfig") - - defer test.MockVariableValue(&setting.Repository.Signing.SigningKey, "")() - defer test.MockVariableValue(&setting.Repository.Signing.SigningFormat, "ssh")() - defer test.MockVariableValue(&setting.Repository.Signing.InitialCommit, []string{"never"})() - defer test.MockVariableValue(&setting.Repository.Signing.CRUDActions, []string{"never"})() - defer test.MockVariableValue(&setting.Git.HomePath, tmpDir)() - - testGitSigning(t) -} - func testGitSigning(t *testing.T) { username := "user2" user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: username}) From c771cd698a3c57f7688ca7eaf8751d1cf6723aa8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 11 Jun 2025 02:43:09 +0800 Subject: [PATCH 37/45] refactor ParseCommitWithSSHSignature --- models/fixtures/branch.yml | 12 --- models/fixtures/repo_unit.yml | 41 ---------- models/fixtures/repository.yml | 31 -------- models/fixtures/user.yml | 2 +- models/repo/repo_list_test.go | 10 +-- services/asymkey/commit.go | 75 +++++++++--------- services/asymkey/commit_test.go | 44 ++++++++++ .../user2/repo-test-trusted-ssh-keys.git/HEAD | 1 - .../repo-test-trusted-ssh-keys.git/config | 6 -- .../description | 1 - .../info/exclude | 6 -- .../9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e | Bin 55 -> 0 bytes .../b7/fca3aa0d80144f9718b0486681944f9c587e33 | 4 - .../b9/04b3dcaada8c26236f096fe337fd3c4cd8048a | Bin 36 -> 0 bytes .../packed-refs | 1 - .../refs/heads/master | 1 - tests/integration/api_repo_test.go | 6 +- tests/integration/gpg_ssh_git_test.go | 20 ----- tests/integration/oauth_test.go | 4 - 19 files changed, 92 insertions(+), 173 deletions(-) create mode 100644 services/asymkey/commit_test.go delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs delete mode 100644 tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index c27ac71bf42d3..6536e1dda7b84 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -201,15 +201,3 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 - -- - id: 25 - repo_id: 63 - name: 'master' - commit_id: 'b7fca3aa0d80144f9718b0486681944f9c587e33' - commit_message: "Initial commit with signed file" - commit_time: 1602935385 - pusher_id: 2 - is_deleted: false - deleted_by_id: 0 - deleted_unix: 0 diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index cd8c3a973fd96..f6b6252da1f88 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -733,44 +733,3 @@ type: 3 config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}" created_unix: 946684810 - -- - id: 111 - repo_id: 63 - type: 4 - config: "{}" - created_unix: 946684810 - -- - id: 112 - repo_id: 63 - type: 5 - config: "{}" - created_unix: 946684810 - -- - id: 113 - repo_id: 63 - type: 1 - config: "{}" - created_unix: 946684810 - -- - id: 114 - repo_id: 63 - type: 2 - config: "{}" - created_unix: 946684810 - -- - id: 115 - repo_id: 63 - type: 8 - created_unix: 946684810 - -- - id: 116 - repo_id: 63 - type: 3 - config: "{}" - created_unix: 946684810 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 93048e44d0a8d..552a78cbd2773 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1786,34 +1786,3 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false - -- - id: 63 - owner_id: 2 - owner_name: user2 - lower_name: repo-test-trusted-ssh-keys - name: repo-test-trusted-ssh-keys - default_branch: master - num_watches: 0 - num_stars: 0 - num_forks: 0 - num_issues: 0 - num_closed_issues: 0 - num_pulls: 0 - num_closed_pulls: 0 - num_milestones: 0 - num_closed_milestones: 0 - num_projects: 0 - num_closed_projects: 0 - is_private: false - is_empty: false - is_archived: false - is_mirror: false - status: 0 - is_fork: false - fork_id: 0 - is_template: false - template_id: 0 - size: 0 - is_fsck_enabled: true - close_issues_via_commit_in_any_branch: false diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index b3bece5589c9b..976a236011cc9 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -67,7 +67,7 @@ num_followers: 2 num_following: 1 num_stars: 2 - num_repos: 15 + num_repos: 14 num_teams: 0 num_members: 0 visibility: 0 diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index bd7fe6da4dc00..7eb76416c2012 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -138,27 +138,27 @@ func getTestCases() []struct { { name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: optional.Some(false)}, - count: 35, + count: 34, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: optional.Some(false)}, - count: 40, + count: 39, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", opts: repo_model.SearchRepoOptions{Keyword: "test", ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true}, - count: 16, + count: 15, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUser2IncludingCollaborativeByName", opts: repo_model.SearchRepoOptions{Keyword: "test", ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 18, Private: true, AllPublic: true}, - count: 14, + count: 13, }, { name: "AllPublic/PublicRepositoriesOfOrganization", opts: repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: optional.Some(false), Template: optional.Some(false)}, - count: 35, + count: 34, }, { name: "AllTemplates", diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 04dbf09d4a320..ae25d427063a8 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -6,6 +6,7 @@ package asymkey import ( "context" "fmt" + "slices" "strings" asymkey_model "code.gitea.io/gitea/models/asymkey" @@ -359,24 +360,39 @@ func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, si return nil } +func verifySSHCommitVerificationByInstanceKey(c *git.Commit, committerUser, signerUser *user_model.User, committerGitEmail, publicKeyContent string) *asymkey_model.CommitVerification { + fingerprint, err := asymkey_model.CalcFingerprint(publicKeyContent) + if err != nil { + log.Error("Error calculating the fingerprint public key %q, err: %v", publicKeyContent, err) + return nil + } + sshPubKey := &asymkey_model.PublicKey{ + Verified: true, + Content: publicKeyContent, + Fingerprint: fingerprint, + HasUsed: true, + } + return verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, sshPubKey, committerUser, signerUser, committerGitEmail) +} + // ParseCommitWithSSHSignature check if signature is good against keystore. -func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { +func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUser *user_model.User) *asymkey_model.CommitVerification { // Now try to associate the signature with the committer, if present - if committer.ID != 0 { + if committerUser.ID != 0 { keys, err := db.Find[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{ - OwnerID: committer.ID, + OwnerID: committerUser.ID, NotKeytype: asymkey_model.KeyTypePrincipal, }) if err != nil { // Skipping failed to get ssh keys of user log.Error("ListPublicKeys: %v", err) return &asymkey_model.CommitVerification{ - CommittingUser: committer, + CommittingUser: committerUser, Verified: false, Reason: "gpg.error.failed_retrieval_gpg_keys", } } - committerEmailAddresses, err := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committer.ID, user_model.GetEmailAddresses) + committerEmailAddresses, err := cache.GetWithContextCache(ctx, cachegroup.UserEmailAddresses, committerUser.ID, user_model.GetEmailAddresses) if err != nil { log.Error("GetEmailAddresses: %v", err) } @@ -391,32 +407,26 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * for _, k := range keys { if k.Verified && activated { - commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, k, committer, committer, c.Committer.Email) + commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, k, committerUser, committerUser, c.Committer.Email) if commitVerification != nil { return commitVerification } } } } - // Trust more than one key for every User + + // Try the pre-set trusted keys (for key-rotation purpose) for _, k := range setting.Repository.Signing.TrustedSSHKeys { - if fingerprint, err := asymkey_model.CalcFingerprint(k); err != nil { - log.Error("Error calculating the fingerprint public key: %s %v", k, err) - } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ - Verified: true, - Content: k, - Fingerprint: fingerprint, - HasUsed: true, - }, committer, committer, c.Committer.Email); commitVerification != nil { + // FIXME: why here uses "commiterUser" as "signerUser" but below don't? why here uses "c.Committer.Email" but below uses "gpgSettings.Email"? + signerUser := committerUser + commitVerification := verifySSHCommitVerificationByInstanceKey(c, committerUser, signerUser, c.Committer.Email, k) + if commitVerification != nil && commitVerification.Verified { return commitVerification } } - defaultReason := asymkey_model.NoKeyFound - - // Covers ssh verification for the default SSH signing key specified in gitea config - if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && setting.Repository.Signing.SigningKey != "" && setting.Repository.Signing.SigningKey != "default" && setting.Repository.Signing.SigningKey != "none" { - // OK we should try the default key + // Try the configured instance-wide SSH public key + if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && !slices.Contains([]string{"", "default", "none"}, setting.Repository.Signing.SigningKey) { gpgSettings := git.GPGSettings{ Sign: true, KeyID: setting.Repository.Signing.SigningKey, @@ -424,31 +434,24 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committer * Email: setting.Repository.Signing.SigningEmail, Format: setting.Repository.Signing.SigningFormat, } - if err := gpgSettings.LoadPublicKeyContent(); err != nil { - log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) - } else if fingerprint, err := asymkey_model.CalcFingerprint(gpgSettings.PublicKeyContent); err != nil { - log.Error("Error calculating the fingerprint public key: %s %v", gpgSettings.KeyID, err) - } else if commitVerification := verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, &asymkey_model.PublicKey{ - Verified: true, - Content: gpgSettings.PublicKeyContent, - Fingerprint: fingerprint, - HasUsed: true, - }, committer, &user_model.User{ + signerUser := &user_model.User{ Name: gpgSettings.Name, Email: gpgSettings.Email, - }, gpgSettings.Email); commitVerification != nil { - if commitVerification.Reason == asymkey_model.BadSignature { - defaultReason = asymkey_model.BadSignature - } else { + } + if err := gpgSettings.LoadPublicKeyContent(); err != nil { + log.Error("Error getting instance-wide SSH signing key %q, err: %v", gpgSettings.KeyID, err) + } else { + commitVerification := verifySSHCommitVerificationByInstanceKey(c, committerUser, signerUser, gpgSettings.Email, gpgSettings.PublicKeyContent) + if commitVerification != nil && commitVerification.Verified { return commitVerification } } } return &asymkey_model.CommitVerification{ - CommittingUser: committer, + CommittingUser: committerUser, Verified: false, - Reason: defaultReason, + Reason: asymkey_model.NoKeyFound, } } diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go new file mode 100644 index 0000000000000..985598ced4906 --- /dev/null +++ b/services/asymkey/commit_test.go @@ -0,0 +1,44 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package asymkey + +import ( + "strings" + "testing" + + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseCommitWithSSHSignature(t *testing.T) { + // Here we only test the TrustedSSHKeys. The complete signing test is in tests/integration/gpg_ssh_git_test.go + t.Run("TrustedSSHKey", func(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.Signing.TrustedSSHKeys, []string{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIH6Y4idVaW3E+bLw1uqoAfJD7o5Siu+HqS51E9oQLPE9"})() + + commit, err := git.CommitFromReader(nil, git.Sha1ObjectFormat.EmptyObjectID(), strings.NewReader(`tree 9a93ffa76e8b72bdb6431910b3a506fa2b39f42e +author User Two 1749230009 +0200 +committer User Two 1749230009 +0200 +gpgsig -----BEGIN SSH SIGNATURE----- + U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgfpjiJ1VpbcT5svDW6qgB8kPujl + KK74epLnUT2hAs8T0AAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5 + AAAAQDX2t2iHuuLxEWHLJetYXKsgayv3c43r0pJNfAzdLN55Q65pC5M7rG6++gT2bxcpOu + Y6EXbpLqia9sunEF3+LQY= + -----END SSH SIGNATURE----- + +Initial commit with signed file +`)) + require.NoError(t, err) + ret := ParseCommitWithSSHSignature(t.Context(), commit, &user_model.User{Name: "foo", Email: "foo@bar.com"}) + require.NotNil(t, ret) + assert.True(t, ret.Verified) + assert.False(t, ret.Warning) + assert.NotNil(t, ret.CommittingUser) // FIXME: test the CommittingUser and SigningUser correctly + assert.NotNil(t, ret.SigningUser) // FIXME: test the CommittingUser and SigningUser correctly + }) +} diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD deleted file mode 100644 index cb089cd89a7d7..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/HEAD +++ /dev/null @@ -1 +0,0 @@ -ref: refs/heads/master diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config deleted file mode 100644 index e6da231579bcc..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/config +++ /dev/null @@ -1,6 +0,0 @@ -[core] - repositoryformatversion = 0 - filemode = true - bare = true - ignorecase = true - precomposeunicode = true diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description deleted file mode 100644 index 498b267a8c781..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/description +++ /dev/null @@ -1 +0,0 @@ -Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude deleted file mode 100644 index a5196d1be8fb5..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/info/exclude +++ /dev/null @@ -1,6 +0,0 @@ -# git ls-files --others --exclude-from=.git/info/exclude -# Lines that start with '#' are comments. -# For a project mostly in C, the following would be a good set of -# exclude patterns (uncomment them if you want to use them): -# *.[oa] -# *~ diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/9a/93ffa76e8b72bdb6431910b3a506fa2b39f42e deleted file mode 100644 index bcf0704cd0cbba115cb93c01e583249df6ed4f46..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 55 zcmV-70LcG%0V^p=O;s?qU@$Z=Ff%bxD9%jJOHI)$sVHIC$+G#*s#`s3%K4o6kInzu N_}pOW0sv$I5hQpJ8C(DW diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 deleted file mode 100644 index 6d64c797ca642..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b7/fca3aa0d80144f9718b0486681944f9c587e33 +++ /dev/null @@ -1,4 +0,0 @@ -x
��[o�0��ٿb�QW����� $�@��˛�`�ͥ���&�k_z���|ҙ�NZ�Ϫ
�55��� -�f�pl9L��ۤ�k�� -ۙ u3�H$��PVײ�VŸ�s䟼��.��<��1]B1�.0�u���t���r��*��^�`2X��� -'���S���c\L�^�n�E�2Z?���X�S��L��x�Iʭ��_�/�hxzl����1��.1'�r�'�)>M�x��&&Aw-����dL�M��b�ぅ��KC��m]��4��f��׹x��I+�g,�n��Y�Ҷ��Z8��rN�k����v�It���[�������}�0��2PxQ�|Uo�9@��E�!S�D?J�� \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/objects/b9/04b3dcaada8c26236f096fe337fd3c4cd8048a deleted file mode 100644 index ee5dcf6ba398b5e76d1df5e90ee6d0ba63ddfcbe..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 36 ucmV+<0Nej~0ZYosPf{>4VhG8|ELH%bM1|ta^t{v*g|y6^R6PKbBMHGIZxA~G diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs deleted file mode 100644 index 250f187384963..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/packed-refs +++ /dev/null @@ -1 +0,0 @@ -# pack-refs with: peeled fully-peeled sorted diff --git a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master b/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master deleted file mode 100644 index 8b4ec6bcd860c..0000000000000 --- a/tests/gitea-repositories-meta/user2/repo-test-trusted-ssh-keys.git/refs/heads/master +++ /dev/null @@ -1 +0,0 @@ -b7fca3aa0d80144f9718b0486681944f9c587e33 diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index df8de083ca087..672c2a2c8bf9b 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -94,9 +94,9 @@ func TestAPISearchRepo(t *testing.T) { }{ { name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{ - nil: {count: 37}, - user: {count: 37}, - user2: {count: 37}, + nil: {count: 36}, + user: {count: 36}, + user2: {count: 36}, }, }, { diff --git a/tests/integration/gpg_ssh_git_test.go b/tests/integration/gpg_ssh_git_test.go index a367720795395..56f9f87783355 100644 --- a/tests/integration/gpg_ssh_git_test.go +++ b/tests/integration/gpg_ssh_git_test.go @@ -300,23 +300,3 @@ func importTestingKey() (*openpgp.Entity, error) { // There should only be one entity in this file. return keyring[0], nil } - -func TestTrustedSSHKeys(t *testing.T) { - defer tests.PrepareTestEnv(t)() - defer test.MockVariableValue(&setting.Repository.Signing.TrustedSSHKeys, []string{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIH6Y4idVaW3E+bLw1uqoAfJD7o5Siu+HqS51E9oQLPE9"})() - - username := "user2" - testCtx := NewAPITestContext(t, username, "repo-test-trusted-ssh-keys", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) - t.Run("CheckMasterBranchSignedVerified", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) { - require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) - require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) - require.True(t, branch.Commit.Verification.Verified) - })) - - setting.Repository.Signing.TrustedSSHKeys = []string{} - t.Run("CheckMasterBranchSignedUnverified", doAPIGetBranch(testCtx, "master", func(t *testing.T, branch api.Branch) { - require.NotNil(t, branch.Commit, "no commit provided with branch! %v", branch) - require.NotNil(t, branch.Commit.Verification, "no verification provided with branch commit! %v", branch.Commit) - require.False(t, branch.Commit.Verification.Verified) - })) -} diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 50c15021ee938..d2228bae7992a 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -691,10 +691,6 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) { FullRepoName: "user2/commitsonpr", Private: false, }, - { - FullRepoName: "user2/repo-test-trusted-ssh-keys", - Private: false, - }, } assert.Equal(t, reposExpected, reposCaptured) From 745a690b3f1bf53f3e40fe59421dfa407941ec74 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 11 Jun 2025 10:38:47 +0200 Subject: [PATCH 38/45] resolve fixme --- custom/conf/app.example.ini | 1 + services/asymkey/commit.go | 6 ++++-- services/asymkey/commit_test.go | 15 ++++++++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 3e9a9e2451621..aa2fcee765507 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1233,6 +1233,7 @@ LEVEL = Info ;; ;; Determines which additional ssh keys are trusted for all signed commits regardless of the user ;; This is useful for ssh signing key rotation. +;; Exposes the provided SIGNING_NAME and SIGNING_EMAIL as the signer, regardless of the SIGNING_FORMAT value. ;; Multiple keys should be comma separated. ;; E.g."ssh- ". or "ssh- , ssh- ". ;TRUSTED_SSH_KEYS = diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index ae25d427063a8..10bb9c8a59474 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -417,8 +417,10 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUs // Try the pre-set trusted keys (for key-rotation purpose) for _, k := range setting.Repository.Signing.TrustedSSHKeys { - // FIXME: why here uses "commiterUser" as "signerUser" but below don't? why here uses "c.Committer.Email" but below uses "gpgSettings.Email"? - signerUser := committerUser + signerUser := &user_model.User{ + Name: setting.Repository.Signing.SigningName, + Email: setting.Repository.Signing.SigningEmail, + } commitVerification := verifySSHCommitVerificationByInstanceKey(c, committerUser, signerUser, c.Committer.Email, k) if commitVerification != nil && commitVerification.Verified { return commitVerification diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go index 985598ced4906..5ad1d014feabd 100644 --- a/services/asymkey/commit_test.go +++ b/services/asymkey/commit_test.go @@ -19,6 +19,8 @@ import ( func TestParseCommitWithSSHSignature(t *testing.T) { // Here we only test the TrustedSSHKeys. The complete signing test is in tests/integration/gpg_ssh_git_test.go t.Run("TrustedSSHKey", func(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.Signing.SigningName, "gitea")() + defer test.MockVariableValue(&setting.Repository.Signing.SigningEmail, "gitea@fake.local")() defer test.MockVariableValue(&setting.Repository.Signing.TrustedSSHKeys, []string{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIH6Y4idVaW3E+bLw1uqoAfJD7o5Siu+HqS51E9oQLPE9"})() commit, err := git.CommitFromReader(nil, git.Sha1ObjectFormat.EmptyObjectID(), strings.NewReader(`tree 9a93ffa76e8b72bdb6431910b3a506fa2b39f42e @@ -34,11 +36,18 @@ gpgsig -----BEGIN SSH SIGNATURE----- Initial commit with signed file `)) require.NoError(t, err) - ret := ParseCommitWithSSHSignature(t.Context(), commit, &user_model.User{Name: "foo", Email: "foo@bar.com"}) + committingUser := &user_model.User{ + ID: 2, + Name: "User Two", + Email: "user2@example.com", + } + ret := ParseCommitWithSSHSignature(t.Context(), commit, committingUser) require.NotNil(t, ret) assert.True(t, ret.Verified) assert.False(t, ret.Warning) - assert.NotNil(t, ret.CommittingUser) // FIXME: test the CommittingUser and SigningUser correctly - assert.NotNil(t, ret.SigningUser) // FIXME: test the CommittingUser and SigningUser correctly + assert.Equal(t, ret.CommittingUser, committingUser) + assert.NotNil(t, ret.SigningUser) + assert.Equal(t, ret.SigningUser.Name, "gitea") + assert.Equal(t, ret.SigningUser.Email, "gitea@fake.local") }) } From f3710a34e27f2f4f06902a9c67c9e34ab9ea5dd1 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 11 Jun 2025 10:54:55 +0200 Subject: [PATCH 39/45] fix test api usage --- services/asymkey/commit_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go index 5ad1d014feabd..56358ca4cd975 100644 --- a/services/asymkey/commit_test.go +++ b/services/asymkey/commit_test.go @@ -45,9 +45,9 @@ Initial commit with signed file require.NotNil(t, ret) assert.True(t, ret.Verified) assert.False(t, ret.Warning) - assert.Equal(t, ret.CommittingUser, committingUser) + assert.Equal(t, committingUser, ret.CommittingUser) assert.NotNil(t, ret.SigningUser) - assert.Equal(t, ret.SigningUser.Name, "gitea") - assert.Equal(t, ret.SigningUser.Email, "gitea@fake.local") + assert.Equal(t, "gitea", ret.SigningUser.Name) + assert.Equal(t, "gitea@fake.local", ret.SigningUser.Email) }) } From 6b67a5440a18a8960292430465ea68c07d7be94d Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 11 Jun 2025 10:56:29 +0200 Subject: [PATCH 40/45] do not crash the test if signinguser is nil --- services/asymkey/commit_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go index 56358ca4cd975..0438209a6192a 100644 --- a/services/asymkey/commit_test.go +++ b/services/asymkey/commit_test.go @@ -46,8 +46,9 @@ Initial commit with signed file assert.True(t, ret.Verified) assert.False(t, ret.Warning) assert.Equal(t, committingUser, ret.CommittingUser) - assert.NotNil(t, ret.SigningUser) - assert.Equal(t, "gitea", ret.SigningUser.Name) - assert.Equal(t, "gitea@fake.local", ret.SigningUser.Email) + if assert.NotNil(t, ret.SigningUser) { + assert.Equal(t, "gitea", ret.SigningUser.Name) + assert.Equal(t, "gitea@fake.local", ret.SigningUser.Email) + } }) } From 6d119a0fea404fea95d47ad1d5faa034cbe45caf Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 11 Jun 2025 17:48:10 +0800 Subject: [PATCH 41/45] merge duplicate code and improve err handling --- routers/api/v1/api.go | 4 +-- routers/api/v1/misc/signing.go | 62 +++++++++++++++------------------- services/asymkey/sign.go | 2 +- 3 files changed, 30 insertions(+), 38 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 1fbca181d24c8..2c138ad61a091 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -971,7 +971,7 @@ func Routes() *web.Router { // Misc (public accessible) m.Group("", func() { m.Get("/version", misc.Version) - m.Get("/signing-key.gpg", misc.SigningKey) + m.Get("/signing-key.gpg", misc.SigningKeyGPG) m.Get("/signing-key.pub", misc.SigningKeySSH) m.Post("/markup", reqToken(), bind(api.MarkupOption{}), misc.Markup) m.Post("/markdown", reqToken(), bind(api.MarkdownOption{}), misc.Markdown) @@ -1428,7 +1428,7 @@ func Routes() *web.Router { m.Combo("/file-contents", reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()). Get(repo.GetFileContentsGet). Post(bind(api.GetFilesOptions{}), repo.GetFileContentsPost) // POST method requires "write" permission, so we also support "GET" method above - m.Get("/signing-key.gpg", misc.SigningKey) + m.Get("/signing-key.gpg", misc.SigningKeyGPG) m.Get("/signing-key.pub", misc.SigningKeySSH) m.Group("/topics", func() { m.Combo("").Get(repo.ListTopics). diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index e2779fc966a64..749d59a822a01 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -9,8 +9,30 @@ import ( "code.gitea.io/gitea/services/context" ) -// SigningKey returns the public key of the default signing key if it exists -func SigningKey(ctx *context.APIContext) { +func getSigningKey(ctx *context.APIContext, expectedFormat string) { + // if the handler is in the repo's route group, get the repo's signing key + // otherwise, get the global signing key + path := "" + if ctx.Repo != nil && ctx.Repo.Repository != nil { + path = ctx.Repo.Repository.RepoPath() + } + content, format, err := asymkey_service.PublicSigningKey(ctx, path) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if format == "" { + ctx.APIErrorNotFound("no signing key") + return + } else if format != expectedFormat { + ctx.APIErrorNotFound("signing key format is " + format) + return + } + _, _ = ctx.Write([]byte(content)) +} + +// SigningKeyGPG returns the public key of the default signing key if it exists +func SigningKeyGPG(ctx *context.APIContext) { // swagger:operation GET /signing-key.gpg miscellaneous getSigningKey // --- // summary: Get default signing-key.gpg @@ -43,25 +65,10 @@ func SigningKey(ctx *context.APIContext) { // description: "GPG armored public key" // schema: // type: string - - path := "" - if ctx.Repo != nil && ctx.Repo.Repository != nil { - path = ctx.Repo.Repository.RepoPath() - } - - content, format, err := asymkey_service.PublicSigningKey(ctx, path) - if err != nil { - ctx.APIErrorInternal(err) - return - } - if format != git.KeyTypeOpenPGP { - ctx.APIErrorNotFound("SSH keys are used for signing, not GPG") - return - } - _, _ = ctx.Write([]byte(content)) + getSigningKey(ctx, git.KeyTypeOpenPGP) } -// SigningKey returns the public key of the default signing key if it exists +// SigningKeySSH returns the public key of the default signing key if it exists func SigningKeySSH(ctx *context.APIContext) { // swagger:operation GET /signing-key.pub miscellaneous getSigningKeySSH // --- @@ -95,20 +102,5 @@ func SigningKeySSH(ctx *context.APIContext) { // description: "ssh public key" // schema: // type: string - - path := "" - if ctx.Repo != nil && ctx.Repo.Repository != nil { - path = ctx.Repo.Repository.RepoPath() - } - - content, format, err := asymkey_service.PublicSigningKey(ctx, path) - if err != nil { - ctx.APIErrorInternal(err) - return - } - if format != git.KeyTypeSSH { - ctx.APIErrorNotFound("GPG keys are used for signing, not SSH") - return - } - _, _ = ctx.Write([]byte(content)) + getSigningKey(ctx, git.KeyTypeSSH) } diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 93e5c691a180a..a9cb88dc3aa50 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -131,7 +131,7 @@ func SigningKey(ctx context.Context, repoPath string) (*git.SigningKey, *git.Sig } // PublicSigningKey gets the public signing key within a provided repository directory -func PublicSigningKey(ctx context.Context, repoPath string) (string, string, error) { +func PublicSigningKey(ctx context.Context, repoPath string) (content string, format string, err error) { signingKey, _ := SigningKey(ctx, repoPath) if signingKey == nil { return "", "", nil From 7655f5d6200bc45ff20d6dbf34cb7022789b08d6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 11 Jun 2025 17:50:52 +0800 Subject: [PATCH 42/45] rename consts to match SigningKey.Format --- modules/git/key.go | 6 ++---- modules/git/repo_gpg.go | 4 ++-- routers/api/v1/misc/signing.go | 4 ++-- services/asymkey/commit.go | 2 +- services/asymkey/sign.go | 4 ++-- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/modules/git/key.go b/modules/git/key.go index a6ea543ef1c5b..2513c048b7cbd 100644 --- a/modules/git/key.go +++ b/modules/git/key.go @@ -5,10 +5,8 @@ package git // Based on https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat const ( - // KeyTypeOpenPGP is the key type for GPG keys, expected default of git cli - KeyTypeOpenPGP = "openpgp" - // KeyTypeSSH is the key type for SSH keys - KeyTypeSSH = "ssh" + SigningKeyFormatOpenPGP = "openpgp" // for GPG keys, the expected default of git cli + SigningKeyFormatSSH = "ssh" ) type SigningKey struct { diff --git a/modules/git/repo_gpg.go b/modules/git/repo_gpg.go index 6eff247f66480..0021a7bda7bf1 100644 --- a/modules/git/repo_gpg.go +++ b/modules/git/repo_gpg.go @@ -14,7 +14,7 @@ import ( // LoadPublicKeyContent will load the key from gpg func (gpgSettings *GPGSettings) LoadPublicKeyContent() error { - if gpgSettings.Format == KeyTypeSSH { + if gpgSettings.Format == SigningKeyFormatSSH { content, err := os.ReadFile(gpgSettings.KeyID) if err != nil { return fmt.Errorf("unable to read SSH public key file: %s, %w", gpgSettings.KeyID, err) @@ -53,7 +53,7 @@ func (repo *Repository) GetDefaultPublicGPGKey(forceUpdate bool) (*GPGSettings, signingKey, _, _ := NewCommand("config", "--get", "user.signingkey").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.KeyID = strings.TrimSpace(signingKey) - format, _, _ := NewCommand("config", "--default", KeyTypeOpenPGP, "--get", "gpg.format").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) + format, _, _ := NewCommand("config", "--default", SigningKeyFormatOpenPGP, "--get", "gpg.format").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) gpgSettings.Format = strings.TrimSpace(format) defaultEmail, _, _ := NewCommand("config", "--get", "user.email").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) diff --git a/routers/api/v1/misc/signing.go b/routers/api/v1/misc/signing.go index 749d59a822a01..db70e04b8f627 100644 --- a/routers/api/v1/misc/signing.go +++ b/routers/api/v1/misc/signing.go @@ -65,7 +65,7 @@ func SigningKeyGPG(ctx *context.APIContext) { // description: "GPG armored public key" // schema: // type: string - getSigningKey(ctx, git.KeyTypeOpenPGP) + getSigningKey(ctx, git.SigningKeyFormatOpenPGP) } // SigningKeySSH returns the public key of the default signing key if it exists @@ -102,5 +102,5 @@ func SigningKeySSH(ctx *context.APIContext) { // description: "ssh public key" // schema: // type: string - getSigningKey(ctx, git.KeyTypeSSH) + getSigningKey(ctx, git.SigningKeyFormatSSH) } diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 10bb9c8a59474..94ea8b50a3003 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -428,7 +428,7 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUs } // Try the configured instance-wide SSH public key - if setting.Repository.Signing.SigningFormat == git.KeyTypeSSH && !slices.Contains([]string{"", "default", "none"}, setting.Repository.Signing.SigningKey) { + if setting.Repository.Signing.SigningFormat == git.SigningKeyFormatSSH && !slices.Contains([]string{"", "default", "none"}, setting.Repository.Signing.SigningKey) { gpgSettings := git.GPGSettings{ Sign: true, KeyID: setting.Repository.Signing.SigningKey, diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index a9cb88dc3aa50..d80ca76abbe9f 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -99,7 +99,7 @@ func SigningKey(ctx context.Context, repoPath string) (*git.SigningKey, *git.Sig return nil, nil } - format, _, _ := git.NewCommand("config", "--default", git.KeyTypeOpenPGP, "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + format, _, _ := git.NewCommand("config", "--default", git.SigningKeyFormatOpenPGP, "--get", "gpg.format").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingKey, _, _ := git.NewCommand("config", "--get", "user.signingkey").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingName, _, _ := git.NewCommand("config", "--get", "user.name").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) signingEmail, _, _ := git.NewCommand("config", "--get", "user.email").RunStdString(ctx, &git.RunOpts{Dir: repoPath}) @@ -136,7 +136,7 @@ func PublicSigningKey(ctx context.Context, repoPath string) (content string, for if signingKey == nil { return "", "", nil } - if signingKey.Format == git.KeyTypeSSH { + if signingKey.Format == git.SigningKeyFormatSSH { content, err := os.ReadFile(signingKey.KeyID) if err != nil { log.Error("Unable to read SSH public key file in %s: %s, %v", repoPath, signingKey, err) From b3997e3e689982cb56e0b1dacbd0ef6e89b62321 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 11 Jun 2025 17:55:48 +0800 Subject: [PATCH 43/45] add some comments --- services/asymkey/commit.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 94ea8b50a3003..148f51fd10e18 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -416,6 +416,8 @@ func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUs } // Try the pre-set trusted keys (for key-rotation purpose) + // At the moment, we still use the SigningName&SigningEmail for the rotated keys. + // Maybe in the future we can extend the key format to "ssh-xxx .... old-user@example.com" to support different signer emails. for _, k := range setting.Repository.Signing.TrustedSSHKeys { signerUser := &user_model.User{ Name: setting.Repository.Signing.SigningName, From 1f4ff813bc607f4e452e7c279923f8f2a7f3307d Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 11 Jun 2025 11:59:29 +0200 Subject: [PATCH 44/45] fix lint --- services/asymkey/sign.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index d80ca76abbe9f..f94462ea4630f 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -131,7 +131,7 @@ func SigningKey(ctx context.Context, repoPath string) (*git.SigningKey, *git.Sig } // PublicSigningKey gets the public signing key within a provided repository directory -func PublicSigningKey(ctx context.Context, repoPath string) (content string, format string, err error) { +func PublicSigningKey(ctx context.Context, repoPath string) (content, format string, err error) { signingKey, _ := SigningKey(ctx, repoPath) if signingKey == nil { return "", "", nil From 70ae40f58fbe29f3c19bf9567e46f047eef14f38 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 11 Jun 2025 11:59:50 +0200 Subject: [PATCH 45/45] Update default setting comment --- modules/setting/repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index bf0ae923d4459..318cf4110855c 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -255,7 +255,7 @@ var ( SigningKey: "default", SigningName: "", SigningEmail: "", - SigningFormat: "openpgp", // git.KeyTypeOpenPGP + SigningFormat: "openpgp", // git.SigningKeyFormatOpenPGP InitialCommit: []string{"always"}, CRUDActions: []string{"pubkey", "twofa", "parentsigned"}, Merges: []string{"pubkey", "twofa", "basesigned", "commitssigned"},