unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] cli/insert: new message file can be world-readable (rely on umask)
@ 2018-02-05  4:37 Daniel Kahn Gillmor
  2018-02-05 11:59 ` Peter Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-05  4:37 UTC (permalink / raw)
  To: Notmuch Mail

There are legitimate cases (public archives) where a user might
actually want their archive to be readable to the world.

"notmuch insert" historically used mode 0600 (unreadable by group or
other), but that choice doesn't appear to have been specifically
justified (perhaps an abundance of caution?).

If the user wants "notmuch insert" to create files that are not
readable by group or other, they can set their umask more
restrictively.
---
 notmuch-insert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 48490b51..167005db 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -227,7 +227,7 @@ maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
 	    return -1;
 	}
 
-	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
+	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0644);
     } while (fd == -1 && errno == EEXIST);
 
     if (fd == -1) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-05  4:37 [PATCH] cli/insert: new message file can be world-readable (rely on umask) Daniel Kahn Gillmor
@ 2018-02-05 11:59 ` Peter Wang
  2018-02-06 19:43   ` [PATCH v2] " Daniel Kahn Gillmor
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Wang @ 2018-02-05 11:59 UTC (permalink / raw)
  To: Daniel Kahn Gillmor; +Cc: Notmuch Mail

On Sun,  4 Feb 2018 23:37:03 -0500, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> There are legitimate cases (public archives) where a user might
> actually want their archive to be readable to the world.
> 
> "notmuch insert" historically used mode 0600 (unreadable by group or
> other), but that choice doesn't appear to have been specifically
> justified (perhaps an abundance of caution?).

I can't remember any specific reason for 0600 instead of 0644.
Probably just assumed that mail is supposed to be private.

> If the user wants "notmuch insert" to create files that are not
> readable by group or other, they can set their umask more
> restrictively.

By calling notmuch through a wrapper shell script, I suppose.

The mode for --create-folder should be reconsidered as well.

Peter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-05 11:59 ` Peter Wang
@ 2018-02-06 19:43   ` Daniel Kahn Gillmor
  2018-02-09  1:40     ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-06 19:43 UTC (permalink / raw)
  To: Notmuch Mail

There are legitimate cases (public archives) where a user might
actually want their archive to be readable to the world.

"notmuch insert" historically used mode 0600 (unreadable by group or
other), but that choice doesn't appear to have been specifically
justified (perhaps an abundance of caution?).

This patch also adjusts the default mode used for --create-folder, to
be mode 0755 before the application of the umask.

If the user wants "notmuch insert" to create files or folders that are
not readable by group or other, they can set their umask more
restrictively.
---
 notmuch-insert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 48490b51..4f1116ed 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -162,7 +162,7 @@ static bool
 maildir_create_folder (const void *ctx, const char *maildir)
 {
     const char *subdirs[] = { "cur", "new", "tmp" };
-    const int mode = 0700;
+    const int mode = 0755;
     char *subdir;
     unsigned int i;
 
@@ -227,7 +227,7 @@ maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
 	    return -1;
 	}
 
-	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
+	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0644);
     } while (fd == -1 && errno == EEXIST);
 
     if (fd == -1) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-06 19:43   ` [PATCH v2] " Daniel Kahn Gillmor
@ 2018-02-09  1:40     ` Daniel Kahn Gillmor
  2018-02-09  1:52       ` Brian Sniffen
                         ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-09  1:40 UTC (permalink / raw)
  To: Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On Tue 2018-02-06 14:43:56 -0500, Daniel Kahn Gillmor wrote:
> There are legitimate cases (public archives) where a user might
> actually want their archive to be readable to the world.
>
> "notmuch insert" historically used mode 0600 (unreadable by group or
> other), but that choice doesn't appear to have been specifically
> justified (perhaps an abundance of caution?).
>
> This patch also adjusts the default mode used for --create-folder, to
> be mode 0755 before the application of the umask.
>
> If the user wants "notmuch insert" to create files or folders that are
> not readable by group or other, they can set their umask more
> restrictively.

I'm now having second thoughts about this.

postfix's local delivery agent has apparently been delivering with mode
0600 for nearly 20 years:

    https://github.com/vdukhovni/postfix/blame/master/postfix/src/local/maildir.c#L188
    
And dovecot's lda defaults to 0600 on delivery:

    https://sources.debian.org/src/dovecot/1:2.2.33.2-1/src/lib-storage/mail-storage.c/?hl=2591#L2591

So maybe there's something i don't know about why a delivery agent would
want to have this restrictive mask?

Perhaps a better way to fix this is with a new option to notmuch insert.

on IRC, bremner suggests something flexible like --mode=0600

I'm more inclined to keep it simpler and more usable (most people don't
know octal, let alone unix permissions bits) and just have a boolean
--world-readable which defaults to false (and switches between modes
0600 and 0644 for files, and 0700 and 0755 for directories).

Any thoughts?

    --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-09  1:40     ` Daniel Kahn Gillmor
@ 2018-02-09  1:52       ` Brian Sniffen
  2018-02-09  2:33         ` Daniel Kahn Gillmor
  2018-02-09  2:00       ` Daniel Kahn Gillmor
  2018-02-09  4:10       ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
  2 siblings, 1 reply; 10+ messages in thread
From: Brian Sniffen @ 2018-02-09  1:52 UTC (permalink / raw)
  To: Daniel Kahn Gillmor; +Cc: Notmuch Mail

If there’s a hidden danger in these modes, better to leave the switch requiring octal tunes!

-- 
Brian Sniffen

> On Feb 8, 2018, at 8:40 PM, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> 
>> On Tue 2018-02-06 14:43:56 -0500, Daniel Kahn Gillmor wrote:
>> There are legitimate cases (public archives) where a user might
>> actually want their archive to be readable to the world.
>> 
>> "notmuch insert" historically used mode 0600 (unreadable by group or
>> other), but that choice doesn't appear to have been specifically
>> justified (perhaps an abundance of caution?).
>> 
>> This patch also adjusts the default mode used for --create-folder, to
>> be mode 0755 before the application of the umask.
>> 
>> If the user wants "notmuch insert" to create files or folders that are
>> not readable by group or other, they can set their umask more
>> restrictively.
> 
> I'm now having second thoughts about this.
> 
> postfix's local delivery agent has apparently been delivering with mode
> 0600 for nearly 20 years:
> 
>    https://github.com/vdukhovni/postfix/blame/master/postfix/src/local/maildir.c#L188
> 
> And dovecot's lda defaults to 0600 on delivery:
> 
>    https://sources.debian.org/src/dovecot/1:2.2.33.2-1/src/lib-storage/mail-storage.c/?hl=2591#L2591
> 
> So maybe there's something i don't know about why a delivery agent would
> want to have this restrictive mask?
> 
> Perhaps a better way to fix this is with a new option to notmuch insert.
> 
> on IRC, bremner suggests something flexible like --mode=0600
> 
> I'm more inclined to keep it simpler and more usable (most people don't
> know octal, let alone unix permissions bits) and just have a boolean
> --world-readable which defaults to false (and switches between modes
> 0600 and 0644 for files, and 0700 and 0755 for directories).
> 
> Any thoughts?
> 
>    --dkg
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-09  1:40     ` Daniel Kahn Gillmor
  2018-02-09  1:52       ` Brian Sniffen
@ 2018-02-09  2:00       ` Daniel Kahn Gillmor
  2018-02-09  4:10       ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
  2 siblings, 0 replies; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-09  2:00 UTC (permalink / raw)
  To: Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

On Thu 2018-02-08 20:40:40 -0500, Daniel Kahn Gillmor wrote:

> postfix's local delivery agent has apparently been delivering with mode
> 0600 for nearly 20 years:
>
>     https://github.com/vdukhovni/postfix/blame/master/postfix/src/local/maildir.c#L188

and even postfix's master process (the one capable of spawning the local
delivery agent, which is ultimately responsible for dropping privileges
to the local user to execute commands in ~/.forward) starts off with a
umask(077):

    https://github.com/vdukhovni/postfix/blame/master/postfix/src/master/master.c#L278

this makes it pretty difficult to attempt safe simple world-readable
mail delivery through the MUA :(

Anyway, this is not on the critical path for me.  For the purposes of
mail delivery to the mailing list archive, i'm now considering just
writing a wrapper script around "notmuch insert" that (as the local
user) chmod on the files that are delivered with overly-restrictive
permissions.

This makes me nervous, because chmods are tricky to do safely,
especially in an automated fashion, but given the tight permissions
we're seeing during message delivery at the moment, this is the simplest
option.

Another option would be to write a mailman3 plugin that delivers to
notmuch, but that's a bigger task than i'm willing to take on right now.

I welcome other suggestions though!

     --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)
  2018-02-09  1:52       ` Brian Sniffen
