Re: [PATCH v2 03/17] test: new test framework to compare json parts
Daniel Kahn Gillmor writes: > + > +Value test: test that object in json data found at address is equal to > specified value: > + > + label:address|value > + should this maybe be label:address=value ? At least looking that the regex and the examples. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 06/17] cli/show: add information about which headers were protected
Daniel Kahn Gillmor writes: > The header-mask member of the per-message crypto object allows a > clever UI frontend to mark whether a header was protected (or not). > And if it was protected, it contains enough information to show useful > detail to an interested user. For example, an MUA could offer a "show > what this message's Subject looked like on the wire" feature in expert > mode. > > As before, we only handle Subject for now, but we might be able to > handle other headers in the future. > > Signed-off-by: Daniel Kahn Gillmor > --- > devel/schemata | 6 ++ > notmuch-show.c | 21 + > test/T356-protected-headers.sh | 4 ++-- > 3 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/devel/schemata b/devel/schemata > index 72feb7b7..9d3c8d30 100644 > --- a/devel/schemata > +++ b/devel/schemata > @@ -88,9 +88,15 @@ crypto = { >status: sigstatus, ># was the set of signatures described under encrypted > cover? >encrypted: bool, > + # which of the headers is covered by sigstatus? > + headers: [header_name*] > }, > decrypted?: { >status: msgdecstatus, > + # map encrypted headers that differed from the outside > headers. > + # the value of each item in the map is what that field > showed externally > + # (maybe null if it was not present in the external > headers). > + header-mask: { header_name: string|null,*} > } I think you also need to add a definition for header_name to schemata (in the same way that messageid is defined as a string). The name "header-mask" is a bit generic, but I don't have my head in this topic like you do. I was thinking of something like "replaced-headers", but it's only a mild suggestion. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 10/17] indexing: record protected subject when indexing cleartext
Daniel Kahn Gillmor writes: > +status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, > GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT); > +_index_mime_part (message, indexopts, clear, msg_crypto); > g_object_unref (clear); If you're going to ignore the return value here (not sure if that's a good idea?) please explicitly cast to void rather than putting in status to ignore. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] NEWS: News for my changes for 0.29
These are pretty terse overall, and could be expanded in future commits. --- NEWS | 37 + 1 file changed, 37 insertions(+) diff --git a/NEWS b/NEWS index d8aa272f..b852efec 100644 --- a/NEWS +++ b/NEWS @@ -1,12 +1,33 @@ Notmuch 0.29 (UNRELEASED) = +General +--- + +Add "body:" field to allow searching for terms that occur only in the +message body. Users will need to reindex their mail to take advantage +of this feature. + +Add support for indexing user specified headers (e.g. List-Id). See +notmuch-config(1) for details. This requires reindexing after changing +the set of headers to be indexed. + +Fix bug for searching in some headers for Xapian keywords in quoted +strings. + +Add support for gzip compressed mail messages (/not/ multi-message +mboxes); e.g. `gzip -9 $MAIL/archive/giant-message && notmuch new` +should work. Note that maildir flag syncing for gzipped messages is +currently untested. + Command Line Interface -- `notmuch show` now supports --body=false and --include-html with --format=text +Fix several performance problems with `notmuch reindex`. + Emacs - @@ -15,6 +36,22 @@ The minimum supported major version of Emacs is now 24. Support for GNU Emacs older than 25.1 is deprecated with this release, and may be removed in a future release. +Notmuch-emacs documentation is somewhat expanded. More contributions +are very welcome. + +Build System + + +Notmuch release tarballs are now compressed with `xz`. + +We now provide conventional detached signatures of the release +tarballs in addition to the signed `sha256sum` files. + +Dependencies + + +Support for GMime 2.6 is removed. + Notmuch 0.28.4 (2019-05-05) === -- 2.20.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 03/17] test: new test framework to compare json parts
On Mon, May 27 2019, David Bremner wrote: > Daniel Kahn Gillmor writes: >> + >> +Value test: test that object in json data found at address is equal to >> specified value: >> + >> + label:address|value >> + > > should this maybe be label:address=value ? At least looking that the > regex and the examples. This description is wrong, the comparison actually uses '=' in the code (and in the example). I'll fix the usage. jamie. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 06/17] cli/show: add information about which headers were protected
On Mon, May 27 2019, David Bremner wrote: > The name "header-mask" is a bit generic, but I don't have my head in > this topic like you do. I was thinking of something like > "replaced-headers", but it's only a mild suggestion. I think the point is that the headers are more accurately "masked" than "replaced", since you can look under the hood and recover what the original header was. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 06/17] cli/show: add information about which headers were protected
"Rollins, Jameson" writes: > On Mon, May 27 2019, David Bremner wrote: >> The name "header-mask" is a bit generic, but I don't have my head in >> this topic like you do. I was thinking of something like >> "replaced-headers", but it's only a mild suggestion. > > I think the point is that the headers are more accurately "masked" than > "replaced", since you can look under the hood and recover what the > original header was. Yes, I see what you mean. It's just that for me a "mask" brings to mind bitwise operations. I guess I'd prefer "masked-headers" to "header-mask", but if the two of you are convinced then I won't block it. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3] test: new test framework to compare json parts
From: Jameson Graef Rollins This makes it easier to write fairly compact, readable tests of json output, without needing to sanitize away parts that we don't care about. Signed-off-by: Daniel Kahn Gillmor --- test/json_check_nodes.py | 114 +++ test/test-lib.sh | 24 + 2 files changed, 138 insertions(+) create mode 100755 test/json_check_nodes.py diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py new file mode 100755 index ..17403c57 --- /dev/null +++ b/test/json_check_nodes.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python +import re +import sys +import json + + +EXPR_RE = re.compile('(?P[a-zA-Z0-9_-]+):(?P[^=!]+)(?:(?P[=!])(?P.*))?', re.DOTALL|re.MULTILINE) + + +if len(sys.argv) < 2: +sys.exit('usage: '+ sys.argv[0] + """ EXPR [EXPR] + +Takes json data on stdin and evaluates test expressions specified in +arguments. Each test is evaluated, and output is printed only if the +test fails. If any test fails the return value of execution will be +non-zero. + +EXPR can be one of following types: + +Value test: test that object in json data found at address is equal to +specified value: + + label:address=value + +Existence test: test that dict or list in json data found at address +does *not* contain the specified key: + + label:address!key + +Extract: extract object from json data found at address and print + + label:address + +Results are printed to stdout prefixed by expression label. In all +cases the test will fail if object does not exist in data. + +Example: + +0 $ echo '["a", "b", {"c": 1}]' | python3 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' +second_d: value not equal: data[1] = 'b' != 'd' +no_c: dict contains key: data[2]["c"] = 1 +1 $ + +""") + + +# parse expressions from arguments +exprs = [] +for expr in sys.argv[1:]: +m = re.match(EXPR_RE, expr) +if not m: +sys.exit("Invalid expression: {}".format(expr)) +exprs.append(m) + +data = json.load(sys.stdin) + +fail = False + +for expr in exprs: +# print(expr.groups(),fail) + +e = 'data{}'.format(expr.group('address')) +try: +val = eval(e) +except SyntaxError: +fail = True +print("{}: syntax error on evaluation of object: {}".format( +expr.group('label'), e)) +continue +except: +fail = True +print("{}: object not found: data{}".format( +expr.group('label'), expr.group('address'))) +continue + +if expr.group('type') == '=': +try: +obj_val = json.loads(expr.group('val')) +except: +fail = True +print("{}: error evaluating value: {}".format( +expr.group('label'), expr.group('address'))) +continue +if val != obj_val: +fail = True +print("{}: value not equal: data{} = {} != {}".format( +expr.group('label'), expr.group('address'), repr(val), repr(obj_val))) + +elif expr.group('type') == '!': +if not isinstance(val, (dict, list)): +fail = True +print("{}: not a dict or a list: data{}".format( +expr.group('label'), expr.group('address'))) +continue +try: +idx = json.loads(expr.group('val')) +if idx in val: +fail = True +print("{}: {} contains key: {}[{}] = {}".format( +expr.group('label'), type(val).__name__, e, expr.group('val'), val[idx])) +except SyntaxError: +fail = True +print("{}: syntax error on evaluation of value: {}".format( +expr.group('label'), expr.group('val'))) +continue + + +elif expr.group('type') is None: +print("{}: {}".format(expr.group('label'), val)) + + +if fail: +sys.exit(1) +sys.exit(0) diff --git a/test/test-lib.sh b/test/test-lib.sh index ff18fae6..616cb674 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -507,6 +507,30 @@ test_sort_json () { "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)" } +# test for json objects: +# read the source of test/json_check_nodes.py (or the output when +# invoking it without arguments) for an explanation of the syntax. +test_json_nodes () { +exec 1>&6 2>&7 # Restore stdout and stderr + if [ -z "$inside_subtest" ]; then + error "bug in the test script: test_json_eval without test_begin_subtest" + fi + inside_subtest= + test "$#" > 0 || + error "bug in the test script: test_json_nodes needs at least 1 parameter" + + if ! test_skip "$test_subtest_name" + then + output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON "$TEST_DIRECTORY"/json_check_nodes.py "$@") + if [ "$?" = 0 ] + then + test_ok_ + else + test_failure_ "$output" +
Re: [PATCH v2 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected
On Sun, May 26 2019, Daniel Kahn Gillmor wrote: > diff --git a/test/T358-emacs-protected-headers.sh > b/test/T358-emacs-protected-headers.sh > new file mode 100755 > index ..56ac06ca > --- /dev/null > +++ b/test/T358-emacs-protected-headers.sh > @@ -0,0 +1,36 @@ > +#!/usr/bin/env bash > + > +test_description="emacs interface" Rather generic test description, but since the output includes the test name, which gives more context, it's probably ok. jamie. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Protected Headers (2nd major revision, more testing!)
On Sun, May 26 2019, Daniel Kahn Gillmor wrote: > Way back in id:20180511055544.13676-1-...@fifthhorseman.net, i > proposed support for protected headers (in particular, for being able > to read and search for subject lines of encrypted messages which > protect the Subject). Although that series was reviewed by Bremner, i > never managed to get it in shape for merging. > > This is a revision of that series, applied against the current master, > having taken into account those reviews and the current state of the > notmuch codebase. I'm hoping that we can get it into 0.29 before the > feature freeze. This series looks good to me. All tests pass for me, and it behaves as expected on my existing emails that include protected headers. Very good test coverage. I don't see any issues with the code on a fairly cursory code review. Would be great to get this in before the 0.29 release. Looking forward to future autocrypt support based on this... jamie. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 03/17] test: new test framework to compare json parts
From: Jameson Graef Rollins This makes it easier to write fairly compact, readable tests of json output, without needing to sanitize away parts that we don't care about. Signed-off-by: Daniel Kahn Gillmor --- test/json_check_nodes.py | 113 +++ test/test-lib.sh | 24 + 2 files changed, 137 insertions(+) create mode 100755 test/json_check_nodes.py diff --git a/test/json_check_nodes.py b/test/json_check_nodes.py new file mode 100755 index ..a622969a --- /dev/null +++ b/test/json_check_nodes.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python +import re +import sys +import json + + +EXPR_RE = re.compile('(?P[a-zA-Z0-9_-]+):(?P[^=!]+)(?:(?P[=!])(?P.*))?', re.DOTALL|re.MULTILINE) + + +if len(sys.argv) < 2: +sys.exit('usage: '+ sys.argv[0] + """ EXPR [EXPR] + +Takes json data on stdin and evaluates test expressions specified in +arguments. Each test is evaluated, and output is printed only if the +test fails. If any test fails the return value of execution will be +non-zero. + +EXPR can be one of following types: + +Value test: test that object in json data found at address is equal to specified value: + + label:address=value + +Existence test: test that dict or list in json data found at address +does *not* contain the specified key: + + label:address!key + +Extract: extract object from json data found at address and print + + label:address + +Results are printed to stdout prefixed by expression label. In all +cases the test will fail if object does not exist in data. + +Example: + +0 $ echo '["a", "b", {"c": 1}]' | python3 json_check_nodes.py 'second_d:[1]="d"' 'no_c:[2]!"c"' +second_d: value not equal: data[1] = 'b' != 'd' +no_c: dict contains key: data[2]["c"] = "c" +1 $ + +""") + + +# parse expressions from arguments +exprs = [] +for expr in sys.argv[1:]: +m = re.match(EXPR_RE, expr) +if not m: +sys.exit("Invalid expression: {}".format(expr)) +exprs.append(m) + +data = json.load(sys.stdin) + +fail = False + +for expr in exprs: +# print(expr.groups(),fail) + +e = 'data{}'.format(expr.group('address')) +try: +val = eval(e) +except SyntaxError: +fail = True +print("{}: syntax error on evaluation of object: {}".format( +expr.group('label'), e)) +continue +except: +fail = True +print("{}: object not found: data{}".format( +expr.group('label'), expr.group('address'))) +continue + +if expr.group('type') == '=': +try: +obj_val = json.loads(expr.group('val')) +except: +fail = True +print("{}: error evaluating value: {}".format( +expr.group('label'), expr.group('address'))) +continue +if val != obj_val: +fail = True +print("{}: value not equal: data{} = {} != {}".format( +expr.group('label'), expr.group('address'), repr(val), repr(obj_val))) + +elif expr.group('type') == '!': +if not isinstance(val, (dict, list)): +fail = True +print("{}: not a dict or a list: data{}".format( +expr.group('label'), expr.group('address'))) +continue +try: +idx = json.loads(expr.group('val')) +if idx in val: +fail = True +print("{}: {} contains key: {}[{}] = {}".format( +expr.group('label'), type(val), e, expr.group('val'), val[idx])) +except SyntaxError: +fail = True +print("{}: syntax error on evaluation of value: {}".format( +expr.group('label'), expr.group('val'))) +continue + + +elif expr.group('type') is None: +print("{}: {}".format(expr.group('label'), val)) + + +if fail: +sys.exit(1) +sys.exit(0) diff --git a/test/test-lib.sh b/test/test-lib.sh index ff18fae6..616cb674 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -507,6 +507,30 @@ test_sort_json () { "import sys, json; json.dump(sorted(json.load(sys.stdin)),sys.stdout)" } +# test for json objects: +# read the source of test/json_check_nodes.py (or the output when +# invoking it without arguments) for an explanation of the syntax. +test_json_nodes () { +exec 1>&6 2>&7 # Restore stdout and stderr + if [ -z "$inside_subtest" ]; then + error "bug in the test script: test_json_eval without test_begin_subtest" + fi + inside_subtest= + test "$#" > 0 || + error "bug in the test script: test_json_nodes needs at least 1 parameter" + + if ! test_skip "$test_subtest_name" + then + output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON "$TEST_DIRECTORY"/json_check_nodes.py "$@") + if [ "$?" = 0 ] + then + test_ok_ + else + test_failure_ "$output" +
[PATCH v3 06/17] cli/show: add information about which headers were protected
This allows a clever UI frontend to mark whether a header was protected (or not), and if it was protected, to show the details to an interested user. As before, we only handle Subject for now, but we might be able to handle other headers in the future. Signed-off-by: Daniel Kahn Gillmor --- devel/schemata | 9 + notmuch-show.c | 21 + test/T356-protected-headers.sh | 4 ++-- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/devel/schemata b/devel/schemata index 72feb7b7..072b8d39 100644 --- a/devel/schemata +++ b/devel/schemata @@ -48,6 +48,9 @@ threadid = string # Message ID, sans "id:" messageid = string +# E-mail header name, sans trailing colon, like "Subject" or "In-Reply-To" +header_name = string + notmuch show schema --- @@ -88,9 +91,15 @@ crypto = { status: sigstatus, # was the set of signatures described under encrypted cover? encrypted: bool, + # which of the headers is covered by sigstatus? + headers: [header_name*] }, decrypted?: { status: msgdecstatus, + # map encrypted headers that differed from the outside headers. + # the value of each item in the map is what that field showed externally + # (maybe null if it was not present in the external headers). + masked-headers: { header_name: string|null,*} } } diff --git a/notmuch-show.c b/notmuch-show.c index b1f6a4bb..583b87f6 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -645,6 +645,12 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->map_key (sp, "encrypted"); sp->boolean (sp, msg_crypto->signature_encrypted); } + if (msg_crypto->payload_subject) { + sp->map_key (sp, "headers"); + sp->begin_list (sp); + sp->string (sp, "Subject"); + sp->end (sp); + } sp->end (sp); } if (msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) { @@ -652,6 +658,21 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->begin_map (sp); sp->map_key (sp, "status"); sp->string (sp, msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL ? "full" : "partial"); + + if (msg_crypto->payload_subject) { + const char *subject = g_mime_message_get_subject GMIME_MESSAGE (node->part); + if (subject == NULL || strcmp (subject, msg_crypto->payload_subject)) { + /* protected subject differs from the external header */ + sp->map_key (sp, "masked-headers"); + sp->begin_map (sp); + sp->map_key (sp, "Subject"); + if (subject == NULL) + sp->null (sp); + else + sp->string (sp, subject); + sp->end (sp); + } + } sp->end (sp); } } diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh index 8a8fef6a..359e245f 100755 --- a/test/T356-protected-headers.sh +++ b/test/T356-protected-headers.sh @@ -22,7 +22,7 @@ test_json_nodes <<<"$output" \ test_begin_subtest "verify protected header is visible with decryption" output=$(notmuch show --decrypt=true --format=json id:protected-hea...@crypto.notmuchmail.org) test_json_nodes <<<"$output" \ -'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \ +'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "Subject Unavailable"}}}' \ 'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"' test_begin_subtest "misplaced protected headers should not be made visible during decryption" @@ -58,7 +58,7 @@ test_json_nodes <<<"$output" \ test_begin_subtest "verify nested message/rfc822 protected header is visible" output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-mess...@crypto.notmuchmail.org) test_json_nodes <<<"$output" \ -'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \ +'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "masked-headers": {"Subject": "Subject Unavailable"}}}' \ 'subject:[0][0][0]["headers"]["Subject"]="This is a message using draft-melnikov-smime-header-signing"' test_done -- 2.20.1
Re: [PATCH v2 06/17] cli/show: add information about which headers were protected
On Mon 2019-05-27 07:12:52 -0300, David Bremner wrote: > I think you also need to add a definition for header_name to schemata > (in the same way that messageid is defined as a string). thanks, done in v3, which you should see shortly. > The name "header-mask" is a bit generic, but I don't have my head in > this topic like you do. I was thinking of something like > "replaced-headers", but it's only a mild suggestion. i went through several variations on this, and settled finally on header-mask. I considered "masked-headers" and "replaced-headers" but ultimately discarded them because they were unclear as to whether the thing they were showing was the thing that was masked/replaced, or the thing that was doing the masking/replacing. I think that "header-mask" is unambiguous in indicating that its contents are the things that are doing the masking-replacing, so i'd prefer to stick with it. (though i'm willing to entertain other proposals that have the same lack of ambiguity) --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 10/17] indexing: record protected subject when indexing cleartext
When indexing the cleartext of an encrypted message, record any protected subject in the database, which should make it findable and visible in search. Signed-off-by: Daniel Kahn Gillmor --- lib/index.cc | 46 +++--- lib/message.cc | 8 ++ lib/notmuch-private.h | 4 +++ test/T356-protected-headers.sh | 16 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/lib/index.cc b/lib/index.cc index f216ae5d..2e0bacb1 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -367,13 +367,15 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part) static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *part); + GMimeMultipartEncrypted *part, + _notmuch_message_crypto_t *msg_crypto); /* 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) + GMimeObject *part, + _notmuch_message_crypto_t *msg_crypto) { GMimeStream *stream, *filter; GMimeFilter *discard_non_term_filter; @@ -403,6 +405,8 @@ _index_mime_part (notmuch_message_t *message, _notmuch_message_add_term (message, "tag", "encrypted"); for (i = 0; i < g_mime_multipart_get_count (multipart); i++) { + notmuch_status_t status; + GMimeObject *child; if (GMIME_IS_MULTIPART_SIGNED (multipart)) { /* Don't index the signature, but index its content type. */ if (i == GMIME_MULTIPART_SIGNED_SIGNATURE) { @@ -419,7 +423,8 @@ _index_mime_part (notmuch_message_t *message, g_mime_multipart_get_part (multipart, i)); if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) { _index_encrypted_mime_part(message, indexopts, - GMIME_MULTIPART_ENCRYPTED (part)); + GMIME_MULTIPART_ENCRYPTED (part), + msg_crypto); } else { if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) { _notmuch_database_log (notmuch_message_get_database (message), @@ -428,8 +433,13 @@ _index_mime_part (notmuch_message_t *message, } continue; } - _index_mime_part (message, indexopts, - g_mime_multipart_get_part (multipart, i)); + child = g_mime_multipart_get_part (multipart, i); + status = _notmuch_message_crypto_potential_payload (msg_crypto, child, part, i); + if (status) + _notmuch_database_log (notmuch_message_get_database (message), + "Warning: failed to mark the potential cryptographic payload (%s).\n", + notmuch_status_to_string (status)); + _index_mime_part (message, indexopts, child, msg_crypto); } return; } @@ -439,7 +449,7 @@ _index_mime_part (notmuch_message_t *message, mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part)); - _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message)); + _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto); return; } @@ -516,7 +526,8 @@ _index_mime_part (notmuch_message_t *message, static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *encrypted_data) + GMimeMultipartEncrypted *encrypted_data, + _notmuch_message_crypto_t *msg_crypto) { notmuch_status_t status; GError *err = NULL; @@ -553,6 +564,10 @@ _index_encrypted_mime_part (notmuch_message_t *message, return; } if (decrypt_result) { + status = _notmuch_message_crypto_successful_decryption (msg_crypto); + if (status) + _notmuch_database_log_append (notmuch, "failed to mark the message as decrypted (%s)\n", + notmuch_status_to_string (status)); if (get_sk) { status = notmuch_message_add_property (message, "session-key", g_mime_decrypt_result_get_session_key (decrypt_result)); @@ -562,7 +577,12 @@ _index_encrypted_mime_part (notmuch_message_t *message, } g_object_unref (decrypt_result); } -_index_mime_part (message, indexopts, clear); +status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_OB
_notmuch_database_log vs _notmuch_database_log_append [was: Re: [PATCH v2 10/17] indexing: record protected subject when indexing cleartext]
On Mon 2019-05-27 07:24:41 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> +status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, >> GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT); >> +_index_mime_part (message, indexopts, clear, msg_crypto); >> g_object_unref (clear); > > If you're going to ignore the return value here (not sure if that's a > good idea?) please explicitly cast to void rather than putting in > status to ignore. Good catch, thanks. I've logged the error with _notmuch_database_log_append in v3 of this patch, i hope that makes sense to you! i note that _index_encrypted_mime_part() itself uses an odd mixture of _notmuch_database_log_append and _notmuch_database_log, which maybe is a sign that more cleanup is due there. I confess i always get a bit confused about when to use one vs. the other, though. _log resets the database's status_string, while _log_append just appends to it. On IRC, you wrote "I'd say it makes sense to append for warnings", which is a plausible rule of thumb, but seems like it might not map to the intent of how we expect the status_string to be used -- is it for the caller of the library? or something else? For example, should every call into the library reset the status string, and then there would only be one _database_log() function? (i don't know whether that's feasible, or what it would mean for internal code that already calls exported functions directly). It'd probably be worthwhile for someone to do an audit of those uses and come up with some normalized way of handling this that we can clearly explain, because i think it's a bit unwieldy now. I'm writing this report with the intent of tagging this e-mail with notmuch::bug, in the hopes that someone interested in doing maintenance work will take this on as a future project :) --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: revision 3: easing access to the cryptographic envelope
On Sun, May 26 2019, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> On Sun 2019-05-26 09:01:46 -0300, David Bremner wrote: >>> Daniel Kahn Gillmor writes: >>> This is the third revision of the series originally posted at id:20190424183113.29242-1-...@fifthhorseman.net (revision 2 was at id:20190520032228.27420-1-...@fifthhorseman.net) This series addresses comments raised by David Bremner in his review. Thanks, Bremner! >>> >>> I pushed this version, with 4 minor whitespace fixups (inserted spaces >>> and/or deleted blank lines) that I missed in my previous review. >> >> Thanks for these fixes, Bremner. If you have a specific set of tooling >> that you use that i can adopt to catch those kinds of coding convention >> mishaps before submitting, i'd be happy to adopt it so things are >> "clean". Bonus points if it integrates into emacs via .dir-locals.el or >> something :P > > To be honest my main mechanism for catching those problems is Tomi > ;). I used to have simple trailing whitespace cleanup hook (with its problems). Since Now 2012 I've used ethan-wspace.el which does cleanups only initially clean content (and higlights if not -- my favorite feature). Like w/ notmuch I also use a bit different version: localhost$ diff -u vc/ext/ethan-wspace/lisp/ethan-wspace.el dotdir/elisp/ethan-wspace.el --- vc/ext/ethan-wspace/lisp/ethan-wspace.el 2019-05-27 23:32:26.625456294 +0300 +++ dotdir/elisp/ethan-wspace.el 2019-05-27 23:38:34.150404797 +0300 @@ -800,8 +800,7 @@ (setq require-final-newline nil)) (when indent-tabs-mode - (setq ethan-wspace-errors -(remove 'tabs ethan-wspace-errors))) + (ethan-wspace-highlight-tabs-mode)) (run-hooks 'ethan-wspace-errors-in-buffer-hook) (ethan-wspace-update-buffer) zsh: exit 1 diff -u ~/vc/ext/ethan-wspace/lisp/ethan-wspace.el (applied latest change from github just now, changed there 2019-05-21) > There is also a reasonable uncrustify configuration that I don't use > as often as I should, mainly because the baseline in various files is > not there. Perhaps if we did more some global whitespace cleanup this > would be more helpful. To try it out run > > % uncrustify -c devel/uncrustify.cfg --replace $files > > then > > % git diff > > to see what changed. > > % find -name .git -prune -o -type f -a -regex '.*[.]\(c\|h\|cc\)' -print > -exec uncrustify -c devel/uncrustify.cfg --replace {} \; > > yields a whopping 1933 insertions and 1903 deletions. Perhaps there are > some places where the uncrustify config needs tuning, but none of the > output looked crazy to me. Two things I noticed causing lots of changes > > 1) "!foo" -> "! foo" > 2) putting a leading * in front of multi-line comments Last time I played w/ uncrustify it was like 2014 -- and I recall there were things that uncrustify just could not do (more than above) -- like indenting differently than emacs would do (and IMO something ugly way). So I wonder if we ever can use uncrustify as tool to verify change formatting (i.e. identical output or FAIL)... ... perhaps newer uncrustify (latest 0.69, in 2014 version was 0.60) (with modified uncrustify.cfg) could help there (I also have done some experiments with "uncrustify postprocessor"; I should take some time to see how that works with latest tool...) > If we do decide to rip off the bandage, that will cause a certain amount > of rebasing pain for any patch series in flight; now (i.e after the > release) is actually a pretty good time from my point of view, but > others (e.g. you) might feel differently. If uncrustifying can be done (even partially), IMO it could be done; patch series authors just have to revisit their changes (keeping those live is their duty anyway). Tomi ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 03/17] test: new test framework to compare json parts
On Mon 2019-05-27 16:34:27 -0400, Daniel Kahn Gillmor wrote: > From: Jameson Graef Rollins > > This makes it easier to write fairly compact, readable tests of json > output, without needing to sanitize away parts that we don't care > about. woops, patches crossed in the ether! feel free to prefer the version directly from Jamie -- they should be the same thing :) --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected
We initially test only notmuch-search; tests for other functionality come in different patchsets later. Signed-off-by: Daniel Kahn Gillmor --- test/T358-emacs-protected-headers.sh | 36 1 file changed, 36 insertions(+) create mode 100755 test/T358-emacs-protected-headers.sh diff --git a/test/T358-emacs-protected-headers.sh b/test/T358-emacs-protected-headers.sh new file mode 100755 index ..5e97918f --- /dev/null +++ b/test/T358-emacs-protected-headers.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +test_description="protected headers in emacs interface" +. $(dirname "$0")/test-lib.sh || exit 1 + +# testing protected headers with emacs +add_gnupg_home +add_email_corpus protected-headers + +test_begin_subtest "notmuch-search should show not unindexed protected subject header in emacs" +test_emacs '(notmuch-search "id:protected-hea...@crypto.notmuchmail.org") + (notmuch-test-wait) + (test-output)' +cat
Re: [PATCH v3 14/17] test: ensure that protected headers appear in notmuch-emacs search as expected
On Mon 2019-05-27 17:35:44 -0400, Daniel Kahn Gillmor wrote: > We initially test only notmuch-search; tests for other functionality > come in different patchsets later. > > Signed-off-by: Daniel Kahn Gillmor sorry, this patch (v3 14/17) is a minor update to the protected header series, correcting a weak test suite title that jrollins pointed out. The v3 patch here was supposed to be in-thread, but i botched the --in-reply-to= when sending. hopefully this message will stitch the threads together. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
stitching threads (v3 14/17)
hm, it appears that notmuch-emacs sends duplicate References: headers during reply when i add that manually to the headers field during compose. and then when notmuch indexes a message, it only indexes the first References: header it finds. These are curious things i find as i try to stitch the threads together. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 06/17] cli/show: add information about which headers were protected
The header-mask member of the per-message crypto object allows a clever UI frontend to mark whether a header was protected (or not). And if it was protected, it contains enough information to show useful detail to an interested user. For example, an MUA could offer a "show what this message's Subject looked like on the wire" feature in expert mode. As before, we only handle Subject for now, but we might be able to handle other headers in the future. Signed-off-by: Daniel Kahn Gillmor --- devel/schemata | 9 + notmuch-show.c | 21 + test/T356-protected-headers.sh | 4 ++-- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/devel/schemata b/devel/schemata index 72feb7b7..c0f69e7e 100644 --- a/devel/schemata +++ b/devel/schemata @@ -48,6 +48,9 @@ threadid = string # Message ID, sans "id:" messageid = string +# E-mail header name, sans trailing colon, like "Subject" or "In-Reply-To" +header_name = string + notmuch show schema --- @@ -88,9 +91,15 @@ crypto = { status: sigstatus, # was the set of signatures described under encrypted cover? encrypted: bool, + # which of the headers is covered by sigstatus? + headers: [header_name*] }, decrypted?: { status: msgdecstatus, + # map encrypted headers that differed from the outside headers. + # the value of each item in the map is what that field showed externally + # (maybe null if it was not present in the external headers). + header-mask: { header_name: string|null,*} } } diff --git a/notmuch-show.c b/notmuch-show.c index b1f6a4bb..4dfe9c1d 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -645,6 +645,12 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->map_key (sp, "encrypted"); sp->boolean (sp, msg_crypto->signature_encrypted); } + if (msg_crypto->payload_subject) { + sp->map_key (sp, "headers"); + sp->begin_list (sp); + sp->string (sp, "Subject"); + sp->end (sp); + } sp->end (sp); } if (msg_crypto->decryption_status != NOTMUCH_MESSAGE_DECRYPTED_NONE) { @@ -652,6 +658,21 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->begin_map (sp); sp->map_key (sp, "status"); sp->string (sp, msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL ? "full" : "partial"); + + if (msg_crypto->payload_subject) { + const char *subject = g_mime_message_get_subject GMIME_MESSAGE (node->part); + if (subject == NULL || strcmp (subject, msg_crypto->payload_subject)) { + /* protected subject differs from the external header */ + sp->map_key (sp, "header-mask"); + sp->begin_map (sp); + sp->map_key (sp, "Subject"); + if (subject == NULL) + sp->null (sp); + else + sp->string (sp, subject); + sp->end (sp); + } + } sp->end (sp); } } diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh index 8a8fef6a..68d431e9 100755 --- a/test/T356-protected-headers.sh +++ b/test/T356-protected-headers.sh @@ -22,7 +22,7 @@ test_json_nodes <<<"$output" \ test_begin_subtest "verify protected header is visible with decryption" output=$(notmuch show --decrypt=true --format=json id:protected-hea...@crypto.notmuchmail.org) test_json_nodes <<<"$output" \ -'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \ +'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \ 'subject:[0][0][0]["headers"]["Subject"]="This is a protected header"' test_begin_subtest "misplaced protected headers should not be made visible during decryption" @@ -58,7 +58,7 @@ test_json_nodes <<<"$output" \ test_begin_subtest "verify nested message/rfc822 protected header is visible" output=$(notmuch show --decrypt=true --format=json id:nested-rfc822-mess...@crypto.notmuchmail.org) test_json_nodes <<<"$output" \ -'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full"}}' \ +'crypto:[0][0][0]["crypto"]={"decrypted": {"status": "full", "header-mask": {"Subject": "Subj
Re: [PATCH v3 06/17] cli/show: add information about which headers were protected
On Mon 2019-05-27 16:40:59 -0400, Daniel Kahn Gillmor wrote: > This allows a clever UI frontend to mark whether a header was > protected (or not), and if it was protected, to show the details to > an interested user. > > As before, we only handle Subject for now, but we might be able to > handle other headers in the future. bah, please excuse this version (v3), i'm juggling too many variant trees locally, and got confused. v4 should be the correct one, as it uses "header-mask", and not "masked-headers". I explained in id:87d0k34oik@fifthhorseman.net why i prefer "header-mask", and that preference still holds. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 10/17] indexing: record protected subject when indexing cleartext
On Mon 2019-05-27 17:17:20 -0400, Daniel Kahn Gillmor wrote: > When indexing the cleartext of an encrypted message, record any > protected subject in the database, which should make it findable and > visible in search. ugh, please ignore v3 of this patch (10/17) as well. v4 should be coming shortly. --dkg, juggling too many branches signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 10/17] indexing: record protected subject when indexing cleartext
When indexing the cleartext of an encrypted message, record any protected subject in the database, which should make it findable and visible in search. Signed-off-by: Daniel Kahn Gillmor --- lib/index.cc | 42 ++ lib/message.cc | 8 +++ lib/notmuch-private.h | 4 test/T356-protected-headers.sh | 16 + 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/lib/index.cc b/lib/index.cc index f216ae5d..1fd9e67e 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -367,13 +367,15 @@ _index_content_type (notmuch_message_t *message, GMimeObject *part) static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *part); + GMimeMultipartEncrypted *part, + _notmuch_message_crypto_t *msg_crypto); /* 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) + GMimeObject *part, + _notmuch_message_crypto_t *msg_crypto) { GMimeStream *stream, *filter; GMimeFilter *discard_non_term_filter; @@ -403,6 +405,8 @@ _index_mime_part (notmuch_message_t *message, _notmuch_message_add_term (message, "tag", "encrypted"); for (i = 0; i < g_mime_multipart_get_count (multipart); i++) { + notmuch_status_t status; + GMimeObject *child; if (GMIME_IS_MULTIPART_SIGNED (multipart)) { /* Don't index the signature, but index its content type. */ if (i == GMIME_MULTIPART_SIGNED_SIGNATURE) { @@ -419,7 +423,8 @@ _index_mime_part (notmuch_message_t *message, g_mime_multipart_get_part (multipart, i)); if (i == GMIME_MULTIPART_ENCRYPTED_CONTENT) { _index_encrypted_mime_part(message, indexopts, - GMIME_MULTIPART_ENCRYPTED (part)); + GMIME_MULTIPART_ENCRYPTED (part), + msg_crypto); } else { if (i != GMIME_MULTIPART_ENCRYPTED_VERSION) { _notmuch_database_log (notmuch_message_get_database (message), @@ -428,8 +433,13 @@ _index_mime_part (notmuch_message_t *message, } continue; } - _index_mime_part (message, indexopts, - g_mime_multipart_get_part (multipart, i)); + child = g_mime_multipart_get_part (multipart, i); + status = _notmuch_message_crypto_potential_payload (msg_crypto, child, part, i); + if (status) + _notmuch_database_log (notmuch_message_get_database (message), + "Warning: failed to mark the potential cryptographic payload (%s).\n", + notmuch_status_to_string (status)); + _index_mime_part (message, indexopts, child, msg_crypto); } return; } @@ -439,7 +449,7 @@ _index_mime_part (notmuch_message_t *message, mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part)); - _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message)); + _index_mime_part (message, indexopts, g_mime_message_get_mime_part (mime_message), msg_crypto); return; } @@ -516,7 +526,8 @@ _index_mime_part (notmuch_message_t *message, static void _index_encrypted_mime_part (notmuch_message_t *message, notmuch_indexopts_t *indexopts, - GMimeMultipartEncrypted *encrypted_data) + GMimeMultipartEncrypted *encrypted_data, + _notmuch_message_crypto_t *msg_crypto) { notmuch_status_t status; GError *err = NULL; @@ -553,6 +564,10 @@ _index_encrypted_mime_part (notmuch_message_t *message, return; } if (decrypt_result) { + status = _notmuch_message_crypto_successful_decryption (msg_crypto); + if (status) + _notmuch_database_log_append (notmuch, "failed to mark the message as decrypted (%s)\n", + notmuch_status_to_string (status)); if (get_sk) { status = notmuch_message_add_property (message, "session-key", g_mime_decrypt_result_get_session_key (decrypt_result)); @@ -562,7 +577,8 @@ _index_encrypted_mime_part (notmuch_message_t *message, } g_object_unref (decrypt_result); } -_index_mime_part (message, indexopts, clear); +status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, GMIME_
Re: Protected Headers (2nd major revision, more testing!)
On Sun 2019-05-26 18:15:53 -0400, Daniel Kahn Gillmor wrote: > this series delivers a concrete improvement: users of notmuch can now > read, index, and search for the subject lines of encrypted messages > sent from MUAs like Enigmail and K-9 mail. Many thanks to jrollins and bremner for their reviews of this series. Apologies for my clumsy handling of the revisions for those changes, but i believe that the latest versions of these patches in this thread are now ready for merging. I've tagged the obsoleted versions appropriately with nmbug, so after "nmbug pull", this query should identify 17 messages with patches for this sequence (choose X for however your own installation identifies this series): tag:notmuch::patch and not tag:notmuch::obsolete and thread:XX i've left v4 of 06/17 and 10/17 tagged notmuch::needs-review because they represent changes under discussion and i don't want to claim that my resolution to the discussion matches Bremner's. This series is also applied on the "protected-headers" branch at https://gitlab.com/dkg/notmuch.git if anyone wants to compare what they've merged from the mailing list with how i've interpreted it. Please let me know if you have any other reviews. all the best, --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] NEWS: News for my changes for 0.29
On Mon 2019-05-27 07:46:55 -0300, David Bremner wrote: > +Dependencies > + > + > +Support for GMime 2.6 is removed. > + I'd add here: The minimum supported version of GMime is now 3.0.3. GMime also needs to have been compiled with cryptographic support. --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] NEWS: note parallel test suite
--- NEWS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/NEWS b/NEWS index 999affc7..60a69936 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,13 @@ The minimum supported major version of Emacs is now 24. Support for GNU Emacs older than 25.1 is deprecated with this release, and may be removed in a future release. +Test Suite +-- + +If either GNU parallel or moreutils parallel is installed, the tests +in the test suite will now be run in parallel (one per available +core). This can be disabled with NOTMUCH_TEST_SERIALIZE=1. + Notmuch 0.28.4 (2019-05-05) === -- 2.20.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] NEWS: include information about per-message cryptographic status
--- NEWS | 4 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index d8aa272f..999affc7 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ Command Line Interface `notmuch show` now supports --body=false and --include-html with --format=text +`notmuch show` and `notmuch reply` now emit per-message cryptographic +status in their json and sexp output formats. See devel/schemata for +more details about what is included there. + Emacs - -- 2.20.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 03/17] test: new test framework to compare json parts
On Mon, May 27 2019, Daniel Kahn Gillmor wrote: > On Mon 2019-05-27 16:34:27 -0400, Daniel Kahn Gillmor wrote: >> From: Jameson Graef Rollins >> >> This makes it easier to write fairly compact, readable tests of json >> output, without needing to sanitize away parts that we don't care >> about. > > woops, patches crossed in the ether! feel free to prefer the version > directly from Jamie -- they should be the same thing :) I actually fixed one other very minor thing in the script, so I would prefer mine. jamie. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch