[PATCH] cli/insert: new message file can be world-readable (rely on umask)

classic Classic list List threaded Threaded
10 messages Options
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

[PATCH] cli/insert: new message file can be world-readable (rely on umask)

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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Peter Wang-2 Peter Wang-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] cli/insert: new message file can be world-readable (rely on umask)

On Sun,  4 Feb 2018 23:37:03 -0500, Daniel Kahn Gillmor <[hidden email]> 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
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] cli/insert: new message file can be world-readable (rely on umask)

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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)

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
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch

signature.asc (847 bytes) Download Attachment
Brian Sniffen-2 Brian Sniffen-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)

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 <[hidden email]> 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
> [hidden email]
> https://notmuchmail.org/mailman/listinfo/notmuch

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)

In reply to this post by Daniel Kahn Gillmor
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch

signature.asc (847 bytes) Download Attachment
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] cli/insert: new message file can be world-readable (rely on umask)

In reply to this post by Brian Sniffen-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch

signature.asc (847 bytes) Download Attachment
Daniel Kahn Gillmor Daniel Kahn Gillmor
Reply | Threaded
Open this post in threaded view
|

[PATCH v3] cli/insert: add --world-readable flag

In reply to this post by Daniel Kahn Gillmor
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 <[hidden email]>
---
 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
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Tomi Ollila-2 Tomi Ollila-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] cli/insert: add --world-readable flag

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 <[hidden email]>
> ---
>  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
> [hidden email]
> https://notmuchmail.org/mailman/listinfo/notmuch
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] cli/insert: add --world-readable flag

In reply to this post by Daniel Kahn Gillmor
Daniel Kahn Gillmor <[hidden email]> 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
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch