PATCH: Less fragile lookup of gpg key

PATCH: Less fragile lookup of gpg key

[Grant Olson]

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

When signing a tag, git will attempt to lookup your gpg key if you don't
provide the key id.  Right now, it's a little fragile.  My gpg key uid
is "Grant T. Olson (Personal Email) <kgo@grant-olson.net>".  My git user
info is "Grant T. Olson <kgo@grant-olson.net>".  Things don't match
because git doesn't have the comment.

However, if I lookup just by email, things work perfectly.

I think doing this would make life much easier for new users, and in the
case that someone has an OpenPGP key without email (e.g. Ubuntu Master
Signing Key) we can safely assume they're an expert and will either add
the key id to their configuration or use -u instead of -s.

Here's a patch that will try to lookup the user by email only if no
signing key is provided.  If there is no email, it will still fall back
to the default generated by git.

From d0fcf1340495045813758f910e8f4d745e28546b Mon Sep 17 00:00:00 2001
From: Grant Olson <kgo@grant-olson.net>
Date: Sat, 1 May 2010 11:02:18 -0400
Subject: [PATCH] Lookup gpg key by email

---
 builtin/tag.c |    2 +-
 ident.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index d311491..4eb3cc5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -165,7 +165,7 @@ static int do_sign(struct strbuf *buffer)
 	int i, j;

 	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
+		if (strlcpy(signingkey, git_committer_email(),
 				sizeof(signingkey)) > sizeof(signingkey) - 1)
 			return error("committer info too long.");
 		bracket = strchr(signingkey, '>');
diff --git a/ident.c b/ident.c
index 9e24388..0e8b78a 100644
--- a/ident.c
+++ b/ident.c
@@ -260,6 +260,15 @@ const char *git_committer_info(int flag)
 			 flag);
 }

+const char *git_committer_email(void)
+{
+	const char *email = getenv("GIT_COMMITTER_EMAIL");
+	if(!email)
+		email = git_default_email;
+
+	return email;
+}
+
 int user_ident_sufficiently_given(void)
 {
 #ifndef WINDOWS
-- 
1.7.0.4



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 554 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[A Large Angry SCM]

Grant Olson wrote:
> When signing a tag, git will attempt to lookup your gpg key if you don't
> provide the key id.  Right now, it's a little fragile.  My gpg key uid
> is "Grant T. Olson (Personal Email) <kgo@grant-olson.net>".  My git user
> info is "Grant T. Olson <kgo@grant-olson.net>".  Things don't match
> because git doesn't have the comment.
> 
> However, if I lookup just by email, things work perfectly.
> 
> I think doing this would make life much easier for new users, and in the
> case that someone has an OpenPGP key without email (e.g. Ubuntu Master
> Signing Key) we can safely assume they're an expert and will either add
> the key id to their configuration or use -u instead of -s.
> 
> Here's a patch that will try to lookup the user by email only if no
> signing key is provided.  If there is no email, it will still fall back
> to the default generated by git.

Why not fall back to just the email if the full lookup fails?

Re: PATCH: Less fragile lookup of gpg key

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Grant Olson wrote:
>> When signing a tag, git will attempt to lookup your gpg key if you don't
>> provide the key id.  Right now, it's a little fragile.  My gpg key uid
>> is "Grant T. Olson (Personal Email) <kgo@grant-olson.net>".  My git user
>> info is "Grant T. Olson <kgo@grant-olson.net>".  Things don't match
>> because git doesn't have the comment.
>>
>> However, if I lookup just by email, things work perfectly.
>>
>> I think doing this would make life much easier for new users, and in the
>> case that someone has an OpenPGP key without email (e.g. Ubuntu Master
>> Signing Key) we can safely assume they're an expert and will either add
>> the key id to their configuration or use -u instead of -s.
>>
>> Here's a patch that will try to lookup the user by email only if no
>> signing key is provided.  If there is no email, it will still fall back
>> to the default generated by git.
>
> Why not fall back to just the email if the full lookup fails?

Thanks; I like that suggestion a lot better.  Grant's suggestion does not
make the lookup "less fragile", but actually makes it less reliable for
people with the same address with different spellings of name and want to
choose which one to use per project.

Re: PATCH: Less fragile lookup of gpg key

[Grant Olson]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On 5/1/2010 1:18 PM, Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
>>
>> Why not fall back to just the email if the full lookup fails?
> 
> Thanks; I like that suggestion a lot better.  Grant's suggestion does not
> make the lookup "less fragile", but actually makes it less reliable for
> people with the same address with different spellings of name and want to
> choose which one to use per project.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Unless I'm mis-understanding you, the does the opposite of that.  It
finds your gpg key based on your git email, ignoring your git name, so
that different spellings of the name between gpg and git become irrelevant.

-- 
Grant

"Can you construct some sort of rudimentary lathe?"


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 552 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[Junio C Hamano]

Grant Olson <kgo@grant-olson.net> writes:

> On 5/1/2010 1:18 PM, Junio C Hamano wrote:
>> A Large Angry SCM <gitzilla@gmail.com> writes:
>>>
>>> Why not fall back to just the email if the full lookup fails?
>> 
>> Thanks; I like that suggestion a lot better.  Grant's suggestion does not
>> make the lookup "less fragile", but actually makes it less reliable for
>> people with the same address with different spellings of name and want to
>> choose which one to use per project.
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> Unless I'm mis-understanding you, the does the opposite of that.  It
> finds your gpg key based on your git email, ignoring your git name, so
> that different spellings of the name between gpg and git become irrelevant.

If I have two keys like these:

    Junio C Hamano <gitster@pobox.com>
    Junio Hamano <gitster@pobox.com>

and I have the latter set in .git/config to use for the project I am
working on, your patch picks one at random, making the process less
reliable.

AFAIU, ALASCM's suggestion was to first try the current method (which
reliably picks what I told git to use by specifying user.name), and only
if that fails, i.e. if I do not have neither of the above two keys but
only have a key named like e.g.

    Git Panda <gitster@pobox.com>

