Re: [PATCH] fix reference to notmuch_message_get_properties

2017-09-23 Thread Tomi Ollila 
On Sat, Sep 23 2017, Daniel Kahn Gillmor wrote:

Looks safe to me

> ---
>  lib/notmuch.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 34bf5899..cbde6a93 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1873,7 +1873,7 @@ notmuch_message_get_properties (notmuch_message_t 
> *message, const char *key, not
>   * says. Whereas when this function returns FALSE, calling any of
>   * these functions results in undefined behaviour.
>   *
> - * See the documentation of notmuch_message_properties_get for example
> + * See the documentation of notmuch_message_get_properties for example
>   * code showing how to iterate over a notmuch_message_properties_t
>   * object.
>   *
> -- 
> 2.14.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 08/10] cli/new: add --try-decrypt=(true|false)

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> Try to decrypt any encrypted parts of newly-discovered messages while
> indexing them.  The cleartext of any successfully-decrypted messages
> will be indexed, with tags applied in the same form as from notmuch
> insert --try-decrypt=true.
>
> Note: if the deprecated crypto.gpg_path configuration option is set to
> anything other than "gpg", we ignore it (and print a warning on
> stderr, if built against gmime < 3.0).
>
> We also add a new test making use of this functionality.  This
> requires a bit of reorganization, because we need to allow passing
> --long-arguments to "notmuch new" via emacs_fcc_message
> ---
>  completion/notmuch-completion.bash | 13 --
>  doc/man1/notmuch-new.rst   | 12 +
>  notmuch-new.c  | 29 +-
>  test/T357-index-decryption.sh  | 51 
> ++
>  test/test-lib.sh   | 11 +++-
>  5 files changed, 112 insertions(+), 4 deletions(-)
>  create mode 100755 test/T357-index-decryption.sh
>
> diff --git a/completion/notmuch-completion.bash 
> b/completion/notmuch-completion.bash
> index 5201be63..17be6b8f 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -311,11 +311,20 @@ _notmuch_insert()
>  _notmuch_new()
>  {
>  local cur prev words cword split
> -_init_completion || return
> +_init_completion -s || return
> +
> +$split &&
> +case "${prev}" in
> + --try-decrypt)
> + COMPREPLY=( $( compgen -W "true false" -- "${cur}" ) )
> + return
> + ;;
> +esac
>  
> +! $split &&
>  case "${cur}" in
>   -*)
> - local options="--no-hooks --quiet ${_notmuch_shared_options}"
> + local options="--no-hooks --try-decrypt= --quiet 
> ${_notmuch_shared_options}"
>   compopt -o nospace
>   COMPREPLY=( $(compgen -W "${options}" -- ${cur}) )
>   ;;
> diff --git a/doc/man1/notmuch-new.rst b/doc/man1/notmuch-new.rst
> index 6acfa112..c255f980 100644
> --- a/doc/man1/notmuch-new.rst
> +++ b/doc/man1/notmuch-new.rst
> @@ -43,6 +43,18 @@ Supported options for **new** include
>  ``--quiet``
>  Do not print progress or results.
>  
> +``--try-decrypt=(true|false)``
> +
> +If true, when encountering an encrypted message, try to
> +decrypt it while indexing.  If decryption is successful, index
> +the cleartext itself.  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 ``--try-decrypt=true`` without
> +considering the security of your index.
> +
> +See also ``index.try_decrypt`` in **notmuch-config(1)**.
> +
>  EXIT STATUS
>  ===
>  
> diff --git a/notmuch-new.c b/notmuch-new.c
> index faeb8f0a..cffcd8bc 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -49,6 +49,7 @@ typedef struct {
>  size_t new_tags_length;
>  const char **new_ignore;
>  size_t new_ignore_length;
> +notmuch_indexopts_t *indexopts;
>  
>  int total_files;
>  int processed_files;
> @@ -267,7 +268,7 @@ add_file (notmuch_database_t *notmuch, const char 
> *filename,
>  if (status)
>   goto DONE;
>  
> -status = notmuch_database_index_file (notmuch, filename, NULL, );
> +status = notmuch_database_index_file (notmuch, filename, 
> state->indexopts, );
>  switch (status) {
>  /* Success. */
>  case NOTMUCH_STATUS_SUCCESS:
> @@ -949,6 +950,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
> char *argv[])
>  unsigned int i;
>  notmuch_bool_t timer_is_active = FALSE;
>  notmuch_bool_t no_hooks = FALSE;
> +int try_decrypt = -1;
>  notmuch_bool_t quiet = FALSE, verbose = FALSE;
>  notmuch_status_t status;
>  
> @@ -957,6 +959,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, 
> char *argv[])
>   { NOTMUCH_OPT_BOOLEAN,  , "verbose", 'v', 0 },
>   { NOTMUCH_OPT_BOOLEAN,  _files_state.debug, "debug", 'd', 0 },
>   { NOTMUCH_OPT_BOOLEAN,  _hooks, "no-hooks", 'n', 0 },
> + { NOTMUCH_OPT_BOOLEAN,  _decrypt, "try-decrypt", 0, 0 },
>   { NOTMUCH_OPT_INHERIT, (void *) _shared_options, NULL, 0, 0 },
>   { 0, 0, 0, 0, 0 }
>  };
> @@ -1074,6 +1077,30 @@ notmuch_new_command (notmuch_config_t *config, int 
> argc, char *argv[])
>  if (notmuch == NULL)
>   return EXIT_FAILURE;
>  
> +add_files_state.indexopts = notmuch_database_get_default_indexopts 
> (notmuch);
> +if (!add_files_state.indexopts) {
> + fprintf (stderr, "Error: could not create index options.\n");
> + return EXIT_FAILURE;
> +}
> +if (try_decrypt == TRUE || try_decrypt == FALSE) {

As discussed in the argument parsing thread, I'm not really fond of
these implicit "tristate booleans".

Re: [PATCH v2 07/10] config: define new option index.try_decrypt

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> By default, notmuch won't try to decrypt on indexing.  With this
> patch, we make it possible to indicate a per-database preference using
> the config variable "index.try_decrypt", which by default will be
> false.
> ---
>  doc/man1/notmuch-config.rst | 12 
>  lib/indexopts.c | 16 +++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
> index 6a51e64f..6f35d127 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -134,6 +134,18 @@ The available configuration items are described below.
>  
>  Default: ``gpg``.
>  
> +**index.try_decrypt**
> +
> +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
> +ensure that the notmuch message index is adequately protected.
> +DO NOT USE ``index.try_decrypt=true`` without considering the
> +security of your index.
> +
> +Default: ``false``.
> +
>  **built_with.**
>  
>  Compile time feature . Current possibilities include
> diff --git a/lib/indexopts.c b/lib/indexopts.c
> index 1162900c..5bd396ff 100644
> --- a/lib/indexopts.c
> +++ b/lib/indexopts.c
> @@ -23,7 +23,21 @@
>  notmuch_indexopts_t *
>  notmuch_database_get_default_indexopts (notmuch_database_t *db)
>  {
> -return talloc_zero (db, notmuch_indexopts_t);
> +notmuch_indexopts_t *ret = talloc_zero (db, notmuch_indexopts_t);
> +if (ret) {
> + char * try_decrypt;
> + notmuch_status_t err;
> + if (!(err = notmuch_database_get_config (db, "index.try_decrypt", 
> _decrypt))) {

I like the style of always separating assigment and conditional.

I wonder if this function would look cleaner by doing early returns
every step of the way instead of nested ifs.

if (!ret)
return ret;

err = notmuch_database_get_config();
if (err)
return ret;

and so on.


> + if (try_decrypt &&
> + ((!(strcasecmp(try_decrypt, "true"))) ||
> +  (!(strcasecmp(try_decrypt, "yes"))) ||
> +  (!(strcasecmp(try_decrypt, "1")
> + notmuch_indexopts_set_try_decrypt (ret, TRUE);
> +
> + free (try_decrypt);
> + }
> +}
> +return ret;
>  }
>  
>  notmuch_status_t
> -- 
> 2.14.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 04/10] index: implement notmuch_indexopts_t with try_decrypt

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> This is currently mostly a wrapper around _notmuch_crypto_t that keeps
> its internals private and doesn't expose any of the GMime API.
> However, non-crypto indexing options might also be added later
> (e.g. filters or other transformations).
> ---
>  lib/add-message.cc|  9 -
>  lib/indexopts.c   | 22 --
>  lib/notmuch-private.h |  7 +++
>  lib/notmuch.h | 19 +++
>  4 files changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/lib/add-message.cc b/lib/add-message.cc
> index 73bde7fa..1fd91c14 100644
> --- a/lib/add-message.cc
> +++ b/lib/add-message.cc
> @@ -460,7 +460,7 @@ _notmuch_database_link_message (notmuch_database_t 
> *notmuch,
>  notmuch_status_t
>  notmuch_database_index_file (notmuch_database_t *notmuch,
>const char *filename,
> -  notmuch_indexopts_t unused (*indexopts),
> +  notmuch_indexopts_t *indexopts,
>notmuch_message_t **message_ret)
>  {
>  notmuch_message_file_t *message_file;
> @@ -468,6 +468,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
>  notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;
>  notmuch_private_status_t private_status;
>  notmuch_bool_t is_ghost = FALSE, is_new = FALSE;
> +notmuch_indexopts_t *def_indexopts = NULL;
>  
>  const char *date;
>  const char *from, *to, *subject;
> @@ -540,6 +541,9 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
>   if (is_new || is_ghost)
>   _notmuch_message_set_header_values (message, date, from, subject);
>  
> + if (!indexopts)
> + indexopts = def_indexopts = notmuch_database_get_default_indexopts 
> (notmuch);
> +
>   ret = _notmuch_message_index_file (message, message_file);
>   if (ret)
>   goto DONE;
> @@ -557,6 +561,9 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
>  }
>  
>DONE:
> +if (def_indexopts)
> + notmuch_indexopts_destroy (def_indexopts);
> +
>  if (message) {
>   if ((ret == NOTMUCH_STATUS_SUCCESS ||
>ret == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) && message_ret)
> diff --git a/lib/indexopts.c b/lib/indexopts.c
> index 2f9b841b..1162900c 100644
> --- a/lib/indexopts.c
> +++ b/lib/indexopts.c
> @@ -21,9 +21,27 @@
>  #include "notmuch-private.h"
>  
>  notmuch_indexopts_t *
> -notmuch_database_get_default_indexopts (notmuch_database_t unused (*db))
> +notmuch_database_get_default_indexopts (notmuch_database_t *db)
>  {
> -return NULL;
> +return talloc_zero (db, notmuch_indexopts_t);

I wonder about the lifetime of indexopts. Should default indexopts be
part of the db object, so that your caller above doesn't have to
alloc/destroy it for every file?

Our library interface has a leaky abstraction of the talloc hierarchical
refcounting. We don't talk about it in any of the docs, some of it is
implied, most of it is completely surprising if the library interface
user assumes a traditional C memory allocation model without
refcounting.

> +}
> +
> +notmuch_status_t
> +notmuch_indexopts_set_try_decrypt (notmuch_indexopts_t *indexopts,
> +notmuch_bool_t try_decrypt)
> +{
> +if (!indexopts)
> + return NOTMUCH_STATUS_NULL_POINTER;
> +indexopts->crypto.decrypt = try_decrypt;
> +return NOTMUCH_STATUS_SUCCESS;
> +}
> +
> +notmuch_bool_t
> +notmuch_indexopts_get_try_decrypt (const notmuch_indexopts_t *indexopts)
> +{
> +if (!indexopts)
> + return FALSE;
> +return indexopts->crypto.decrypt;
>  }
>  
>  void
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index b187a80f..3168cf3c 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -51,6 +51,7 @@ NOTMUCH_BEGIN_DECLS
>  #include "xutil.h"
>  #include "error_util.h"
>  #include "string-util.h"
> +#include "crypto.h"
>  
>  #ifdef DEBUG
>  # define DEBUG_DATABASE_SANITY 1
> @@ -632,6 +633,12 @@ _notmuch_thread_create (void *ctx,
>   notmuch_exclude_t omit_exclude,
>   notmuch_sort_t sort);
>  
> +/* param.c */
> +
> +typedef struct _notmuch_indexopts {
> +_notmuch_crypto_t crypto;
> +} notmuch_indexopts_t;
> +
>  NOTMUCH_END_DECLS
>  
>  #ifdef __cplusplus
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 6c76fb40..8baa76ab 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -2214,6 +2214,25 @@ notmuch_config_list_destroy (notmuch_config_list_t 
> *config_list);
>  notmuch_indexopts_t *
>  notmuch_database_get_default_indexopts (notmuch_database_t *db);
>  
> +/**
> + * Specify whether to decrypt encrypted parts while indexing.
> + *
> + * 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 SET THIS FLAG TO TRUE
> +

Re: [PATCH v2 05/10] crypto: index encrypted parts when indexopts try_decrypt is set.

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> If we see index options that ask us to decrypt when indexing a
> message, and we encounter an encrypted part, we'll try to descend into
> it.
>
> If we can decrypt, we add the property index-decryption=success.
>
> If we can't decrypt (or recognize the encrypted type of mail), we add
> the property index-decryption=failure.
>
> Note that a single message may have both values of the
> "index-decryption" property: "success" and "failure".  For example,
> consider a message that includes multiple layers of encryption.  If we
> manage to decrypt the outer layer ("index-decryption=success"), but
> fail on the inner layer ("index-decryption=failure").
>
> Before re-indexing, we wipe this automatically-added property, so that
> it will subsequently correspond to the actual semantics of the stored
> index.
> ---
>  lib/add-message.cc|   2 +-
>  lib/index.cc  | 103 
> +-
>  lib/message.cc|  14 ++-
>  lib/notmuch-private.h |   1 +
>  4 files changed, 107 insertions(+), 13 deletions(-)
>
> diff --git a/lib/add-message.cc b/lib/add-message.cc
> index 1fd91c14..66eb0a1f 100644
> --- a/lib/add-message.cc
> +++ b/lib/add-message.cc
> @@ -544,7 +544,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
>   if (!indexopts)
>   indexopts = def_indexopts = notmuch_database_get_default_indexopts 
> (notmuch);
>  
> - ret = _notmuch_message_index_file (message, message_file);
> + ret = _notmuch_message_index_file (message, indexopts, message_file);
>   if (ret)
>   goto DONE;
>  
> diff --git a/lib/index.cc b/lib/index.cc
> index ceb444df..285928f7 100644
> --- a/lib/index.cc
> +++ b/lib/index.cc
> @@ -364,9 +364,17 @@ _index_content_type (notmuch_message_t *message, 
> GMimeObject *part)
>  }
>  }
>  
> +static void
> +_index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t 
> *indexopts,
> +#if (GMIME_MAJOR_VERSION < 3)
> + GMimeContentType *content_type,
> +#endif

I don't think adding this within ifdefs servers a useful purpose. It
just makes the code harder to read all over the place. Please just pass
it in unconditionally.

> + GMimeMultipartEncrypted *part);
> +
>  /* Callback to generate terms for each mime part of a message. */
>  static void
>  _index_mime_part (notmuch_message_t *message,
> +   notmuch_indexopts_t *indexopts,
> GMimeObject *part)
>  {
>  GMimeStream *stream, *filter;
> @@ -409,17 +417,26 @@ _index_mime_part (notmuch_message_t *message,
>   }
>   }
>   if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) {
> - /* Don't index encrypted parts, but index their content type. */
> - _index_content_type (message,
> -  g_mime_multipart_get_part (multipart, i));
> - if ((i != GMIME_MULTIPART_ENCRYPTED_VERSION) &&
> - (i != GMIME_MULTIPART_ENCRYPTED_CONTENT)) {
> - _notmuch_database_log (_notmuch_message_database (message),
> -"Warning: Unexpected extra parts of 
> multipart/encrypted.\n");
> + if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) {
> + _index_encrypted_mime_part(message, indexopts,
> +#if (GMIME_MAJOR_VERSION < 3)
> +content_type,
> +#endif
> +GMIME_MULTIPART_ENCRYPTED 
> (part));
> + } else {
> + /* Don't index the non-content parts of an
> +  * encrypted message, but do index their content
> +  * type. */
> + _index_content_type (message,
> +  g_mime_multipart_get_part (multipart, 
> i));
> + if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) {
> + _notmuch_database_log (_notmuch_message_database 
> (message),
> +"Warning: Unexpected extra parts 
> of multipart/encrypted.\n");
> + }
>   }
>   continue;
>   }
> - _index_mime_part (message,
> + _index_mime_part (message, indexopts,
> g_mime_multipart_get_part (multipart, i));
>   }
>   return;
> @@ -430,7 +447,7 @@ _index_mime_part (notmuch_message_t *message,
>  
>   mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART 
> (part));
>  
> - _index_mime_part (message, g_mime_message_get_mime_part (mime_message));
> + _index_mime_part (message, indexopts, g_mime_message_get_mime_part 
> (mime_message));
>  
>   return;
>  }
> @@ -502,8 +519,74 @@ _index_mime_part (notmuch_message_t *message,
>  }
>  }
>  
> +/* descend (if desired) into the cleartext part of an encrypted MIME
>

Re: [PATCH v2 03/10] tests: prepare for more crypto tests (using add_gnupg_home)

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> Move add_gnupg_home to test-lib.sh to prepare it for reuse.
> ---
>  test/T350-crypto.sh | 17 -
>  test/test-lib.sh| 15 +++
>  2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
> index 1d408af7..e1b8fd83 100755
> --- a/test/T350-crypto.sh
> +++ b/test/T350-crypto.sh
> @@ -7,23 +7,6 @@
>  test_description='PGP/MIME signature verification and decryption'
>  . ./test-lib.sh || exit 1
>  
> -add_gnupg_home ()
> -{
> -local output
> -[ -d ${GNUPGHOME} ] && return
> -_gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
> -at_exit_function _gnupg_exit

The above lines get dropped. Rebase fail?

BR,
Jani.

> -mkdir -m 0700 "$GNUPGHOME"
> -gpg --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc 
> >"$GNUPGHOME"/import.log 2>&1
> -test_debug "cat $GNUPGHOME/import.log"
> -if (gpg --quick-random --version >/dev/null 2>&1) ; then
> - echo quick-random >> "$GNUPGHOME"/gpg.conf
> -elif (gpg --debug-quick-random --version >/dev/null 2>&1) ; then
> - echo debug-quick-random >> "$GNUPGHOME"/gpg.conf
> -fi
> -echo no-emit-version >> "$GNUPGHOME"/gpg.conf
> -}
> -
>  ##
>  
>  add_gnupg_home
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 35024649..b8427d97 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -93,6 +93,21 @@ unset GREP_OPTIONS
>  # For emacsclient
>  unset ALTERNATE_EDITOR
>  
> +add_gnupg_home ()
> +{
> +local output
> +[ -d ${GNUPGHOME} ] && return
> +mkdir -m 0700 "$GNUPGHOME"
> +gpg --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc 
> >"$GNUPGHOME"/import.log 2>&1
> +test_debug "cat $GNUPGHOME/import.log"
> +if (gpg --quick-random --version >/dev/null 2>&1) ; then
> + echo quick-random >> "$GNUPGHOME"/gpg.conf
> +elif (gpg --debug-quick-random --version >/dev/null 2>&1) ; then
> + echo debug-quick-random >> "$GNUPGHOME"/gpg.conf
> +fi
> +echo no-emit-version >> "$GNUPGHOME"/gpg.conf
> +}
> +
>  # Each test should start with something like this, after copyright notices:
>  #
>  # test_description='Description of this test...
> -- 
> 2.14.1
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 02/10] crypto: make shared crypto code behave library-like

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> If we're going to reuse the crypto code across both the library and
> the client, then it needs to report error states properly and not
> write to stderr.
> ---
>  lib/database.cc |  6 
>  lib/notmuch.h   | 17 +++
>  mime-node.c |  7 -
>  util/crypto.c   | 89 
> -
>  util/crypto.h   |  6 ++--
>  5 files changed, 76 insertions(+), 49 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 79eb3d69..82a3d463 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -413,6 +413,12 @@ notmuch_status_to_string (notmuch_status_t status)
>   return "Operation requires a database upgrade";
>  case NOTMUCH_STATUS_PATH_ERROR:
>   return "Path supplied is illegal for this function";
> +case NOTMUCH_STATUS_MALFORMED_CRYPTO_PROTOCOL:
> + return "Crypto protocol missing, malformed, or unintelligible";
> +case NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION:
> + return "Crypto engine initialization failure";
> +case NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL:
> + return "Unknown crypto protocol";
>  default:
>  case NOTMUCH_STATUS_LAST_STATUS:
>   return "Unknown error status value";
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index f26565f3..6c76fb40 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -191,6 +191,23 @@ typedef enum _notmuch_status {
>   * function, in a way not covered by a more specific argument.
>   */
>  NOTMUCH_STATUS_ILLEGAL_ARGUMENT,
> +/**
> + * A MIME object claimed to have cryptographic protection which
> + * notmuch tried to handle, but the protocol was not specified in
> + * an intelligible way.
> + */
> +NOTMUCH_STATUS_MALFORMED_CRYPTO_PROTOCOL,
> +/**
> + * Notmuch attempted to do crypto processing, but could not
> + * initialize the engine needed to do so.
> + */
> +NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION,
> +/**
> + * A MIME object claimed to have cryptographic protection, and
> + * notmuch attempted to process it, but the specific protocol was
> + * something that notmuch doesn't know how to handle.
> + */
> +NOTMUCH_STATUS_UNKNOWN_CRYPTO_PROTOCOL,
>  /**
>   * Not an actual status value. Just a way to find out how many
>   * valid status values there are.
> diff --git a/mime-node.c b/mime-node.c
> index d9ff7de1..6cd7d2de 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -265,7 +265,12 @@ _mime_node_create (mime_node_t *parent, GMimeObject 
> *part)
>   || (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
>   GMimeContentType *content_type = g_mime_object_get_content_type (part);
>   const char *protocol = g_mime_content_type_get_parameter (content_type, 
> "protocol");
> - cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, 
> protocol);
> + notmuch_status_t status;
> + status = _notmuch_crypto_get_gmime_ctx_for_protocol (node->ctx->crypto,
> +  protocol, 
> );
> + if (status) /* this is a warning, not an error */
> + fprintf (stderr, "Warning: %s (%s).\n", notmuch_status_to_string 
> (status),
> +  protocol ? protocol : "(NULL)");

For NULL protocol this will print "((NULL))".

>   if (!cryptoctx)
>   return NULL;

I guess this will work because we initialize cryptoctx to NULL, but if
we return the status, I think we should trust status == success means
cryptoctx is fine, and otherwise we shouldn't touch or look at
cryptoctx.

Other than that, LGTM.


BR,
Jani.

>  }
> diff --git a/util/crypto.c b/util/crypto.c
> index 97e8c8f4..e7908197 100644
> --- a/util/crypto.c
> +++ b/util/crypto.c
> @@ -27,86 +27,86 @@
>  #define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
>  
>  #if (GMIME_MAJOR_VERSION < 3)
> -/* Create a GPG context (GMime 2.6) */
> -static GMimeCryptoContext*
> -create_gpg_context (_notmuch_crypto_t *crypto)
> +/* Create or pass on a GPG context (GMime 2.6) */
> +static notmuch_status_t
> +get_gpg_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
>  {
> -GMimeCryptoContext *gpgctx;
> +if (ctx == NULL || crypto == NULL)
> + return NOTMUCH_STATUS_NULL_POINTER;
>  
>  if (crypto->gpgctx) {
> - return crypto->gpgctx;
> + *ctx = crypto->gpgctx;
> + return NOTMUCH_STATUS_SUCCESS;
>  }
>  
>  /* TODO: GMimePasswordRequestFunc */
> -gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath 
> : "gpg");
> -if (! gpgctx) {
> - fprintf (stderr, "Failed to construct gpg context.\n");
> - return NULL;
> +crypto->gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? 
> crypto->gpgpath : "gpg");
> +if (! crypto->gpgctx) {
> + return NOTMUCH_STATUS_FAILED_CRYPTO_CONTEXT_CREATION;
>  }
> -crypto->gpgctx = gpgctx;
>  
> -

Re: [PATCH v2 01/10] crypto: Move crypto.c into libutil

2017-09-23 Thread Jani Nikula 
On Fri, 15 Sep 2017, Daniel Kahn Gillmor  wrote:
> This prepares us for using the crypto object in both the library and
> the client.
>
> i've prefixed notmuch_crypto with _ to indicate that while this can be
> built into the library when needed, it's not something to be exported
> or used externally.

You know, this would be considerably easier to review if this were split
to separate patches:

- prefixing notmuch_crypto_t and friends with _
- dropping notmuch_crypto_context_t in favour of using
  GMimeCryptoContext directly
- moving the stuff to util
- changing the notmuch_crypto_cleanup() return type

I think the patch is fine, but I'd have much more confidence in each
individual patch if this were split up than I have in everything
together.

BR,
Jani.

> ---
>  Makefile.local|  1 -
>  mime-node.c   | 12 ++--
>  notmuch-client.h  | 27 +++
>  notmuch-reply.c   |  2 +-
>  notmuch-show.c|  2 +-
>  util/Makefile.local   |  2 +-
>  crypto.c => util/crypto.c | 45 +
>  util/crypto.h | 31 +++
>  8 files changed, 68 insertions(+), 54 deletions(-)
>  rename crypto.c => util/crypto.c (79%)
>  create mode 100644 util/crypto.h
>
> diff --git a/Makefile.local b/Makefile.local
> index 9d9c52c2..9505b7fe 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -246,7 +246,6 @@ notmuch_client_srcs = \
>   sprinter-text.c \
>   query-string.c  \
>   mime-node.c \
> - crypto.c\
>   tag-util.c
>  
>  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
> diff --git a/mime-node.c b/mime-node.c
> index 24d73afa..d9ff7de1 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,7 +33,7 @@ typedef struct mime_node_context {
>  GMimeMessage *mime_message;
>  
>  /* Context provided by the caller. */
> -notmuch_crypto_t *crypto;
> +_notmuch_crypto_t *crypto;
>  } mime_node_context_t;
>  
>  static int
> @@ -56,7 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>  
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> - notmuch_crypto_t *crypto, mime_node_t **root_out)
> + _notmuch_crypto_t *crypto, mime_node_t **root_out)
>  {
>  const char *filename = notmuch_message_get_filename (message);
>  mime_node_context_t *mctx;
> @@ -171,7 +171,7 @@ set_signature_list_destructor (mime_node_t *node)
>  /* Verify a signed mime node (GMime 2.6) */
>  static void
>  node_verify (mime_node_t *node, GMimeObject *part,
> -  g_mime_3_unused(notmuch_crypto_context_t *cryptoctx))
> +  g_mime_3_unused(GMimeCryptoContext *cryptoctx))
>  {
>  GError *err = NULL;
>  
> @@ -192,7 +192,7 @@ node_verify (mime_node_t *node, GMimeObject *part,
>  /* Decrypt and optionally verify an encrypted mime node (GMime 2.6) */
>  static void
>  node_decrypt_and_verify (mime_node_t *node, GMimeObject *part,
> -  g_mime_3_unused(notmuch_crypto_context_t *cryptoctx))
> +  g_mime_3_unused(GMimeCryptoContext *cryptoctx))
>  {
>  GError *err = NULL;
>  GMimeDecryptResult *decrypt_result = NULL;
> @@ -227,7 +227,7 @@ static mime_node_t *
>  _mime_node_create (mime_node_t *parent, GMimeObject *part)
>  {
>  mime_node_t *node = talloc_zero (parent, mime_node_t);
> -notmuch_crypto_context_t *cryptoctx = NULL;
> +GMimeCryptoContext *cryptoctx = NULL;
>  
>  /* Set basic node properties */
>  node->part = part;
> @@ -265,7 +265,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>   || (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
>   GMimeContentType *content_type = g_mime_object_get_content_type (part);
>   const char *protocol = g_mime_content_type_get_parameter (content_type, 
> "protocol");
> - cryptoctx = notmuch_crypto_get_context (node->ctx->crypto, protocol);
> + cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, 
> protocol);
>   if (!cryptoctx)
>   return NULL;
>  }
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 9d0f367d..76e69501 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -31,10 +31,6 @@
>  
>  #include "gmime-extra.h"
>  
> -typedef GMimeCryptoContext notmuch_crypto_context_t;
> -/* This is automatically included only since gmime 2.6.10 */
> -#include 
> -
>  #include "notmuch.h"
>  
>  /* This is separate from notmuch-private.h because we're trying to
> @@ -54,6 +50,7 @@ typedef GMimeCryptoContext notmuch_crypto_context_t;
>  #include 
>  
>  #include "talloc-extra.h"
> +#include "crypto.h"
>  
>  #define unused(x) x __attribute__ ((unused))
>  
> @@ -71,22 +68,12 @@ typedef struct notmuch_show_format {
> const struct notmuch_show_params *params);
>  } notmuch_show_format_t;
>

Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-09-23 Thread Arun Isaac 

Mark Walters  writes:

> Alternatively, does just using / as the folder work?

notmuch complains about absolute paths if / is used as the folder. Look
at function `notmuch-maildir-add-notmuch-insert-style-fcc-header' in
notmuch-maildir-fcc.el.

David Bremner  writes:

> Do you happen to know if it calls with an empty string as the folder
> name? It would be consistent with searching for that to insert at the
> top level.

Setting notmuch-fcc-dirs to "\"\"" fails, if that's what you are asking.

I also tried "." as the folder. This works, but if I search using
`notmuch search --output=files ...', the filename is of the form
"database.path/./cur/some.mail.file". This is not so clean.

> In any case I've noted your feature request/bug-report. It doesn't sound
> terribly difficult to change, but it will need someone motivated to
> think about all of the related details like updating the test suite and
> changing docstrings.

I can contribute this patch. Shall I make it such that if the folder
part of `notmuch-fcc-dirs' is an empty string, then `notmuch insert' is
called without a `--folder=' argument?
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] fix reference to notmuch_message_get_properties

2017-09-23 Thread Daniel Kahn Gillmor 
---
 lib/notmuch.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 34bf5899..cbde6a93 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1873,7 +1873,7 @@ notmuch_message_get_properties (notmuch_message_t 
*message, const char *key, not
  * says. Whereas when this function returns FALSE, calling any of
  * these functions results in undefined behaviour.
  *
- * See the documentation of notmuch_message_properties_get for example
+ * See the documentation of notmuch_message_get_properties for example
  * code showing how to iterate over a notmuch_message_properties_t
  * object.
  *
-- 
2.14.1

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-09-23 Thread Mark Walters 

On Sat, 23 Sep 2017, David Bremner  wrote:
> Arun Isaac  writes:
>
>> I am using notmuch-emacs, and I would like to Fcc my sent mail to the
>> top-level directory given by database.path, instead of using a folder
>> like "sent" relative to the top-level directory.
>>
>> The function `notmuch-maildir-notmuch-insert-current-buffer' in
>> notmuch-maildir-fcc.el calls `notmuch insert' with the `--folder='
>> argument. I would like it to not do this.
>
> Do you happen to know if it calls with an empty string as the folder
> name? It would be consistent with searching for that to insert at the
> top level.
>
> In any case I've noted your feature request/bug-report. It doesn't sound
> terribly difficult to change, but it will need someone motivated to
> think about all of the related details like updating the test suite and
> changing docstrings.

Alternatively, does just using / as the folder work?

Best wishes

Mark

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: notmuch-emacs: Fcc to top-level directory given by database.path

2017-09-23 Thread David Bremner 
Arun Isaac  writes:

> I am using notmuch-emacs, and I would like to Fcc my sent mail to the
> top-level directory given by database.path, instead of using a folder
> like "sent" relative to the top-level directory.
>
> The function `notmuch-maildir-notmuch-insert-current-buffer' in
> notmuch-maildir-fcc.el calls `notmuch insert' with the `--folder='
> argument. I would like it to not do this.

Do you happen to know if it calls with an empty string as the folder
name? It would be consistent with searching for that to insert at the
top level.

In any case I've noted your feature request/bug-report. It doesn't sound
terribly difficult to change, but it will need someone motivated to
think about all of the related details like updating the test suite and
changing docstrings.

d


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

notmuch-emacs: Fcc to top-level directory given by database.path

2017-09-23 Thread Arun Isaac 

I am using notmuch-emacs, and I would like to Fcc my sent mail to the
top-level directory given by database.path, instead of using a folder
like "sent" relative to the top-level directory.

The function `notmuch-maildir-notmuch-insert-current-buffer' in
notmuch-maildir-fcc.el calls `notmuch insert' with the `--folder='
argument. I would like it to not do this.

For my immediate purposes, I have overridden
`notmuch-maildir-notmuch-insert-current-buffer' with the following
advice. But, it would be nice if this feature existed out of the box.

(define-advice notmuch-maildir-notmuch-insert-current-buffer
  (:override (folder  create tags)
 insert-into-root)
(apply 'notmuch-call-notmuch-process
   :stdin-string (buffer-string)
   "insert" tags))
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Is there a prefix that performs from: and to: simultaneously?

2017-09-23 Thread Attic Hermit 
> I'm not sure it's worth the extra complexity.
Maybe you're right. I'll take the option, combinding `from' and `to'
headers to get the jobs done.

Thanks!

-- 
Attic Hermit
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch