-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Improve instance wide ssh commit signing #34341
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
base: main
Are you sure you want to change the base?
Improve instance wide ssh commit signing #34341
Conversation
* 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 = [email protected] 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
What do you think @brtwrst about this? Except of an absent automatic setup this should now be even easier, by just editing a single file. I found out that gpg supported global key verification for all users, but ssh not, this PR aims to change that. No I have no idea how to write tests for this |
That looks awesome. Makes it super simple to set up and the |
I tested this like this Since the ssh keys are so simple idk if a double quote are even needed / supported. File paths are not supported in this PR for this list. |
Ok, can't wait for this to make it in :) Thank you for your work. |
app.example.ini needs to be updated. |
Co-authored-by: Lunny Xiao <[email protected]>
…stopherHX/34341
…m/christopherhx/gitea into pr/ChristopherHX/34341
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
…stopherHX/34341
Made some changes in c771cd6 and reverted unnecessary test changes. I have some new questions and left some FIXMEs in code. |
// 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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIXME: about "signerUser"
Actually I have one more question: is it fine to use "TrustedSSHKeys" like this? For example:
- Set SigningKey=K1, SigningEmail=Email1
- Then rotate, set SigningKey=K2, SigningEmail=Email2, add K1 to TrustedSSHKeys
- Now, when use K1 to verify, it knows nothing about Email1, so some "verification trust models" would always fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct this is some part that is very likely incorrect, it might be better to use SIGNING_USER and SIGNING_EMAIL.
why here uses "commiterUser" as "signerUser" but below don't?
The original reason, I updated the code below to use "gpgSettings.Email" was that enabling the existing gpg git signing integration test caused failures while verifying the signer email.
I look into that tomorrow to make both use gpgSettings.Email, but the TrustedSSHKeys does not need to check if the SIGNING_FORMAT e.g. ssh to gpg migration.
some "verification trust models" would always fail?
I made a script to verify this and my answer for the time being is No.
- Step 1 auto setup gitea, with initial commit signing enabled with the first key
- Step 2 call rest apis
await github.rest.repos.createForAuthenticatedUser({
name: 'test-repo',
private: false,
auto_init: true,
default_branch: 'main',
trust_model: 'committer',
})
await github.rest.repos.createForAuthenticatedUser({
name: 'test-repo2',
private: false,
auto_init: true,
default_branch: 'main',
trust_model: 'collaboratorcommitter',
})
await github.rest.repos.createForAuthenticatedUser({
name: 'test-repo3',
private: false,
auto_init: true,
default_branch: 'main',
trust_model: 'collaborator',
})
- Step 3 Rotate keys move old pubkey into TrustedSSHKeys
- Step 4 Open your Local Gitea
Yes how I did this here has exactly one possible odd thing in test-repo3 See here
The other trust models have no visible change before and after rotation for me, but maybe I am wrong here and there are more problems.
assert.NotNil(t, ret.CommittingUser) // FIXME: test the CommittingUser and SigningUser correctly | ||
assert.NotNil(t, ret.SigningUser) // FIXME: test the CommittingUser and SigningUser correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the FIXME above: what we expect the "committer" and "signer" to be when using instance-wide SSH key and the rotated trusted key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the Committer is given by the fixture e.g. committer User Two <[email protected]> 1749230009 +0200
We can assert this, I am going to update this test soon.
e.g. the following is all needed after ssh-keygen, no trouble with installing and setting up gpg or hacking around a hidden .gitconfig for ssh key usage
Where /data/id_ed25519 is the private key.
TRUSTED_SSH_KEYS
can be a list of additional ssh public key contents to trust for every user of this instanceCloses #34329
Related #31392