then use only the e-mail as you wanted to, but do so purely as a
fallback.

Which I found quite reasonable.

Re: PATCH: Less fragile lookup of gpg key

[Grant Olson]

[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]

On 05/01/2010 03:54 PM, Junio C Hamano wrote:
> Grant Olson <kgo@grant-olson.net> writes:
>>
>> Unless I'm mis-understanding you, the does the opposite of that.  It
>> finds your gpg key based on your git email, ignoring your git name, so
>> that different spellings of the name between gpg and git become irrelevant.
> 
> If I have two keys like these:
> 
>     Junio C Hamano <gitster@pobox.com>
>     Junio Hamano <gitster@pobox.com>
> 
> and I have the latter set in .git/config to use for the project I am
> working on, your patch picks one at random, making the process less
> reliable.
> 
> AFAIU, ALASCM's suggestion was to first try the current method (which
> reliably picks what I told git to use by specifying user.name), and only
> if that fails, i.e. if I do not have neither of the above two keys but
> only have a key named like e.g.
> 
>     Git Panda <gitster@pobox.com>
> 
> then use only the e-mail as you wanted to, but do so purely as a
> fallback.
> 
> Which I found quite reasonable.

Fair enough.  This version of the patch will try to gpg sign by email
address only, if (and only if) you try to sign a tag without explicitly
providing a key id (-s) and the lookup by "user.name <user.email>" fails.

From 791a110dc4d362b2cd11b19ae25a86bf91710e34 Mon Sep 17 00:00:00 2001
From: Grant Olson <kgo@grant-olson.net>
Date: Sun, 2 May 2010 19:33:41 -0400
Subject: [PATCH] Lookup gpg key by email address if user+email lookup
fails with -s

---
 builtin/tag.c |   67
+++++++++++++++++++++++++++++++++++++++++++++------------
 cache.h       |    1 +
 ident.c       |    9 +++++++
 3 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index d311491..45dd43d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -156,22 +156,13 @@ static int verify_tag(const char *name, const char
*ref,
 	return 0;
 }

-static int do_sign(struct strbuf *buffer)
+static int do_gpg(struct strbuf *buffer)
 {
+	/* retval can be standard -1 for error, 0 for ok, or 1 for a warning
+	 * so that we can attempt to recover by running gpg again. */
 	struct child_process gpg;
 	const char *args[4];
-	char *bracket;
 	int len;
-	int i, j;
-
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
-			return error("committer info too long.");
-		bracket = strchr(signingkey, '>');
-		if (bracket)
-			bracket[1] = '\0';
-	}

 	/* When the username signingkey is bad, program could be terminated
 	 * because gpg exits without reading and then write gets SIGPIPE. */
