Re: [PATCH 07/18] crypto: new decryption policy "auto"
Daniel Kahn Gillmor writes:
> GMimeObject *
> -_notmuch_crypto_decrypt (notmuch_message_t *message,
> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
> + notmuch_message_t *message,
>g_mime_3_unused(GMimeCryptoContext* crypto_ctx),
>GMimeMultipartEncrypted *part,
>GMimeDecryptResult **decrypt_result,
>GError **err)
> {
> GMimeObject *ret = NULL;
> +if (decrypt == NOTMUCH_DECRYPT_FALSE)
> + return NULL;
I'm going to assume that all is well and no return value from
_notmuch_crypto_decrypt is used without guarding for NULL.
d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 07/18] crypto: new decryption policy "auto"
On Sun, Nov 12 2017, Daniel Kahn Gillmor wrote:
> On Sat 2017-11-11 15:14:03 -0800, Jameson Graef Rollins wrote:
>> On Wed, Oct 25 2017, Daniel Kahn Gillmor wrote:
>>> diff --git a/util/crypto.h b/util/crypto.h
>>> index b23ca747..dc95b4ca 100644
>>> --- a/util/crypto.h
>>> +++ b/util/crypto.h
>>> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
>>> } _notmuch_crypto_t;
>>>
>>> GMimeObject *
>>> -_notmuch_crypto_decrypt (notmuch_message_t *message,
>>> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
>>> +notmuch_message_t *message,
>>> GMimeCryptoContext* crypto_ctx,
>>> GMimeMultipartEncrypted *part,
>>> GMimeDecryptResult **decrypt_result,
>>
>> Why does _notmuch_crypt_decrypt need to have
>> "notmuch_decryption_policy_t decrypt" as an input argument? Isn't
>> notmuch_decryption_policy_t already an attribute of the crypto_ctx? Is
>> there some situation where the policy would differ from what's specified
>> in the crypto_ctx?
>
> crypto_ctx here is just a GMimeCryptoContext, which doesn't know
> anything about notmuch_decryption_policy_t. Maybe i'm misunderstanding
> your question?
>
> I'd be happy to streamline the interface to this internal function, but
> given that it's not an exported API, i'm not as concerned about things
> like future cleanliness -- the notmuch source contains all invocations
> of the function anywhere, so if we find a nicer way to streamline it in
> the future, we can do that cleanup across the codebase in a single
> commit.
I guess I'm confusing how things were before, when the crypto_ctx was a
notmuch-defined thing that included the GMimeCryptoContext.
It seems like _notmuch_crypto_t could just hold the GMimeCryptoContext,
as it does for earlier versions of GMime, which would make things easier
to pass around. But this discussion is tangent to this patch series.
jamie.
signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 07/18] crypto: new decryption policy "auto"
On Sat 2017-11-11 15:14:03 -0800, Jameson Graef Rollins wrote:
> On Wed, Oct 25 2017, Daniel Kahn Gillmor wrote:
>> diff --git a/util/crypto.h b/util/crypto.h
>> index b23ca747..dc95b4ca 100644
>> --- a/util/crypto.h
>> +++ b/util/crypto.h
>> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
>> } _notmuch_crypto_t;
>>
>> GMimeObject *
>> -_notmuch_crypto_decrypt (notmuch_message_t *message,
>> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
>> + notmuch_message_t *message,
>> GMimeCryptoContext* crypto_ctx,
>> GMimeMultipartEncrypted *part,
>> GMimeDecryptResult **decrypt_result,
>
> Why does _notmuch_crypt_decrypt need to have
> "notmuch_decryption_policy_t decrypt" as an input argument? Isn't
> notmuch_decryption_policy_t already an attribute of the crypto_ctx? Is
> there some situation where the policy would differ from what's specified
> in the crypto_ctx?
crypto_ctx here is just a GMimeCryptoContext, which doesn't know
anything about notmuch_decryption_policy_t. Maybe i'm misunderstanding
your question?
I'd be happy to streamline the interface to this internal function, but
given that it's not an exported API, i'm not as concerned about things
like future cleanliness -- the notmuch source contains all invocations
of the function anywhere, so if we find a nicer way to streamline it in
the future, we can do that cleanup across the codebase in a single
commit.
Thanks for the review!
--dkg
signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 07/18] crypto: new decryption policy "auto"
On Wed, Oct 25 2017, Daniel Kahn Gillmor wrote:
> diff --git a/util/crypto.h b/util/crypto.h
> index b23ca747..dc95b4ca 100644
> --- a/util/crypto.h
> +++ b/util/crypto.h
> @@ -16,7 +16,8 @@ typedef struct _notmuch_crypto {
> } _notmuch_crypto_t;
>
> GMimeObject *
> -_notmuch_crypto_decrypt (notmuch_message_t *message,
> +_notmuch_crypto_decrypt (notmuch_decryption_policy_t decrypt,
> + notmuch_message_t *message,
>GMimeCryptoContext* crypto_ctx,
>GMimeMultipartEncrypted *part,
>GMimeDecryptResult **decrypt_result,
Why does _notmuch_crypt_decrypt need to have
"notmuch_decryption_policy_t decrypt" as an input argument? Isn't
notmuch_decryption_policy_t already an attribute of the crypto_ctx? Is
there some situation where the policy would differ from what's specified
in the crypto_ctx?
jamie.
signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 07/18] crypto: new decryption policy "auto"
This new automatic decryption policy should make it possible to
decrypt messages that we have stashed session keys for, without
incurring a call to the user's asymmetric keys.
---
doc/man1/notmuch-config.rst | 11 ---
lib/index.cc | 3 ++-
lib/indexopts.c | 13 -
lib/notmuch.h | 1 +
mime-node.c | 7 ---
notmuch-client.h | 4 +++-
notmuch.c | 3 ++-
test/T357-index-decryption.sh | 12 +++-
util/crypto.c | 8 +++-
util/crypto.h | 3 ++-
10 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 6961737f..14642062 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -142,9 +142,14 @@ The available configuration items are described below.
**[STORED IN DATABASE]**
When indexing an encrypted e-mail message, if this variable is
-set to true, notmuch will try to decrypt the message and index
-the cleartext. Be aware that the index is likely sufficient
-to reconstruct the cleartext of the message itself, so please
+set to ``true``, notmuch will try to decrypt the message and
+index the cleartext. If ``auto``, it will try to index the
+cleartext if a stashed session key is already known for the message,
+but will not try to access your secret keys. Use ``false`` to
+avoid decrypting even when a session key is already known.
+
+Be aware that the notmuch index is likely sufficient to
+reconstruct the cleartext of the message itself, so please
ensure that the notmuch message index is adequately protected.
DO NOT USE ``index.try_decrypt=true`` without considering the
security of your index.
diff --git a/lib/index.cc b/lib/index.cc
index 483fe31d..d9a0018c 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -548,7 +548,8 @@ _index_encrypted_mime_part (notmuch_message_t *message,
}
}
#endif
-clear = _notmuch_crypto_decrypt (message, crypto_ctx, encrypted_data,
NULL, &err);
+clear = _notmuch_crypto_decrypt (notmuch_indexopts_get_try_decrypt
(indexopts),
+message, crypto_ctx, encrypted_data, NULL,
&err);
if (err) {
_notmuch_database_log (notmuch, "Failed to decrypt during indexing.
(%d:%d) [%s]\n",
err->domain, err->code, err->message);
diff --git a/lib/indexopts.c b/lib/indexopts.c
index 23557143..93a2c9eb 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -33,11 +33,14 @@ notmuch_database_get_default_indexopts (notmuch_database_t
*db)
if (err)
return ret;
-if (try_decrypt &&
- ((!(strcasecmp(try_decrypt, "true"))) ||
-(!(strcasecmp(try_decrypt, "yes"))) ||
-(!(strcasecmp(try_decrypt, "1")
- notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
+if (try_decrypt) {
+ if ((!(strcasecmp(try_decrypt, "true"))) ||
+ (!(strcasecmp(try_decrypt, "yes"))) ||
+ (!(strcasecmp(try_decrypt, "1"
+ notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_TRUE);
+ else if (!strcasecmp(try_decrypt, "auto"))
+ notmuch_indexopts_set_try_decrypt (ret, NOTMUCH_DECRYPT_AUTO);
+}
free (try_decrypt);
return ret;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 26fe8f81..7b1c61ad 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2241,6 +2241,7 @@ notmuch_database_get_default_indexopts
(notmuch_database_t *db);
typedef enum {
NOTMUCH_DECRYPT_FALSE,
NOTMUCH_DECRYPT_TRUE,
+NOTMUCH_DECRYPT_AUTO,
} notmuch_decryption_policy_t;
/**
diff --git a/mime-node.c b/mime-node.c
index eb6a16c0..815c1787 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -205,7 +205,8 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject
*part,
break;
node->decrypt_attempted = true;
- node->decrypted_child = _notmuch_crypto_decrypt (parent ?
parent->envelope_file : NULL,
+ node->decrypted_child = _notmuch_crypto_decrypt
(node->ctx->crypto->decrypt,
+parent ?
parent->envelope_file : NULL,
cryptoctx,
encrypteddata, &decrypt_result, &err);
}
if (! node->decrypted_child) {
@@ -270,7 +271,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
}
#if (GMIME_MAJOR_VERSION < 3)
-if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt ==
NOTMUCH_DECRYPT_TRUE))
+if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && (node->ctx->crypto->decrypt !=
NOTMUCH_DECRYPT_FALSE))
|| (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
GMimeContentType *content_type = g_mime_object_get_content_type (part);
const char
