[Dovecot] v1.2: can't access other users shared INBOX
Hi *,
when a user A shares his INBOX with another user B, the user B can't access its content:
User A:
g getacl INBOX
- ACL INBOX A@example.com lrswipkxtecda B@example.com lrswipkxtecd g OK Completed
User B:
l list "" "*"
- LIST (\Noselect \HasChildren) "/" "user"
- LIST (\Noselect \HasChildren) "/" "user/A@example.com"
- LIST (\HasChildren) "/" "INBOX"
- LIST (\HasNoChildren) "/" "INBOX/Gesendet"
- LIST (\HasChildren) "/" "user/A@example.com/foobar"
- LIST (\HasNoChildren) "/" "user/A@example.com/INBOX" l OK List completed. s1 select "user/A@example.com" s1 NO [CANNOT] Invalid mailbox name s2 select "user/A@example.com/INBOX" s2 NO [NONEXISTENT] Mailbox doesn't exist: INBOX
Actually there are two bugs to observe here:
"user/A@example.com" really should be accessible to user B. Why is it listed with "\Noselect"?
"user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On 04.03.2009, Sascha Wilde wrote:
Hi *,
when a user A shares his INBOX with another user B, the user B can't access its content:
User A:
g getacl INBOX * ACL INBOX A@example.com lrswipkxtecda B@example.com lrswipkxtecd g OK Completed
User B:
l list "" "*" * LIST (\Noselect \HasChildren) "/" "user" * LIST (\Noselect \HasChildren) "/" "user/A@example.com" * LIST (\HasChildren) "/" "INBOX" * LIST (\HasNoChildren) "/" "INBOX/Gesendet" * LIST (\HasChildren) "/" "user/A@example.com/foobar" * LIST (\HasNoChildren) "/" "user/A@example.com/INBOX" l OK List completed. s1 select "user/A@example.com" s1 NO [CANNOT] Invalid mailbox name s2 select "user/A@example.com/INBOX" s2 NO [NONEXISTENT] Mailbox doesn't exist: INBOX
Actually there are two bugs to observe here:
1) "user/A@example.com" really should be accessible to user B. Why is it listed with "\Noselect"?
I'm not sure it should be accessible. This is most likely not A's INBOX. That's the other folder you're trying to access:
2) "user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
That's A's INBOX, most likely, so it should be accessible. That it's listed but not accessible is AFAICT a combination of two bugs. One is that the INBOX's ACL is used as default, so if B as l-permission in A's INBOX all of A's mailboxes that do not set an ACL for B are listed. The other is that dovecot does not determine B's permissions correctly when it comes to A's INBOX. The first bug is the topic of the other thread ("ACLs are applied recursively to sub mailboxes"). The second comes about as follows, at least for the maildir storage backend: The core of the issue lies again in how acl_backend_vfile_object_init determines the value of the local_path. If the name given to the function is "INBOX", the outcome depends not only on who the owner of the actual storage is, but also on whether the current user is the owner. If the user is the owner, the filename is determined correctly as "dovecot-acl" in the root directory of the user's maildir hierarchy. If the user is not the owner, local_path is set to ".INBOX/dovecot-acl" in the root directory, which is not correct. The reason for this difference are lines 217f in mailbox-list-maildir.c (rev. 5284f45c249a, function maildir_list_get_path): if (strcmp(name, "INBOX") == 0 && _list->set.inbox_path != NULL) return _list->set.inbox_path; i.e. "INBOX" is a special case, but only if inbox_path has been set, and it will be set in maildir_create only if the user is the owner of the mailbox: if (list_set.inbox_path == NULL && strcmp(layout, MAILDIR_PLUSPLUS_DRIVER_NAME) == 0 && (_storage->ns->flags & NAMESPACE_FLAG_INBOX) != 0) { /* Maildir++ INBOX is the Maildir base itself */ list_set.inbox_path = list_set.root_dir; } The solution I'm testing is to simply remove the test for the NAMESPACE_FLAG_INBOX flag (see patch below). So far it seems to work fine. I'm not sure it will have some negative consequences in other parts of dovecot, but I didn't see anything obvious in the code and the inbox_path is conceptually a property of the storage backend and not of the namespace configuration, so whether it's set or not, should probably not depend NAMESPACE_FLAG_INBOX. Regards, Bernhard diff -r 5284f45c249a src/lib-storage/index/maildir/maildir-storage.c --- a/src/lib-storage/index/maildir/maildir-storage.c Sun Mar 15 20:06:45 2009 -0400 +++ b/src/lib-storage/index/maildir/maildir-storage.c Tue Mar 17 16:38:57 2009 +0100 @@ -201,8 +201,7 @@ maildir_create(struct mail_storage *_sto list_set.lock_method = &_storage->lock_method; if (list_set.inbox_path == NULL && - strcmp(layout, MAILDIR_PLUSPLUS_DRIVER_NAME) == 0 && - (_storage->ns->flags & NAMESPACE_FLAG_INBOX) != 0) { + strcmp(layout, MAILDIR_PLUSPLUS_DRIVER_NAME) == 0) { /* Maildir++ INBOX is the Maildir base itself */ list_set.inbox_path = list_set.root_dir; } -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Bernhard Herzog bh@intevation.de writes:
On 04.03.2009, Sascha Wilde wrote: [...]
User B:
l list "" "*"
- LIST (\Noselect \HasChildren) "/" "user"
- LIST (\Noselect \HasChildren) "/" "user/A@example.com"
- LIST (\HasChildren) "/" "INBOX"
- LIST (\HasNoChildren) "/" "INBOX/Gesendet"
- LIST (\HasChildren) "/" "user/A@example.com/foobar"
- LIST (\HasNoChildren) "/" "user/A@example.com/INBOX" l OK List completed. s1 select "user/A@example.com" s1 NO [CANNOT] Invalid mailbox name s2 select "user/A@example.com/INBOX" s2 NO [NONEXISTENT] Mailbox doesn't exist: INBOX
Actually there are two bugs to observe here:
- "user/A@example.com" really should be accessible to user B. Why is it listed with "\Noselect"?
I'm not sure it should be accessible. This is most likely not A's INBOX.
That's the other folder you're trying to access:
- "user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
This might very well be true, but in this case dovecot behaves different From cyrus -- which might still be RfC conforming (I haven't checked, but from my memories the RfC is very unspecific on these topics anyway).
I only hope that this difference is not to confusing to (Kolab) clients...
[...]
The solution I'm testing is to simply remove the test for the NAMESPACE_FLAG_INBOX flag (see patch below).
Thanks! I'll give it a try.
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Tue, 2009-03-17 at 18:20 +0100, Sascha Wilde wrote:
- "user/A@example.com" really should be accessible to user B. Why is it listed with "\Noselect"?
I'm not sure it should be accessible. This is most likely not A's INBOX.
That's the other folder you're trying to access:
- "user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
This might very well be true, but in this case dovecot behaves different From cyrus -- which might still be RfC conforming (I haven't checked, but from my memories the RfC is very unspecific on these topics anyway).
Are you talking about 1) or 2)? If 1), RFC doesn't talk about it. And I'm not really sure if it's a good idea to default the A@example.com to be the same as INBOX. If Cyrus does that, does it then not show the A@example.com/INBOX?
Hi *,
Before going back to the details in this discussion I want to point out that the whole thing turned out to be really relevant with existing clients: The Horde based Kolab WebClient expects the behavior as shown by Cyrus IMAP and fails to show "user/A@example.com/INBOX" as dovecot currently lists A's INBOX.
While this might be considered a bug in Horde it shows that existing clients actually highly depend on the behavior as seen in Cyrus IMAP.
Timo Sirainen tss@iki.fi writes:
On Tue, 2009-03-17 at 18:20 +0100, Sascha Wilde wrote:
- "user/A@example.com" really should be accessible to user B. Why is it listed with "\Noselect"?
I'm not sure it should be accessible. This is most likely not A's INBOX.
That's the other folder you're trying to access:
- "user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
This might very well be true, but in this case dovecot behaves different From cyrus -- which might still be RfC conforming (I haven't checked, but from my memories the RfC is very unspecific on these topics anyway).
Are you talking about 1) or 2)?
I'm talking about 1 vs 2 if you will. ;-) I expected "user/A@example.com" to be A's INBOX, while BH pointed out, that with the current implementation it is more likely that "user/A@example.com/INBOX" actually refers to A's INBOX.
If 1), RFC doesn't talk about it. And I'm not really sure if it's a good idea to default the A@example.com to be the same as INBOX. If Cyrus does that,
Yes, cyrus does that.
does it then not show the A@example.com/INBOX?
No, it doesn't as, the INBOX of A is referred to as "user/A@example.com", so "A@example.com/INBOX" would actually be an folder named "INBOX". Which would be displayed as "INBOX/INBOX" from A's point of view:
User A: when shared to User B maps to: INBOX user/A@example.com INBOX/foo user/A@example.com/foo INBOX/foo/bar user/A@example.com/foo/bar
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Sascha Wilde wilde@intevation.de writes:
Before going back to the details in this discussion I want to point out that the whole thing turned out to be really relevant with existing clients: The Horde based Kolab WebClient expects the behavior as shown by Cyrus IMAP and fails to show "user/A@example.com/INBOX" as dovecot currently lists A's INBOX.
This analysis was plain wrong. The true problem was that a001 LIST "" "user/A@example.com/%" did not list "user/A@example.com/INBOX"...
See Bernhard's mail on this subject, including a fix.
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On 17.03.2009, Bernhard Herzog wrote:
That's A's INBOX, most likely, so it should be accessible. That it's listed but not accessible is AFAICT a combination of two bugs. One is that the INBOX's ACL is used as default, so if B as l-permission in A's INBOX all of A's mailboxes that do not set an ACL for B are listed. The other is that dovecot does not determine B's permissions correctly when it comes to A's INBOX.
Even if this is fixed (e.g. with the patch from the above post), there's still one other problem with the INBOX. If A gives B list rights in A's INBOX but not on any other folder, B doesn't see A's INBOX when doing a LIST. The reason for that is maildir_fill_readdir always adds the virtual name of the INBOX even when MAILBOX_LIST_ITER_VIRTUAL_NAMES isn't set. In lines 260ff of mailbox-list-maildir-iter.c, rev. 5284f45c249a it unconditionally adds the prefix to "INBOX" when adding it to the tree: node = mailbox_tree_get(ctx->tree_ctx, t_strconcat(ns->prefix, "INBOX", NULL), NULL); The patch below fixes this, by only adding the virtual name of the INBOX if virtual_names is true, basically in the same way as earlier in the loop. I'm not sure whether it's really the correct fix, but in my tests so far it seems to work correctly. Regards, Bernhard diff -r 5284f45c249a src/lib-storage/list/mailbox-list-maildir-iter.c --- a/src/lib-storage/list/mailbox-list-maildir-iter.c Sun Mar 15 20:06:45 2009 -0400 +++ b/src/lib-storage/list/mailbox-list-maildir-iter.c Thu Mar 19 20:29:19 2009 +0100 @@ -257,8 +257,17 @@ maildir_fill_readdir(struct maildir_list iter_is_mailbox(&ctx->ctx, ctx->dir, "", "INBOX", MAILBOX_LIST_FILE_TYPE_UNKNOWN, &flags); if (ret > 0) { - node = mailbox_tree_get(ctx->tree_ctx, - t_strconcat(ns->prefix, "INBOX", NULL), NULL); + if (!virtual_names) { + str_truncate(mailbox, 0); + str_append(mailbox, "INBOX"); + mailbox_name = str_c(mailbox); + } else { + mailbox_name = + mail_namespace_get_vname(ns, mailbox, + "INBOX"); + } + node = mailbox_tree_get(ctx->tree_ctx, mailbox_name, + NULL); node->flags = MAILBOX_NOCHILDREN | MAILBOX_MATCHED; } } -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On 19.03.2009, Bernhard Herzog wrote:
The reason for that is maildir_fill_readdir always adds the virtual name of the INBOX even when MAILBOX_LIST_ITER_VIRTUAL_NAMES isn't set. In lines 260ff of mailbox-list-maildir-iter.c, rev. 5284f45c249a it unconditionally adds the prefix to "INBOX" when adding it to the tree:
node = mailbox_tree_get(ctx->tree_ctx, t_strconcat(ns->prefix, "INBOX", NULL), NULL);
The patch below fixes this, by only adding the virtual name of the INBOX if virtual_names is true, basically in the same way as earlier in the loop. I'm not sure whether it's really the correct fix, but in my tests so far it seems to work correctly.
As it turns out, there is one problem the patch doesn't address. It fixes the problem of listing the INBOX when the search pattern is simply "*", but dovecot still doesn't list the inbox of user fred if the pattern is "users/fred/%" (or however the shared namespace is configured). The reason is that "INBOX" simply doesn't match the pattern. The namespace prefix has to be taken into account when doing the match. The updated patch below fixes this. Regards, Bernhard diff -r d401e8f95bdc src/lib-storage/list/mailbox-list-maildir-iter.c --- a/src/lib-storage/list/mailbox-list-maildir-iter.c Tue Mar 24 18:04:18 2009 -0400 +++ b/src/lib-storage/list/mailbox-list-maildir-iter.c Wed Mar 25 17:39:07 2009 +0100 @@ -250,16 +250,29 @@ maildir_fill_readdir(struct maildir_list if (!update_only) node->flags |= MAILBOX_MATCHED; } - } else if (mailbox_tree_lookup(ctx->tree_ctx, "INBOX") == NULL && - imap_match(glob, "INBOX") == IMAP_MATCH_YES) { - /* see if INBOX exists. */ - ret = ctx->ctx.list->v. - iter_is_mailbox(&ctx->ctx, ctx->dir, "", "INBOX", - MAILBOX_LIST_FILE_TYPE_UNKNOWN, &flags); - if (ret > 0) { - node = mailbox_tree_get(ctx->tree_ctx, - t_strconcat(ns->prefix, "INBOX", NULL), NULL); - node->flags = MAILBOX_NOCHILDREN | MAILBOX_MATCHED; + } else { + const char * inbox_name; + if (!virtual_names) { + inbox_name = "INBOX"; + } else { + inbox_name = mail_namespace_get_vname(ns, mailbox, + "INBOX"); + } + + if (mailbox_tree_lookup(ctx->tree_ctx, inbox_name) == NULL && + imap_match(glob, inbox_name) == IMAP_MATCH_YES) { + /* see if INBOX exists. */ + ret = ctx->ctx.list->v. + iter_is_mailbox(&ctx->ctx, ctx->dir, "", + "INBOX", + MAILBOX_LIST_FILE_TYPE_UNKNOWN, + &flags); + if (ret > 0) { + node = mailbox_tree_get(ctx->tree_ctx, + inbox_name, NULL); + node->flags = (MAILBOX_NOCHILDREN | + MAILBOX_MATCHED); + } } } return 0; -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Bernhard Herzog bh@intevation.de writes: [...]
As it turns out, there is one problem the patch doesn't address. It fixes the problem of listing the INBOX when the search pattern is simply "*", but dovecot still doesn't list the inbox of user fred if the pattern is "users/fred/%" (or however the shared namespace is configured). The reason is that "INBOX" simply doesn't match the pattern. The namespace prefix has to be taken into account when doing the match. The updated patch below fixes this.
Thanks, that fixes the reported problem as well as the problems I experienced with the Horde-WebClient (and falsely blamed on the use of "INBOX" in the mailbox name).
Timo, is it possible to merge Bernhard's fixes in dovecot-1.2 tip?
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On 25.03.2009, Bernhard Herzog wrote:
On 19.03.2009, Bernhard Herzog wrote:
The reason for that is maildir_fill_readdir always adds the virtual name of the INBOX even when MAILBOX_LIST_ITER_VIRTUAL_NAMES isn't set. In lines 260ff of mailbox-list-maildir-iter.c, rev. 5284f45c249a it unconditionally adds the prefix to "INBOX" when adding it to the tree:
node = mailbox_tree_get(ctx->tree_ctx, t_strconcat(ns->prefix, "INBOX", NULL), NULL);
The patch below fixes this, by only adding the virtual name of the INBOX if virtual_names is true, basically in the same way as earlier in the loop. I'm not sure whether it's really the correct fix, but in my tests so far it seems to work correctly.
As it turns out, there is one problem the patch doesn't address.
There's one other problem that the patch doesn't fix. If user fred gives dave read permission on INBOX but not on any other folder and the inbox has children, the INBOX is not always listed for dave. OTOH, if dave has read permissions on one of the children, or the INBOX does not have children at all, the INBOX is listed. What happens is that if INBOX has children maildir_fill_readdir will add INBOX to the tree indirectly when it encounters the children and later when the INBOX special cases are handled, INBOX is already in the tree and it won't be matched against the mailbox name pattern and thus it's MAILBOX_MATCHED flag will not be set. If INBOX is the only visible mailbox that would match the search pattern, no mailbox in the tree has the MAILBOX_MATCHED flag, and dovecot will consider the whole users/fred namespace invisible to dave. The patch below addresses this. Bernhard diff -r 643a96aec996 src/lib-storage/list/mailbox-list-maildir-iter.c --- a/src/lib-storage/list/mailbox-list-maildir-iter.c Thu Mar 26 18:36:36 2009 -0400 +++ b/src/lib-storage/list/mailbox-list-maildir-iter.c Fri Mar 27 17:46:53 2009 +0200 @@ -250,16 +250,29 @@ maildir_fill_readdir(struct maildir_list if (!update_only) node->flags |= MAILBOX_MATCHED; } - } else if (mailbox_tree_lookup(ctx->tree_ctx, "INBOX") == NULL && - imap_match(glob, "INBOX") == IMAP_MATCH_YES) { + } else { + const char * inbox_name; + if (!virtual_names) { + inbox_name = "INBOX"; + } else { + inbox_name = mail_namespace_get_vname(ns, mailbox, + "INBOX"); + } + /* see if INBOX exists. */ ret = ctx->ctx.list->v. - iter_is_mailbox(&ctx->ctx, ctx->dir, "", "INBOX", - MAILBOX_LIST_FILE_TYPE_UNKNOWN, &flags); - if (ret > 0) { - node = mailbox_tree_get(ctx->tree_ctx, - t_strconcat(ns->prefix, "INBOX", NULL), NULL); - node->flags = MAILBOX_NOCHILDREN | MAILBOX_MATCHED; + iter_is_mailbox(&ctx->ctx, ctx->dir, "", + "INBOX", + MAILBOX_LIST_FILE_TYPE_UNKNOWN, + &flags); + if (ret > 0 && imap_match(glob, inbox_name) == IMAP_MATCH_YES) { + + node = mailbox_tree_get(ctx->tree_ctx, inbox_name, + &created); + if (created) + node->flags = MAILBOX_NOCHILDREN; + + node->flags |= MAILBOX_MATCHED; } } return 0; -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Wed, 2009-03-04 at 17:12 +0100, Sascha Wilde wrote:
- "user/A@example.com" really should be accessible to user B.
Do you still want that this would point to user's INBOX?
- "user/A@example.com/INBOX" does not exist, so the error message is correct, but why does it appear in the listing in the first place?
I committed several fixes related to handling shared INBOXes and also some other mailbox listing bugfixes.
Am Freitag, 3. April 2009 00:16:51 schrieb Timo Sirainen:
On Wed, 2009-03-04 at 17:12 +0100, Sascha Wilde wrote:
- "user/A at example.com" really should be accessible to user B.
Do you still want that this would point to user's INBOX?
IIRC this is Cyrus' behaviour and it is sane, so I think dovecot should do it the same.
As Sascha pointed out the mapping with Cyrus is User A: when shared to User B maps to: INBOX user/A at example.com INBOX/foo user/A at example.com/foo INBOX/foo/bar user/A at example.com/foo/bar
I committed several fixes related to handling shared INBOXes and also some other mailbox listing bugfixes.
Thanks, I saw that Bernhard (H.)'s patch is in 1.2rc2.
(Note that Sascha is unavailable this week. He'll be back next week.)
Best, Bernhard R.
Managing Director - Owner: www.intevation.net (Free Software Company) Germany Coordinator: fsfeurope.org. Coordinator: www.Kolab-Konsortium.com. Intevation GmbH, Osnabrück, DE; Amtsgericht Osnabrück, HRB 18998 Geschäftsführer Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
participants (4)
-
Bernhard Herzog
-
Bernhard Reiter
-
Sascha Wilde
-
Timo Sirainen