Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 

On 12/2/2016 7:48 PM, Daniel Kahn Gillmor wrote:

On Fri 2016-12-02 18:52:04 -0500, Jeffrey Stedfast wrote:

On 12/2/2016 3:31 PM, Daniel Kahn Gillmor wrote:

(and hm, actually, maybe that means we should be zeroing the RAM before
releasing the stored session key; i can make that change if that sounds
good to you)

Yes, please :)

thanks for scrubbing the ram already :)


the other advantage for keeping this command is that it makes it clearer
in the API which cryptocontexts are capable of this kind of operation.
Would it be better to make this function part of the GMimeCryptoContext
API instead of the GMimeGpgContext, though?

If the plan is to also add it to the S/MIME context, then yes.

I have no idea whether it makes sense to add it to the S/MIME context --
it might, but i haven't tried.  i don't think S/MIME can do decryption
anyway yet, can it?


What API would you prefer for a "try-this-session-key" approach?  I see
two mechanisms:

(a) give the session key to the context for any number of subsequent
  decryptions until it is cleared or re-set:

  g_mime_gpg_context_set_session_key_for_decryption()
  g_mime_gpg_context_clear_session_key_for_decryption()

  This is a bit awkward, though: what should happen if the wrong
  session key is given; should it fall back to trying to use the
  available public keys?  if so, then we'd need to do some fancy
  footwork, because gpg simply fails to decrypt if you give it the
  wrong session key.


(b) have some variation on g_mime_multipart_encrypted_decrypt() or
  g_mime_crypto_context_decrypt(), which has an additional parameter
  of "session_key".

(b) makes the most sense.

Perhaps g_mime_crypto_context_decrypt_session?

and a g_mime_multipart_encrypted_decrypt_session() as well?

I can work on this patch if it's something that you'd like.

   --dkg



I am not in a rush to have it since at this moment, you are the only one 
using the session_key at all and you don't seem to need it in any sort 
of immediate timeframe.



I was more just curious.


Feel free to implement as time and/or desire permits.


Jeff

___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 18:52:04 -0500, Jeffrey Stedfast wrote:
> On 12/2/2016 3:31 PM, Daniel Kahn Gillmor wrote:
>> (and hm, actually, maybe that means we should be zeroing the RAM before
>> releasing the stored session key; i can make that change if that sounds
>> good to you)
>
> Yes, please :)

thanks for scrubbing the ram already :)

>> the other advantage for keeping this command is that it makes it clearer
>> in the API which cryptocontexts are capable of this kind of operation.
>> Would it be better to make this function part of the GMimeCryptoContext
>> API instead of the GMimeGpgContext, though?
>
> If the plan is to also add it to the S/MIME context, then yes.

I have no idea whether it makes sense to add it to the S/MIME context --
it might, but i haven't tried.  i don't think S/MIME can do decryption
anyway yet, can it?

>> What API would you prefer for a "try-this-session-key" approach?  I see
>> two mechanisms:
>>
>> (a) give the session key to the context for any number of subsequent
>>  decryptions until it is cleared or re-set:
>>
>>  g_mime_gpg_context_set_session_key_for_decryption()
>>  g_mime_gpg_context_clear_session_key_for_decryption()
>>
>>  This is a bit awkward, though: what should happen if the wrong
>>  session key is given; should it fall back to trying to use the
>>  available public keys?  if so, then we'd need to do some fancy
>>  footwork, because gpg simply fails to decrypt if you give it the
>>  wrong session key.
>>
>>
>> (b) have some variation on g_mime_multipart_encrypted_decrypt() or
>>  g_mime_crypto_context_decrypt(), which has an additional parameter
>>  of "session_key".
>
> (b) makes the most sense.
>
> Perhaps g_mime_crypto_context_decrypt_session?

and a g_mime_multipart_encrypted_decrypt_session() as well?

I can work on this patch if it's something that you'd like.

  --dkg


signature.asc
Description: PGP signature
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

[gmime-devel] [PATCH] Scrub the session key from RAM where possible

2016-12-02 Thread Daniel Kahn Gillmor 
We won't need the session key after this copy, so we should get rid of
it in case the slice gets reused.
---
 gmime/gmime-gpg-context.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index 74f8a4e..43059ff 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -1338,7 +1338,11 @@ gpg_ctx_parse_status (struct _GpgCtx *gpg, GError **err)
} else if (!strncmp (status, "BADMDC", 6)) {
/* nothing to do, this will only be sent after 
DECRYPTION_FAILED */
} else if (!strncmp (status, "SESSION_KEY", 11)) {
+   char *begin = status;
status = next_token (status, >session_key);
+   /* scrub the session key from RAM */
+   if (status > begin)
+   memset (begin, 'X', status - begin);
} else {
gpg_ctx_parse_signer_info (gpg, status);
}
-- 
2.10.2

___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH v2 2/2] Support extraction of session keys during decryption.

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 19:37:22 -0500, Daniel Kahn Gillmor wrote:
> On Fri 2016-12-02 19:21:25 -0500, Jeffrey Stedfast wrote:
>> Committed with the memset fixes.
>
> Thanks!
>
> i see one test you dropped during commit, just wanted to make sure this
> was intentionally left out (- is the committed copy, + is my suggestion):
>
> --- a/gmime/gmime-gpg-context.c
> +++ b/gmime/gmime-gpg-context.c
> @@ -1337,7 +1339,7 @@ gpg_ctx_parse_status (struct _GpgCtx *gpg, GError **err)
>   /* nothing to do... we'll grab the MDC used in 
> DECRYPTION_INFO */
>   } else if (!strncmp (status, "BADMDC", 6)) {
>   /* nothing to do, this will only be sent after 
> DECRYPTION_FAILED */
> - } else if (!strncmp (status, "SESSION_KEY", 11)) {
> + } else if (gpg->ctx->retrieve_session_key && !strncmp 
> (status, "SESSION_KEY", 11)) {
>   status = next_token (status, >session_key);
>   } else {
>   gpg_ctx_parse_signer_info (gpg, status);
>
>
>
> I don't much care one way or the other, but i'd be happy to understand
> your reasoning to try to make future contributions fit better.

On second thought, i do care, and i prefer your method, because it gives
us a chance to scrub the RAM in this stream too.  I'll send another
patch shortly.

  --dkg
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH v2 2/2] Support extraction of session keys during decryption.

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 19:21:25 -0500, Jeffrey Stedfast wrote:
> Committed with the memset fixes.

Thanks!

i see one test you dropped during commit, just wanted to make sure this
was intentionally left out (- is the committed copy, + is my suggestion):

--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -1337,7 +1339,7 @@ gpg_ctx_parse_status (struct _GpgCtx *gpg, GError **err)
/* nothing to do... we'll grab the MDC used in 
DECRYPTION_INFO */
} else if (!strncmp (status, "BADMDC", 6)) {
/* nothing to do, this will only be sent after 
DECRYPTION_FAILED */
-   } else if (!strncmp (status, "SESSION_KEY", 11)) {
+   } else if (gpg->ctx->retrieve_session_key && !strncmp 
(status, "SESSION_KEY", 11)) {
status = next_token (status, >session_key);
} else {
gpg_ctx_parse_signer_info (gpg, status);



I don't much care one way or the other, but i'd be happy to understand
your reasoning to try to make future contributions fit better.

thanks for the rapid turnaround, Jeffrey!

   --dkg


signature.asc
Description: PGP signature
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH v2 2/2] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 

Committed with the memset fixes.

Jeff

