Mailing List Archive

[PATCH GnuPG 2/2] gpg: allow import of previously known keys, even without UIDs
* g10/import.c (import_one): Allow import of keys that have no user ids,
if we already know them. Keys are still rejected if they contained
invalid user ids, or none that pass a given filter criteria.
---
g10/import.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/g10/import.c b/g10/import.c
index 00bc47cc1..89ec18840 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -1806,16 +1806,6 @@ import_one (ctrl_t ctrl,
log_printf ("\n");
}

-
- /* Unless import-drop-uids has been requested we don't allow import
- * of a key without UIDs. */
- if (!uidnode && !(options & IMPORT_DROP_UIDS))
- {
- if (!silent)
- log_error( _("key %s: no user ID\n"), keystr_from_pk(pk));
- return 0;
- }
-
if (screener && screener (keyblock, screener_arg))
{
log_error (_("key %s: %s\n"), keystr_from_pk (pk),
@@ -1888,9 +1878,9 @@ import_one (ctrl_t ctrl,
}

/* Delete invalid parts and without the drop option bail out if
- * there are no user ids. */
+ * there were user ids, but none was actually valid. */
if (!delete_inv_parts (ctrl, keyblock, keyid, options)
- && !(options & IMPORT_DROP_UIDS) )
+ && uidnode && !(options & IMPORT_DROP_UIDS) )
{
if (!silent)
{
@@ -1985,6 +1975,13 @@ import_one (ctrl_t ctrl,
err = 0;
stats->skipped_new_keys++;
}
+ else if (err && !uidnode && !(options & IMPORT_DROP_UIDS) )
+ {
+ if (!silent)
+ log_info( _("key %s: new key but contains no user ID - skipped\n"), keystr(keyid));
+ err = 0;
+ stats->no_user_id++;
+ }
else if (err) /* Insert this key. */
{
/* Note: ERR can only be NO_PUBKEY or UNUSABLE_PUBKEY. */
--
2.20.1


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH GnuPG 2/2] gpg: allow import of previously known keys, even without UIDs [ In reply to ]
Thanks for this series, Vincent! I like the motivation behind it. I
think it is concretely useful to import an OpenPGP certificate as part
of a merge even if the incoming cert does not have a
cryptographically-valid self-sig over any user ID.

This permits the use of public keystores that perform the "redacting
User IDs" mitigation described at
https://tools.ietf.org/html/draft-dkg-openpgp-abuse-resistant-keystore-03#section-5.1

In particular, it allows gpg to perform "Certificate Update with
Redacted User IDs":
https://tools.ietf.org/html/draft-dkg-openpgp-abuse-resistant-keystore-03#section-5.1.1

(other operations later in section 5.1 that might involve synthesizing
user IDs are not yet supported, even with this patch, and that's ok)

It would be great to add a test to the test suite for this behavior --
to show what does *not* work before the patch is applied, and then to
have the test suite succeed after the patches are applied. This would
also help us avoid regressions on this behavior in the future.

For example, here is a severely stripped down OpenPGP certificate. Just
a primary key and one user ID and a self-sig:

-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: [A] primary key, user ID, and self-sig expiring in 2021

mDMEXNmUGRYJKwYBBAHaRw8BAQdA75R8VlchvmEd2Iz/8l07RoKUaUPDB71Ao1zZ
631VAN20CHRlc3Qga2V5iJYEExYIAD4WIQS0aY+VvNh/4EjMyikIQ9qWmqja+wUC
XNmUGQIbAwUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRAIQ9qWmqja
+0G1AQDdQiwhXxjXLMqoth+D4SigVHTJK8ORwifzsy3UE7mPGwD/aZ67XbAF/lgI
kv2O1Jo0u9BL9RNNF+L0DM7rAFbfMAs=
=1eII
-----END PGP PUBLIC KEY BLOCK-----

Here is a cert for the same primary key, but with a subkey and no user
ID attached:

-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: [B] primary key, subkey, subkey binding sig (no user ID)

mDMEXNmUGRYJKwYBBAHaRw8BAQdA75R8VlchvmEd2Iz/8l07RoKUaUPDB71Ao1zZ
631VAN24OARc2ZQhEgorBgEEAZdVAQUBAQdABsd5ha0AWXdXcSmfeiWIfrNcGqQK
j++lwwWDAOlkVicDAQgHiHgEGBYIACAWIQS0aY+VvNh/4EjMyikIQ9qWmqja+wUC
XNmUIQIbDAAKCRAIQ9qWmqja++vFAP98G1L+1/rWTGbsnxOAV2RocBYIroAvsbkR
Ly6FdP8YNwEA7jOgT05CoKIe37MstpOz23mM80AK369Ca3JMmKKCQgg=
=xuDu
-----END PGP PUBLIC KEY BLOCK-----

And here is a cert for the same primary key, with the user ID packet
stripped entirely, but with updated self-sig (issued after the initial
one) that contains a later expiration date over that same user ID:

-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: [C] primary key and self-sig expiring in 2024 (no user ID)

mDMEXNmUGRYJKwYBBAHaRw8BAQdA75R8VlchvmEd2Iz/8l07RoKUaUPDB71Ao1zZ
631VAN2IlgQTFggAPgIbAwULCQgHAgYVCgkICwIEFgIDAQIeAQIXgBYhBLRpj5W8
2H/gSMzKKQhD2paaqNr7BQJc2ZR1BQkJZgHcAAoJEAhD2paaqNr79soA/0lWkUsu
3NLwgbni6EzJxnTzgeNMpljqNpipHAwfix9hAP93AVtFdC8g7hdUZxawobl9lnSN
9ohXOEBWvdJgVv2YAg==
=KWIK
-----END PGP PUBLIC KEY BLOCK-----


If i do "gpg --import" on A, then B, then C, and then i do "gpg
--edit-key test clean save" and "gpg --list-keys", i think gpg display
have a merged certificate that expires in 2024, and has a subkey, like
this:

pub ed25519 2019-05-13 [SC] [expires: 2024-05-11]
B4698F95BCD87FE048CCCA290843DA969AA8DAFB
uid [ unknown] test key
sub cv25519 2019-05-13 [E]

but in practice, the imports of both B and C fail with:

gpg: key 0843DA969AA8DAFB: no user ID

and the result is instead the same as if only A were present:

pub ed25519 2019-05-13 [SC] [expires: 2021-05-12]
B4698F95BCD87FE048CCCA290843DA969AA8DAFB
uid [ unknown] test key


Feel free to use these example objects in the test suite if that's
useful.

On Sun 2019-05-12 12:36:56 +0200, Vincent Breitmoser wrote:
> * g10/import.c (import_one): Allow import of keys that have no user ids,
> if we already know them. Keys are still rejected if they contained
> invalid user ids, or none that pass a given filter criteria.

Your commit message here doesn't seem to match the intent of this patch,
for several reasons:

* It talks about several different keys, but it would be good to
clarify in each circumstance whether we're talking about:

a) the known copy of the OpenPGP certificate,
b) the incoming copy of the OpenPGP certificate, or
c) the merged copy of the OpenPGP certificate.

* "contained invalid user ids" is different from "contained no valid
user IDs"

* "validity" for user IDs in the OpenPGP typically means something
subtly different from "has a cryptographically correct self-sig from
the primary key". It would be good to avoid mixing those up.

As a nit-pick, i'd also convert the description to refer to the
certificate in the singular instead of plural where possible, because
it's easier to read and make sense of (esp. when the cert itself might
have multiple elements internally).

rewriting to what i *think* the intent of the series is, i'd have
written it this way:


* g10/import.c (import_one): Accept an incoming OpenPGP certificate that
has no user id, as long as we already have a local variant of the cert
that matches the primary key. Import still fails if the resulting
merged certificate contains no self-signed user ids that pass any given
filter criteria.


Is this your intent?

--dkg
Re: [PATCH GnuPG 2/2] gpg: allow import of previously known keys, even without UIDs [ In reply to ]
On Mon 2019-05-13 12:19:20 -0400, Daniel Kahn Gillmor wrote:
> It would be great to add a test to the test suite for this behavior --
> to show what does *not* work before the patch is applied, and then to
> have the test suite succeed after the patches are applied. This would
> also help us avoid regressions on this behavior in the future.

Following up on this test regime, here are 2 additional OpenPGP
certificates that use the same primary key and lack user IDs, but
dealing with revocations of key material:

-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: [D] primary key, subkey, subkey revocation (no user ID)

mDMEXNmUGRYJKwYBBAHaRw8BAQdA75R8VlchvmEd2Iz/8l07RoKUaUPDB71Ao1zZ
631VAN24OARc2ZQhEgorBgEEAZdVAQUBAQdABsd5ha0AWXdXcSmfeiWIfrNcGqQK
j++lwwWDAOlkVicDAQgHiHgEKBYIACAWIQS0aY+VvNh/4EjMyikIQ9qWmqja+wUC
XNmnkAIdAgAKCRAIQ9qWmqja+ylaAQDmIKf86BJEq4OpDqU+V9D+wn2cyuxbyWVQ
3r9LiL9qNwD/QAjyrhSN8L3Mfq+wdTHo5i0yB9ZCCpHLXSbhCqfWZwQ=
=dwx2
-----END PGP PUBLIC KEY BLOCK-----


