Hi Elli, Quoting ellie timoney <ellie@xxxxxxxxxxxx>:
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 serverI 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-5912Zentrum für Datenverarbeitung mail: michael.menge@xxxxxxxxxxxxxxxxxxxx
Wächterstraße 76 72074 Tübingen
Attachment:
smime.p7s
Description: S/MIME-Signatur
------------------------------------------ Cyrus: Info Permalink: https://cyrus.topicbox.com/groups/info/T5a7b6ffa3591f516-M68085830d937588d53dd7fe9 Delivery options: https://cyrus.topicbox.com/groups/info/subscription