On 12/2/2016 6:58 PM, Jeffrey Stedfast wrote:
Did you send the wrong patch file? This doesn't seem to bzero() the 
session_key before freeing.


Jeff

On 12/2/2016 1:37 PM, Daniel Kahn Gillmor wrote:

Some message stores can be optimized by caching session keys of
encrypted messages, rather than incurring asymmetric crypto operations
on each subsequent decryption.

This patch allows gmime to perform extraction of session keys during
message decryption to support that use case.

See the discussion starting here for more detail on the API/design
choices:
https://mail.gnome.org/archives/gmime-devel-list/2016-July/msg5.html
---
  gmime/gmime-crypto-context.c | 41 
  gmime/gmime-crypto-context.h |  4 
  gmime/gmime-gpg-context.c| 50 
+++-

  gmime/gmime-gpg-context.h|  4 
  tests/test-pgp.c |  5 +
  tests/test-pgpmime.c |  9 +++-
  6 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gmime/gmime-crypto-context.c b/gmime/gmime-crypto-context.c
index 5269440..90fa390 100644
--- a/gmime/gmime-crypto-context.c
+++ b/gmime/gmime-crypto-context.c
@@ -573,6 +573,7 @@ g_mime_decrypt_result_init (GMimeDecryptResult 
*result, GMimeDecryptResultClass

  result->mdc = GMIME_DIGEST_ALGO_DEFAULT;
  result->recipients = NULL;
  result->signatures = NULL;
+result->session_key = NULL;
  }
static void
@@ -586,6 +587,8 @@ g_mime_decrypt_result_finalize (GObject *object)
  if (result->signatures)
  g_object_unref (result->signatures);

+g_free (result->session_key);
+
  G_OBJECT_CLASS (result_parent_class)->finalize (object);
  }
  @@ -755,3 +758,41 @@ g_mime_decryption_result_get_mdc 
(GMimeDecryptResult *result)


  return result->mdc;
  }
+
+
+/**
+ * g_mime_decrypt_result_set_session_key:
+ * @result: a #GMimeDecryptResult
+ * @session_key: a pointer to a null-terminated string representing 
the session key

+ *
+ * Set the session_key to be returned by this decryption result.
+ **/
+void
+g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, 
const char *session_key)

+{
+g_return_if_fail (GMIME_IS_DECRYPT_RESULT (result));
+
+g_free (result->session_key);
+result->session_key = g_strdup(session_key);
+}
+
+
+/**
+ * g_mime_decrypt_result_get_session_key:
+ * @result: a #GMimeDecryptResult
+ *
+ * Get the session_key used for this decryption, if the underlying
+ * crypto context is capable of and (configured to) retrieve session
+ * keys during decryption.  See, for example,
+ * g_mime_gpg_context_set_retrieve_session_key().
+ *
+ * Returns: the session_key digest algorithm used, or NULL if no
+ * session key was requested or found.
+ **/
+const char*
+g_mime_decryption_result_get_session_key (GMimeDecryptResult *result)
+{
+g_return_val_if_fail (GMIME_IS_DECRYPT_RESULT (result), 
GMIME_DIGEST_ALGO_DEFAULT);

+
+return result->session_key;
+}
diff --git a/gmime/gmime-crypto-context.h b/gmime/gmime-crypto-context.h
index cd38760..72573ed 100644
--- a/gmime/gmime-crypto-context.h
+++ b/gmime/gmime-crypto-context.h
@@ -206,6 +206,7 @@ struct _GMimeDecryptResult {
  GMimeSignatureList *signatures;
  GMimeCipherAlgo cipher;
  GMimeDigestAlgo mdc;
+char *session_key;
  };
struct _GMimeDecryptResultClass {
@@ -229,6 +230,9 @@ GMimeCipherAlgo g_mime_decrypt_result_get_cipher 
(GMimeDecryptResult *result);
  void g_mime_decrypt_result_set_mdc (GMimeDecryptResult *result, 
GMimeDigestAlgo mdc);
  GMimeDigestAlgo g_mime_decrypt_result_get_mdc (GMimeDecryptResult 
*result);
  +void g_mime_decrypt_result_set_session_key (GMimeDecryptResult 
*result, const char *session_key);
+const char *g_mime_decrypt_result_get_session_key 
(GMimeDecryptResult *result);

+
  G_END_DECLS
#endif /* __GMIME_CRYPTO_CONTEXT_H__ */
diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index afc1d52..ac12c41 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -172,6 +172,7 @@ g_mime_gpg_context_init (GMimeGpgContext *ctx, 
GMimeGpgContextClass *klass)

  ctx->always_trust = FALSE;
  ctx->use_agent = FALSE;
  ctx->path = NULL;
+ctx->retrieve_session_key = FALSE;
  }
static void
@@ -311,6 +312,7 @@ struct _GpgCtx {
  GMimeStream *diagnostics;

  GMimeCertificateList *encrypted_to;  /* full list of 
encrypted-to recipients */

+char *session_key;
  GMimeSignatureList *signatures;
  GMimeSignature *signature;

@@ -375,6 +377,7 @@ gpg_ctx_new (GMimeGpgContext *ctx)
  gpg->need_id = NULL;

  gpg->encrypted_to = NULL;
+gpg->session_key = NULL;
  gpg->signatures = NULL;
  gpg->signature = NULL;

@@ -555,6 +558,8 @@ gpg_ctx_free (struct _GpgCtx *gpg)
  if (gpg->encrypted_to)
  g_object_unref (gpg->encrypted_to);

+g_free (gpg->session_key);
+
  if (gpg->signatures)

Re: [gmime-devel] [PATCH v2 2/2] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 
Did you send the wrong patch file? This doesn't seem to bzero() the 
session_key before freeing.


Jeff

On 12/2/2016 1:37 PM, Daniel Kahn Gillmor wrote:

Some message stores can be optimized by caching session keys of
encrypted messages, rather than incurring asymmetric crypto operations
on each subsequent decryption.

This patch allows gmime to perform extraction of session keys during
message decryption to support that use case.

See the discussion starting here for more detail on the API/design
choices:
https://mail.gnome.org/archives/gmime-devel-list/2016-July/msg5.html
---
  gmime/gmime-crypto-context.c | 41 
  gmime/gmime-crypto-context.h |  4 
  gmime/gmime-gpg-context.c| 50 +++-
  gmime/gmime-gpg-context.h|  4 
  tests/test-pgp.c |  5 +
  tests/test-pgpmime.c |  9 +++-
  6 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gmime/gmime-crypto-context.c b/gmime/gmime-crypto-context.c
index 5269440..90fa390 100644
--- a/gmime/gmime-crypto-context.c
+++ b/gmime/gmime-crypto-context.c
@@ -573,6 +573,7 @@ g_mime_decrypt_result_init (GMimeDecryptResult *result, 
GMimeDecryptResultClass
result->mdc = GMIME_DIGEST_ALGO_DEFAULT;
result->recipients = NULL;
result->signatures = NULL;
+   result->session_key = NULL;
  }
  
  static void

@@ -586,6 +587,8 @@ g_mime_decrypt_result_finalize (GObject *object)
if (result->signatures)
g_object_unref (result->signatures);

+   g_free (result->session_key);
+   
G_OBJECT_CLASS (result_parent_class)->finalize (object);
  }
  
@@ -755,3 +758,41 @@ g_mime_decryption_result_get_mdc (GMimeDecryptResult *result)


return result->mdc;
  }
+
+
+/**
+ * g_mime_decrypt_result_set_session_key:
+ * @result: a #GMimeDecryptResult
+ * @session_key: a pointer to a null-terminated string representing the 
session key
+ *
+ * Set the session_key to be returned by this decryption result.
+ **/
+void
+g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const char 
*session_key)
+{
+   g_return_if_fail (GMIME_IS_DECRYPT_RESULT (result));
+
+   g_free (result->session_key);
+   result->session_key = g_strdup(session_key);
+}
+
+
+/**
+ * g_mime_decrypt_result_get_session_key:
+ * @result: a #GMimeDecryptResult
+ *
+ * Get the session_key used for this decryption, if the underlying
+ * crypto context is capable of and (configured to) retrieve session
+ * keys during decryption.  See, for example,
+ * g_mime_gpg_context_set_retrieve_session_key().
+ *
+ * Returns: the session_key digest algorithm used, or NULL if no
+ * session key was requested or found.
+ **/
+const char*
+g_mime_decryption_result_get_session_key (GMimeDecryptResult *result)
+{
+   g_return_val_if_fail (GMIME_IS_DECRYPT_RESULT (result), 
GMIME_DIGEST_ALGO_DEFAULT);
+   
+   return result->session_key;
+}
diff --git a/gmime/gmime-crypto-context.h b/gmime/gmime-crypto-context.h
index cd38760..72573ed 100644
--- a/gmime/gmime-crypto-context.h
+++ b/gmime/gmime-crypto-context.h
@@ -206,6 +206,7 @@ struct _GMimeDecryptResult {
GMimeSignatureList *signatures;
GMimeCipherAlgo cipher;
GMimeDigestAlgo mdc;
+   char *session_key;
  };
  
  struct _GMimeDecryptResultClass {

@@ -229,6 +230,9 @@ GMimeCipherAlgo g_mime_decrypt_result_get_cipher 
(GMimeDecryptResult *result);
  void g_mime_decrypt_result_set_mdc (GMimeDecryptResult *result, 
GMimeDigestAlgo mdc);
  GMimeDigestAlgo g_mime_decrypt_result_get_mdc (GMimeDecryptResult *result);
  
+void g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const char *session_key);

+const char *g_mime_decrypt_result_get_session_key (GMimeDecryptResult *result);
+
  G_END_DECLS
  
  #endif /* __GMIME_CRYPTO_CONTEXT_H__ */

diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index afc1d52..ac12c41 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -172,6 +172,7 @@ g_mime_gpg_context_init (GMimeGpgContext *ctx, 
GMimeGpgContextClass *klass)
ctx->always_trust = FALSE;
ctx->use_agent = FALSE;
ctx->path = NULL;
+   ctx->retrieve_session_key = FALSE;
  }
  
  static void

@@ -311,6 +312,7 @@ struct _GpgCtx {
GMimeStream *diagnostics;

GMimeCertificateList *encrypted_to;  /* full list of encrypted-to 
recipients */
+   char *session_key;
GMimeSignatureList *signatures;
GMimeSignature *signature;

@@ -375,6 +377,7 @@ gpg_ctx_new (GMimeGpgContext *ctx)
gpg->need_id = NULL;

gpg->encrypted_to = NULL;
+   gpg->session_key = NULL;
gpg->signatures = NULL;
gpg->signature = NULL;

@@ -555,6 +558,8 @@ gpg_ctx_free (struct _GpgCtx *gpg)
if (gpg->encrypted_to)
g_object_unref (gpg->encrypted_to);

+

Re: [gmime-devel] [PATCH 5/6] Use pinentry-mode loopback in test suite when using "modern" GnuPG

2016-12-02 Thread Jeffrey Stedfast 

On 12/2/2016 11:21 AM, Daniel Kahn Gillmor wrote:

On Fri 2016-12-02 11:05:59 -0500, Daniel Kahn Gillmor wrote:

+   if (strncmp (vstring, vheader, sizeof (vheader) - 1))
+   return 0;

Same. In fact, I'd probably recommend pclose()ing vpipe as soon as you
finish reading the output of gpg --version (no reason to keep it open
after reading it).


+   ret = (vstring[sizeof (vheader) - 1] > '2') ||
+   (vstring[sizeof (vheader) - 1] == '2' &&
+vstring[sizeof (vheader)] == '.' &&
+vstring[sizeof (vheader) + 1] >= '1');

This has the potential of reading past the end of the buffer.

ah, right.  maybe we should first assert that vlen >= sizeof (vheader) ?

hm, i take it back -- how can this read past the end of the buffer if
the strncmp test above already succeeded?  the first thing it reads is
at sizeof (vheader - 1), and we already know that the first vheader-1
octets match.  So in the event that the buffer is too short,
vstring[sizeof (vheader) - 1] will be NULL, which is < '2', so "ret"
will be set to 0 and will never test vstring[sizeof (vheader)] or later.

  --dkg



Hmmm, yea, you're right. I blame it on my lack of coffee before replying :)


Jeff


___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 

On 12/2/2016 3:31 PM, Daniel Kahn Gillmor wrote:

On Fri 2016-12-02 10:37:38 -0500, Jeffrey Stedfast wrote:

I just read the man page for gpg's --show-session-key and
--override-session-key.

It looks like the gpg authors advise against using it unless you need to
(while providing what they feel is a valuable use-case).

So I guess that answers my question.

I think the way to think about this is that the session key is a bit of
secret key material, capable on its own of decrypting the secret
message.  If a user deletes the resultant cleartext MIME part but
somehow fails to delete the DecryptResult, then someone who gets access
to the DecryptResult can decrypt the ciphertext again.  That seems like
"opt in" behavior to me.


Right.



(and hm, actually, maybe that means we should be zeroing the RAM before
releasing the stored session key; i can make that change if that sounds
good to you)


Yes, please :)



the other advantage for keeping this command is that it makes it clearer
in the API which cryptocontexts are capable of this kind of operation.
Would it be better to make this function part of the GMimeCryptoContext
API instead of the GMimeGpgContext, though?


If the plan is to also add it to the S/MIME context, then yes.




However, I have another question which is: Do we want to have an API to
allow the use of a session-key when decrypting?

yes, ultimately, we'll want something like that, but it's something
that'd belong in a follow-on patch.  just extracting the session key
data is the first step.

What API would you prefer for a "try-this-session-key" approach?  I see
two mechanisms:

(a) give the session key to the context for any number of subsequent
 decryptions until it is cleared or re-set:

 g_mime_gpg_context_set_session_key_for_decryption()
 g_mime_gpg_context_clear_session_key_for_decryption()

 This is a bit awkward, though: what should happen if the wrong
 session key is given; should it fall back to trying to use the
 available public keys?  if so, then we'd need to do some fancy
 footwork, because gpg simply fails to decrypt if you give it the
 wrong session key.


(b) have some variation on g_mime_multipart_encrypted_decrypt() or
 g_mime_crypto_context_decrypt(), which has an additional parameter
 of "session_key".


(b) makes the most sense.

Perhaps g_mime_crypto_context_decrypt_session?