@@ -199,8 +190,56 @@ static int do_sign(struct strbuf *buffer)
 	len = strbuf_read(buffer, gpg.out, 1024);
 	close(gpg.out);

-	if (finish_command(&gpg) || !len || len < 0)
-		return error("gpg failed to sign the tag");
+	if(finish_command(&gpg))
+		{
+			warning("gpg failed to sign the tag");
+			return 1;
+		}
+
+	return 0;
+}
+
+static int do_sign(struct strbuf *buffer)
+{
+	char *bracket;
+	int i, j;
+	int uid_for_key = 0;
+	int err_ok_warning;
+
+	if (!*signingkey) {
+		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
+				sizeof(signingkey)) > sizeof(signingkey) - 1)
+			return error("committer info too long.");
+		bracket = strchr(signingkey, '>');
+		if (bracket)
+			bracket[1] = '\0';
+		uid_for_key = 1;
+	}
+
+	err_ok_warning = do_gpg(buffer);
+
+	if(!err_ok_warning || err_ok_warning != 1)
+		return -1;
+
+	if (err_ok_warning == 1 || !buffer->len || buffer->len < 0)
+	{
+		if (uid_for_key)
+		{
+			warning("couldn't find key for '%s'...", signingkey);
+			warning("Trying key lookup by email address only.");
+
+			if (strlcpy(signingkey, git_committer_email(),
+				sizeof(signingkey)) > sizeof(signingkey) - 1)
+					return error("committer info too long.");
+
+				err_ok_warning = do_gpg(buffer);
+
+			if (err_ok_warning || !buffer->len || buffer-> len < 0)
+				return error("gpg failed to sign the tag");
+		} else {
+			return error("gpg failed to sign the tag");
+		}
+	}

 	/* Strip CR from the line endings, in case we are on Windows. */
 	for (i = j = 0; i < buffer->len; i++)
diff --git a/cache.h b/cache.h
index 5eb0573..90a3067 100644
--- a/cache.h
+++ b/cache.h
@@ -789,6 +789,7 @@ enum date_mode parse_date_format(const char *format);
 #define IDENT_NO_DATE	       4
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
+extern const char *git_committer_email();
 extern const char *fmt_ident(const char *name, const char *email, const
char *date_str, int);
 extern const char *fmt_name(const char *name, const char *email);
 extern const char *git_editor(void);
diff --git a/ident.c b/ident.c
index 9e24388..0e8b78a 100644
--- a/ident.c
+++ b/ident.c
@@ -260,6 +260,15 @@ const char *git_committer_info(int flag)
 			 flag);
 }