@ 2018-02-09  2:33         ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-09  2:33 UTC (permalink / raw)
  To: Brian Sniffen; +Cc: Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Thu 2018-02-08 20:52:41 -0500, Brian Sniffen wrote:
> If there’s a hidden danger in these modes, better to leave the switch
> requiring octal tunes!

eh?  i'm not sure i understand the argument.  if there's a hidden
danger, we want them to really clearly say on the tin what the hidden
danger is.  i think --world-readable=true is a much clearer warning than
--mode=MYSTERYMEAT.  right?  what am i missing?

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3] cli/insert: add --world-readable flag
  2018-02-09  1:40     ` Daniel Kahn Gillmor
  2018-02-09  1:52       ` Brian Sniffen
  2018-02-09  2:00       ` Daniel Kahn Gillmor
@ 2018-02-09  4:10       ` Daniel Kahn Gillmor
  2018-02-15  6:08         ` Tomi Ollila
  2018-03-24 23:17         ` David Bremner
  2 siblings, 2 replies; 10+ messages in thread
From: Daniel Kahn Gillmor @ 2018-02-09  4:10 UTC (permalink / raw)
  To: Notmuch Mail

In some cases (e.g. when building a publicly-visible e-mail archive)
it doesn't make any sense to restrict visibility of the message to the
current user account.

This adds a --world-readable boolean option for "notmuch insert", so
that those who want to archive their mail publicly can feed their
archiver with:

    notmuch insert --world-readable

Other local delivery agents (postfix's local, and dovecot's lda) all
default to delivery in mode 0600 rather than relying on the user's
umask, so this fix doesn't change the default.

Also, this does not override the user's umask.  if the umask is
already set tight, it will not become looser as the result of passing
--world-readable.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 doc/man1/notmuch-insert.rst |  6 ++++++
 notmuch-insert.c            | 25 ++++++++++++++-----------
 test/T070-insert.sh         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 47884515..86e2f567 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -51,6 +51,12 @@ Supported options for **insert** include
 ``--no-hooks``
     Prevent hooks from being run.
 
+``--world-readable``
+    When writing mail to the mailbox, allow it to be read by users
+    other than the current user.  Note that this does not override
+    umask.  By default, delivered mail is only readable by the current
+    user.
+
 ``--decrypt=(true|nostash|auto|false)``
     If ``true`` and the message is encrypted, try to decrypt the
     message while indexing, stashing any session keys discovered.  If
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 48490b51..d229c9dc 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -159,10 +159,10 @@ mkdir_recursive (const void *ctx, const char *path, int mode)
  * otherwise. Partial results are not cleaned up on errors.
  */
 static bool
-maildir_create_folder (const void *ctx, const char *maildir)
+maildir_create_folder (const void *ctx, const char *maildir, bool world_readable)
 {
     const char *subdirs[] = { "cur", "new", "tmp" };
-    const int mode = 0700;
+    const int mode = (world_readable ? 0755 : 0700);
     char *subdir;
     unsigned int i;
 
@@ -211,10 +211,11 @@ tempfilename (const void *ctx)
  * is not touched).
  */
 static int
-maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
+maildir_mktemp (const void *ctx, const char *maildir, bool world_readable, char **path_out)
 {
     char *filename, *path;
     int fd;
+    const int mode = (world_readable ? 0644 : 0600);
 
     do {
 	filename = tempfilename (ctx);
@@ -227,7 +228,7 @@ maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
 	    return -1;
 	}
 
-	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
+	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode);
     } while (fd == -1 && errno == EEXIST);
 
     if (fd == -1) {
@@ -289,12 +290,12 @@ copy_fd (int fdout, int fdin)
  * the file, or NULL on errors.
  */
 static char *
-maildir_write_tmp (const void *ctx, int fdin, const char *maildir)
+maildir_write_tmp (const void *ctx, int fdin, const char *maildir, bool world_readable)
 {
     char *path;
     int fdout;
 
-    fdout = maildir_mktemp (ctx, maildir, &path);
+    fdout = maildir_mktemp (ctx, maildir, world_readable, &path);
     if (fdout < 0)
 	return NULL;
 
@@ -323,11 +324,11 @@ FAIL:
  * errors.
  */
 static char *
-maildir_write_new (const void *ctx, int fdin, const char *maildir)
+maildir_write_new (const void *ctx, int fdin, const char *maildir, bool world_readable)
 {
     char *cleanpath, *tmppath, *newpath, *newdir;
 
-    tmppath = maildir_write_tmp (ctx, fdin, maildir);
+    tmppath = maildir_write_tmp (ctx, fdin, maildir, world_readable);
     if (! tmppath)
 	return NULL;
     cleanpath = tmppath;
@@ -457,6 +458,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     bool create_folder = false;
     bool keep = false;
     bool hooks = true;
+    bool world_readable = false;
     bool synchronize_flags;
     char *maildir;
     char *newpath;
@@ -467,7 +469,8 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_string = &folder, .name = "folder", .allow_empty = true },
 	{ .opt_bool = &create_folder, .name = "create-folder" },
 	{ .opt_bool = &keep, .name = "keep" },
-	{ .opt_bool =  &hooks, .name = "hooks" },
+	{ .opt_bool = &hooks, .name = "hooks" },
+	{ .opt_bool = &world_readable, .name = "world-readable" },
 	{ .opt_inherit = notmuch_shared_indexing_options },
 	{ .opt_inherit = notmuch_shared_options },
 	{ }
@@ -523,7 +526,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     strip_trailing (maildir, '/');
-    if (create_folder && ! maildir_create_folder (config, maildir))
+    if (create_folder && ! maildir_create_folder (config, maildir, world_readable))
 	return EXIT_FAILURE;
 
     /* Set up our handler for SIGINT. We do not set SA_RESTART so that copying
@@ -535,7 +538,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     sigaction (SIGINT, &action, NULL);
 
     /* Write the message to the Maildir new directory. */
-    newpath = maildir_write_new (config, STDIN_FILENO, maildir);
+    newpath = maildir_write_new (config, STDIN_FILENO, maildir, world_readable);
     if (! newpath) {
 	return EXIT_FAILURE;
     }
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 40519bb2..05be473a 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -4,6 +4,10 @@ test_description='"notmuch insert"'
 
 test_require_external_prereq gdb
 
+# subtests about file permissions assume that we're working with umask
+# 022 by default.
+umask 022
+
 # Create directories and database before inserting.
 mkdir -p "$MAIL_DIR"/{cur,new,tmp}
 mkdir -p "$MAIL_DIR"/Drafts/{cur,new,tmp}
@@ -37,6 +41,9 @@ notmuch insert < "$gen_msg_filename"
 cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
 test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
 
+test_begin_subtest "Permissions on inserted message should be 0600"
+test_expect_equal "600" "$(stat -c %a "$cur_msg_filename")"
+
 test_begin_subtest "Insert message adds default tags"
 output=$(notmuch show --format=json "subject:insert-subject")
 expected='[[[{
@@ -73,6 +80,27 @@ notmuch insert +custom < "$gen_msg_filename"
 output=$(notmuch search --output=messages tag:custom)
 test_expect_equal "$output" "id:$gen_msg_id"
 
+test_begin_subtest "Insert tagged world-readable message"
+gen_insert_msg
+notmuch insert --world-readable +world-readable-test < "$gen_msg_filename"
+cur_msg_filename=$(notmuch search --output=files "tag:world-readable-test")
+test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
+
+test_begin_subtest "Permissions on inserted world-readable message should be 0644"
+test_expect_equal "644" "$(stat -c %a "$cur_msg_filename")"
+
+test_begin_subtest "Insert tagged world-readable message with group-only umask"
+oldumask=$(umask)
+umask 027
+gen_insert_msg
+notmuch insert --world-readable +world-readable-umask-test < "$gen_msg_filename"
+cur_msg_filename=$(notmuch search --output=files "tag:world-readable-umask-test")
+umask "$oldumask"
+test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
+
+test_begin_subtest "Permissions on inserted world-readable message with funny umask should be 0640"
+test_expect_equal "640" "$(stat -c %a "$cur_msg_filename")"
+
 test_begin_subtest "Insert message, add/remove tags"
 gen_insert_msg
 notmuch insert +custom -unread < "$gen_msg_filename"
@@ -170,6 +198,23 @@ output=$(notmuch search --output=files path:F/G/H/I/J/new tag:folder)
 basename=$(basename "$output")
 test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/new/${basename}"
 
+test_begin_subtest "Created subfolder should have permissions 0700"
+test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J")"
+test_begin_subtest "Created subfolder new/ should also have permissions 0700"
+test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/new")"
+
+test_begin_subtest "Insert message, create world-readable subfolder"
+gen_insert_msg
+notmuch insert --folder=F/G/H/I/J/K --create-folder --world-readable +folder-world-readable < "$gen_msg_filename"
+output=$(notmuch search --output=files path:F/G/H/I/J/K/new tag:folder-world-readable)
+basename=$(basename "$output")
+test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/K/new/${basename}"
+
+test_begin_subtest "Created world-readable subfolder should have permissions 0755"
+test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K")"
+test_begin_subtest "Created world-readable subfolder new/ should also have permissions 0755"
+test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K/new")"
+
 test_begin_subtest "Insert message, create existing subfolder"
 gen_insert_msg
 notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] cli/insert: add --world-readable flag
  2018-02-09  4:10       ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
@ 2018-02-15  6:08         ` Tomi Ollila
  2018-03-24 23:17         ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: Tomi Ollila @ 2018-02-15  6:08 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

On Thu, Feb 08 2018, Daniel Kahn Gillmor wrote:

> In some cases (e.g. when building a publicly-visible e-mail archive)
> it doesn't make any sense to restrict visibility of the message to the
> current user account.
>
> This adds a --world-readable boolean option for "notmuch insert", so
> that those who want to archive their mail publicly can feed their
> archiver with:
>
>     notmuch insert --world-readable
>
> Other local delivery agents (postfix's local, and dovecot's lda) all
> default to delivery in mode 0600 rather than relying on the user's
> umask, so this fix doesn't change the default.
>
> Also, this does not override the user's umask.  if the umask is
> already set tight, it will not become looser as the result of passing
> --world-readable.

Code looks good to me and tests ... also, +1

testing is too hard atm...

Tomi

>
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
>  doc/man1/notmuch-insert.rst |  6 ++++++
>  notmuch-insert.c            | 25 ++++++++++++++-----------
>  test/T070-insert.sh         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+), 11 deletions(-)
>
> diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
> index 47884515..86e2f567 100644
> --- a/doc/man1/notmuch-insert.rst
> +++ b/doc/man1/notmuch-insert.rst
> @@ -51,6 +51,12 @@ Supported options for **insert** include
>  ``--no-hooks``
>      Prevent hooks from being run.
>  
> +``--world-readable``
> +    When writing mail to the mailbox, allow it to be read by users
> +    other than the current user.  Note that this does not override
> +    umask.  By default, delivered mail is only readable by the current
> +    user.
> +
>  ``--decrypt=(true|nostash|auto|false)``
>      If ``true`` and the message is encrypted, try to decrypt the
>      message while indexing, stashing any session keys discovered.  If
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 48490b51..d229c9dc 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -159,10 +159,10 @@ mkdir_recursive (const void *ctx, const char *path, int mode)
>   * otherwise. Partial results are not cleaned up on errors.
>   */
>  static bool
> -maildir_create_folder (const void *ctx, const char *maildir)
> +maildir_create_folder (const void *ctx, const char *maildir, bool world_readable)
>  {
>      const char *subdirs[] = { "cur", "new", "tmp" };
> -    const int mode = 0700;
> +    const int mode = (world_readable ? 0755 : 0700);
>      char *subdir;
>      unsigned int i;
>  
> @@ -211,10 +211,11 @@ tempfilename (const void *ctx)
>   * is not touched).
>   */
>  static int
> -maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
> +maildir_mktemp (const void *ctx, const char *maildir, bool world_readable, char **path_out)
>  {
>      char *filename, *path;
>      int fd;
> +    const int mode = (world_readable ? 0644 : 0600);
>  
>      do {
>  	filename = tempfilename (ctx);
> @@ -227,7 +228,7 @@ maildir_mktemp (const void *ctx, const char *maildir, char **path_out)
>  	    return -1;
>  	}
>  
> -	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
> +	fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, mode);
>      } while (fd == -1 && errno == EEXIST);
>  
>      if (fd == -1) {
> @@ -289,12 +290,12 @@ copy_fd (int fdout, int fdin)
>   * the file, or NULL on errors.
>   */
>  static char *
> -maildir_write_tmp (const void *ctx, int fdin, const char *maildir)
> +maildir_write_tmp (const void *ctx, int fdin, const char *maildir, bool world_readable)
>  {
>      char *path;
>      int fdout;
>  
> -    fdout = maildir_mktemp (ctx, maildir, &path);
> +    fdout = maildir_mktemp (ctx, maildir, world_readable, &path);
>      if (fdout < 0)
>  	return NULL;
>  
> @@ -323,11 +324,11 @@ FAIL:
>   * errors.
>   */
>  static char *
> -maildir_write_new (const void *ctx, int fdin, const char *maildir)
> +maildir_write_new (const void *ctx, int fdin, const char *maildir, bool world_readable)
>  {
>      char *cleanpath, *tmppath, *newpath, *newdir;
>  
> -    tmppath = maildir_write_tmp (ctx, fdin, maildir);
> +    tmppath = maildir_write_tmp (ctx, fdin, maildir, world_readable);
>      if (! tmppath)
>  	return NULL;
>      cleanpath = tmppath;
> @@ -457,6 +458,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
>      bool create_folder = false;
>      bool keep = false;
>      bool hooks = true;
> +    bool world_readable = false;
>      bool synchronize_flags;
>      char *maildir;
>      char *newpath;
> @@ -467,7 +469,8 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
>  	{ .opt_string = &folder, .name = "folder", .allow_empty = true },
>  	{ .opt_bool = &create_folder, .name = "create-folder" },
>  	{ .opt_bool = &keep, .name = "keep" },
> -	{ .opt_bool =  &hooks, .name = "hooks" },
> +	{ .opt_bool = &hooks, .name = "hooks" },
> +	{ .opt_bool = &world_readable, .name = "world-readable" },
>  	{ .opt_inherit = notmuch_shared_indexing_options },
>  	{ .opt_inherit = notmuch_shared_options },
>  	{ }
> @@ -523,7 +526,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
>      }
>  
>      strip_trailing (maildir, '/');
> -    if (create_folder && ! maildir_create_folder (config, maildir))
> +    if (create_folder && ! maildir_create_folder (config, maildir, world_readable))
>  	return EXIT_FAILURE;
>  
>      /* Set up our handler for SIGINT. We do not set SA_RESTART so that copying
> @@ -535,7 +538,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
>      sigaction (SIGINT, &action, NULL);
>  
>      /* Write the message to the Maildir new directory. */
> -    newpath = maildir_write_new (config, STDIN_FILENO, maildir);
> +    newpath = maildir_write_new (config, STDIN_FILENO, maildir, world_readable);
>      if (! newpath) {
>  	return EXIT_FAILURE;
>      }
> diff --git a/test/T070-insert.sh b/test/T070-insert.sh
> index 40519bb2..05be473a 100755
> --- a/test/T070-insert.sh
> +++ b/test/T070-insert.sh
> @@ -4,6 +4,10 @@ test_description='"notmuch insert"'
>  
>  test_require_external_prereq gdb
>  
> +# subtests about file permissions assume that we're working with umask
> +# 022 by default.
> +umask 022
> +
>  # Create directories and database before inserting.
>  mkdir -p "$MAIL_DIR"/{cur,new,tmp}
>  mkdir -p "$MAIL_DIR"/Drafts/{cur,new,tmp}
> @@ -37,6 +41,9 @@ notmuch insert < "$gen_msg_filename"
>  cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
>  test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
>  
> +test_begin_subtest "Permissions on inserted message should be 0600"
> +test_expect_equal "600" "$(stat -c %a "$cur_msg_filename")"
> +
>  test_begin_subtest "Insert message adds default tags"
>  output=$(notmuch show --format=json "subject:insert-subject")
>  expected='[[[{
> @@ -73,6 +80,27 @@ notmuch insert +custom < "$gen_msg_filename"
>  output=$(notmuch search --output=messages tag:custom)
>  test_expect_equal "$output" "id:$gen_msg_id"
>  
> +test_begin_subtest "Insert tagged world-readable message"
> +gen_insert_msg
> +notmuch insert --world-readable +world-readable-test < "$gen_msg_filename"
> +cur_msg_filename=$(notmuch search --output=files "tag:world-readable-test")
> +test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
> +
> +test_begin_subtest "Permissions on inserted world-readable message should be 0644"
> +test_expect_equal "644" "$(stat -c %a "$cur_msg_filename")"
> +
> +test_begin_subtest "Insert tagged world-readable message with group-only umask"
> +oldumask=$(umask)
> +umask 027
> +gen_insert_msg
> +notmuch insert --world-readable +world-readable-umask-test < "$gen_msg_filename"
> +cur_msg_filename=$(notmuch search --output=files "tag:world-readable-umask-test")
> +umask "$oldumask"
> +test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
> +
> +test_begin_subtest "Permissions on inserted world-readable message with funny umask should be 0640"
> +test_expect_equal "640" "$(stat -c %a "$cur_msg_filename")"
> +
>  test_begin_subtest "Insert message, add/remove tags"
>  gen_insert_msg
>  notmuch insert +custom -unread < "$gen_msg_filename"
> @@ -170,6 +198,23 @@ output=$(notmuch search --output=files path:F/G/H/I/J/new tag:folder)
>  basename=$(basename "$output")
>  test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/new/${basename}"
>  
> +test_begin_subtest "Created subfolder should have permissions 0700"
> +test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J")"
> +test_begin_subtest "Created subfolder new/ should also have permissions 0700"
> +test_expect_equal "700" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/new")"
> +
> +test_begin_subtest "Insert message, create world-readable subfolder"
> +gen_insert_msg
> +notmuch insert --folder=F/G/H/I/J/K --create-folder --world-readable +folder-world-readable < "$gen_msg_filename"
> +output=$(notmuch search --output=files path:F/G/H/I/J/K/new tag:folder-world-readable)
> +basename=$(basename "$output")
> +test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/K/new/${basename}"
> +
> +test_begin_subtest "Created world-readable subfolder should have permissions 0755"
> +test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K")"
> +test_begin_subtest "Created world-readable subfolder new/ should also have permissions 0755"
> +test_expect_equal "755" "$(stat -c %a "${MAIL_DIR}/F/G/H/I/J/K/new")"
> +
>  test_begin_subtest "Insert message, create existing subfolder"
>  gen_insert_msg
>  notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
> -- 
> 2.15.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] cli/insert: add --world-readable flag
  2018-02-09  4:10       ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
  2018-02-15  6:08         ` Tomi Ollila
@ 2018-03-24 23:17         ` David Bremner
  1 sibling, 0 replies; 10+ messages in thread
From: David Bremner @ 2018-03-24 23:17 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> In some cases (e.g. when building a publicly-visible e-mail archive)
> it doesn't make any sense to restrict visibility of the message to the
> current user account.

pushed to master,

d

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-24 23:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-05  4:37 [PATCH] cli/insert: new message file can be world-readable (rely on umask) Daniel Kahn Gillmor
2018-02-05 11:59 ` Peter Wang
2018-02-06 19:43   ` [PATCH v2] " Daniel Kahn Gillmor
2018-02-09  1:40     ` Daniel Kahn Gillmor
2018-02-09  1:52       ` Brian Sniffen
2018-02-09  2:33         ` Daniel Kahn Gillmor
2018-02-09  2:00       ` Daniel Kahn Gillmor
2018-02-09  4:10       ` [PATCH v3] cli/insert: add --world-readable flag Daniel Kahn Gillmor
2018-02-15  6:08         ` Tomi Ollila
2018-03-24 23:17         ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).