Suggested feature and contribution

[Egoitz Aurrekoetxea]

Thanks a lot Bron!

Yeah I saw your commit in 2015… I think the idea is fine but perhaps an alert should be better for that instead of a restriction…. Normally people is not going to do this kind of silly things… And when done is normally a mua driven crazy (we know everyone which mua I’m talking about :p )….

I’ll tell about this fix to a customer of us who removed tons of folder… weren’t at saremail_restore (our deleted items recover system) and I started taking a look at what was going and arrived to this lines of mboxlist.c (I think it was..)…


Cheers!


Egoitz Aurrekoetxea
Dpto. de sistemas
944 209 470
Parque Tecnológico. Edificio 103
48170 Zamudio (Bizkaia)
egoitz at sarenet.es <mailto:undefined>
www.sarenet.es <http://www.sarenet.es/>
Antes de imprimir este correo electrónico piense si es necesario hacerlo.

> El 19 jul 2019, a las 2:13, Bron Gondwana <brong at fastmailteam.com> escribió:
> 
> 
> 
> On Fri, Jul 19, 2019, at 10:09, Bron Gondwana wrote:
>> On Thu, Jul 18, 2019, at 19:09, Egoitz Aurrekoetxea wrote:
>>> Good morning,
>>> 
>>> When using delete_delayed if someone removes a big folder (that one with more than 20 subfolders anywhere below it) in mboxlist_delayed_deletemailbox() only last 20 are preserved. We think it could be a good idea to preserve all and to have a parameter for configuring it. The reason for that, is that we use delete_delayed for storing the removed content remotely with the customer hired retention period in slow disk space. Perhaps could be a good idea something like : 
>>> 
>>> In mboxlist_delayed_deletemailbox() : 
>>> 
>>> If (!preserve_delete_delayed_folders_always)
>>> {
>>>     /* keep the last 19, so the new one is the 20th */
>>>     for (i = 0; i < (int)existing.count - 19; i++) {
>>>         const char *subname = strarray_nth(&existing, i);
>>>         syslog(LOG_NOTICE, "too many subfolders for %s, deleting %s (%d / %d)",
>>>                newname, subname, i+1, (int)existing.count);
>>>         r = mboxlist_deletemailbox(subname, 1, userid, auth_state, NULL, 0, 1, 1,
>>>                                    keep_intermediaries);
>>>         if (r) goto done;
>>>     }
>>> }
>> 
>> Hmm.... yeah, OK.  This is actually buggy in that case!  The intended behaviour was to avoid a Denial of Service attack where you would create and delete the same mailbox name millions of times - however, the whole concept is bogus because there's nothing stopping somebody creating and deleting folder000001 through folderFFFFFF and creating the same attack.
>> 
>> I suggest that we just remove this whole silly check entirely, and if we want a similar level of attack protection we do something smarter like a quota for total folders+deleted folders that haven't been cleaned up yet - set it high enough that anybody hitting that is clearly doing something wrong, and require the administrator semi-manually clean up the deleted folders in order to re-allow folder creation.
>> 
>> Cheers,
>> 
>> Bron.
> 
> I've pushed commits to master and 3.0 to remove this check.
> 
> --
>   Bron Gondwana, CEO, Fastmail Pty Ltd
>   brong at fastmailteam.com <mailto:brong at fastmailteam.com>
> 
> 
> ----
> Cyrus Home Page: http://www.cyrusimap.org/ <http://www.cyrusimap.org/>
> List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/ <http://lists.andrew.cmu.edu/pipermail/info-cyrus/>
> To Unsubscribe:
> https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus <https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/info-cyrus/attachments/20190719/d5d52903/attachment-0001.html>