From 29648a137c5807135ab168917b4a51d5e19e51c2 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Fri, 8 Dec 2017 01:24:01 -0500 Subject: [PATCH] crypto: actually stash session keys when decrypt=true If you're going to store the cleartext index of an encrypted message, in most situations you might just as well store the session key. Doing this storage has efficiency and recoverability advantages. Combined with a schedule of regular OpenPGP subkey rotation and destruction, this can also offer security benefits, like "deletable e-mail", which is the store-and-forward analog to "forward secrecy". But wait, i hear you saying, i have a special need to store cleartext indexes but it's really bad for me to store session keys! Maybe (let's imagine) i get lots of e-mails with incriminating photos attached, and i want to be able to search for them by the text in the e-mail, but i don't want someone with access to the index to be actually able to see the photos themselves. Fret not, the next patch in this series will support your wacky uncommon use case. --- doc/man1/notmuch-config.rst | 10 ++++++---- doc/man1/notmuch-insert.rst | 13 +++++++------ doc/man1/notmuch-new.rst | 19 ++++++++++--------- doc/man1/notmuch-reindex.rst | 10 +++++----- doc/man7/notmuch-properties.rst | 4 ++++ lib/index.cc | 18 +++++++++++++++++- test/T357-index-decryption.sh | 24 +++++++++++++++++++++++- util/crypto.c | 16 +++++++++++++++- 8 files changed, 87 insertions(+), 27 deletions(-) diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst index 1a9e08a3..dabf269f 100644 --- a/doc/man1/notmuch-config.rst +++ b/doc/man1/notmuch-config.rst @@ -143,10 +143,12 @@ 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. 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. + index the cleartext, stashing a copy of any discovered session + keys for the message. If ``auto``, it will try to index the + cleartext if a stashed session key is already known for the message + (e.g. from a previous copy), but will not try to access your + secret keys. Use ``false`` to avoid decrypting even when a + stashed session key is already present. Be aware that the notmuch index is likely sufficient to reconstruct the cleartext of the message itself, so please diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst index b22be863..214f261b 100644 --- a/doc/man1/notmuch-insert.rst +++ b/doc/man1/notmuch-insert.rst @@ -54,12 +54,13 @@ Supported options for **insert** include ``--decrypt=(true|auto|false)`` If ``true`` and the message is encrypted, try to decrypt the - message while indexing. If ``auto``, and notmuch already - knows about a session key for the message, it will try - decrypting using that session key but will not try to access - the user's secret keys. If decryption is successful, index - the cleartext itself. Either way, the message is always - stored to disk in its original form (ciphertext). + message while indexing, storing any session keys discovered. + If ``auto``, and notmuch already knows about a session key for + the message, it will try decrypting using that session key but + will not try to access the user's secret keys. If decryption + is successful, index the cleartext itself. Either way, the + message is always stored to disk in its original form + (ciphertext). Be aware that the index is likely sufficient to reconstruct the cleartext of the message itself, so please ensure that the diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst index 71df31d7..27676a19 100644 --- a/doc/man1/notmuch-new.rst +++ b/doc/man1/notmuch-new.rst @@ -46,16 +46,17 @@ Supported options for **new** include ``--decrypt=(true|auto|false)`` If ``true``, when encountering an encrypted message, try to - decrypt it while indexing. If decryption is successful, index - the cleartext itself. If ``auto``, try to use any session key - already known to belong to this message, but do not attempt to - use the user's secret keys. + decrypt it while indexing, and store any discovered session + keys. If ``auto``, try to use any session key already known + to belong to this message, but do not attempt to use the + user's secret keys. If decryption is successful, index the + cleartext of the message. - Be aware that the 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 ``--decrypt=true`` without - considering the security of your index. + Be aware that the index is likely sufficient (and the session + key is certainly sufficient) to reconstruct the cleartext of + the message itself, so please ensure that the notmuch message + index is adequately protected. DO NOT USE ``--decrypt=true`` + without considering the security of your index. See also ``index.decrypt`` in **notmuch-config(1)**. diff --git a/doc/man1/notmuch-reindex.rst b/doc/man1/notmuch-reindex.rst index e8174f39..47790871 100644 --- a/doc/man1/notmuch-reindex.rst +++ b/doc/man1/notmuch-reindex.rst @@ -24,11 +24,11 @@ Supported options for **reindex** include ``--decrypt=(true|auto|false)`` If ``true``, when encountering an encrypted message, try to - decrypt it while reindexing. If ``auto``, and notmuch already - knows about a session key for the message, it will try - decrypting using that session key but will not try to access - the user's secret keys. If decryption is successful, index - the cleartext itself. + decrypt it while reindexing, storing any session keys + discovered. If ``auto``, and notmuch already knows about a + session key for the message, it will try decrypting using that + session key but will not try to access the user's secret keys. + If decryption is successful, index the cleartext itself. If ``false``, notmuch reindex will also delete any stashed session keys for all messages matching the search terms. diff --git a/doc/man7/notmuch-properties.rst b/doc/man7/notmuch-properties.rst index 1a3f690e..07d36a1a 100644 --- a/doc/man7/notmuch-properties.rst +++ b/doc/man7/notmuch-properties.rst @@ -98,6 +98,10 @@ of its normal activity. message. This includes attachments, cryptographic signatures, and other material that cannot be reconstructed from the index alone. + See ``index.decrypt`` in **notmuch-config(1)** for more + details about how to set notmuch's policy on when to store session + keys. + The session key should be in the ASCII text form produced by GnuPG. For OpenPGP, that consists of a decimal representation of the hash algorithm used (identified by number from RFC 4880, diff --git a/lib/index.cc b/lib/index.cc index 3914012a..0ad683fa 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -549,11 +549,15 @@ _index_encrypted_mime_part (notmuch_message_t *message, } #endif bool attempted = false; + GMimeDecryptResult *decrypt_result = NULL; + bool get_sk = (HAVE_GMIME_SESSION_KEYS && notmuch_indexopts_get_decrypt_policy (indexopts) == NOTMUCH_DECRYPT_TRUE); clear = _notmuch_crypto_decrypt (&attempted, notmuch_indexopts_get_decrypt_policy (indexopts), - message, crypto_ctx, encrypted_data, NULL, &err); + message, crypto_ctx, encrypted_data, get_sk ? &decrypt_result : NULL, &err); if (!attempted) return; if (err || !clear) { + if (decrypt_result) + g_object_unref (decrypt_result); if (err) { _notmuch_database_log (notmuch, "Failed to decrypt during indexing. (%d:%d) [%s]\n", err->domain, err->code, err->message); @@ -568,6 +572,18 @@ _index_encrypted_mime_part (notmuch_message_t *message, "property (%d)\n", status); return; } + if (decrypt_result) { +#if HAVE_GMIME_SESSION_KEYS + if (get_sk) { + status = notmuch_message_add_property (message, "session-key", + g_mime_decrypt_result_get_session_key (decrypt_result)); + if (status) + _notmuch_database_log (notmuch, "failed to add session-key " + "property (%d)\n", status); + } +#endif + g_object_unref (decrypt_result); + } _index_mime_part (message, indexopts, clear); g_object_unref (clear); diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh index 9f46a01b..fcecb1d9 100755 --- a/test/T357-index-decryption.sh +++ b/test/T357-index-decryption.sh @@ -48,6 +48,17 @@ test_expect_equal \ "$output" \ "$expected" +test_begin_subtest "show the message body of the encrypted message" +notmuch dump wumpus +output=$(notmuch show wumpus | awk '/^\014part}/{ f=0 }; { if (f) { print $0 } } /^\014part{ ID: 3/{ f=1 }') +expected='This is a test encrypted message with a wumpus.' +if [ $NOTMUCH_HAVE_GMIME_SESSION_KEYS -eq 0 ]; then + test_subtest_known_broken +fi +test_expect_equal \ + "$output" \ + "$expected" + test_begin_subtest "message should go away after deletion" # cache the message in an env var and remove it: @@ -129,10 +140,21 @@ test_expect_equal \ "$output" \ "$expected" +# try a simple reindex +test_begin_subtest 'reindex in auto mode' +test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success' +test_begin_subtest "reindexed encrypted messages, should not have changed" +output=$(notmuch search wumpus) +if [ $NOTMUCH_HAVE_GMIME_SESSION_KEYS -eq 0 ]; then + test_subtest_known_broken +fi +test_expect_equal \ + "$output" \ + "$expected" # try to remove cleartext indexing test_begin_subtest 'reindex without cleartext' -test_expect_success 'notmuch reindex tag:encrypted and property:index.decryption=success' +test_expect_success 'notmuch reindex --decrypt=false tag:encrypted and property:index.decryption=success' test_begin_subtest "reindexed encrypted messages, without cleartext" output=$(notmuch search wumpus) expected='' diff --git a/util/crypto.c b/util/crypto.c index 338f1d5d..066dea6e 100644 --- a/util/crypto.c +++ b/util/crypto.c @@ -197,10 +197,24 @@ _notmuch_crypto_decrypt (bool *attempted, if (attempted) *attempted = true; #if (GMIME_MAJOR_VERSION < 3) +#if HAVE_GMIME_SESSION_KEYS + gboolean oldgetsk = g_mime_crypto_context_get_retrieve_session_key (crypto_ctx); + gboolean newgetsk = (decrypt_result); + if (newgetsk != oldgetsk) + /* This could return an error, but we can't do anything about it, so ignore it */ + g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, newgetsk, NULL); +#endif ret = g_mime_multipart_encrypted_decrypt(part, crypto_ctx, decrypt_result, err); +#if HAVE_GMIME_SESSION_KEYS + if (newgetsk != oldgetsk) + g_mime_crypto_context_set_retrieve_session_key (crypto_ctx, oldgetsk, NULL); +#endif #else - ret = g_mime_multipart_encrypted_decrypt(part, GMIME_DECRYPT_NONE, NULL, + GMimeDecryptFlags flags = GMIME_DECRYPT_NONE; + if (decrypt_result) + flags |= GMIME_DECRYPT_EXPORT_SESSION_KEY; + ret = g_mime_multipart_encrypted_decrypt(part, flags, NULL, decrypt_result, err); #endif return ret;