Skip to content

Support signed pushes #13454

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

Closed
silverwind opened this issue Nov 7, 2020 · 20 comments · Fixed by #24860
Closed

Support signed pushes #13454

silverwind opened this issue Nov 7, 2020 · 20 comments · Fixed by #24860
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@silverwind
Copy link
Member

silverwind commented Nov 7, 2020

Git supports signing pushes since 2.2.0, we should enable it server side if git is at least that version as it's a backwards-compatible feature. Essentially we need to configure each repo or git globally with:

[receive]
    advertisePushOptions = true
    certNonceSeed = "<uniquerandomstring>"

Maybe the UI can also indicate push signatures, but I guess that can come later.

certNonceSeed could be set to a hash derived from security.SECRET_KEY.

https://people.kernel.org/monsieuricon/signed-git-pushes
https://github.com/git/git/blob/7f7ebe054af6d831b999d6c2241b9227c4e4e08d/Documentation/RelNotes/2.2.0.txt#L81-L87

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Nov 7, 2020
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

first of all - cheers.
I wanted to ask.
If I already have this enabled on my server, would I have to edit the app.ini config (namely the certNonceSeed value) before upgrading once this option is ready or would it leave .gitconfig unchanged (that is - working as it is)?

@silverwind
Copy link
Member Author

I'm actually also wondering what happens if certNonceSeed changes on the server. Does anyone know if that will that invalidate anything?

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

You yourself as the operator would not be able to verify the validity of the old certificates with the new nonce seed.

I think it does not matter if you are just recording the certs or rejecting invalid or unsigned pushes - that's done using the current nonce seed, others would probably barely notice, since they don't have access to the nonce seed in the first place, otherwise they'd be able to forge certs.
Until you need to verify something.
At which point you'd have to recall when exactly you changed it, what was the original (and/or all of the previous values) and go about trying all your different past nonce seeds to see if the cert can finally be verified

So the seed probably shouldn't be changed in a random way as no mechanism to keep track of the changes exists (other than your pass).
Similarly to your GPG key, you don't normally change it every day.
You could maybe change it on a yearly basis knowing when it's done and saving previous values safely.

@silverwind
Copy link
Member Author

Right, so ideally we should expose a option to allow users to set their own certNonceSeed to allow them keep the verifyability of old signatures. If there is no user-defined nonce, generate one derived from security.SECRET_KEY.

I guess if really necessary one could also expose a option to input past nonces for verification purpose, but I think in the general use case we can assume a user either sets their previous nonce or sets none at all, getting the default.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

Right, so ideally we should expose a option to allow users to set their own certNonceSeed to allow them keep the verifyability of old signatures. If there is no user-defined nonce, generate one derived from security.SECRET_KEY.

I don't think it works like that.
It's set server-wide, like a TLS cert for a domain/service rather than for the specific user.
It's more of a mechanism for server to keep stateful-ish track of the pushes, really, and (in case you want it) to act on the info.

I guess if really necessary one could also expose a option to input past nonces for verification purpose, but I think in the general use case we can assume a user either sets their previous nonce or sets none at all, getting the default.

This would then probably be a non-standard and non-intended use of the thingy, while I agree it's a nice idea.
Still, would probably require more thought on the subject.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

I also have no idea how hard resets are solved in the scenario of changed nonce seed, much less of an idea if we consider per-user nonce seed.
Which is probably why IMO it'd be best to first stick to the default, per-server nonce seed that is not supposed to change and maybe experiment later.

@silverwind
Copy link
Member Author

I never said per-user nonce. I of course meant per-server (or if a cluster of servers, shared among them).

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

I never said per-user nonce. I of course meant per-server (or if a cluster of servers, shared among them).

right, my bad...I got the users part as general instance users, not as Gitea users == instance admins

@silverwind
Copy link
Member Author

Yeah, I meant admins, not actual gitea users 😉

@silverwind
Copy link
Member Author

silverwind commented Sep 1, 2021

So I think we can close this, the preferred way should be to configure it in /etc/gitconfig and the git server process will pick it up there. Docker users can mount that file into the container. No explicit support from gitea required.

@6543
Copy link
Member

6543 commented Sep 1, 2021

I think we should at least document it ?

@silverwind
Copy link
Member Author

Hmm yes, some docs regarding this and /etc/gitconfig in general would be nice.

@silverwind silverwind reopened this Sep 2, 2021
@silverwind
Copy link
Member Author

Testing this again, I think we can improve the error message seen when signed push is not enabled in gitconfig:

$ git push --signed gitea master
fatal: the receiving end does not support --signed push
fatal: the remote end hung up unexpectedly
Gitea: Internal error

Compare to GitHub which also does not support signed push but does not show the latter two messages:

$ git push --signed github master
fatal: the receiving end does not support --signed push

@a1012112796
Copy link
Member

maybe we can provide option to check and block unsigned git push, which is similar with "require signed commits"

@Mikaela
Copy link
Contributor

Mikaela commented Jan 16, 2022

Is the procedure the same for SSH signed pushes or are those even a thing?

Edit: the answer appears to be yes, as long as the server has a SSH-signing capable git version.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

Is the procedure the same for SSH signed pushes or are those even a thing?

Edit: the answer appears to be yes, as long as the server has a SSH-signing capable git version.

I think this is a perfectly valid question, it'd perhaps be nice to have this mentioned in the docs.

@OdinVex
Copy link

OdinVex commented Mar 9, 2022

Would prefer to be able to do it over HTTPS if possible.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

Would prefer to be able to do it over HTTPS if possible.

I believe there's a confusion, I'll try to clarify.
This thread discusses (commit) push signing methods, which could either be gpg or ssh (you can now sign stuffTM using your ssh keys, too), irrespective of the actual push transfer method used, that could be either SSH or HTTP/HTTPS.

@silverwind
Copy link
Member Author

Exactly. I think this is a pure documentation issue. It may be possible for gitea to automatically configure git if it supports signed pushes, but I'm not sure we have interfaces for doing that.

@OdinVex
Copy link

OdinVex commented Mar 10, 2022

I wasn't able to get push advertising working, so I've stuck to simple gpgsign-ing.

silverwind added a commit that referenced this issue May 23, 2023
…stogram` by default (#24860)

Close #13454 , Close #23255, Close #14697 (and maybe more related
issues)

Many users have the requirement to customize the git config. This PR
introduces an easy way: put the options in Gitea's app.ini
`[git.config]`, then the config options will be applied to git config.

And it can support more flexible default config values, eg: now
`diff.algorithm=histogram` by default. According to:
https://stackoverflow.com/a/32367597/4754037 , `histogram diff` is
efficient and doesn't like to cause server-side problems.

---------

Co-authored-by: silverwind <[email protected]>
Co-authored-by: KN4CK3R <[email protected]>
Co-authored-by: Giteabot <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
7 participants