I really wish C allowed method overloading :(

Jeff
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 10:37:38 -0500, Jeffrey Stedfast wrote:
> I just read the man page for gpg's --show-session-key and 
> --override-session-key.
>
> It looks like the gpg authors advise against using it unless you need to 
> (while providing what they feel is a valuable use-case).
>
> So I guess that answers my question.

I think the way to think about this is that the session key is a bit of
secret key material, capable on its own of decrypting the secret
message.  If a user deletes the resultant cleartext MIME part but
somehow fails to delete the DecryptResult, then someone who gets access
to the DecryptResult can decrypt the ciphertext again.  That seems like
"opt in" behavior to me.

(and hm, actually, maybe that means we should be zeroing the RAM before
releasing the stored session key; i can make that change if that sounds
good to you)

the other advantage for keeping this command is that it makes it clearer
in the API which cryptocontexts are capable of this kind of operation.
Would it be better to make this function part of the GMimeCryptoContext
API instead of the GMimeGpgContext, though?

> However, I have another question which is: Do we want to have an API to 
> allow the use of a session-key when decrypting?

yes, ultimately, we'll want something like that, but it's something
that'd belong in a follow-on patch.  just extracting the session key
data is the first step.

What API would you prefer for a "try-this-session-key" approach?  I see
two mechanisms:

(a) give the session key to the context for any number of subsequent
decryptions until it is cleared or re-set:

g_mime_gpg_context_set_session_key_for_decryption()
g_mime_gpg_context_clear_session_key_for_decryption()

This is a bit awkward, though: what should happen if the wrong
session key is given; should it fall back to trying to use the
available public keys?  if so, then we'd need to do some fancy
footwork, because gpg simply fails to decrypt if you give it the
wrong session key.


(b) have some variation on g_mime_multipart_encrypted_decrypt() or
g_mime_crypto_context_decrypt(), which has an additional parameter
of "session_key".

Any other suggestions?

--dkg
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

[gmime-devel] [PATCH v2 1/2] Use pinentry-mode loopback in test suite when using "modern" GnuPG

2016-12-02 Thread Daniel Kahn Gillmor 
The "Modern" GnuPG suite (2.1.x or higher) defaults to relying on the
gpg-agent for secret key material access, which can prompt the user
for a passphrase.  The test suite uses callbacks to supply the
passphrase, so in these modern versions it should use "pinentry-mode
loopback".

Many users of GMime are likely to avoid using the passphrase callback
and instead to rely on permission from the cryptographic agent
directly.  We do not currently test these scenarios, though we could
do so with a fake pinentry.  If we do that, then those scenarios
should *not* use the loopback pinentry-mode, and we'd need to adjust
this setup.

In the longer term, the right way to resolve all of this would be to
use gpgme directly, instead of having our own wrapper that invokes gpg
manually.
---
 tests/testsuite.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/tests/testsuite.c b/tests/testsuite.c
index fa4fd9c..559d7dc 100644
--- a/tests/testsuite.c
+++ b/tests/testsuite.c
@@ -27,6 +27,10 @@
 #ifdef ENABLE_THREADS
 #include 
 #endif
+#include 
+#include 
+#include 
+#include 
 
 #include "testsuite.h"
 
@@ -346,9 +350,40 @@ g_throw (Exception *ex)
longjmp (env->env, 1);
 }
 
+static int
+is_gpg_modern()
+{
+   const char vheader[] = "gpg (GnuPG) ";
+   FILE *vpipe;
+   char *vstring;
+   size_t vlen;
+   int ret;
+
+   if ((vpipe = popen ("gpg --version", "r")) == NULL)
+   return 0;
+   vlen = 0;
+   vstring = NULL;
+   if (getline (, , vpipe) == -1) {
+   pclose (vpipe);
+   return 0;
+   }
+   pclose (vpipe);
+   if (strncmp (vstring, vheader, sizeof (vheader) - 1))
+   return 0;
+   ret = (vstring[sizeof (vheader) - 1] > '2') ||
+   (vstring[sizeof (vheader) - 1] == '2' &&
+vstring[sizeof (vheader)] == '.' &&
+vstring[sizeof (vheader) + 1] >= '1');
+   free (vstring);
+   return ret;
+}
+
 int
 testsuite_setup_gpghome (void)
 {
+   FILE *gpgconf;
+   const char directive[] = "pinentry-mode loopback\n";
+   
/* reset .gnupg config directory */
if (system ("/bin/rm -rf ./tmp") != 0)
return EXIT_FAILURE;
@@ -365,6 +400,18 @@ testsuite_setup_gpghome (void)
if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
return EXIT_FAILURE;

+   if (is_gpg_modern()) {
+   if ((gpgconf = fopen ("./tmp/.gnupg/gpg.conf", "w")) == NULL)
+   return EXIT_FAILURE;
+   if (fwrite (directive, sizeof(directive) - 1, 1, gpgconf) != 1) 
{
+   fclose (gpgconf);
+   return EXIT_FAILURE;
+   }
+   if (fclose (gpgconf))
+   return EXIT_FAILURE;
+   }
+   
+   
return EXIT_SUCCESS;
 }
 
-- 
2.10.2

___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

[gmime-devel] [PATCH v2 2/2] Support extraction of session keys during decryption.

2016-12-02 Thread Daniel Kahn Gillmor 
Some message stores can be optimized by caching session keys of
encrypted messages, rather than incurring asymmetric crypto operations
on each subsequent decryption.

This patch allows gmime to perform extraction of session keys during
message decryption to support that use case.

See the discussion starting here for more detail on the API/design
choices:
https://mail.gnome.org/archives/gmime-devel-list/2016-July/msg5.html
---
 gmime/gmime-crypto-context.c | 41 
 gmime/gmime-crypto-context.h |  4 
 gmime/gmime-gpg-context.c| 50 +++-
 gmime/gmime-gpg-context.h|  4 
 tests/test-pgp.c |  5 +
 tests/test-pgpmime.c |  9 +++-
 6 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gmime/gmime-crypto-context.c b/gmime/gmime-crypto-context.c
index 5269440..90fa390 100644
--- a/gmime/gmime-crypto-context.c
+++ b/gmime/gmime-crypto-context.c
@@ -573,6 +573,7 @@ g_mime_decrypt_result_init (GMimeDecryptResult *result, 
GMimeDecryptResultClass
result->mdc = GMIME_DIGEST_ALGO_DEFAULT;
result->recipients = NULL;
result->signatures = NULL;
+   result->session_key = NULL;
 }
 
 static void
@@ -586,6 +587,8 @@ g_mime_decrypt_result_finalize (GObject *object)
if (result->signatures)
g_object_unref (result->signatures);

+   g_free (result->session_key);
+   
G_OBJECT_CLASS (result_parent_class)->finalize (object);
 }
 
@@ -755,3 +758,41 @@ g_mime_decryption_result_get_mdc (GMimeDecryptResult 
*result)

return result->mdc;
 }
+
+
+/**
+ * g_mime_decrypt_result_set_session_key:
+ * @result: a #GMimeDecryptResult
+ * @session_key: a pointer to a null-terminated string representing the 
session key
+ *
+ * Set the session_key to be returned by this decryption result.
+ **/
+void
+g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const char 
*session_key)
+{
+   g_return_if_fail (GMIME_IS_DECRYPT_RESULT (result));
+
+   g_free (result->session_key);
+   result->session_key = g_strdup(session_key);
+}
+
+
+/**
+ * g_mime_decrypt_result_get_session_key:
+ * @result: a #GMimeDecryptResult
+ *
+ * Get the session_key used for this decryption, if the underlying
+ * crypto context is capable of and (configured to) retrieve session
+ * keys during decryption.  See, for example,
+ * g_mime_gpg_context_set_retrieve_session_key().
+ *
+ * Returns: the session_key digest algorithm used, or NULL if no
+ * session key was requested or found.
+ **/
+const char*
+g_mime_decryption_result_get_session_key (GMimeDecryptResult *result)
+{
+   g_return_val_if_fail (GMIME_IS_DECRYPT_RESULT (result), 
GMIME_DIGEST_ALGO_DEFAULT);
+   
+   return result->session_key;
+}
diff --git a/gmime/gmime-crypto-context.h b/gmime/gmime-crypto-context.h
index cd38760..72573ed 100644
--- a/gmime/gmime-crypto-context.h
+++ b/gmime/gmime-crypto-context.h
@@ -206,6 +206,7 @@ struct _GMimeDecryptResult {
GMimeSignatureList *signatures;
GMimeCipherAlgo cipher;
GMimeDigestAlgo mdc;
+   char *session_key;
 };
 
 struct _GMimeDecryptResultClass {
@@ -229,6 +230,9 @@ GMimeCipherAlgo g_mime_decrypt_result_get_cipher 
(GMimeDecryptResult *result);
 void g_mime_decrypt_result_set_mdc (GMimeDecryptResult *result, 
GMimeDigestAlgo mdc);
 GMimeDigestAlgo g_mime_decrypt_result_get_mdc (GMimeDecryptResult *result);
 
+void g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const 
char *session_key);
+const char *g_mime_decrypt_result_get_session_key (GMimeDecryptResult *result);
+
 G_END_DECLS
 
 #endif /* __GMIME_CRYPTO_CONTEXT_H__ */
diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index afc1d52..ac12c41 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -172,6 +172,7 @@ g_mime_gpg_context_init (GMimeGpgContext *ctx, 
GMimeGpgContextClass *klass)
ctx->always_trust = FALSE;
ctx->use_agent = FALSE;
ctx->path = NULL;
+   ctx->retrieve_session_key = FALSE;
 }
 
 static void
@@ -311,6 +312,7 @@ struct _GpgCtx {
GMimeStream *diagnostics;

GMimeCertificateList *encrypted_to;  /* full list of encrypted-to 
recipients */
+   char *session_key;
GMimeSignatureList *signatures;
GMimeSignature *signature;

@@ -375,6 +377,7 @@ gpg_ctx_new (GMimeGpgContext *ctx)
gpg->need_id = NULL;

gpg->encrypted_to = NULL;
+   gpg->session_key = NULL;
gpg->signatures = NULL;
gpg->signature = NULL;

@@ -555,6 +558,8 @@ gpg_ctx_free (struct _GpgCtx *gpg)
if (gpg->encrypted_to)
g_object_unref (gpg->encrypted_to);

+   g_free (gpg->session_key);
+   
if (gpg->signatures)
g_object_unref (gpg->signatures);

@@ -691,6 +696,9 @@ gpg_ctx_get_argv (struct _GpgCtx *gpg, int

Re: [gmime-devel] [PATCH 5/6] Use pinentry-mode loopback in test suite when using "modern" GnuPG

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 11:05:59 -0500, Daniel Kahn Gillmor wrote:
>>> +   if (strncmp (vstring, vheader, sizeof (vheader) - 1))
>>> +   return 0;
>>
>> Same. In fact, I'd probably recommend pclose()ing vpipe as soon as you 
>> finish reading the output of gpg --version (no reason to keep it open 
>> after reading it).
>>
>>> +   ret = (vstring[sizeof (vheader) - 1] > '2') ||
>>> +   (vstring[sizeof (vheader) - 1] == '2' &&
>>> +vstring[sizeof (vheader)] == '.' &&
>>> +vstring[sizeof (vheader) + 1] >= '1');
>>
>> This has the potential of reading past the end of the buffer.
>
> ah, right.  maybe we should first assert that vlen >= sizeof (vheader) ?

hm, i take it back -- how can this read past the end of the buffer if
the strncmp test above already succeeded?  the first thing it reads is
at sizeof (vheader - 1), and we already know that the first vheader-1
octets match.  So in the event that the buffer is too short,
vstring[sizeof (vheader) - 1] will be NULL, which is < '2', so "ret"
will be set to 0 and will never test vstring[sizeof (vheader)] or later.

 --dkg
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 5/6] Use pinentry-mode loopback in test suite when using "modern" GnuPG

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 10:11:18 -0500, Jeffrey Stedfast wrote:
> I think my resistance to moving to gpgme has worn out, so I'm on board 
> with that.

cool!  I think that's a patch for another day, though :)

>> ---
>>   tests/testsuite.c | 43 +++
>>   1 file changed, 43 insertions(+)
>>
>> diff --git a/tests/testsuite.c b/tests/testsuite.c
>> index 9422e93..84079d1 100644
>> --- a/tests/testsuite.c
>> +++ b/tests/testsuite.c
>> @@ -27,6 +27,10 @@
>>   #ifdef ENABLE_THREADS
>>   #include 
>>   #endif
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>>   
>>   #include "testsuite.h"
>>   
>> @@ -346,9 +350,38 @@ g_throw (Exception *ex)
>>  longjmp (env->env, 1);
>>   }
>>   
>> +static int
>> +is_gpg_modern()
>> +{
>> +const char vheader[] = "gpg (GnuPG) ";
>> +FILE *vpipe;
>> +char *vstring;
>> +size_t vlen;
>> +int ret;
>> +
>> +if ((vpipe = popen ("gpg --version", "r")) == NULL)
>> +return 0;
>> +vlen = 0;
>> +vstring = NULL;
>> +if (getline (, , vpipe) == -1)
>> +return 0;
>
> Shouldn't vpipe be closed in this error condition?

yep, i think you're right.

>> +if (strncmp (vstring, vheader, sizeof (vheader) - 1))
>> +return 0;
>
> Same. In fact, I'd probably recommend pclose()ing vpipe as soon as you 
> finish reading the output of gpg --version (no reason to keep it open 
> after reading it).
>
>> +ret = (vstring[sizeof (vheader) - 1] > '2') ||
>> +(vstring[sizeof (vheader) - 1] == '2' &&
>> + vstring[sizeof (vheader)] == '.' &&
>> + vstring[sizeof (vheader) + 1] >= '1');
>
> This has the potential of reading past the end of the buffer.

ah, right.  maybe we should first assert that vlen >= sizeof (vheader) ?

>> @@ -362,6 +395,16 @@ testsuite_setup_gpghome ()
>>   
>>  if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
>>  return EXIT_FAILURE;
>> +
>> +if (is_gpg_modern()) {
>> +if ((gpgconf = open ("./tmp/.gnupg/gpg.conf", O_WRONLY | 
>> O_CREAT | O_TRUNC, 0400)) == -1)
>> +return EXIT_FAILURE;
>> +if (write (gpgconf, directive, sizeof(directive) - 1) != 
>> sizeof(directive) - 1)
>> +return EXIT_FAILURE;
>> +if (close (gpgconf) != 0)
>> +return EXIT_FAILURE;
>> +}
>
> This might be cleaner if you used GMimeStreamFs or FILE* since the 
> correct thing to do is to loop the write() call until all data is 
> written and/or until -1 is returned where errno is not EINTR.

fine, i'm happy to make those changes.  a new patch is forthcoming :)

  --dkg


signature.asc
Description: PGP signature
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 2/6] Default to using "gpg"

2016-12-02 Thread Daniel Kahn Gillmor 
On Fri 2016-12-02 09:19:29 -0500, Jeffrey Stedfast wrote:
> I've commited this patch with a slight modification...

thanks!  a bit more commentary interleaved below:

>> diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
>> index 0a05ed2..2ca280a 100644
>> --- a/gmime/gmime-gpg-context.c
>> +++ b/gmime/gmime-gpg-context.c
>> @@ -763,7 +763,7 @@ gpg_ctx_op_start (struct _GpgCtx *gpg)
>>  }
>>  
>>  /* run gpg */
>> -execvp (gpg->ctx->path, argv);
>> +execvp (gpg->ctx->path ? gpg->ctx->path : "gpg", argv);
>
> If g_mime_gpg_context_new () always sets path, then we don't need to do 
> this messy hack.

In other software projects, i've found it's useful to distinguish
between the "default" case and the case where something has been
explicitly set by the programmer.  For example, the config settings in
firefox's about:config for boolean values actually end up being a
tri-state: Default, User-set:True, and User-set:False.  This distinction
allows an update to firefox to change the defaults while not disturbing
people who've explicitly stated a commitment to one setting, even if it
happens to be the current default.

I admit that i don't have a specific case where that's useful in gmime
here (certainly we're not talking about the firefox code upgrade path
for an object that lives in memory), but preserving ctx->path == NULL
for the "default" case would provide a way for any code that wants to
find out whether this is "default" or not to know.

at any rate, i'm happy with either approach, just wanted to explain why
i don't think the above is actually a "messy hack" any more than the
other approach :)

Regards,

--dkg


signature.asc
Description: PGP signature
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 
I just read the man page for gpg's --show-session-key and 
--override-session-key.


It looks like the gpg authors advise against using it unless you need to 
(while providing what they feel is a valuable use-case).


So I guess that answers my question.

However, I have another question which is: Do we want to have an API to 
allow the use of a session-key when decrypting?


Jeff

On 12/2/2016 10:20 AM, Jeffrey Stedfast wrote:
Overall this patch looks good to me, but I am wondering if there is a 
reason to have the retrieve_session_key API?


I guess what I am asking is why would we *not* want to default to 
*always* retrieving the session key in GMime?


Does it pose a security risk?

I think that if it's not a risk, we might as well just always request 
it to simplify things.


Do you agree?

Jeff

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

Some message stores can be optimized by caching session keys of
encrypted messages, rather than incurring asymmetric crypto operations
on each subsequent decryption.

This patch allows gmime to perform extraction of session keys during
message decryption to support that use case.

See the discussion starting here for more detail on the API/design
choices:
https://mail.gnome.org/archives/gmime-devel-list/2016-July/msg5.html
---
  gmime/gmime-crypto-context.c | 41 
  gmime/gmime-crypto-context.h |  4 
  gmime/gmime-gpg-context.c| 50 
+++-

  gmime/gmime-gpg-context.h|  4 
  tests/test-pgp.c |  5 +
  tests/test-pgpmime.c |  9 +++-
  6 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gmime/gmime-crypto-context.c b/gmime/gmime-crypto-context.c
index 5269440..90fa390 100644
--- a/gmime/gmime-crypto-context.c
+++ b/gmime/gmime-crypto-context.c
@@ -573,6 +573,7 @@ g_mime_decrypt_result_init (GMimeDecryptResult 
*result, GMimeDecryptResultClass

  result->mdc = GMIME_DIGEST_ALGO_DEFAULT;
  result->recipients = NULL;
  result->signatures = NULL;
+result->session_key = NULL;
  }
static void
@@ -586,6 +587,8 @@ g_mime_decrypt_result_finalize (GObject *object)
  if (result->signatures)
  g_object_unref (result->signatures);

+g_free (result->session_key);
+
  G_OBJECT_CLASS (result_parent_class)->finalize (object);
  }
  @@ -755,3 +758,41 @@ g_mime_decryption_result_get_mdc 
(GMimeDecryptResult *result)


  return result->mdc;
  }
+
+
+/**
+ * g_mime_decrypt_result_set_session_key:
+ * @result: a #GMimeDecryptResult
+ * @session_key: a pointer to a null-terminated string representing 
the session key

+ *
+ * Set the session_key to be returned by this decryption result.
+ **/
+void
+g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, 
const char *session_key)

+{
+g_return_if_fail (GMIME_IS_DECRYPT_RESULT (result));
+
+g_free (result->session_key);
+result->session_key = g_strdup(session_key);
+}
+
+
+/**
+ * g_mime_decrypt_result_get_session_key:
+ * @result: a #GMimeDecryptResult
+ *
+ * Get the session_key used for this decryption, if the underlying
+ * crypto context is capable of and (configured to) retrieve session
+ * keys during decryption.  See, for example,
+ * g_mime_gpg_context_set_retrieve_session_key().
+ *
+ * Returns: the session_key digest algorithm used, or NULL if no
+ * session key was requested or found.
+ **/
+const char*
+g_mime_decryption_result_get_session_key (GMimeDecryptResult *result)
+{
+g_return_val_if_fail (GMIME_IS_DECRYPT_RESULT (result), 
GMIME_DIGEST_ALGO_DEFAULT);

+
+return result->session_key;
+}
diff --git a/gmime/gmime-crypto-context.h b/gmime/gmime-crypto-context.h
index cd38760..72573ed 100644
--- a/gmime/gmime-crypto-context.h
+++ b/gmime/gmime-crypto-context.h
@@ -206,6 +206,7 @@ struct _GMimeDecryptResult {
  GMimeSignatureList *signatures;
  GMimeCipherAlgo cipher;
  GMimeDigestAlgo mdc;
+char *session_key;
  };
struct _GMimeDecryptResultClass {
@@ -229,6 +230,9 @@ GMimeCipherAlgo g_mime_decrypt_result_get_cipher 
(GMimeDecryptResult *result);
  void g_mime_decrypt_result_set_mdc (GMimeDecryptResult *result, 
GMimeDigestAlgo mdc);
  GMimeDigestAlgo g_mime_decrypt_result_get_mdc (GMimeDecryptResult 
*result);
  +void g_mime_decrypt_result_set_session_key (GMimeDecryptResult 
*result, const char *session_key);
+const char *g_mime_decrypt_result_get_session_key 
(GMimeDecryptResult *result);

+
  G_END_DECLS
#endif /* __GMIME_CRYPTO_CONTEXT_H__ */
diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index 2ca280a..6f7d374 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -172,6 +172,7 @@ g_mime_gpg_context_init (GMimeGpgContext *ctx, 
GMimeGpgContextClass *klass)

  ctx->always_trust = FALSE;
  ctx->use_agent = FALSE;
  ctx->path = NULL;
+ctx->retrieve_session_key = FALSE;
  }
static void
@@ -311,6 +312,7 @@ struct _GpgCtx {

Re: [gmime-devel] [PATCH 6/6] Support extraction of session keys during decryption.

2016-12-02 Thread Jeffrey Stedfast 
Overall this patch looks good to me, but I am wondering if there is a 
reason to have the retrieve_session_key API?


I guess what I am asking is why would we *not* want to default to 
*always* retrieving the session key in GMime?


Does it pose a security risk?

I think that if it's not a risk, we might as well just always request it 
to simplify things.


Do you agree?

Jeff

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

Some message stores can be optimized by caching session keys of
encrypted messages, rather than incurring asymmetric crypto operations
on each subsequent decryption.

This patch allows gmime to perform extraction of session keys during
message decryption to support that use case.

See the discussion starting here for more detail on the API/design
choices:
https://mail.gnome.org/archives/gmime-devel-list/2016-July/msg5.html
---
  gmime/gmime-crypto-context.c | 41 
  gmime/gmime-crypto-context.h |  4 
  gmime/gmime-gpg-context.c| 50 +++-
  gmime/gmime-gpg-context.h|  4 
  tests/test-pgp.c |  5 +
  tests/test-pgpmime.c |  9 +++-
  6 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/gmime/gmime-crypto-context.c b/gmime/gmime-crypto-context.c
index 5269440..90fa390 100644
--- a/gmime/gmime-crypto-context.c
+++ b/gmime/gmime-crypto-context.c
@@ -573,6 +573,7 @@ g_mime_decrypt_result_init (GMimeDecryptResult *result, 
GMimeDecryptResultClass
result->mdc = GMIME_DIGEST_ALGO_DEFAULT;
result->recipients = NULL;
result->signatures = NULL;
+   result->session_key = NULL;
  }
  
  static void

@@ -586,6 +587,8 @@ g_mime_decrypt_result_finalize (GObject *object)
if (result->signatures)
g_object_unref (result->signatures);

+   g_free (result->session_key);
+   
G_OBJECT_CLASS (result_parent_class)->finalize (object);
  }
  
@@ -755,3 +758,41 @@ g_mime_decryption_result_get_mdc (GMimeDecryptResult *result)


return result->mdc;
  }
+
+
+/**
+ * g_mime_decrypt_result_set_session_key:
+ * @result: a #GMimeDecryptResult
+ * @session_key: a pointer to a null-terminated string representing the 
session key
+ *
+ * Set the session_key to be returned by this decryption result.
+ **/
+void
+g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const char 
*session_key)
+{
+   g_return_if_fail (GMIME_IS_DECRYPT_RESULT (result));
+
+   g_free (result->session_key);
+   result->session_key = g_strdup(session_key);
+}
+
+
+/**
+ * g_mime_decrypt_result_get_session_key:
+ * @result: a #GMimeDecryptResult
+ *
+ * Get the session_key used for this decryption, if the underlying
+ * crypto context is capable of and (configured to) retrieve session
+ * keys during decryption.  See, for example,
+ * g_mime_gpg_context_set_retrieve_session_key().
+ *
+ * Returns: the session_key digest algorithm used, or NULL if no
+ * session key was requested or found.
+ **/
+const char*
+g_mime_decryption_result_get_session_key (GMimeDecryptResult *result)
+{
+   g_return_val_if_fail (GMIME_IS_DECRYPT_RESULT (result), 
GMIME_DIGEST_ALGO_DEFAULT);
+   
+   return result->session_key;
+}
diff --git a/gmime/gmime-crypto-context.h b/gmime/gmime-crypto-context.h
index cd38760..72573ed 100644
--- a/gmime/gmime-crypto-context.h
+++ b/gmime/gmime-crypto-context.h
@@ -206,6 +206,7 @@ struct _GMimeDecryptResult {
GMimeSignatureList *signatures;
GMimeCipherAlgo cipher;
GMimeDigestAlgo mdc;
+   char *session_key;
  };
  
  struct _GMimeDecryptResultClass {

@@ -229,6 +230,9 @@ GMimeCipherAlgo g_mime_decrypt_result_get_cipher 
(GMimeDecryptResult *result);
  void g_mime_decrypt_result_set_mdc (GMimeDecryptResult *result, 
GMimeDigestAlgo mdc);
  GMimeDigestAlgo g_mime_decrypt_result_get_mdc (GMimeDecryptResult *result);
  
+void g_mime_decrypt_result_set_session_key (GMimeDecryptResult *result, const char *session_key);

+const char *g_mime_decrypt_result_get_session_key (GMimeDecryptResult *result);
+
  G_END_DECLS
  
  #endif /* __GMIME_CRYPTO_CONTEXT_H__ */

diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index 2ca280a..6f7d374 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -172,6 +172,7 @@ g_mime_gpg_context_init (GMimeGpgContext *ctx, 
GMimeGpgContextClass *klass)
ctx->always_trust = FALSE;
ctx->use_agent = FALSE;
ctx->path = NULL;
+   ctx->retrieve_session_key = FALSE;
  }
  
  static void

@@ -311,6 +312,7 @@ struct _GpgCtx {
GMimeStream *diagnostics;

GMimeCertificateList *encrypted_to;  /* full list of encrypted-to 
recipients */
+   char *session_key;
GMimeSignatureList *signatures;
GMimeSignature *signature;

@@ -375,6 +377,7 @@ gpg_ctx_new (GMimeGpgContext *ctx)
gpg->need_id = NULL;

Re: [gmime-devel] [PATCH 5/6] Use pinentry-mode loopback in test suite when using "modern" GnuPG

2016-12-02 Thread Jeffrey Stedfast 

Commentary inline...

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

The "Modern" GnuPG suite (2.1.x or higher) defaults to relying on the
gpg-agent for secret key material access, which can prompt the user
for a passphrase.  The test suite uses callbacks to supply the
passphrase, so in these modern versions it should use "pinentry-mode
loopback".

Many users of GMime are likely to avoid using the passphrase callback
and instead to rely on permission from the cryptographic agent
directly.  We do not currently test these scenarios, though we could
do so with a fake pinentry.  If we do that, then those scenarios
should *not* use the loopback pinentry-mode, and we'd need to adjust
this setup.

In the longer term, the right way to resolve all of this would be to
use gpgme directly, instead of having our own wrapper that invokes gpg
manually.


I think my resistance to moving to gpgme has worn out, so I'm on board 
with that.



---
  tests/testsuite.c | 43 +++
  1 file changed, 43 insertions(+)

diff --git a/tests/testsuite.c b/tests/testsuite.c
index 9422e93..84079d1 100644
--- a/tests/testsuite.c
+++ b/tests/testsuite.c
@@ -27,6 +27,10 @@
  #ifdef ENABLE_THREADS
  #include 
  #endif
+#include 
+#include 
+#include 
+#include 
  
  #include "testsuite.h"
  
@@ -346,9 +350,38 @@ g_throw (Exception *ex)

longjmp (env->env, 1);
  }
  
+static int

+is_gpg_modern()
+{
+   const char vheader[] = "gpg (GnuPG) ";
+   FILE *vpipe;
+   char *vstring;
+   size_t vlen;
+   int ret;
+
+   if ((vpipe = popen ("gpg --version", "r")) == NULL)
+   return 0;
+   vlen = 0;
+   vstring = NULL;
+   if (getline (, , vpipe) == -1)
+   return 0;


Shouldn't vpipe be closed in this error condition?


+   if (strncmp (vstring, vheader, sizeof (vheader) - 1))
+   return 0;


Same. In fact, I'd probably recommend pclose()ing vpipe as soon as you 
finish reading the output of gpg --version (no reason to keep it open 
after reading it).



+   ret = (vstring[sizeof (vheader) - 1] > '2') ||
+   (vstring[sizeof (vheader) - 1] == '2' &&
+vstring[sizeof (vheader)] == '.' &&
+vstring[sizeof (vheader) + 1] >= '1');


This has the potential of reading past the end of the buffer.


+   free (vstring);
+   pclose (vpipe);
+   return ret;
+}
+
  int
  testsuite_setup_gpghome ()
  {
+   int gpgconf;
+   const char directive[] = "pinentry-mode loopback\n";
+   
/* reset .gnupg config directory */
if (system ("/bin/rm -rf ./tmp") != 0)
return EXIT_FAILURE;
@@ -362,6 +395,16 @@ testsuite_setup_gpghome ()
  
  	if (system ("gpg --list-keys > /dev/null 2>&1") != 0)

return EXIT_FAILURE;
+   
+   if (is_gpg_modern()) {
+   if ((gpgconf = open ("./tmp/.gnupg/gpg.conf", O_WRONLY | 
O_CREAT | O_TRUNC, 0400)) == -1)
+   return EXIT_FAILURE;
+   if (write (gpgconf, directive, sizeof(directive) - 1) != 
sizeof(directive) - 1)
+   return EXIT_FAILURE;
+   if (close (gpgconf) != 0)
+   return EXIT_FAILURE;
+   }


This might be cleaner if you used GMimeStreamFs or FILE* since the 
correct thing to do is to loop the write() call until all data is 
written and/or until -1 is returned where errno is not EINTR.


Jeff
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 3/6] consolidate setup of GnuPG homedir for test suites

2016-12-02 Thread Jeffrey Stedfast 

Hah. I was just thinking this after committing your previous patch :)

This patch has now also been committed.

Thanks!

Jeff

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

three different test suites all set up and tear down the GnuPG homedir
in the same way.  By consolidating that code in testsuite.c we can
improve it in a single place.
---
  tests/test-pgp.c | 10 ++
  tests/test-pgpmime.c | 10 ++
  tests/test-smime.c   | 10 ++
  tests/testsuite.c| 21 +
  tests/testsuite.h|  3 +++
  5 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/tests/test-pgp.c b/tests/test-pgp.c
index 3e7a2a4..8fc755c 100644
--- a/tests/test-pgp.c
+++ b/tests/test-pgp.c
@@ -283,13 +283,7 @@ int main (int argc, char **argv)

testsuite_init (argc, argv);

-   /* reset .gnupg config directory */
-   if (system ("/bin/rm -rf ./tmp") != 0)
-   return EXIT_FAILURE;
-   if (system ("/bin/mkdir ./tmp") != 0)
-   return EXIT_FAILURE;
-   setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
+   if (testsuite_setup_gpghome ())
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
@@ -404,7 +398,7 @@ int main (int argc, char **argv)

g_mime_shutdown ();

-   if (system ("/bin/rm -rf ./tmp") != 0)
+   if (testsuite_destroy_gpghome ())
return EXIT_FAILURE;

return testsuite_exit ();
diff --git a/tests/test-pgpmime.c b/tests/test-pgpmime.c
index b5a8b21..4cead27 100644
--- a/tests/test-pgpmime.c
+++ b/tests/test-pgpmime.c
@@ -419,13 +419,7 @@ int main (int argc, char *argv[])

testsuite_init (argc, argv);

-   /* reset .gnupg config directory */
-   if (system ("/bin/rm -rf ./tmp") != 0)
-   return EXIT_FAILURE;
-   if (system ("/bin/mkdir ./tmp") != 0)
-   return EXIT_FAILURE;
-   setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
+   if (testsuite_setup_gpghome ())
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
@@ -489,7 +483,7 @@ int main (int argc, char *argv[])

g_mime_shutdown ();

-   if (system ("/bin/rm -rf ./tmp") != 0)
+   if (testsuite_destroy_gpghome ())
return EXIT_FAILURE;

return testsuite_exit ();
diff --git a/tests/test-smime.c b/tests/test-smime.c
index f05e03a..f35fbb7 100644
--- a/tests/test-smime.c
+++ b/tests/test-smime.c
@@ -420,13 +420,7 @@ int main (int argc, char *argv[])

testsuite_init (argc, argv);

-   /* reset .gnupg config directory */
-   if (system ("/bin/rm -rf ./tmp") != 0)
-   return EXIT_FAILURE;
-   if (system ("/bin/mkdir ./tmp") != 0)
-   return EXIT_FAILURE;
-   g_setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
+   if (testsuite_setup_gpghome ())
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
@@ -490,7 +484,7 @@ int main (int argc, char *argv[])

g_mime_shutdown ();

-   if (system ("/bin/rm -rf ./tmp") != 0)
+   if (testsuite_destroy_gpghome ())
return EXIT_FAILURE;

return testsuite_exit ();
diff --git a/tests/testsuite.c b/tests/testsuite.c
index 2b72301..acd36a5 100644
--- a/tests/testsuite.c
+++ b/tests/testsuite.c
@@ -346,6 +346,27 @@ g_throw (Exception *ex)
longjmp (env->env, 1);
  }
  
+int

+testsuite_setup_gpghome ()
+{
+   /* reset .gnupg config directory */
+   if (system ("/bin/rm -rf ./tmp") != 0)
+   return EXIT_FAILURE;
+   if (system ("/bin/mkdir ./tmp") != 0)
+   return EXIT_FAILURE;
+   g_setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
+   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
+   return EXIT_FAILURE;
+   return EXIT_SUCCESS;
+}
+
+int
+testsuite_destroy_gpghome ()
+{
+   if (system ("/bin/rm -rf ./tmp") != 0)
+   return EXIT_FAILURE;
+   return EXIT_SUCCESS;
+}
  
  #ifdef BUILD

  int main (int argc, char **argv)
diff --git a/tests/testsuite.h b/tests/testsuite.h
index ece9014..6c92a5a 100644
--- a/tests/testsuite.h
+++ b/tests/testsuite.h
@@ -45,6 +45,9 @@ void testsuite_check_passed (void);
  
  int testsuite_total_errors (void);
  
+/* for those parts of the test suite that use GnuPG */

+int testsuite_setup_gpghome (void);
+int testsuite_destroy_gpghome (void);
  
  /*#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 96)

  #define G_GNUC_NORETURN __attribute__((noreturn))



___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 4/6] avoid trying to interact with the user session during test suites

2016-12-02 Thread Jeffrey Stedfast 

Committed.

Thanks!

Jeff

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

---
  tests/testsuite.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/tests/testsuite.c b/tests/testsuite.c
index acd36a5..9422e93 100644
--- a/tests/testsuite.c
+++ b/tests/testsuite.c
@@ -355,6 +355,11 @@ testsuite_setup_gpghome ()
if (system ("/bin/mkdir ./tmp") != 0)
return EXIT_FAILURE;
g_setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
+   /* disable the usual mechanisms gpg-agent uses for pinentry */
+   g_unsetenv ("DISPLAY");
+   g_unsetenv ("DBUS_SESSION_BUS_ADDRESS");
+   g_unsetenv ("GPG_TTY");
+
if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
return EXIT_FAILURE;
return EXIT_SUCCESS;



___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

Re: [gmime-devel] [PATCH 1/6] Ship extra data for tests.

2016-12-02 Thread Jeffrey Stedfast 
I've committed this patch with a slight modification (added the 
test*.eml to the wildcard grouping instead of listing them individually).


Thanks!

Jeff

On 12/2/2016 2:02 AM, Daniel Kahn Gillmor wrote:

Running "make check" from the tarball currently doesn't run as many of
the tests as it should, because the data files for the tests aren't
shipped.

This patch includes those files in the generated upstream tarball.
---
  tests/Makefile.am | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 93e4150..d0c0be9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -112,7 +112,9 @@ test_smime_DEPENDENCIES = $(DEPS)
  test_smime_LDADD = $(LDADDS)
  endif
  
-EXTRA_DIST = test1.eml test2.eml test3.eml

+EXTRA_DIST = test1.eml test2.eml test3.eml $(wildcard  \
+ data/pgp*/gmime.gpg.* data/mbox/*put/substring.mbox   \
+ empty*.msg message-partial.* rfc2060.msg)
  
  VERBOSITY=-v
  



___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list