Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

GPG signatures show as untrusted for non-admins #18501

Closed
parnic-sks opened this issue Jan 31, 2022 · 7 comments · Fixed by #18539
Closed

GPG signatures show as untrusted for non-admins #18501

parnic-sks opened this issue Jan 31, 2022 · 7 comments · Fixed by #18539
Labels
Milestone

Comments

@parnic-sks
Copy link

parnic-sks commented Jan 31, 2022

Gitea Version

1.16.0

Git Version

2.35.0

Operating System

Ubuntu 20.04.3, aarch64/arm64

How are you running Gitea?

Built myself from tag v1.16.0
Also reproducible on https://try.gitea.io

Database

PostgreSQL

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Description

If a commit is signed from a collaborator with the GPG key added to the collaborator's account, it will show as "untrusted" in the commit list.

git log shows good signature:

>git log --show-signature
──────────────────────────────────────────────────────────────────────────────────┐
commit 1f4ffc3844456d7d577801f2d09039c682bd195e (HEAD -> test, origin/test, main) │
──────────────────────────────────────────────────────────────────────────────────┴
gpg: Signature made 1/31/2022 2:38:23 PM Central Standard Time
gpg:                using RSA key D00D9F014F64FD0D6B9E469D4D9DED295F42CF55
gpg: Good signature from "Parnic <parnic@parnic.com>" [ultimate]
Author: Parnic <parnic@parnic.com>
Date:   Mon Jan 31 14:38:23 2022 -0600

    Test

That key is added to that user's try.gitea.io account, but the commit shows untrusted (using try.gitea.io's default trust model):
https://try.gitea.io/parnic-sks/signature-test/commit/1c09133de06bb343eb9ed090ca7a37e6eac46bb1
edit: updated to a commit with the correct committer address: https://try.gitea.io/parnic-sks/signature-test/commit/1f4ffc3844456d7d577801f2d09039c682bd195e

Commits from the repo admin do still show as trusted, however. I suspect that's because the code seems to only be trusting repo admins. There are 4 similar calls to CalculateTrustStatus(), but all use this IsUserRepoAdmin func for the isCodeReader argument, and pass either nil or an empty map[string]bool{} as the final keyMap argument, e.g.:

