Bug#928963: gpg-agent READKEY emits an invalid S-expression when private key file has comment (on 64-bit big-endian platforms)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#928963: gpg-agent READKEY emits an invalid S-expression when private key file has comment (on 64-bit big-endian platforms)

Daniel Kahn Gillmor-3
Package: gpg-agent
Severity: serious
Version: 2.2.12-1
Control: found -1 2.2.13-1
Control: found -1 2.2.14-1
Control: found -1 2.2.15-1
Control: affects -1 src:monkeysphere
Control: tags -1 patch upstream
Control: forwarded -1 https://dev.gnupg.org/T4501

On sparc64, powerpc64, and s390x (debian's three 64-bit big-endian
platforms), gpg is unable to create a new OpenPGP certificate from some
secret keys that it already knows about.

In particular, "gpg --batch --generate" from a Key-Grip: line that
refers to a key file in private-keys-v1.d/ that contains a comment
sublist will fail with "Invalid S-expression" on those platforms.

This is due to a buggy invocation of gcry_sexp_build_array that is only
tickled when int is smaller than size_t and the platform is big-endian,
which causes the comment string to be set to zero length, which itself
is interpreted as an error of GPG_ERR_SEXP_ZERO_PREFIX.

However, this failure causes necessary functionality for
"monkeysphere-host import-key" as of monkeysphere version 0.43-3 to
break on these platforms, making monkeysphere FTBFS because the failure
is caught by its test suite.

The attached patch resolves the issue when i test it on
zelenka.debian.org (s390x), and should also work on the other two
platforms.

        --dkg


From e4a158faacd67e15e87183fb48e8bd0cc70f90a8 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <[hidden email]>
Date: Tue, 14 May 2019 00:05:42 -0400
Subject: [PATCH] agent: correct length for uri and comment on 64-bit
 big-endian platforms

* agent/findkey.c (agent_public_key_from_file): pass size_t as int to
gcry_sexp_build_array's %b.

--

This is only a problem on big-endian systems where size_t is not the
same size as an int.  It was causing failures on debian's s390x,
powerpc64, and sparc64 platforms.

There may well be other failures with %b on those platforms in the
codebase, and it probably needs an audit.

Once you have a key in private-keys-v1.d/$KEYGRIP.key with a comment
or a uri of reasonable length associated with it, this fix can be
tested with:

   gpg-agent --server <<<"READKEY $KEYGRIP"

On the failing platforms, the printed comment will be of length 0.

Gnupg-bug-id: 4501
Signed-off-by: Daniel Kahn Gillmor <[hidden email]>
---
 agent/findkey.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/agent/findkey.c b/agent/findkey.c
index 755a90be1..20c9dc56a 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -1366,6 +1366,7 @@ agent_public_key_from_file (ctrl_t ctrl,
   gcry_sexp_t uri_sexp, comment_sexp;
   const char *uri, *comment;
   size_t uri_length, comment_length;
+  int uri_intlen, comment_intlen;
   char *format, *p;
   void *args[2+7+2+2+1]; /* Size is 2 + max. # of elements + 2 for uri + 2
                             for comment + end-of-list.  */
@@ -1447,14 +1448,16 @@ agent_public_key_from_file (ctrl_t ctrl,
     {
       p = stpcpy (p, "(uri %b)");
       assert (argidx+1 < DIM (args));
-      args[argidx++] = (void *)&uri_length;
+      uri_intlen = (int)uri_length;
+      args[argidx++] = (void *)&uri_intlen;
       args[argidx++] = (void *)&uri;
     }
   if (comment)
     {
       p = stpcpy (p, "(comment %b)");
       assert (argidx+1 < DIM (args));
-      args[argidx++] = (void *)&comment_length;
+      comment_intlen = (int)comment_length;
+      args[argidx++] = (void *)&comment_intlen;
       args[argidx++] = (void*)&comment;
     }
   *p++ = ')';
--
2.20.1


signature.asc (233 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#928963: fixed in gnupg2 2.2.13-2

Paul Gevers-4
Hi Daniel,

On Tue, 14 May 2019 06:18:31 +0000 Daniel Kahn Gillmor
<[hidden email]> wrote:
>  gnupg2 (2.2.13-2) unstable; urgency=medium
>  .
>    * Correct gpg-wks-server manpage (Closes: #927431) Thanks, ju xor!
>    * Fix handling private keys with comments (Closes: #928963, #928964)
>    * clean up logcheck rules for gpg-agent (Closes: #918466)
>    * Update gpg-wks-client.1 (Closes: #918586)
>    * cherry-pick more patches from upstream STABLE-BRANCH-2-2

Is there any reason that we shouldn't want to unblock this for buster
(i.e. is there any reason why you didn't file an unblock bug request)?

Paul


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#928963: [pkg-gnupg-maint] Bug#928963: fixed in gnupg2 2.2.13-2

Daniel Kahn Gillmor-3
Hi Paul--

On Sat 2019-06-22 20:51:00 +0200, Paul Gevers wrote:

> On Tue, 14 May 2019 06:18:31 +0000 Daniel Kahn Gillmor
> <[hidden email]> wrote:
>>  gnupg2 (2.2.13-2) unstable; urgency=medium
>>  .
>>    * Correct gpg-wks-server manpage (Closes: #927431) Thanks, ju xor!
>>    * Fix handling private keys with comments (Closes: #928963, #928964)
>>    * clean up logcheck rules for gpg-agent (Closes: #918466)
>>    * Update gpg-wks-client.1 (Closes: #918586)
>>    * cherry-pick more patches from upstream STABLE-BRANCH-2-2
>
> Is there any reason that we shouldn't want to unblock this for buster
> (i.e. is there any reason why you didn't file an unblock bug request)?
Filing an unblock for gnupg2 version 2.2.13-2 for buster is on my stack
of things to do, but i'm quite far behind on other work.  I do think it
would be useful to have, and i welcome any help in filing such an
unblock request.

This change includes several upstream cleanup changes beyond the
2.2.12-1 that is in buster right now, in particular (from upstream's
NEWS):

  * gpg: Implement key lookup via keygrip (using the & prefix).

  * gpg: Allow generating Ed25519 key from existing key.

  * gpg: Emit an ERROR status line if no key was found with -k.

  * gpg: Stop early when trying to create a primary Elgamal key.  [#4329]

  * gpgsm: Print the card's key algorithms along with their keygrips
    in interactive key generation.

  * agent: Clear bogus pinentry cache in the error case.  [#4348]

  * scd: Support "acknowledge button" feature.

  * scd: Fix for USB INTERRUPT transfer.  [#4308]

  * wks: Do no use compression for the the encrypted challenge and
    response.


Since the gnupg2 source produces a udeb for gpgv, there are likely to be
additional hurdles to clearing the queue. :/

           --dkg

signature.asc (233 bytes) Download Attachment