-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: [E] primary key, revocation signature over primary (no user ID)

mDMEXNmUGRYJKwYBBAHaRw8BAQdA75R8VlchvmEd2Iz/8l07RoKUaUPDB71Ao1zZ
631VAN2IeAQgFggAIBYhBLRpj5W82H/gSMzKKQhD2paaqNr7BQJc2ZQZAh0AAAoJ
EAhD2paaqNr7qAwA/2jBUpnN0BxwRO/4CrxvrLIsL+C9aSXJUOTv8XkP4lvtAQD3
XsDFfFNgEueiTfF7HtOGt5LPmRqVvUpQSMVgJJW6CQ==
=tM90
-----END PGP PUBLIC KEY BLOCK-----

I believe that failing to import these revocation certificates
represents a security risk to users of GnuPG, because it means leaving
the user with a certificate that GnuPG knows is revoked, but it is left
unrevoked.

--dkg
[PATCH GnuPG 2/2] gpg: allow import of previously known keys, even without UIDs [ In reply to ]
* g10/import.c (import_one): Accept an incoming OpenPGP certificate that
has no user id, as long as we already have a local variant of the cert
that matches the primary key.
---
g10/import.c | 49 +++++++++++--------------------------------------
1 file changed, 11 insertions(+), 38 deletions(-)

diff --git a/g10/import.c b/g10/import.c
index 00bc47cc1..2be214e63 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -1769,7 +1769,6 @@ import_one (ctrl_t ctrl,
size_t an;
char pkstrbuf[PUBKEY_STRING_SIZE];
int merge_keys_done = 0;
- int any_filter = 0;
KEYDB_HANDLE hd = NULL;

if (r_valid)
@@ -1806,16 +1805,6 @@ import_one (ctrl_t ctrl,
log_printf ("\n");
}

-
- /* Unless import-drop-uids has been requested we don't allow import
- * of a key without UIDs. */
- if (!uidnode && !(options & IMPORT_DROP_UIDS))
- {
- if (!silent)
- log_error( _("key %s: no user ID\n"), keystr_from_pk(pk));
- return 0;
- }
-
if (screener && screener (keyblock, screener_arg))
{
log_error (_("key %s: %s\n"), keystr_from_pk (pk),
@@ -1887,20 +1876,10 @@ import_one (ctrl_t ctrl,
}
}

- /* Delete invalid parts and without the drop option bail out if
- * there are no user ids. */
- if (!delete_inv_parts (ctrl, keyblock, keyid, options)
- && !(options & IMPORT_DROP_UIDS) )
- {
- if (!silent)
- {
- log_error( _("key %s: no valid user IDs\n"), keystr_from_pk(pk));
- if (!opt.quiet )
- log_info(_("this may be caused by a missing self-signature\n"));
- }
- stats->no_user_id++;
- return 0;
- }
+ /* Delete invalid parts, and note if we have any valid ones left.
+ * We will later abort import if this key is new but contains
+ * no valid uids. */
+ delete_inv_parts (ctrl, keyblock, keyid, options);

/* Get rid of deleted nodes. */
commit_kbnode (&keyblock);
@@ -1910,24 +1889,11 @@ import_one (ctrl_t ctrl,
{
apply_keep_uid_filter (ctrl, keyblock, import_filter.keep_uid);
commit_kbnode (&keyblock);
- any_filter = 1;
}
if (import_filter.drop_sig)
{
apply_drop_sig_filter (ctrl, keyblock, import_filter.drop_sig);
commit_kbnode (&keyblock);
- any_filter = 1;
- }
-
- /* If we ran any filter we need to check that at least one user id
- * is left in the keyring. Note that we do not use log_error in
- * this case. */
- if (any_filter && !any_uid_left (keyblock))
- {
- if (!opt.quiet )
- log_info ( _("key %s: no valid user IDs\n"), keystr_from_pk (pk));
- stats->no_user_id++;
- return 0;
}

/* The keyblock is valid and ready for real import. */
@@ -1985,6 +1951,13 @@ import_one (ctrl_t ctrl,
err = 0;
stats->skipped_new_keys++;
}
+ else if (err && !any_uid_left (keyblock) && !(options & IMPORT_DROP_UIDS) )
+ {
+ if (!silent)
+ log_info( _("key %s: new key but contains no user ID - skipped\n"), keystr(keyid));
+ err = 0;
+ stats->no_user_id++;
+ }
else if (err) /* Insert this key. */
{
/* Note: ERR can only be NO_PUBKEY or UNUSABLE_PUBKEY. */
--
2.20.1


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel