On Thu, Sep 07, 2017 at 05:43:19PM +0000, Stefan Beller wrote: > On Thu, Sep 7, 2017 at 2:11 AM, Shikher Verma <root@xxxxxxxxxxxxxxxx> wrote: > > On Wed, Sep 06, 2017 at 02:31:49PM -0700, Stefan Beller wrote: > >> On Wed, Sep 6, 2017 at 2:39 AM, Shikher Verma <root@xxxxxxxxxxxxxxxx> wrote: > >> > Currently, git only stores push certificates if there is a receive hook > >> > present. This may violate the principle of least surprise (e.g., I > >> > pushed with --signed, and I don't see anything in upstream). > >> > Additionally, push certificates could be more versatile if they are not > >> > tightly bound to git hooks. Finally, it would be useful to verify the > >> > signed pushes at later points of time with ease. > >> > > >> > A named ref is added for ease of access/tooling around push > >> > certificates. If the last push was signed, ref/PUSH_CERT stores the > >> > ref of the latest push cert otherwise it is empty. > >> > > >> > Sending patches as RFC since the documentation would have to be > >> > updated and git gc might have to be patched to not garbage collect > >> > the latest push certificate. > >> > > >> > This patch applies on master (3ec7d702a) > >> > >> What are performance implications for busy repositories at busy hosts? > >> (think kernel.org / github) They may want to disable this new feature > >> for performance reasons or because they don't want to clutter the > >> object store. So at least a config option to turn it off would be useful. > > > > Any typical git push would write several objects to disk, > > (or just one pack file, [which may be renamed eventually, see 722ff7f8]) > > > this patch > > would only add one more object per push so I think the performance > > penalty is not that high. But I agree that we can have a config to turn > > it off. > > I personally do not run a high performance server, so I do not terribly mind, > but thought it would be nice for them to have at least an option ready made > instead of a potential performance regression. > > >> On the ref to store the push certs: > >> (a) Currently the ref points at the blob, I wonder if we'd rather want to > >> point at a commit? (Then we can build up a history of > >> push certs, instead of relying on the reflog to show all > >> push certs. Also the gc issue might be easier to solve using this) > > > > I am not sure how that would work. The ref points at the blob of push > > certificate. Since each push can update multiple refs, each push > > certificate can point to mutiple commits (tip of the updated refs). > > Also if the named ref points at the commit then how will we get the > > corresponding push certificate? > > > > I did think about keeping a history of push certificates but the problem > > is new pushes can delete refs and commits which were pointed to by > > previous push certificates. This makes it really difficult to decide > > which push certificates to keep and which to gc. Also this history would > > be different for different clones of the same repo. Since push > > certificate are only meta data of the git workflow I think its best to > > just keep the latest push certificate and gc the old ones. People can > > use the recieve hook if they want to do advance things like logging a > > history of push certificates. I think git should provide a builtin > > solution for the simple case. > > What I had in mind was what would be achieved with a > hook like this (untested): > > #!/bin/sh > if test -z GIT_PUSH_CERT ; then > exit 0 > fi > > # add a new worktree 'tmp', checking out the magic ref: > git worktree add tmp refs/PUSH_CERT > > cp $GIT_PUSH_CERT tmp/cert > git -C tmp add cert > git -C tmp commit -m "new push cert" > # maybe include GIT_PUSH_CERT_[NONCE_]STATUS > # in commit message? > > # clean up, command doesn't exist yet: > git worktree delete tmp > This might be a good starting point for a sample hook if we choose to go that way. As Junio suggested. > This would not deal with concurrency as it re-uses the > same worktree, but illustrates what I had in mind > for the git history of that special ref. I personally feel that we should decouple push certificates from hooks and serve the push certificate whenever someone does `git pull --signed important-ref`. That way we remove trust from services like Github, Gitlab, Bitbucket. This could be done if git stores a map of refs to last push certificate that updated that ref. Push certificates are preventing MiTM attack between pusher and server. If we start serving push certificates on pull, it would prevent MiTM attack between pusher and puller! Compromise of server wouldn't mean vulnerabilities in your project. A sample hook solves the immediate problem of making push certs more accessible. But going with `git pull --signed` make push certs tremendously more accessible and useful. What are your thoughts on having git signed pull? What do you guys think I should do in regards to this patch series?
Attachment:
signature.asc
Description: PGP signature
