+Dave Borowitz, who implemented push cert handling in JGit and Gerrit Hi Ian, Ian Jackson wrote[1]: > I have been investigating git signed pushes. I found a number of > infelicities in the server side implementation which make using this > in practice rather difficult. I'm emailing here (before writing > patches) to see what people think of my proposed changes. > > 1. PUSH_CERT_KEY has truncated keyid (Debian #852647) > > I see this: > GIT_PUSH_CERT_KEY=A3DBCBC039B13D8A > > There is almost no purpose for which this 64-bit keyid can be safely > used. The full key fingerprint should be provided instead. > > Proposed change: provide the full fingerprint instead. Do this > for every caller of gpg-interface.c. Sounds sane. > 2. git-receive-pack calls gpg (Debian #852684) > > It would be better if it called gpgv. gpg does all sorts of > complicated things, including automatically starting or connecting to > a gpg-agent, which are not appropriate for use in a daemon on a > server. > > Additionally, I find that passing -c gpg.program=/usr/bin/gpgv > to git receive-pack is not effective, and there seems to be no > sensible way to specify the keyrings to use (although that could be > done by setting GNUPGHOME perhaps). > > Proposed change: call gpgv instead (and make any needed changes to > adapt to gpgv). Do this only when we are in git-receive-pack; other > call sites of gpg-interface.c will continue to use gpg. I think respecting gpg.program would be nicer. Is there a reason not to do that? I suspect receive-pack just forgot to call git_gpg_config. > 3. No way to specify keyring (Debian #852684, side note) > > There should be a way to specify the keyring used by > git-receive-pack's gpgv invocation. This should probably be done with > a config option, receive.certKeyring perhaps. How is the keyring configured for other commands that use GPG, like "git tag -v"? (Forgive my laziness in not looking it up.) > 4. Trouble with the nonce (Debian #852688 part 2) > > To use the signed push feature it is necessary to provide a nonce seed > to git-receive-pack. > > The docs say the seed must be secret but there is no documented way to > pass this seed to git that does not either write it to a git > configuration file somewhere, or pass it on a command line. The git > configuration system is unsuited to keeping secrets. Command lines > can be seen in ps etc. [...] > Proposed fix (in two parts): > > (i) Provide a new config option receive.certNonceSeedsFile. It > contains seeds, one per line. When stateless_rpc, we send a nonce > computed from the first seed. We accept nonces computed from any of > the listed seeds. The documentation will say that the file should > normally contain two seeds; rollover is achieved by mving into place a > new seed at the top, and dropping one from the bottom. An example > script will be provided. I like it. I also wonder why you say the git configuration system is unsuited to keeping secrets. E.g. passing an include.path setting with -c or GIT_CONFIG_PARAMETERS should avoid the kinds of trouble you described. Is there a change we could make to make it work better? That said, I think being able to name a file is a good idea. > (ii) At some later point, the following enhancement: When > !stateless_rpc, certNonceSeedsFile is ignored except that if neither > it nor the old certNonceSeed is set, the signed push feature is > disabled. That seems like an awkward interface. Shouldn't there be at least another config variable to enable signed push without making up a seed or filename? > In this state we always get a fresh nonce (from a suitable > system random source). How does this work with stateless_rpc? (See "Session State" in Documentation/technical/http-protocol.txt.) > Nontrivial because current git doesn't seem to > have a "get suitable random number" function, and the mess that is the > semantics of /dev/*random* files means that providing one is going to > be controversial. I think you're overestimating how much pushback adding such a thing would get. > 5. There are no docs on how to use this feature properly > (Debian #852695, #852688 part 1) > > Using the signed push feature requires careful programming on the > server side. There should be a doc explaining how to do this. > > Proposed fix: provide a .txt file containing much the same contents as > seen here: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=852695 Yes, that sounds like a very welcome kind of thing to add. More references: - JGit's push cert handling: https://git.eclipse.org/r/#/q/message:cert - Gerrit's push cert handling: https://gerrit-review.googlesource.com/q/project:gerrit+message:gpg I haven't been able to find much in terms of docs for the feature. There is https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#receive.trustedKey and https://gerrit-review.googlesource.com/Documentation/config-project-config.html#receive.enableSignedPush. If signed push is enabled but not required for a repository then if I remember correctly it is able to show whether an upload was signed by a trusted key, as context during a review. Thanks and hope that helps, Jonathan [1] https://public-inbox.org/git/22944.38288.91698.811743@xxxxxxxxxxxxxxxxxxxxxx/
