Quantcast

[PATCH] g10: Skip signing keys where no secret key is available.

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] g10: Skip signing keys where no secret key is available.

Daniel Kahn Gillmor-7
From: Simon Arlott <[hidden email]>

* g10/getkey.c (finish_lookup): When requiring PUBKEY_USAGE_SIG, skip
over keys where no signing key is available.

--

This should only be relevant when gpg is required to choose which key
to sign with -- if verifying signatures, we already know which subkey
to look at, and indeed gpg doesn't seem to have a problem with this.

This patch comes from
https://bugs.gnupg.org/gnupg/file793/sign-fix.patch

I (dkg) have reviewed and tested it with missing local keys, and it
makes sense to me as the default behavior.  If the user has the secret
key for a signing-capable subkey available and the command is --sign,
it should be used.

If the user has explicitly specified a subkey that happens to be
missing (e.g. with the trailing ! for --default-key 0x${FPR}!) then
this does not override that behavior (the signature will still fail).

GnuPG-bug-id: 1967
Debian-bug-id: 834922

Signed-off-by: Daniel Kahn Gillmor <[hidden email]>
---
 g10/getkey.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/g10/getkey.c b/g10/getkey.c
index e39de28ae..d2349ee6c 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -3523,6 +3523,13 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact,
       continue;
     }
 
+  if ((req_usage & PUBKEY_USAGE_SIG) && agent_probe_secret_key (NULL, pk))
+    {
+      if (DBG_LOOKUP)
+ log_debug ("\tno secret key for signing\n");
+      continue;
+    }
+
   if (DBG_LOOKUP)
     log_debug ("\tsubkey might be fine\n");
   /* In case a key has a timestamp of 0 set, we make sure
--
2.11.0


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] g10: Skip signing keys where no secret key is available.

Patrick Brunschwig-2
On 05.02.17 22:31, Daniel Kahn Gillmor wrote:

> From: Simon Arlott <[hidden email]>
>
> * g10/getkey.c (finish_lookup): When requiring PUBKEY_USAGE_SIG, skip
> over keys where no signing key is available.
>
> --
>
> This should only be relevant when gpg is required to choose which key
> to sign with -- if verifying signatures, we already know which subkey
> to look at, and indeed gpg doesn't seem to have a problem with this.
>
> This patch comes from
> https://bugs.gnupg.org/gnupg/file793/sign-fix.patch
>
> I (dkg) have reviewed and tested it with missing local keys, and it
> makes sense to me as the default behavior.  If the user has the secret
> key for a signing-capable subkey available and the command is --sign,
> it should be used.
>
> If the user has explicitly specified a subkey that happens to be
> missing (e.g. with the trailing ! for --default-key 0x${FPR}!) then
> this does not override that behavior (the signature will still fail).
>
> GnuPG-bug-id: 1967
> Debian-bug-id: 834922
>
> Signed-off-by: Daniel Kahn Gillmor <[hidden email]>
> ---
>  g10/getkey.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/g10/getkey.c b/g10/getkey.c
> index e39de28ae..d2349ee6c 100644
> --- a/g10/getkey.c
> +++ b/g10/getkey.c
> @@ -3523,6 +3523,13 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact,
>        continue;
>      }
>  
> +  if ((req_usage & PUBKEY_USAGE_SIG) && agent_probe_secret_key (NULL, pk))
> +    {
> +      if (DBG_LOOKUP)
> + log_debug ("\tno secret key for signing\n");
> +      continue;
> +    }
> +
>    if (DBG_LOOKUP)
>      log_debug ("\tsubkey might be fine\n");
>    /* In case a key has a timestamp of 0 set, we make sure
Would this patch still issue a "MISSING_KEY" line for --status-fd? If
no, you break existing logic (which for example Enigmail relies on).

-Patrick


_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] g10: Skip signing keys where no secret key is available.

Daniel Kahn Gillmor-7
On Mon 2017-02-06 09:57:59 +0100, Patrick Brunschwig wrote:
> Would this patch still issue a "MISSING_KEY" line for --status-fd? If
> no, you break existing logic (which for example Enigmail relies on).

in what case does enigmail expect a "MISSING_KEY" line?  the scenario is
that the user has a primary key A and two valid, non-expired,
signing-capable subkeys, B and C.  C is the newer subkey, and the user
has specified that they want to sign with A.  We'd like to go ahead and
sign with B if it is available and C is missing.

What should enigmail do in that case with a MISSING_KEY line?  shouldn't
it just accept that a valid signature has been made?

I've pushed the proposed fix to a new git branch dkg/T1967, and updated
https://dev.gnupg.org/T1967 to note that branch.

     --dkg

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] g10: Skip signing keys where no secret key is available.

Patrick Brunschwig-2
On 26.04.17 03:27, Daniel Kahn Gillmor wrote:

> On Mon 2017-02-06 09:57:59 +0100, Patrick Brunschwig wrote:
>> Would this patch still issue a "MISSING_KEY" line for --status-fd? If
>> no, you break existing logic (which for example Enigmail relies on).
>
> in what case does enigmail expect a "MISSING_KEY" line?  the scenario is
> that the user has a primary key A and two valid, non-expired,
> signing-capable subkeys, B and C.  C is the newer subkey, and the user
> has specified that they want to sign with A.  We'd like to go ahead and
> sign with B if it is available and C is missing.
>
> What should enigmail do in that case with a MISSING_KEY line?  shouldn't
> it just accept that a valid signature has been made?
>
> I've pushed the proposed fix to a new git branch dkg/T1967, and updated
> https://dev.gnupg.org/T1967 to note that branch.
Sorry, I meant INV_SGNR. Enigmail looks for this when a message is
supposed to be signed but there is no suitable key.

-Patrick



_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] g10: Skip signing keys where no secret key is available.

Daniel Kahn Gillmor-7
On Wed 2017-04-26 08:22:31 +0200, Patrick Brunschwig wrote:

> On 26.04.17 03:27, Daniel Kahn Gillmor wrote:
>> On Mon 2017-02-06 09:57:59 +0100, Patrick Brunschwig wrote:
>>> Would this patch still issue a "MISSING_KEY" line for --status-fd? If
>>> no, you break existing logic (which for example Enigmail relies on).
>>
>> in what case does enigmail expect a "MISSING_KEY" line?  the scenario is
>> that the user has a primary key A and two valid, non-expired,
>> signing-capable subkeys, B and C.  C is the newer subkey, and the user
>> has specified that they want to sign with A.  We'd like to go ahead and
>> sign with B if it is available and C is missing.
>>
>> What should enigmail do in that case with a MISSING_KEY line?  shouldn't
>> it just accept that a valid signature has been made?
>>
>> I've pushed the proposed fix to a new git branch dkg/T1967, and updated
>> https://dev.gnupg.org/T1967 to note that branch.
>
> Sorry, I meant INV_SGNR. Enigmail looks for this when a message is
> supposed to be signed but there is no suitable key.
So you're asking whether INV_SGNR shows up in the case where there
really is *no* signing subkey, right?  because if gpg finds a different
signing subkey available and uses it successfully, you don't want it to
appear.  right?

I just did the following test in a fresh $GNUPGHOME (manually extracting
$PRIMARY_FPR and $SUBKEY0_GRIP from the relevant places), with the patch
under discussion applied here.  Note that i'm renaming
private-keys-v1.d/*.key files to emulate removal of secret key material
by any other means:


  gpg --quick-gen-key 'testing <[hidden email]>' rsa cert
  gpg --quick-add-key ${PRIMARY_FPR} rsa sign # signing-capable subkey 0
  gpg --with-keygrip --list-keys
  echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
     ## makes valid signature with subkey 0, no INV_SGNR  
  mv $GNUPGHOME/private-keys-v1.d/${SUBKEY0_GRIP}{,.bak}
  echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
     ## produces INV_SGNR on stderr
  gpg --quick-add-key ${PRIMARY_FPR} rsa sign  # signing-capable subkey 1
  echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
     ## makes valid signature with subkey 1, no INV_SGNR  
  mv $GNUPGHOME/private-keys-v1.d/${SUBKEY1_GRIP}{,.bak}
  echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
     ## produces INV_SGNR on stderr
  mv $GNUPGHOME/private-keys-v1.d/${SUBKEY1_GRIP}{,.bak}
  echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
     ## makes valid signature with subkey 0, no INV_SGNR  

This seems like the sensible logic to me -- as long as some
signing-capable subkey is available.  Does enigmail need something
different?

        --dkg

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] g10: Skip signing keys where no secret key is available.

Patrick Brunschwig-2
On 27.04.17 04:02, Daniel Kahn Gillmor wrote:

> On Wed 2017-04-26 08:22:31 +0200, Patrick Brunschwig wrote:
>> On 26.04.17 03:27, Daniel Kahn Gillmor wrote:
>>> On Mon 2017-02-06 09:57:59 +0100, Patrick Brunschwig wrote:
>>>> Would this patch still issue a "MISSING_KEY" line for --status-fd? If
>>>> no, you break existing logic (which for example Enigmail relies on).
>>>
>>> in what case does enigmail expect a "MISSING_KEY" line?  the scenario is
>>> that the user has a primary key A and two valid, non-expired,
>>> signing-capable subkeys, B and C.  C is the newer subkey, and the user
>>> has specified that they want to sign with A.  We'd like to go ahead and
>>> sign with B if it is available and C is missing.
>>>
>>> What should enigmail do in that case with a MISSING_KEY line?  shouldn't
>>> it just accept that a valid signature has been made?
>>>
>>> I've pushed the proposed fix to a new git branch dkg/T1967, and updated
>>> https://dev.gnupg.org/T1967 to note that branch.
>>
>> Sorry, I meant INV_SGNR. Enigmail looks for this when a message is
>> supposed to be signed but there is no suitable key.
>
> So you're asking whether INV_SGNR shows up in the case where there
> really is *no* signing subkey, right?  because if gpg finds a different
> signing subkey available and uses it successfully, you don't want it to
> appear.  right?
>
> I just did the following test in a fresh $GNUPGHOME (manually extracting
> $PRIMARY_FPR and $SUBKEY0_GRIP from the relevant places), with the patch
> under discussion applied here.  Note that i'm renaming
> private-keys-v1.d/*.key files to emulate removal of secret key material
> by any other means:
>
>
>   gpg --quick-gen-key 'testing <[hidden email]>' rsa cert
>   gpg --quick-add-key ${PRIMARY_FPR} rsa sign # signing-capable subkey 0
>   gpg --with-keygrip --list-keys
>   echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
>      ## makes valid signature with subkey 0, no INV_SGNR  
>   mv $GNUPGHOME/private-keys-v1.d/${SUBKEY0_GRIP}{,.bak}
>   echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
>      ## produces INV_SGNR on stderr
>   gpg --quick-add-key ${PRIMARY_FPR} rsa sign  # signing-capable subkey 1
>   echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
>      ## makes valid signature with subkey 1, no INV_SGNR  
>   mv $GNUPGHOME/private-keys-v1.d/${SUBKEY1_GRIP}{,.bak}
>   echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
>      ## produces INV_SGNR on stderr
>   mv $GNUPGHOME/private-keys-v1.d/${SUBKEY1_GRIP}{,.bak}
>   echo test | gpg -u 'testing <[hidden email]>' --status-fd=2 --clearsign
>      ## makes valid signature with subkey 0, no INV_SGNR  
>
> This seems like the sensible logic to me -- as long as some
> signing-capable subkey is available.  Does enigmail need something
> different?
Perfect - that's exactly what Enigmail would expect.

Thanks,
Patrick



_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (849 bytes) Download Attachment
Loading...