+const char *git_committer_email(void)
+{
+	const char *email = getenv("GIT_COMMITTER_EMAIL");
+	if(!email)
+		email = git_default_email;
+
+	return email;
+}
+
 int user_ident_sufficiently_given(void)
 {
 #ifndef WINDOWS
-- 
1.7.0.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 554 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[Greg A. Woods]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

At Sat, 01 May 2010 12:54:24 -0700, Junio C Hamano <gitster@pobox.com> wrote:
Subject: Re: PATCH:  Less fragile lookup of gpg key
> 
> If I have two keys like these:
> 
>     Junio C Hamano <gitster@pobox.com>
>     Junio Hamano <gitster@pobox.com>

I'm not an expert on PGP internals or such, but I think that's a really
bad thing to do.  I'm surprised you were able to get gpg to do it in the
first place.  I would have hoped it wouldn't allow it.  As far as I can
tell it's _not_ compatible with other implementations of PGP.

PGP keys normally are searched by the e-mail portion only.  All the
other stuff (comments and the display name, etc.) is for decoration
only.  This is just as it is in e-mail routing too of course.

You can of course have more than one e-mail address per key, but you
should NEVER have more than one key per e-mail.

I.e. it's less reliable in the first place to have two different keys
which can be found using the same e-mail address.

-- 
						Greg A. Woods
						Planix, Inc.

<woods@planix.com>       +1 416 218 0099        http://www.planix.com/

[-- Attachment #2: Type: application/pgp-signature, Size: 186 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[Grant Olson]

[-- Attachment #1: Type: text/plain, Size: 3502 bytes --]

On 5/2/2010 8:59 PM, Greg A. Woods wrote:
> At Sat, 01 May 2010 12:54:24 -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Subject: Re: PATCH:  Less fragile lookup of gpg key
>>
>> If I have two keys like these:
>>
>>     Junio C Hamano <gitster@pobox.com>
>>     Junio Hamano <gitster@pobox.com>
> 
> I'm not an expert on PGP internals or such, but I think that's a really
> bad thing to do.  I'm surprised you were able to get gpg to do it in the
> first place.  I would have hoped it wouldn't allow it.  As far as I can
> tell it's _not_ compatible with other implementations of PGP.
> 
> PGP keys normally are searched by the e-mail portion only.  All the
> other stuff (comments and the display name, etc.) is for decoration
> only.  This is just as it is in e-mail routing too of course.
> 
> You can of course have more than one e-mail address per key, but you
> should NEVER have more than one key per e-mail.
> 
> I.e. it's less reliable in the first place to have two different keys
> which can be found using the same e-mail address.
> 

This might be getting a little off-topic for the git list, but...

It's a weird thing to do, which is why I didn't account for it in the
original patch, but the RFC doesn't have any specific requirements
regarding what a UID is:

5.11. User ID Packet (Tag 13)

   A User ID packet consists of UTF-8 text that is intended to represent
   the name and email address of the key holder.  By convention, it
   includes an RFC 2822 [RFC2822] mail name-addr, but there are no
   restrictions on its content.  The packet length in the header
   specifies the length of the User ID.

Ultimately the key ID is the unique identifier, so there's nothing
technically wrong with creating multiple keys with the same email.  It
will cause some email clients to flip out, but that's a non-issue with
git tags.

But if someone is a regular user of OpenPGP, and they get two signatures
from the same email address with different keys, the assumption is going
to be that at least one of them is a forgery.  Maybe the keys are
cross-signed if, for example, someone is phasing out an old 1024-bit key
with a new 2048-bit one.  But that's still going to raise a few
eyebrows.  And even in that case, you still probably want to sign
everything with the same key.

If you look at any day-to-day usage of gpg, or examples posted around,
they're almost always going to just pass an email address only with the
-u flag, if they use the -u flag at all.  I don't think you're going to
find any examples or tutorials that suggest you type:

gpg -u "Junio Hamano <gitster@pobox.com>" --armor --sign file.txt

they'll all use:

gpg -u gitster@pobox.com --armor --sign file.txt

And normally a user wouldn't even use the -u flag to begin with.  They
would just go with the default secret key on your keyring.  But that
might have a different email address than your git settings, so it does
make sense to use -u within git.

I personally think that using multiple keys with the same UID falls into
the 'advanced user' category, where you can expect the advanced user to
figure out how to deal with the exceptional usage, and have the defaults
set out to cover the general case.

But all that being said, I don't have a problem with the proposed
solution of falling back to a straight email search after the username +
email search fails.

-- 
Grant

"Can you construct some sort of rudimentary lathe?"


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 552 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[Theodore Tso]

On May 2, 2010, at 8:59 PM, Greg A. Woods wrote:

> I'm not an expert on PGP internals or such, but I think that's a really
> bad thing to do.  I'm surprised you were able to get gpg to do it in the
> first place.  I would have hoped it wouldn't allow it.  As far as I can
> tell it's _not_ compatible with other implementations of PGP.

> You can of course have more than one e-mail address per key, but you
> should NEVER have more than one key per e-mail.

This is pretty common actually.  At the very least it will happen if people are trying to transition between an older and a newer key --- for example, if they are trying to move from a less secure crypto algorithm to a more secure crypto algorithm.

And most versions of PGP support it _just_ _fine_.

They may not pick the key I want, but that's why I have environment or config files set up in various programs which use PGP to use a specific PGP KeyID.

-- Ted

Re: PATCH: Less fragile lookup of gpg key

[Greg A. Woods]

[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]

At Mon, 3 May 2010 07:16:55 -0400, Theodore Tso <tytso@MIT.EDU> wrote:
Subject: Re: PATCH:  Less fragile lookup of gpg key
> 
> On May 2, 2010, at 8:59 PM, Greg A. Woods wrote:
> > 
> > You can of course have more than one e-mail address per key, but you
> > should NEVER have more than one key per e-mail.
> 
> This is pretty common actually.  At the very least it will happen if
> people are trying to transition between an older and a newer key ---
> for example, if they are trying to move from a less secure crypto
> algorithm to a more secure crypto algorithm.

As I understand things the best way to manage these kinds of things is
to use sub-keys.  You can change the expire time on a sub-key, and then
eventually you can revoke it, all the while preserving your one primary
public key for signing.  Indeed it's a good idea to regularly change
your sub-key and expire the older ones.

Any time I've ever encountered anyone with more than one published key
associated with any given e-mail address, confusion has inevitably
ensued.

Normally the only time I've ever seen anyone end up with multiple
published keys associated with the same e-mail address it has happened
when they have accidentally lost their private key somehow and therefore
they were unable to revoke it properly.

If you must regenerate your primary public key, and you have control of
your old public key then the right thing to do is to set the old one to
expire ASAP, and/or to revoke it, upon generating a new one, then
publish the updates together.  This way there doesn't have to be any
window of confusion.

So, as Grant Olson has also explained, publishing multiple keys with the
same e-mail address in one of their UIDs (even if the entire UID is not
identical), is only for advanced users who are willing to deal with the
exceptional usage that results.  Not all Git users are advanced users
who will be willing and/or able to deal with these issues.

Meanwhile the original problem here appears to me to be that Git
effectively encourages use of multiple valid keys that may have the same
e-mail address attached to multiple key-IDs.

If I understand correctly from the GnuPG documentation, the desired way
to search for a key has a very well defined algorithm based on the
syntax identifying the format of the "key".  I think Git should use that
same algorithm at minimum, but by default if there's no hint based on
the expressed syntax of the key given it should follow the example of
most/all(?) MUA interfaces to PGP, which if I'm not mistaken is to
search by exact match of the e-mail address stripped of any display name
and all comments.

-- 
						Greg A. Woods
						Planix, Inc.

<woods@planix.com>       +1 416 218 0099        http://www.planix.com/

[-- Attachment #2: Type: application/pgp-signature, Size: 186 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[tytso]

On Mon, May 03, 2010 at 06:19:17PM -0400, Greg A. Woods wrote:
> 
> Normally the only time I've ever seen anyone end up with multiple
> published keys associated with the same e-mail address it has happened
> when they have accidentally lost their private key somehow and therefore
> they were unable to revoke it properly.

Well, I suspect this case happens fairly often.  (And there are other
cases; where you're still gathering enough signatures so you can use
your new key, and the old key hasn't been compromised, but people have
started getting paranoid about the crypto algorithms involved, etc.)
So I'd argue that saying this is only something that Advanced GPG
users will use is probably a bit short-sighted.

> Meanwhile the original problem here appears to me to be that Git
> effectively encourages use of multiple valid keys that may have the same
> e-mail address attached to multiple key-IDs.

Yes, I think that *is* the problem.  If you want to optimize for the
common case, that's fine, but it's also useful to have a way for users
to specify in their gitconfig files that a specific KeyID should be
used if they are signing with a particular e-mail ID.

							- Ted

Re: PATCH: Less fragile lookup of gpg key

[Grant Olson]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On 5/3/2010 10:19 PM, tytso@mit.edu wrote:
> On Mon, May 03, 2010 at 06:19:17PM -0400, Greg A. Woods wrote:
>> Meanwhile the original problem here appears to me to be that Git
>> effectively encourages use of multiple valid keys that may have the same
>> e-mail address attached to multiple key-IDs.
> 
> Yes, I think that *is* the problem.  If you want to optimize for the
> common case, that's fine, but it's also useful to have a way for users
> to specify in their gitconfig files that a specific KeyID should be
> used if they are signing with a particular e-mail ID.
> 

That's already there:

git config user.signingkey 0xDEADBEEF

-Grant


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 552 bytes --]

Re: PATCH: Less fragile lookup of gpg key

[Andreas Ericsson]

On 05/04/2010 04:19 AM, tytso@mit.edu wrote:
> On Mon, May 03, 2010 at 06:19:17PM -0400, Greg A. Woods wrote:
>>
>> Meanwhile the original problem here appears to me to be that Git
>> effectively encourages use of multiple valid keys that may have the same
>> e-mail address attached to multiple key-IDs.
> 
> Yes, I think that *is* the problem.  If you want to optimize for the
> common case, that's fine, but it's also useful to have a way for users
> to specify in their gitconfig files that a specific KeyID should be
> used if they are signing with a particular e-mail ID.
> 

Or even better, use the patch with Junio's suggested improvements, so
nothing changes from how things stand today for people with multiple
keys and where git_UID == gpg_UID, but for those where that's not so,
try again with just the mail part when no signing key is configured.

Everything else is theoretical discussion that would best be dealt
with on some crypto-related mailing list where people actually care.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.