Re: Cyrus imap 3.4.8: synchronizing local mailbox list with MUPDATE server does not work

Hi Elli,

Quoting ellie timoney <[email protected]>:

>
> So I think getxstring's new pin->isclient behaviour is correct here,  
> but is more strict than it used to be.  The other end is behaving  
> badly and ctl_mboxlist no longer tolerates it.  Interesting.
>
> Ahh yep.  static int sendupdate(const mbentry_t *mbentry, void  
> *rock) in imap/mupdate.c is what sends the LIST response, and it has  
> hardcoded '+' in all its literal outputs.  Perhaps this function  
> sometimes outputs to a server, and sometimes to a client.  In which  
> case it ought to check and omit the '+' when pout->isclient is true,  
> and it might need to check for the LITERAL+ capability (or whatever  
> it's called) when talking to a server.  But if it never talks to a  
> server, then we should just remove the '+'s.
>

I am not sure how the information flow is handled, exactly, and if  
there are other cases where LITERAL+ is used,
but I know of the following mupdate connections.

1. backend (client) to mupdate master (server): initial update on  
startup (ctl_mboxlist -m)
2. backend (client) to mupdate master (server): connection if mailbox  
is created / renamed / deleted
3. frontend mupdate (client) to  mupdate master (server) update of  
frontend mailbox list.
    not sure if this is a pull be the client or a push by the server

I am unsure about the 2nd but I suspect that the 3rd type of  
connection might have the same problem.

If you fix this on the mupdate server side please ensure that the fix is
backported to 3.0 and maybe 2.5 branch as this is needed to migrate from older
cyrus versions to any version with the recent security fix.

>> Cyrus Imapd docs : « Generally accepted wisdom when upgrading a  
>> Murder configuration is to upgrade all your back end servers first.  
>> This can be done one at a time. Upgrade your mupdate master and  
>> front ends last. »

Or provide an config option to disable a the strict check (for "pin")  
in mupdate_client connections.
As the mupdate connections are only used between trusted servers and  
(at least in our setup)
not exposed to the world the risk of CVE-2024-34055 is IMHO minimal  
for mupdate connections

> What's been baffling me so far though, is that our tests don't  
> detect any problem.  But!  That's because our tests only run  
> ctl_mboxlist -m on startup, before any mailboxes have been created,  
> and the LIST response is empty.  As part of fixing this, I think  
> I'll probably add a ctl_mboxlist -m call on each backend as part of  
> our post-test sanity checks.  That should catch this sort of thing  
> coming back in the future.

more test coverage is good to catch these kind of problems.
Can you also add checks for the other two types of mupdate connections
to check of there are other cases where LITERAL+ is used where it should not
have been used.

>
> Thanks heaps for your initial diagnosis, that was a big help!
>

glad I could help, so that this gets fixed soon

Michael

--------------------------------------------------------------------------------
Michael Menge                          Tel.: (49) 7071 / 29-70316
Universität Tübingen                   Fax.: (49) 7071 / 29-5912
Zentrum für Datenverarbeitung          mail:  
[email protected]
Wächterstraße 76
72074 Tübingen


------------------------------------------
Cyrus: Info
Permalink: https://cyrus.topicbox.com/groups/info/T5a7b6ffa3591f516-M68085830d937588d53dd7fe9
Delivery options: https://cyrus.topicbox.com/groups/info/subscription