diff mbox series

[RFC,2/2] crypto sign: add cryptoSign.* config

Message ID 20211220140928.1205586-3-fs@gigacodes.de (mailing list archive)
State New, archived
Headers show
Series cryptoSign flag & config | expand

Commit Message

Fabian Stelzer Dec. 20, 2021, 2:09 p.m. UTC
Since git now supports multiple cryptographic methods/formats to sign
objects, the `gpg.` configuration prefix is misleading.
Add `cryptoSign.`, but keep `gpg.` as a compatibility alias at least for
all existing options.
`gpg.mintrustlevel` is moved to `cryptosign.gpg.mintrustlevel` while
also still allowing the former.
---
 Documentation/config/gpg.txt | 31 ++++++++++++++++++++-----------
 gpg-interface.c              | 30 ++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 19 deletions(-)

Comments

Eric Sunshine Dec. 20, 2021, 10:07 p.m. UTC | #1
`On Mon, Dec 20, 2021 at 9:09 AM Fabian Stelzer <fs@gigacodes.de> wrote:
> Since git now supports multiple cryptographic methods/formats to sign
> objects, the `gpg.` configuration prefix is misleading.
> Add `cryptoSign.`, but keep `gpg.` as a compatibility alias at least for
> all existing options.
> `gpg.mintrustlevel` is moved to `cryptosign.gpg.mintrustlevel` while
> also still allowing the former.
> ---
> diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
> @@ -1,6 +1,17 @@
> +cryptoSign.format::
> +gpg.format::
> +       Specifies which key format to use when signing with `--crypto-sign`.
> +       Default is "openpgp". Other possible values are "x509", "ssh".
> +
> +cryptoSign.<format>.program::
> +gpg.<format>.program::
> +       Use this to customize the program used for the signing format you
> +       chose (see `cryptoSign.format`). The default value for

This is a somewhat minor comment, but I find that grouping these
config keys together like this gives too much weight to the old
`gpg.foo` ones, making it seem as if they're still first-class
citizens which people can use freely. If you instead organize them as
below, then it is easier to see at a glance that the old keys
shouldn't be used:

    cryptoSign.format::
        Specifies which key format to use when signing...

    cryptoSign.<format>.program::
        Use this to customize the program used...

    ...

    gpg.format::
        Deprecated synonym of `cryptoSign.format`.

    gpg.<format>.program::
        Deprecated synonym of `cryptoSign.<format>.program`.

The same observation about grouping of config keys applies to the
remainder of the documentation changes in this patch.
Fabian Stelzer Dec. 21, 2021, 9:39 a.m. UTC | #2
On 20.12.2021 17:07, Eric Sunshine wrote:
>`On Mon, Dec 20, 2021 at 9:09 AM Fabian Stelzer <fs@gigacodes.de> wrote:
>> Since git now supports multiple cryptographic methods/formats to sign
>> objects, the `gpg.` configuration prefix is misleading.
>> Add `cryptoSign.`, but keep `gpg.` as a compatibility alias at least for
>> all existing options.
>> `gpg.mintrustlevel` is moved to `cryptosign.gpg.mintrustlevel` while
>> also still allowing the former.
>> ---
>> diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
>> @@ -1,6 +1,17 @@
>> +cryptoSign.format::
>> +gpg.format::
>> +       Specifies which key format to use when signing with `--crypto-sign`.
>> +       Default is "openpgp". Other possible values are "x509", "ssh".
>> +
>> +cryptoSign.<format>.program::
>> +gpg.<format>.program::
>> +       Use this to customize the program used for the signing format you
>> +       chose (see `cryptoSign.format`). The default value for
>
>This is a somewhat minor comment, but I find that grouping these
>config keys together like this gives too much weight to the old
>`gpg.foo` ones, making it seem as if they're still first-class
>citizens which people can use freely. If you instead organize them as
>below, then it is easier to see at a glance that the old keys
>shouldn't be used:
>
>    cryptoSign.format::
>        Specifies which key format to use when signing...
>
>    cryptoSign.<format>.program::
>        Use this to customize the program used...
>
>    ...
>
>    gpg.format::
>        Deprecated synonym of `cryptoSign.format`.
>
>    gpg.<format>.program::
>        Deprecated synonym of `cryptoSign.<format>.program`.
>
>The same observation about grouping of config keys applies to the
>remainder of the documentation changes in this patch.

I wasn't sure how much we want to already deprecate the `gpg.` keys so I 
tried a gentle approach :)
But I would be in favor of your variant.

Thanks
diff mbox series

Patch

diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
index 4f30c7dbdd..ef21eb8249 100644
--- a/Documentation/config/gpg.txt
+++ b/Documentation/config/gpg.txt
@@ -1,6 +1,17 @@ 
 gpg.program::