if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) {
    return models.IsUserRepoAdmin(ctx.Repo.Repository, user)
}, nil); err != nil {
    [...]

The only way to get the commit to show as Trusted is to change the trust model to Committer, but that causes commits from Gitea itself (such as PR squash-merges) to show as untrusted.

Screenshots

Commit:
image

Key added to committer's account:
image

Committer's collaborator status on the repo:
image

@parnic-sks parnic-sks changed the title Signatures show as untrusted for non-admins GPG signatures show as untrusted for non-admins Jan 31, 2022
@zeripath
Copy link
Contributor

git cat-file commit 1c09133de0                                                             [20:35:47]
tree 1cd0051f9ce1ec215946d61436216383bea4be08
author Parnic <parnic@parnic.com> 1643659575 -0600
committer Parnic <github@parnic.com> 1643659926 -0600
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEE0A2fAU9k/Q1rnkadTZ3tKV9Cz1UFAmH4QpoACgkQTZ3tKV9C
 z1WLJAf8DRlHYYOPQ/AXvrZXebKXUE9YIXEch+qXPUb7eWs+PBU6TplLXrEi+u1P
 70OSVSsFr4lh5xLZ4/KDV4Wc4CU8BoepuuCYaNDAMv2Ol4xVV84yKGv3uEnGLmG6
 v+0L8fFrwJ86RtEGjCfmS646RjcBi5QqwKdTBo1Ec32zi0tT+SOTN2DIOltZFPJe
 dD07TeZ+8cuPQcvhfHSkz2aT/AFujDXIMu7Hi0OHhC5hzNTtaFKBE7DsgPJWD4JG
 ab8wLsJngVgmamUp3M+gEbio3gOoYEebRKkw7OI+eLi4Dz5dHBwr3nb5JbWcXDlQ
 FgzFO86uQ7PlMPn4J7Npy7ajDfshjA==
 =MPKR
 -----END PGP SIGNATURE-----

Test

@zeripath
Copy link
Contributor

is github@parnic.com a verified email address for the parnic user?

@parnic-sks
Copy link
Author

parnic-sks commented Jan 31, 2022

@zeripath sorry, you're right, I screwed that up.

I've added a commit that uses the correct committer, same problem: https://try.gitea.io/parnic-sks/signature-test/commit/1f4ffc3844456d7d577801f2d09039c682bd195e
Branch: https://try.gitea.io/parnic-sks/signature-test/src/branch/test

>git cat-file commit 1f4ffc3844456d7d577801f2d09039c682bd195e
tree 1cd0051f9ce1ec215946d61436216383bea4be08
author Parnic <parnic@parnic.com> 1643661503 -0600
committer Parnic <parnic@parnic.com> 1643661503 -0600
gpgsig -----BEGIN PGP SIGNATURE-----

 iQEzBAABCAAdFiEE0A2fAU9k/Q1rnkadTZ3tKV9Cz1UFAmH4SL8ACgkQTZ3tKV9C
 z1XISgf+InCCP9AXf2fnwAgVlTJgu6hD4poV2e0KBp9GfztroPQhdHH54Rhqaa6K
 hjDCE19pJT0mG/tIF6ZqeOD2HZ5jSW409Ze91WXOhAErF7Q5MHbfQEfnUpDsTrmF
 Ru6NspsB7FvhedsBCd5Gg5NHghQ5T3hgtBpwdD/x6uSemOqwcPgskhVfKTBw8Y9O
 fAP3IgxBFMwSG6vhDwhUFXXFB6B17o88PyY9CCzhy5uqk4kyV7/59aVRcc76utDW
 m5XuB4Cl2YRHoa1m08IGgA7pz8ncaaQRjUSpiQMLLsQCfNFC0LfE/4MjaWAGutgK
 ZHAUD7zIN7tx9Qem4V37i7WzY0Fttg==
 =M/eV
 -----END PGP SIGNATURE-----

Test

@zeripath
Copy link
Contributor

You are using the collaborator or collaboratorcommiter trustmodel.

You are probably expecting to use the github compatible committer trustmodel.

Change your default trustmodel to committer or change the trust model of the repository committer.


(The default trustmodel for new installs will be committer from 1.17 onwards to account for this confusion.)

@parnic-sks
Copy link
Author

No, I want to use the collaborator trust model. In this case, the user who made that commit is a collaborator on the repo, so that trust model should allow the collaborator's commit to show as signed. Based on the code, it looks like collaborator can never show a collaborator's commit as "trusted" because only repo admins are considered to be trusted.

If I use the "committer" trust model, then commits signed by Gitea on behalf of a user (e.g. from PR squash-merges) will show as untrusted. Here's an example from one of our internal repos showing that:
image

I understand that new commits from Gitea using the committer trust model will come from Gitea, rather than the original author, to avoid this issue, but that doesn't fix our existing commits.

I also want to reiterate that this Collaborator trust model worked fine in 1.15.10, which we upgraded from.

@zeripath zeripath reopened this Jan 31, 2022
@zeripath
Copy link
Contributor

zeripath commented Jan 31, 2022

Ag! That shouldn't be happening something has changed.

Apologies that I misunderstood your issue.

@zeripath
Copy link
Contributor

zeripath commented Feb 1, 2022

regression from #17917

zeripath added a commit to zeripath/gitea that referenced this issue Feb 1, 2022
There was an unintended regression in go-gitea#17917 which leads to only
repository admin commits being trusted. This PR restores the old logic.

Fix go-gitea#18501

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.16.1 milestone Feb 1, 2022
6543 pushed a commit that referenced this issue Feb 2, 2022
* Collaborator trust model should trust collaborators

There was an unintended regression in #17917 which leads to only
repository admin commits being trusted. This PR restores the old logic.

Fix #18501

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this issue Feb 2, 2022
Backport go-gitea#18539

There was an unintended regression in go-gitea#17917 which leads to only
repository admin commits being trusted. This PR restores the old logic.

Fix go-gitea#18501

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick pushed a commit that referenced this issue Feb 3, 2022
Backport #18539

There was an unintended regression in #17917 which leads to only
repository admin commits being trusted. This PR restores the old logic.

Fix #18501

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
* Collaborator trust model should trust collaborators

There was an unintended regression in go-gitea#17917 which leads to only
repository admin commits being trusted. This PR restores the old logic.

Fix go-gitea#18501

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants