Re: [gmime-devel] [PATCH 6/5 :P] Avoid trying to replay the session key unless we have gpg >= 2.1.16

[Jeffrey Stedfast <fejj gnome org>]

Just committed these 6 patches, thanks!

I kept the secret_fd name, though, as I'm not a fan of the 
multipurpose_fd name.

2 out of the 3 uses are secret, anyway :)

meh!

I did add the extended comment documenting secret_fd, though. I can 
definitely see how that is useful for understanding the code.

Jeff

On 12/5/2016 2:49 AM, Daniel Kahn Gillmor wrote:
> ---
>   tests/test-pgpmime.c | 20 +++++++++++------
>   tests/testsuite.c    | 63 +++++++++++++++++++++++++++++++++++++++++++---------
>   tests/testsuite.h    |  1 +
>   3 files changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/tests/test-pgpmime.c b/tests/test-pgpmime.c
> index c671c9b..e3ca614 100644
> --- a/tests/test-pgpmime.c
> +++ b/tests/test-pgpmime.c
> @@ -443,7 +443,7 @@ int main (int argc, char *argv[])
>         char *key;
>         int i;
>         GMimeStream *stream = NULL, *cleartext = NULL;
> -       char *session_key;
> +       char *session_key = NULL;
>         
>         g_mime_init (0);
>         
> @@ -496,13 +496,16 @@ int main (int argc, char *argv[])
>         try {
>                 _construct_example (ctx, FALSE, &cleartext, &stream);
>                 session_key = test_multipart_encrypted (ctx, FALSE, cleartext, stream, NULL);
> -               g_free (test_multipart_encrypted (ctx, FALSE, cleartext, stream, session_key));
> +               if (testsuite_can_safely_override_session_key ())
> +                       g_free (test_multipart_encrypted (ctx, FALSE, cleartext, stream, session_key));
>                 testsuite_check_passed ();
>         } catch (ex) {
>                 testsuite_check_failed ("multipart/encrypted failed: %s", ex->message);
>         } finally;
> -       g_object_unref (cleartext);
> -       g_object_unref (stream);
> +       if (cleartext)
> +               g_object_unref (cleartext);
> +       if (stream)
> +               g_object_unref (stream);
>         cleartext = stream = NULL;
>         if (session_key) {
>                 memset (session_key, 0, strlen (session_key));
> @@ -514,13 +517,16 @@ int main (int argc, char *argv[])
>         try {
>                 _construct_example (ctx, TRUE, &cleartext, &stream);
>                 session_key = test_multipart_encrypted (ctx, TRUE, cleartext, stream, NULL);
> -               g_free (test_multipart_encrypted (ctx, TRUE, cleartext, stream, session_key));
> +               if (testsuite_can_safely_override_session_key ())
> +                       g_free (test_multipart_encrypted (ctx, TRUE, cleartext, stream, session_key));
>                 testsuite_check_passed ();
>         } catch (ex) {
>                 testsuite_check_failed ("multipart/encrypted+sign failed: %s", ex->message);
>         } finally;
> -       g_object_unref (cleartext);
> -       g_object_unref (stream);
> +       if (cleartext)
> +               g_object_unref (cleartext);
> +       if (stream)
> +               g_object_unref (stream);
>         cleartext = stream = NULL;
>         if (session_key) {
>                 memset (session_key, 0, strlen (session_key));
> diff --git a/tests/testsuite.c b/tests/testsuite.c
> index 68f9522..6d6e6aa 100644
> --- a/tests/testsuite.c
> +++ b/tests/testsuite.c
> @@ -350,39 +350,80 @@ g_throw (Exception *ex)
>         longjmp (env->env, 1);
>   }
>   
> -static int
> -is_gpg_modern (void)
> +static const char *
> +get_gpg_version (void)
>   {
> +       static int found = 0;
> +       static char version[1024];
>         const char vheader[] = "gpg (GnuPG) ";
>         char *vstring = NULL;
>         size_t vlen = 0;
>         FILE *vpipe;
>         int ret;
> +
> +       if (found)
> +               return version;
>         
>         if ((vpipe = popen ("gpg --version", "r")) == NULL)
> -               return 0;
> +               return NULL;
>         
>         if (getline (&vstring, &vlen, vpipe) == -1) {
>                 free (vstring);
>                 pclose (vpipe);
> -               return 0;
> +               return NULL;
>         }
>         
>         pclose (vpipe);
>         
>         if (strncmp (vstring, vheader, sizeof (vheader) - 1)) {
>                 free (vstring);
> -               return 0;
> +               return NULL;
>         }
> -       
> -       ret = (vstring[sizeof (vheader) - 1] > '2') ||
> -               (vstring[sizeof (vheader) - 1] == '2' &&
> -                vstring[sizeof (vheader)] == '.' &&
> -                vstring[sizeof (vheader) + 1] >= '1');
> +
> +       strncpy (version, vstring + sizeof (vheader) - 1, sizeof (version) - 1);
> +       version[sizeof (version) - 1] = '\0';
>         
>         free (vstring);
> +
> +       found = 1;
> +       return version;
> +}
> +
> +static int
> +is_gpg_modern (void)
> +{
> +       const char *ver = get_gpg_version ();
> +
> +       if (!ver)
> +               return 0;
> +       
> +       return (ver[0] > '2') ||
> +               (ver[0] == '2' &&
> +                ver[1] == '.' &&
> +                ver[2] >= '1');
> +}
> +
> +/* in versions of gpg before 2.1.16, the only mechanism to override
> +   the session key was --override-session-key, which leaks its
> +   argument to the process table.
> +
> +   in 2.1.16, gpg introduced --override-session-key-fd, which is what
> +   gmime uses to be safe.
> +*/
> +
> +int
> +testsuite_can_safely_override_session_key (void)
> +{
> +       const char *ver = get_gpg_version ();
> +
> +       if (!ver)
> +               return 0;
>         
> -       return ret;
> +       return (ver[0] > '2') ||
> +               ((ver[0] == '2' && ver[1] == '.' ) &&
> +                ((ver[2] > '1') ||
> +                 ((ver[2] == '1' && ver[3] == '.') &&
> +                  (atoi(ver+4) >= 16))));
>   }
>   
>   int
> diff --git a/tests/testsuite.h b/tests/testsuite.h
> index a5f9392..87d3126 100644
> --- a/tests/testsuite.h
> +++ b/tests/testsuite.h
> @@ -48,6 +48,7 @@ int testsuite_total_errors (void);
>   /* GnuPG test suite utility functions */
>   int testsuite_setup_gpghome (void);
>   int testsuite_destroy_gpghome (void);
> +int testsuite_can_safely_override_session_key (void);
>   
>   /*#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)
>   #define G_GNUC_NORETURN __attribute__((noreturn))