From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 871986DE01ED for ; Wed, 14 Feb 2018 22:08:17 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.48 X-Spam-Level: X-Spam-Status: No, score=0.48 tagged_above=-999 required=5 tests=[AWL=-0.172, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Z5uiwn7-bpgO for ; Wed, 14 Feb 2018 22:08:16 -0800 (PST) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id 381666DE01EC for ; Wed, 14 Feb 2018 22:08:16 -0800 (PST) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id 7B6EB100033; Thu, 15 Feb 2018 08:08:18 +0200 (EET) From: Tomi Ollila To: Daniel Kahn Gillmor , Notmuch Mail Subject: Re: [PATCH v3] cli/insert: add --world-readable flag In-Reply-To: <20180209041058.4037-1-dkg@fifthhorseman.net> References: <87k1vnuehz.fsf@fifthhorseman.net> <20180209041058.4037-1-dkg@fifthhorseman.net> User-Agent: Notmuch/0.26~rc0+5~gaec2eb0 (https://notmuchmail.org) Emacs/25.2.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Feb 2018 06:08:17 -0000 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 > --- > 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