Re: [PATCH] Add commit & tag signing/verification via SSH keys using ssh-keygen

From: Fabian Stelzer <fs@gigacodes.de>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Fabian Stelzer via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] Add commit & tag signing/verification via SSH keys using ssh-keygen
Date: Tue, 6 Jul 2021 17:33:51 +0200	[thread overview]
Message-ID: <a45b940f-490b-0b8a-a994-f78d1bb8f90a@gigacodes.de> (raw)
In-Reply-To: <YORsVNVC6lVEA7yD@camp.crustytoothpaste.net>

From: Fabian Stelzer <fs@gigacodes.de>
>> set gpg.format = ssh and user.signingkey to a ssh public key string (like from an
>> authorized_keys file) and commits/tags can be signed using the private
>> key from your ssh-agent.
>>
>> Verification uses a allowed_signers_file (see ssh-keygen(1)) which
>> defaults to .gitsigners but can be set via gpg.ssh.allowedsigners
>> A possible gpg.ssh.revocationfile is also passed to ssh-keygen on
>> verification.
>>
>> needs openssh>8.2p1
> Usually we'll want to write the explanation here in full sentences with
> typical capitalization.
Thanks, i was unsure about what to put in the commit and what into the 
cover letter.
I'll fix this with the next patch update and move some of it into the 
commit message.
In our env the commit messages are usually kept quite short.
>
>> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
>> ---
>>      RFC: Add commit & tag signing/verification via SSH keys using ssh-keygen
>>      
>>      Support for using private keyfiles directly is still missing and i'm
>>      unsure on how to configure it or if the pubkey in the signingkey field
>>      is such a good idea. A SSH Fingerprint as signingkey would be nicer, but
>>      key lookup would be quite cumbersome. Maybe storing the fingerprint in
>>      signingkey and then have a gpg.ssh.$FINGERPRINT.publickey/privatekeyfile
>>      setting? As a default we could get the first ssh key from ssh-add and
>>      store it in the config to avoid unintentional changes of the used
>>      signing key. I've started with some tests for SSH Signing but having
>>      static private keyfiles would make this a lot easier. So still on my
>>      TODO.
> I think user.signingKey could be helpful for signing here.  That could
> be a file name, not just a fingerprint, although we'd probably want to
> have support for tilde expansion.  You could add an additional option,
> gpg.ssh.keyring, that specifies the signatures to verify.  That would be
> named the same thing as a potential option of gpg.openpgp.keyring,
> which would be convenient.  Also, gpg.ssh.revokedKeyring could maybe be
> the name for revoked keys?
The problem ist that looking up a key by fingerprint alone is not really 
possible with ssh :/
A referenced file (which could contain a public or private key) would be 
fine and i could return the fingerprint in the get_signing_key api which 
the pushcerts code uses as "pusher" info in the cert.
I'll change the keyring naming to what you suggested. Makes sense to 
have this option for gpg as well.

>
>>      This feature makes git signing much more accessible to the average user.
>>      Usually they have a SSH Key for pushing code already. Using it for
>>      signing commits allows us to verify not only the transport but the
>>      pushed code as well. The allowed_signers file could be kept in the
>>      repository if all receives are verified (allowing only useris with valid
>>      signatures to add/change them) or outside if generated/managed
>>      differently. Tools like gitolite could optionally generate and enforce
>>      them from the already existing user ssh keys for example.
>>      
>>      In our corporate environemnt we use PIV x509 Certs on Yubikeys for email
>>      signing/encryption and ssh keys which i think is quite common (at least
>>      for the email part). This way we can establish the correct trust for the
>>      SSH Keys without setting up a separate GPG Infrastructure (which is
>>      still quite painful for users) or implementing x509 signing support for
>>      git (which lacks good forwarding mechanisms). Using ssh agent forwarding
>>      makes this feature easily usable in todays development environments
>>      where code is often checked out in remote VMs / containers.
> I think some of this rationale would work well in the commit message,
> especially the part about the fact that using an SSH key may be easier
> for users and the fact that it can be well supported by smart cards.
> Those are compelling arguments about why this is a desirable change, and
> should be in the commit message.
>
> I haven't looked too deeply at the intricacies of the change, but I'm in
> favor of it.  I would, however, like to see some tests here, including
> for commits, tags, and push certificates.  Note that you'll probably
> need to run the testsuite both with and without
> GIT_TEST_DEFAULT_HASH=sha256 to verify everything works.
I'm working on some tests but there are lots of GPG / GPGSM tests in the 
suite and i'm unsure of how many i should duplicate.
Thanks for the info with the hash setting.
>
>> diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
>> index d94025cb368..fd71bd782ec 100644
>> --- a/Documentation/config/gpg.txt
>> +++ b/Documentation/config/gpg.txt
>> @@ -11,13 +11,13 @@ gpg.program::
>>   
>>   gpg.format::
>>   	Specifies which key format to use when signing with `--gpg-sign`.
>> -	Default is "openpgp" and another possible value is "x509".
>> +	Default is "openpgp". Other possible values are "x509", "ssh".
>>   
>>   gpg.<format>.program::
>>   	Use this to customize the program used for the signing format you
>>   	chose. (see `gpg.program` and `gpg.format`) `gpg.program` can still
>>   	be used as a legacy synonym for `gpg.openpgp.program`. The default
>> -	value for `gpg.x509.program` is "gpgsm".
>> +	value for `gpg.x509.program` is "gpgsm" and `gpg.ssh.program` is "ssh-keygen".
>>   
>>   gpg.minTrustLevel::
>>   	Specifies a minimum trust level for signature verification.  If
>> @@ -27,6 +27,15 @@ gpg.minTrustLevel::
>>   	with at least `undefined` trust.  Setting this option overrides
>>   	the required trust-level for all operations.  Supported values,
>>   	in increasing order of significance:
>> +
>> +gpg.ssh.allowedSigners::
>> +	A file containing all valid SSH signing principals.
>> +	Similar to an .ssh/authorized_keys file. See ssh-keygen(1) for details.
>> +	Defaults to .gitsigners
> We probably don't want to store this in the repository.  If OpenSSH has
> a standard location for this, then we can default to that; otherwise, we
> should pick something in .ssh or in $XDG_CONFIG_HOME/git.
I'm not aware of a standard location. I think there are use cases to 
store this in the repo, but i'm of course fine not defaulting to it.