git signed push server-side

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.

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.

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.

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.

Additionally, the seed should be changed occasionally (since unchangeable secrets are a bad idea). Ideally this should be done automatically.

Analysis: the seed is used to allow the server to mostly-statelessly
verify the freshness of a client's nonce.  (This is necessary only in
connectionless transport protocols, "stateless_rpc" as
builtin/receive-pack.c has it.)

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.

(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. In this state we always get a fresh nonce (from a suitable system random source). 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.

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

If I send patches like the above, would they be welcomed (subject to detailed review, of course) ?

Thanks, Ian.

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

When I did the signed push, the primary focus was to get the protocol extension right, and what the server end does with the received certificate was left as something that can be refined later by those who are really into it.

So I do find it a welcome development that you are looking into it (but remember, I do not represent "people"---I am merely one of them).

I and others may or may not have objections and may or may not offer better ideas over what you are going to write in your design docs and patches---I cannot promise anything before seeing them.

+Dave Borowitz, who implemented push cert handling in JGit and Gerrit Hi Ian,

Ian Jackson wrote[1]:

Sounds sane.

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.

How is the keyring configured for other commands that use GPG, like "git tag -v"? (Forgive my laziness in not looking it up.)

[...]

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.

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?

How does this work with stateless_rpc? (See "Session State" in Documentation/technical/http-protocol.txt.)

I think you're overestimating how much pushback adding such a thing would get.

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@chiark.greenend.org.uk/

Jonathan Nieder <jrnieder@gmail.com> writes:

I probably should react a bit stronger against that "instead", as Ian will not be writing the world's first server side hook that uses this interface. A different variable that lets you read the full length "in addition" I wouldn't have a problem with, as existing scripts will continue working the same way if you did so.

But on the other hand, the value of this environment is not meant to be used to make decision by the hook anyway, so it perhaps is OK to change it in a backward incompatible way to break those who have been using the value for any serious purpose.

The purpose of the signed push is not to replace authentication and authorization. The primary goal behind the signed push mechanism is to allow server side hook to implement a way to store these certs in the order it receives without losing them, and that gives a way for the server operators to protect against claims that they are showing what the pusher did not intend to publish. They can say "the tip of these branches are at this commit, because, see this, a signed cert by the pusher asking us to put these commits at these branches" with such a mechanism---as opposed to "we authenticated the user and here is our server log that says that user pushed to update these refs", which is much weaker claim that they can make without the signed push mechanism.

Jonathan Nieder <jrnieder@gmail.com> writes:

That would be a good change.

AFAIR we never do anything special, so you should be able to point GNUPGHOME to wherever you like to use the desired configuration.

I also wonder that too. The configuration file that has the filename could be made just as secret and unreadable from public as the new file that stores the seed with the same mechanism, I would imagine.

This was rather deliberately left underspecified, hoping that the BCP would emerge after people gain experience. As Ian is looking into this and hopefully gain real-world experience, we can have a good BCP description after he is done with his project ;-)

Indeed.

Hi.  Thanks to both of you for your helpful comments.

Jonathan Nieder writes ("Re: git signed push server-side"):

...

I think it very unlikely that anyone would want git-receive-pack's signed push facility to end up running gpg rather than gpgv. But, the git-receive-pack functions here are building blocks which need quite a lot of extra work to use, anyway. So having all callers have to pass -c gpg.program would be quite tolerable, if a bit ugly.

So, I will send a patch to fix that.

It's not. You just get whatever keyring and trust db etc. your gpg has as the default. As Junio says, being able to set the keyring explicitly is not essential to use the signed push feature; one can make a wrapper for gpgv, or set GNUPGHOME.

It just seemed to me that the current interface is not very convenient. If it is controversial to provide a more convenient interface, then I will work with what there is.

...

I was not aware of include.path. That would solve this aspect of the problem (but not the rollover aspect, of course). GIT_CONFIG_PARAMETERS is not documented.

Perhaps. I don't have a strong opinion about the config parameter names and semantics. This seems like a matter of taste and I'm happy to just do whatever others want.

It doesn't, which is why I propose this only if !stateless_rpc.

Well, I'm happy to give it ago. (NB: I will use /dev/urandom on Linux.)

Really helpful, thanks. I will read these. (Shame that I can't even view that information without running their javascript!)

I ended up reading this
 https://github.com/sitaramc/gitolite/blob/cf062b8bb6b21a52f7c5002d33fbc950762c1aa7/contrib/hooks/repo-specific/save-push-signatures

(This is an empty page for me, even with javascript turned on.)

I'll probably implement the refs/meta/push-certs:REF@{cert} special branch thing from JGit. Can anyone point me at a public repo which has such push-certs recorded, so I can check the details and make my thing do stuff the same way ?

Junio C Hamano writes ("Re: git signed push server-side"):

Anyone who is verifying signatures and doing anything with the 16-character key id other than logging it, has a serious security bug.

Precisely.

I don't understand why the signed push system could (or should) not be used for A&A. It seems eminently suited to it, in circumstances where traceability is important (and the pushers can be expected to be gpg users).

Junio C Hamano writes ("Re: git signed push server-side"):

Right. I looked at the protocol in some detail (I have a background in crypto protocol design) and it seemed good to me. (Although, I have not done a formal verification.)

Of course. But it is helpful for me to get some input on what direction to go in, before I write and test code.

FTR, I don't expect to do anything particularly quickly. And, of course, I'd welcome any other suggestions/input in the meantime.

Thanks, Ian.