-	Use this custom program instead of "`gpg`" found on `$PATH` when
-	making or verifying a PGP signature. The program must support the
+	Deprecated alias for `cryptoSign.<format>.program`.
+
+cryptoSign.format::
+gpg.format::
+	Specifies which key format to use when signing with `--crypto-sign`.
+	Default is "openpgp". Other possible values are "x509", "ssh".
+
+cryptoSign.<format>.program::
+gpg.<format>.program::
+	Use this to customize the program used for the signing format you
+	chose (see `cryptoSign.format`). The default value for
+	`gpg.x509.program` is "gpgsm" and `gpg.ssh.program` is "ssh-keygen".
+	With the format set to "opengpg" or "x509" the program must support the
 	same command-line interface as GPG, namely, to verify a detached
 	signature, "`gpg --verify $signature - <$file`" is run, and the
 	program is expected to signal a good signature by exiting with
@@ -8,17 +19,12 @@  gpg.program::
 	standard input of "`gpg -bsau $key`" is fed with the contents to be
 	signed, and the program is expected to send the result to its
 	standard output.
+	If the format is "ssh", then the configured program must implement the
+	`ssh-keygen -Y find-principals|check-novalidate|verify|sign` commands
+	(see ssh-keygen(1) man page).
 
-gpg.format::
-	Specifies which key format to use when signing with `--gpg-sign`.
-	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" and `gpg.ssh.program` is "ssh-keygen".
 
+crpytoSign.gpg.minTrustLevel::
 gpg.minTrustLevel::
 	Specifies a minimum trust level for signature verification.  If
 	this option is unset, then signature verification for merge
@@ -34,12 +40,14 @@  gpg.minTrustLevel::
 * `fully`
 * `ultimate`
 
+cryptoSign.ssh.defaultKeyCommand::
 gpg.ssh.defaultKeyCommand:
 	This command that will be run when user.signingkey is not set and a ssh
 	signature is requested. On successful exit a valid ssh public key is
 	expected in the	first line of its output. To automatically use the first
 	available key from your ssh-agent set this to "ssh-add -L".
 
+cryptoSign.ssh.allowedSignersFile::
 gpg.ssh.allowedSignersFile::
 	A file containing ssh public keys which you are willing to trust.
 	The file consists of one or more lines of principals followed by an ssh
@@ -67,6 +75,7 @@  This way only committers with an already valid key can add or change keys in the
 Using a SSH CA key with the cert-authority option
 (see ssh-keygen(1) "CERTIFICATES") is also valid.
 
+cryptoSign.ssh.revocationFile::
 gpg.ssh.revocationFile::
 	Either a SSH KRL or a list of revoked public keys (without the principal prefix).
 	See ssh-keygen(1) for details.
diff --git a/gpg-interface.c b/gpg-interface.c
index 3e7255a2a9..eacafcd56e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -638,6 +638,7 @@  int git_gpg_config(const char *var, const char *value, void *cb)
 	struct gpg_format *fmt = NULL;
 	char *fmtname = NULL;
 	char *trust;
+	const char *crypto_var = NULL;
 	int ret;
 
 	if (!strcmp(var, "user.signingkey")) {
@@ -647,7 +648,17 @@  int git_gpg_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(var, "gpg.format")) {
+	/*
+	 * `gpg.` is a backwards compatibility prefix alias for `cryptosign.`
+	 * All following vars expect a prefix so we can return early if
+	 * there is none
+	 */
+	if (!skip_prefix(var, "gpg.", &crypto_var) &&
+	    !skip_prefix(var, "cryptosign.", &crypto_var))
+		return 0;
+
+
+	if (!strcmp(crypto_var, "format")) {
 		if (!value)
 			return config_error_nonbool(var);
 		fmt = get_format_by_name(value);
@@ -658,7 +669,9 @@  int git_gpg_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(var, "gpg.mintrustlevel")) {
+	/* `gpg.mintrustlevel` moved to `cryptosign.gpg.mintrustlevel` */
+	if (!strcmp(crypto_var, "mintrustlevel") ||
+	    !strcmp(crypto_var, "gpg.mintrustlevel")) {
 		if (!value)
 			return config_error_nonbool(var);
 
@@ -672,31 +685,32 @@  int git_gpg_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(var, "gpg.ssh.defaultkeycommand")) {
+	if (!strcmp(crypto_var, "ssh.defaultkeycommand")) {
 		if (!value)
 			return config_error_nonbool(var);
 		return git_config_string(&ssh_default_key_command, var, value);
 	}
 
-	if (!strcmp(var, "gpg.ssh.allowedsignersfile")) {
+	if (!strcmp(crypto_var, "ssh.allowedsignersfile")) {
 		if (!value)
 			return config_error_nonbool(var);
 		return git_config_pathname(&ssh_allowed_signers, var, value);
 	}
 
-	if (!strcmp(var, "gpg.ssh.revocationfile")) {
+	if (!strcmp(crypto_var, "ssh.revocationfile")) {
 		if (!value)
 			return config_error_nonbool(var);
 		return git_config_pathname(&ssh_revocation_file, var, value);
 	}
 
-	if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program"))
+	if (!strcmp(crypto_var, "program") ||
+	    !strcmp(crypto_var, "openpgp.program"))
 		fmtname = "openpgp";
 
-	if (!strcmp(var, "gpg.x509.program"))
+	if (!strcmp(crypto_var, "x509.program"))
 		fmtname = "x509";
 
-	if (!strcmp(var, "gpg.ssh.program"))
+	if (!strcmp(crypto_var, "ssh.program"))
 		fmtname = "ssh";
 
 	if (fmtname) {