cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
544 stars 148 forks source link

bug renaming/deleting special use folders in murder setup #2599

Open MichaelMenge opened 5 years ago

MichaelMenge commented 5 years ago

This bug was reported on info-cyrus Mailinglist with the subject "renameing/deliting special use folders in murder setup." on 13. Nov 2018, but i received no response.

we discovered a bug in cyrus imapd 3.0.8 in murder/aggregator configuration.

We have the following default structure:

a LIST "" "*"
* LIST (\HasNoChildren) "/" INBOX
* LIST (\HasChildren) "/" Mail
* LIST (\HasNoChildren \Drafts) "/" Mail/drafts
* LIST (\HasNoChildren) "/" Mail/s-spam
* LIST (\HasNoChildren \Sent) "/" Mail/sent
* LIST (\HasNoChildren \Trash) "/" Mail/trash
* LIST (\HasNoChildren \Junk) "/" Mail/v-spam
a OK Completed (0.000 secs 7 calls)

ctl_mboxlist -d | grep ^user.zrstes1 user.zrstes1 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.drafts 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.s-spam 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.sent 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.trash 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.v-spam 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan

Renaming/Deleting a Mailbox with Special-Use Flag fails

b RENAME Mail/v-spam Mail/spam
b NO SPECIAL-USE flag conflict
c DELETE Mail/v-spam
c NO SPECIAL-USE flag conflict

but Renaming/Deleting the Folder above will succeed

d RENAME Mail Test
* OK rename Mail Test
* OK rename Mail/drafts Test/drafts
* OK rename Mail/s-spam Test/s-spam
* OK rename Mail/sent Test/sent
* OK rename Mail/trash Test/trash
* OK rename Mail/v-spam Test/v-spam
d OK Completed

but results in broken entries in the mailboxdb on mupdate master and frontends

ctl_mboxlist -C /etc/imapd_fe.conf -d | grep ^user.zrstes1 user.zrstes1 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Mail.drafts 1 ma05.mail.localhost! user.zrstes1.Mail.sent 1 ma05.mail.localhost! user.zrstes1.Mail.trash 1 ma05.mail.localhost! user.zrstes1.Mail.v-spam 1 ma05.mail.localhost! user.zrstes1.Test 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Test.drafts 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Test.s-spam 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Test.sent 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Test.trash 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan user.zrstes1.Test.v-spam 1 ma05.mail.localhost!ssd zrstes1 lrswipkxtecdan

see the missing partition for old names of the special use folders. it seams that the sepecial use annotation was moved with the folders without a problem

e LIST (SPECIAL-USE) "" "*"
* LIST (\HasNoChildren \Drafts) "/" Test/drafts
* LIST (\HasNoChildren \Sent) "/" Test/sent
* LIST (\HasNoChildren \Trash) "/" Test/trash
* LIST (\HasNoChildren \Junk) "/" Test/v-spam
e OK Completed (0.000 secs 7 calls)

On the backend the mailboxdb entries look fine. But the user is unable to rename/move the folders back, or create the new folders with the old name because the folder are still reserved on the mupdate server.

f RENAME Test Mail
* OK rename Test Mail
* NO rename Test/drafts Mail/drafts: unable to reserve mailbox on mupdate server
f NO unable to reserve mailbox on mupdate server
g LIST "" "*"
* LIST (\HasNoChildren) "/" INBOX
* LIST (\HasNoChildren) "/" Mail
* LIST (\HasNoChildren \Drafts) "/" Test/drafts
* LIST (\HasNoChildren) "/" Test/s-spam
* LIST (\HasNoChildren \Sent) "/" Test/sent
* LIST (\HasNoChildren \Trash) "/" Test/trash
* LIST (\HasNoChildren \Junk) "/" Test/v-spam
g OK Completed (0.010 secs 7 calls)

a "ctl_mboxlist -m -a" on the backend will fix the mailboxdb on the mupdate master/frontend

I am not sure if moving a special use folder should work or not this should be consistent to renaming a top folder.

At the moment our users are unable to restore their folder structure if they renamed/moved their folders unintended.

dilyanpalauzov commented 5 years ago

Per https://github.com/cyrusimap/cyrus-imapd/issues/2406 and https://github.com/cyrusimap/cyrus-imapd/commit/657b4ce15f160dd831d03070f5c31b290fdaf905 special use folders can only be direct children of INBOX. This was not the case with older Cyrus Imap versions. I guess current cyrus imap assumes that SPECIAL-USE can only be within INBOX and any other configuration leads to undefined behaviour.

MichaelMenge commented 5 years ago

Thanks for pointing to the issiue/commit. I did read the RFC 6154 but was unable to find the limitation to direct folders of the INBOX is stated, or that special use on shared folders is not allowed. Can you tell me where read this in RFC 6154?

What i did find where indirect indications that special use on sub- and shared folders are possible.

2. New Mailbox Attributes Identifying Special-Use Mailboxes ... (last paragraph) Special-use attributes are likely to be user-specific. User Adam might share his \Sent mailbox with user Barb, but that mailbox is unlikely to also serve as Barb's \Sent mailbox. It's certainly possible for Adam and Barb to each set the \Sent use on the same mailbox, but that would be done by specific action (see the sections below).

5.2. Example of an Extended IMAP LIST Command ... Also note that we've used the wildcard "*", rather than "%", to make sure we see all special-use mailboxes, even ones that might not be at the namespace's root.

7. Security Considerations ... Example: If a user is allowed to give the "\Junk" attribute to a shared mailbox, legitimate mail that's misclassified as junk (false positives) will be put into that shared mailbox, exposing the user's private mail to others. The server might warn a user of that possibility, or might refuse to allow the specification to be made on a shared mailbox. (Note that this problem exists independent of this specification, if the server allows a user to share a mailbox that's already in use for such a function.)

and that the server is responsible to verify that the changes to special use are allowed before setting them.

4. IMAP METADATA Entry for Special-Use Attributes ... A server that supports this MUST check the validity of changes to the special-use attributes that are done through the metadata in the same way that it checks validity for the CREATE command and for any internal mechanisms for setting special uses on mailboxes. It MUST NOT just blindly accept setting of these metadata by clients, which might result in the setting of special uses that the implementation does not support, multiple mailboxes with the same special use, or other situations that the implementation considers invalid.

dilyanpalauzov commented 5 years ago

https://github.com/cyrusimap/cyrus-imapd/issues/2406 is about writing down that Cyrus IMAP deviates from the RFC.

elliefm commented 4 years ago

[assigning everyone]

This is tagged 3.2, but I'm not really sure what to do with it w.r.t. upcoming 3.2 release.

I wonder: does this affect non-murder setups? And if not, why does it behave differently in a murder setup? Does it affect the master branch, or only 3.0?

I think I've convinced myself I wanna make some tests to try and reproduce it, but guidance from people who know special-use a bit better than I do would be appreciated?

MichaelMenge commented 4 years ago

I am not a expert for SPECIAL-USE, but as far as i understand the RFC 6154 the limitation not to allow renaming of a special-use folder is self imposed by cyrus or the developer that implemented the feature in the first place.

The special-use metadata/annotation was handled correctly on the backend. So for me the easiest solution sees to be to remove the limitation and allow renaming of special use folders.

There is one other limitation for SPECIAL-USE with the ANNOTATE Extension (RFC 5257) which limits the SPECIAL-USE Folders to direct folders of the IMBOX. I suspect that this limitation is also self imposed. We did use SPECIAL-USE with ANNOTATE in 2.4 on subfolders without problem, but as the support for the ANNOTATE Extension reintroduced for 3.0 and 3.2 (Bring back ANNOTATEMORE support) it should be confirmed that this has no unintended side effect.

I can try to create a patch to remove the restrictions, but I don't have the knowledge to ensure that there are no negative effects on the ANNOTATE Extension. One option would be to disable the limitations only if the "annotate_enable_legacy_commands" option is off.

ksmurchison commented 4 years ago

@MichaelMenge We would greatly appreciate a patch to get things started.

MichaelMenge commented 4 years ago

while testing/debugging my patch to allow renaming of special-use folders I discovered the reason why the mupdate master had the old mailbox with an incomplete entry (missing partition and ACLs)

The Backends sends a "ACTIVATE OLDMAILBOX" with incomplete data to the mupdate master. I could trace this to EXPORTED modseq_t mboxlist_foldermodseq_dirty(struct mailbox *mailbox) imap/mboxlist.c:3177 where mboxlist_update(mbentry, 0) is called.

As far as I know mupdate master does not track modseq information and I suspected mboxlist_update(mbentry, 1) (localonly update) should be used.

Changing this did fixes the problem, but I don't know if this is the correct solution or if there are side effects. So can somebody confirm that mboxlist_foldermodseq_dirty only requires a localonly mboxlist_update

alecpl commented 11 months ago

This is still an issue in 3.6.